Re: [PATCH v5 0/7] Add AMD ISP4 driver

[Sultan Alsawaf]

 Hi Bin,

On Wed, Nov 05, 2025 at 01:25:58AM -0800, Sultan Alsawaf wrote:
> Hi Bin,
> 
> To expedite review, I've attached a patch containing a bunch of fixes I've made
> on top of v5. Most of my changes should be self-explanatory; feel free to ask
> further about specific changes if you have any questions.
> 
> I haven't had a chance to review all of the v4 -> v5 changes yet, but I figured
> I should send what I've got so far.
> 
> FYI, there is a regression in isp4if_dequeue_buffer() where the bufq lock isn't
> protecting the list_del() anymore. I also checked the compiler output when using
> guard() versus scoped_guard() in that function and there is no difference:
> 
>   sha1sum:
>   5652a0306da22ea741d80a9e03a787d0f71758a8  isp4_interface.o // guard()
>   5652a0306da22ea741d80a9e03a787d0f71758a8  isp4_interface.o // scoped_guard()
> 
> So guard() should be used there again, which I've done in my patch.
> 
> I also have a few questions:
> 
> 1. Does ISP FW provide a register interface to disable the IRQ? If so, it is
>    faster to use that than using disable_irq_nosync() to disable the IRQ from
>    the CPU's side.
> 
> 2. When the IRQ is re-enabled in isp4sd_fw_resp_func(), is there anything
>    beforehand to mask all pending interrupts from the ISP so that there isn't a
>    bunch of stale interrupts firing as soon the IRQ is re-enabled?
> 
> 3. Is there any risk of deadlock due to the stream kthread racing with the ISP
>    when the ISP posts a new response _after_ the kthread determines there are no
>    more new responses but _before_ the enable_irq() in isp4sd_fw_resp_func()?
> 
> 4. Why are some lines much longer than before? It seems inconsistent that now
>    there is a mix of several lines wrapped to 80 cols and many lines going
>    beyond 80 cols. And there are multiple places where code is wrapped before
>    reaching 80 cols even with lots of room left, specifically for cases where it
>    wouldn't hurt readability to put more characters onto each line.

I've attached a new, complete patch of changes to apply on top of v5. You may
ignore the incomplete patch from my previous email and use the new one instead.

I made many changes and also answered questions 1-3 myself.

Please apply this on top of v5 and let me know if you have any questions.

BTW, I noticed a strange regression in v5 even without any of my changes: every
time you use cheese after using it one time, the video will freeze after 30-60
seconds with this message printed to dmesg:
  [ 2032.716559] amd_isp_capture amd_isp_capture: -><- fail respid unknown respid(0x30002)

And the video never unfreezes. I haven't found the cause for the regression yet;
can you try to reproduce it?

Thanks,
Sultan