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

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: <aRGjX1pv0y_lVext@sultan-box>
Date: Mon, 10 Nov 2025 00:33:35 -0800
From: Sultan Alsawaf <sultan@...neltoast.com>
To: Bin Du <Bin.Du@....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
Subject: Re: [PATCH v5 0/7] Add AMD ISP4 driver

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

Content of type "text/plain" skipped