Message-ID: <aJA2S0EY7HhVSSzc@sultan-box>
Date: Sun, 3 Aug 2025 21:25:47 -0700
From: Sultan Alsawaf <sultan@...neltoast.com>
To: "Du, Bin" <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
Subject: Re: [PATCH v2 0/8] Add AMD ISP4 driver

On Mon, Aug 04, 2025 at 11:32:11AM +0800, Du, Bin wrote:
> On 7/31/2025 6:04 PM, Du, Bin wrote:
> > Thanks Sultan for your test
> > 
> > On 7/31/2025 8:30 AM, Sultan Alsawaf wrote:
> > > On Wed, Jul 30, 2025 at 05:53:58PM +0800, Du, Bin wrote:
> > > > On 7/30/2025 1:38 PM, Sultan Alsawaf wrote:
> > > > > On Tue, Jul 29, 2025 at 06:13:50PM +0800, Du, Bin wrote:
> > > > > > On 7/29/2025 3:45 PM, Sultan Alsawaf wrote:
> > > > > > > On Tue, Jul 29, 2025 at 12:42:16AM -0700, Sultan Alsawaf wrote:
> > > > > > > > On Tue, Jul 29, 2025 at 11:32:23AM +0800, Du, Bin wrote:
> > > > > > > > > Thanks Sultan, please see my comments
> > > > > > > > > 
> > > > > > > > > On 7/27/2025 6:31 AM, Sultan Alsawaf wrote:
> > > > > > > > > > On Fri, Jul 25, 2025 at 06:22:03PM +0800, Du, Bin wrote:
> > > > > > > > > > > > I have the Ryzen AI MAX+ 395 SKU of the HP ZBook Ultra G1a 14.
> > > > > > > > > > > > 
> > > > > > > > > > > > I cannot for the life of me get
> > > > > > > > > > > > the webcam working under Linux.
> > > > > > > > > > > > The webcam works
> > > > > > > > > > > > under Windows so it's not a hardware issue.
> > > > > > > > > > > > 
> > > > > > > > > > > > With this patchset and all of
> > > > > > > > > > > > the patches you link here
> > > > > > > > > > > > applied to 6.15, I get
> > > > > > > > > > > > the following errors:
> > > > > > > > > > > >        [   11.970038]
> > > > > > > > > > > > amd_isp_i2c_designware
> > > > > > > > > > > > amd_isp_i2c_designware: Unknown
> > > > > > > > > > > > Synopsys component type:
> > > > > > > > > > > > 0xffffffff
> > > > > > > > > > > >        [   11.973162]
> > > > > > > > > > > > amd_isp_i2c_designware
> > > > > > > > > > > > amd_isp_i2c_designware: error
> > > > > > > > > > > > -19: i2c_dw_probe failed
> > > > > > > > > > > > 
> > > > > > > > > > > > With the old ispkernel code from
> > > > > > > > > > > > February [1] applied on 6.15,
> > > > > > > > > > > > the webcam
> > > > > > > > > > > > indicator LED lights up but
> > > > > > > > > > > > there's no image. I see these
> > > > > > > > > > > > messages at boot:
> > > > > > > > > > > >        [    9.449005]
> > > > > > > > > > > > amd_isp_capture
> > > > > > > > > > > > amd_isp_capture.1.auto: amdgpu:
> > > > > > > > > > > > AMD ISP v4l2 device registered
> > > > > > > > > > > >        [    9.489005]
> > > > > > > > > > > > amd_isp_i2c_designware
> > > > > > > > > > > > amd_isp_i2c_designware.2.auto:
> > > > > > > > > > > > The OV05 sensor device is added
> > > > > > > > > > > > to the ISP I2C bus
> > > > > > > > > > > >        [    9.529012]
> > > > > > > > > > > > amd_isp_i2c_designware
> > > > > > > > > > > > amd_isp_i2c_designware.2.auto:
> > > > > > > > > > > > timeout while trying to abort
> > > > > > > > > > > > current transfer
> > > > > > > > > > > >        [    9.554046]
> > > > > > > > > > > > amd_isp_i2c_designware
> > > > > > > > > > > > amd_isp_i2c_designware.2.auto:
> > > > > > > > > > > > timeout in disabling adapter
> > > > > > > > > > > >        [    9.554174]
> > > > > > > > > > > > amd_isp_i2c_designware
> > > > > > > > > > > > amd_isp_i2c_designware.2.auto:
> > > > > > > > > > > > timeout while trying to abort
> > > > > > > > > > > > current transfer
> > > > > > > > > > > >        [    9.580022]
> > > > > > > > > > > > amd_isp_i2c_designware
> > > > > > > > > > > > amd_isp_i2c_designware.2.auto:
> > > > > > > > > > > > timeout in disabling adapter
> > > > > > > > > > > > 
> > > > > > > > > > > > And then the kernel crashes due
> > > > > > > > > > > > to the same use-after-free
> > > > > > > > > > > > issues I pointed out
> > > > > > > > > > > > in my other email [2].
> > > > > > > > > > > > 
> > > > > > > > > > > > Any idea what's going on?
> > > > > > > > > > > > 
> > > > > > > > > > > > [1]
> > > > > > > > > > > > https://github.com/amd/Linux_ISP_Kernel/commit/
> > > > > > > > > > > > c6d42584fbd0aa42cc91ecf16dc5c4f3dfea0bb4
> > > > > > > > > > > > [2] https://lore.kernel.org/r/aIEiJL83pOYO8lUJ@sultan-box
> > > > > > > > > > > Hi Sultan,
> > > > > > > > > > > 
> > > > > > > > > > > [1] is for kernel 6.8, believe it
> > > > > > > > > > > can't be applied to 6.15. We didn't
> > > > > > > > > > > verify
> > > > > > > > > > > on 6.15 but we are really glad to
> > > > > > > > > > > help, would you please provide some
> > > > > > > > > > > info,
> > > > > > > > > > > 1. Suppose you are using Ubuntu, right? What's the version?
> > > > > > > > > > > 2. 6.15, do you mean
> > > > > > > > > > > https://github.com/torvalds/linux/tree/
> > > > > > > > > > > v6.15 ?
> > > > > > > > > > > 
> > > > > > > > > > > After your confirmation, we'll see
> > > > > > > > > > > what we can do to enable your camera
> > > > > > > > > > > quickly and easily
> > > > > > > > > > > 
> > > > > > > > > > > Regards,
> > > > > > > > > > > Bin
> > > > > > > > > > 
> > > > > > > > > > Thank you, Bin!
> > > > > > > > > > 
> > > > > > > > > > 1. I'm using Arch Linux with the ISP4-patched libcamera [1].
> > > > > > > > > > 2. Yes, here is my kernel source [2].
> > > > > > > > > > 
> > > > > > > > > > I have some more findings:
> > > > > > > > > > 
> > > > > > > > > > Currently, the first blocking issue is
> > > > > > > > > > that the I2C adapter fails to
> > > > > > > > > > initialize.
> > > > > > > > > > This is because the ISP tile isn't powered on.
> > > > > > > > > > 
> > > > > > > > > > I noticed that in the old version of
> > > > > > > > > > amd_isp_i2c_designware [3], there were
> > > > > > > > > > calls to isp_power_set(), which is
> > > > > > > > > > available in the old ISP4 sources [4].
> > > > > > > > > > Without isp_power_set(), the I2C adapter
> > > > > > > > > > always fails to initialize for me.
> > > > > > > > > > 
> > > > > > > > > > How is the ISP tile supposed to get
> > > > > > > > > > powered on in the current ISP4 code?
> > > > > > > > > > 
> > > > > > > > > You are correct, yes, i believe the I2C
> > > > > > > > > adapter failure is caused by ISP not
> > > > > > > > > being powered up. Currently in latest code,
> > > > > > > > > isp_power_set is no longer
> > > > > > > > > available, instead, we implemented genPD for ISP in amdgpu
> > > > > > > > > https://lore.kernel.org/all/20250618221923.3944751-1-
> > > > > > > > > pratap.nirujogi@....com/
> > > > > > > > > Both amd_isp_i2c and amd_isp_capture are in
> > > > > > > > > the power domain and use the
> > > > > > > > > standard runtime PM API to do the power control
> > > > > > > > 
> > > > > > > > Thanks for that link, I found it along with
> > > > > > > > another patch on the list to make
> > > > > > > > the fwnode work ("drm/amd/amdgpu: Initialize
> > > > > > > > swnode for ISP MFD device").
> > > > > > > > 
> > > > > > > > > > Also, I noticed that the driver init
> > > > > > > > > > ordering matters between all of the
> > > > > > > > > > drivers
> > > > > > > > > > needed for the ISP4 camera. In
> > > > > > > > > > particular, amd_isp_i2c_designware and
> > > > > > > > > > amd_isp4
> > > > > > > > > > must be initialized before amd_capture,
> > > > > > > > > > otherwise amd_capture will fail to find
> > > > > > > > > > the fwnode properties for the OV05C10
> > > > > > > > > > device attached to the I2C bus.
> > > > > > > > > > 
> > > > > > > > > > But there is no driver init ordering
> > > > > > > > > > enforced, which also caused some issues
> > > > > > > > > > for
> > > > > > > > > > me until I figured it out. Maybe probe
> > > > > > > > > > deferral (-EPROBE_DEFER) should be used
> > > > > > > > > > to ensure each driver waits for its dependencies to init first?
> > > > > > > > > > 
> > > > > > > > > amd_isp_capture only has dependency on
> > > > > > > > > amd_isp4 which is the ACPI platform
> > > > > > > > > driver, it is init before amd_isp_catpure.
> > > > > > > > > Do you see in your side the amd_capture
> > > > > > > > > probe failure caused by failing to
> > > > > > > > > read fwnode properties? If that's the case
> > > > > > > > > please help to check if amd_isp4
> > > > > > > > > is loaded successfully
> > > > > > > > 
> > > > > > > > I got much further now: there aren't any driver
> > > > > > > > initialization errors, but when
> > > > > > > > I open the camera, there's no image. The camera
> > > > > > > > LED turns on so it's active.
> > > > > > > > 
> > > > > > > > And then shortly afterwards, amdgpu dies and the
> > > > > > > > entire system freezes.
> > > > > > > > 
> > > > > > > > I've attached my full dmesg, please let me know
> > > > > > > > what you think. Thanks!
> > > > > > > 
> > > > > > > I almost forgot, here is my current kernel tree:
> > > > > > > https://github.com/kerneltoast/kernel_x86_laptop/tree/v6.16-
> > > > > > > sultan-isp4
> > > > > > > 
> > > > > > > Sultan
> > > > > > 
> > > > > > Thanks Sultan, yes, seems much close to the final
> > > > > > success. Will have some
> > > > > > internal discussion.
> > > > > 
> > > > > I got the webcam working. The same bug happened when I tried
> > > > > Ubuntu's linux-oem
> > > > > kernel, which made me think that the issue was firmware.
> > > > > 
> > > > > And indeed, the culprit was a firmware update from February. I bisected
> > > > > linux-firmware and found the commit which broke the webcam for me:
> > > > > 
> > > > >     commit 1cc8c1bfa11251ce8bfcc97d1f15e312f7fe4df0 (HEAD)
> > > > >     Author: Pratap Nirujogi <pratap.nirujogi@....com>
> > > > >     Date:   Wed Feb 19 12:16:51 2025 -0500
> > > > > 
> > > > >         amdgpu: Update ISP FW for isp v4.1.1
> > > > > 
> > > > >         From internal git commit:
> > > > >         5058202443e08a673b6772ea6339efb50853be28
> > > > > 
> > > > >         Signed-off-by: Pratap Nirujogi <pratap.nirujogi@....com>
> > > > > 
> > > > >      amdgpu/isp_4_1_1.bin | Bin 4543184 -> 6083536 bytes
> > > > >      1 file changed, 0 insertions(+), 0 deletions(-)
> > > > > 
> > > > > Downgrading firmware to before that commit fixes the webcam.
> > > > > Any idea why?
> > > > > 
> > > > > Thanks,
> > > > > Sultan
> > > > 
> > > > So, can i say the working firmware binary is this one?
> > > > 
> > > > Commit 8f070131
> > > > amdgpu: Update ISP FW for isp v4.1.1
> > > > 
> > > >  From internal git commit:
> > > > 39b007366cc76ef8c65e3bc6220ccb213f4861fb
> > > > 
> > > > Signed-off-by: Pratap Nirujogi <pratap.nirujogi@....com>
> > > 
> > > Correct.
> > > 
> > > > There are too many changes between them, so i can't tell exactly which
> > > > change caused this. So, from my side
> > > > 1. Will try these two firmware to see if we have the same issue.
> > > > 2. It has been quite a long time since last release, will see if need to
> > > > release a latest one.
> > > 
> > > Thanks. It was a quick bisect for me, so I'm happy to help test if a
> > > bisect
> > > between those two internal git commits is needed.
> > > 
> > Really appreciate your test.
> > > In case it makes a difference, I have the laptop with the 2.8K OLED
> > > display. I'm
> > > aware there is one other display variant on other SKUs, which is a
> > > WUXGA IPS.
> > > 
> > Good to know, I believe it won't make any difference for ISP
> > 
> > > Also, with that old firmware, my camera only works with the old isp4
> > > driver from
> > > that Linux_ISP_Kernel repo (which is the same isp4 driver used in
> > > Ubuntu's
> > > linux-oem kernel). Does the new isp4 driver you've submitted here
> > > require newer
> > > firmware than the old driver located in Linux_ISP_Kernel?
> > > 
> > > Sultan
> > 
> > We had a try, yes, both of the old FW can't work on the new ISP4 driver,
> > as you know, for the last months, we did lots of driver modifications
> > for upstream and cause it incompatible with old FW.
> > Now, under internal discussion to upstream a new FW to support the new
> > ISP driver
> > 
> > Regards,
> > Bin
> > 
> > Hi Sultan,
> 
> This is the conclusion of your test,
> Driver: https://github.com/amd/Linux_ISP_Kernel/commit/c6d42584fbd0aa42cc91ecf16dc5c4f3dfea0bb4
> [1] It works on FW      8f070131(ext):39b00736(int)
> [2] It can't work on FW 1cc8c1bf(ext):50582024(int)

Correct.

> Would you please help to check if CONFIG_VIDEO_OV05C is defined not in the
> .config file when building the kernel? Our assumption is to make [1] work,
> CONFIG_VIDEO_OV05C shouldn't be defined. to make [2] work,
> CONFIG_VIDEO_OV05C should be defined.

Yes, it is enabled and I have only tested with it enabled:

    $ rg CONFIG_VIDEO_OV05C linux-oem-6.14/.config 
    CONFIG_VIDEO_OV05C=m

That's the Ubuntu linux-oem-6.14 kernel. You can get the full source and .config
I tested by running the following commands:

    git clone https://git.launchpad.net/ubuntu/+source/linux-oem-6.14 -b applied/6.14.0-1006.6
    cd linux-oem-6.14
    python debian/scripts/misc/annotations -e --arch amd64 > .config
    make olddefconfig

Let me know if that works.

Sultan

Powered by blists - more mailing lists