linux-kernel - Re: [PATCH v4 4/7] media: platform: amd: isp4 subdev and firmware loading handling added

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b433fb73-1652-4b12-8631-dd7059f72566@amd.com>
Date: Sat, 11 Oct 2025 16:27:14 +0800
From: "Du, Bin" <bin.du@....com>
To: Sultan Alsawaf <sultan@...neltoast.com>
Cc: mchehab@...nel.org, hverkuil@...all.nl,
 laurent.pinchart+renesas@...asonboard.com, bryan.odonoghue@...aro.org,
 sakari.ailus@...ux.intel.com, prabhakar.mahadev-lad.rj@...renesas.com,
 linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
 pratap.nirujogi@....com, benjamin.chan@....com, king.li@....com,
 gjorgji.rosikopulos@....com, Phil.Jawich@....com, Dominic.Antony@....com,
 mario.limonciello@....com, richard.gong@....com, anson.tsao@....com,
 Alexey Zagorodnikov <xglooom@...il.com>
Subject: Re: [PATCH v4 4/7] media: platform: amd: isp4 subdev and firmware
 loading handling added

On 10/11/2025 3:18 PM, Sultan Alsawaf wrote:
> On Fri, Oct 10, 2025 at 06:25:48PM +0800, Du, Bin wrote:
>> Thanks, Sultan. sorry for the delayed response due to the long public
>> holiday here.
> 
> No worries, hope you had a good holiday. :)
> 

Thank you!

>> On 10/1/2025 3:24 PM, Sultan Alsawaf wrote:
>>> On Tue, Sep 30, 2025 at 03:30:49PM +0800, Du, Bin wrote:
>>>> On 9/23/2025 3:23 PM, Sultan Alsawaf wrote:
>>>>> On Thu, Sep 11, 2025 at 06:08:44PM +0800, Bin Du wrote:
>>>>>> +	u32 r1;
>>>>>> +
>>>>>> +	if (!isp_dev)
>>>>>> +		goto error_drv_data;
>>>>>> +
>>>>>> +	isp = &isp_dev->isp_sdev;
>>>>>> +	/* check ISP_SYS interrupts status */
>>>>>> +	r1 = isp4hw_rreg(ISP4_GET_ISP_REG_BASE(isp), ISP_SYS_INT0_STATUS);
>>>>>> +
>>>>>> +	isp_sys_irq_status = r1 & ISP4_FW_RESP_RB_IRQ_STATUS_MASK;
>>>>>
>>>>> There are four IRQs (one for each stream) but any one of the IRQs can result in
>>>>> a notification for _all_ streams. Each IRQ should only do the work of its own
>>>>> stream.
>>>>>
>>>>> You can do this by passing devm_request_irq() a private pointer to indicate the
>>>>> mapping between a stream and its IRQ, so that isp4_irq_handler() can know which
>>>>> stream it should look at.
>>>>>
>>>>
>>>> Will do optimization to remove unused IRQs and keep only necessary ones
>>>> (reduce from 4 to 2), actually an IRQ won't result in notification to all
>>>> streams, please check the implementation of isp4_resp_interrupt_notify, it
>>>> will only wake up IRQ corresponding stream processing thread.
>>>
>>> What I mean is that the IRQ for one stream can wake a different stream if it is
>>> also ready at the same time according to the interrupt status register.
>>>
>>
>> Yes, you are correct, besides its own stream, the IRQ may wake a different
>> stream if it is ready too in the IRQ status register. But i believe the
>> shared irq handler can improve the performance without negative effects. The
>> peseudo code of isp4_irq_handler works like this (take your below example)
>> irqreturn_t isp4_irq_handler(...)
>> {
>> 	status = read_irq_status();
>> 	if(status & WPT9)
>> 		isp4_wake_up_resp_thread(isp, 1);
>> 	if(status & WPT10)
>> 		isp4_wake_up_resp_thread(isp, 2)
>>          ack_irq_status(status);
>> 	return IRQ_HANDLED;
>> }
>> Which means the first isp4_irq_handler can process all IRQs at that time.
>> For the second isp4_irq_handler, because the irq status is cleared by the
>> first isp4_irq_handler, it just does nothing and quit. So even if
>> isp4_irq_handler doen't know exactly which IRQ triggers it, there's no harm
>> as far as I can tell, not sure if I missed something.
> 
> My thinking was that it's possible for duplicate wakeups to occur when the
> stream IRQs are affined to different CPUs and they fire around the same time in
> parallel.
> 
> But now that I see how the ISP interrupts are actually GPU interrupts, it means
> that the stream IRQs will always be affined to the same CPU as each other.
> 
> So my concern does not apply here, and you should disregard it. :)
> 
> Sultan

Thanks for the further clarification.

-- 
Regards,
Bin