Message-ID: <ddaf56e2-f065-4cfb-8a06-28479065713c@amd.com>
Date: Thu, 15 Jan 2026 14:47:10 +0800
From: "Du, Bin" <bin.du@....com>
To: Kate Hsuan <hpa@...hat.com>, "Nirujogi, Pratap" <pnirujog@....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,
 sultan@...neltoast.com, 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 v7 0/7] Add AMD ISP4 driver



On 1/14/2026 4:59 PM, Kate Hsuan wrote:
> Hi Pratap,
> 
> On Wed, Jan 14, 2026 at 1:14 AM Nirujogi, Pratap <pnirujog@....com> wrote:
>>
>> Hi Kate,
>>
>> On 1/13/2026 9:11 AM, Kate Hsuan wrote:
>>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

[snip]

>>>> --
>>>> Regards,
>>>> Bin
>>>>
>>> The ISP driver works perfectly with a clear and sharp video. I tested
>>> it again, and I found a suspend/resume issue.
>>> The ISP can't suspend when the system is set to suspend. The privacy
>>> LED is on when the system is suspended. Therefore, the user will see a
>>> luminous privacy LED when the system is set to suspend.
>>> Today, I made a work to move all the power control to use the runtime
>>> PM, including suspend/resume. This work may be humble and may break
>>> the finite state machine but it works. The major changes of it
>>> include:
>>> 1. Support suspend/resume.
>>> 2. The power is managed by the runtime PM so the s_power and the related
>>>      callback function were dropped.
>>> 3. The enable_isp GPIO pin is controlled by the runtime PM.
>>> 4. pm_runtime_get_noresume() is used to get the runtime PM at probe()
>>>      since the device doesn't have to be set to power on when initialising.
>>>
>>> This work stops the video stream on suspend and starts the stream on
>>> resume so the privacy LED is turned on and turned off with the changes
>>> of suspend and resume.
>>>
>>> Could you please consider this patch and idea?
>>>
>>> Thank you :)
>>
>> Thanks for reporting this issue and also providing the patch.
>>
>> We have addressed this issue recently. I suspect the below fix in AMDGPU
>> available in v6.19-rc5 is missing in your build.
>>
>> https://github.com/torvalds/linux/commit/7ed51e3a1381422278933d0d3ebda0268b6825de
>>
>> I have tested locally and this issue is not observed with this change
>> included. Can you please check and feedback if this solves the problem?
>>
>> This change takes care of handling isp suspend-resume as part of amdgpu
>> device suspend-resume instead of genpd, and uses the pm rumtime as you
>> have suggested.
>>
>> Thanks,
>>
>> Pratap
>>
>>
>>>
>>> --
>>> BR,
>>> Kate
>>
> 
> I tested again with the patch you mentioned. It works but I found some issues.
> I also tested it with my work and the test results were shown as follows.
> 
> The test step is
> 1. start the camera app -> 2. set to suspend -> 3. resume the laptop
> 
> The results:
> 1. rebase to 6.19-RC5 with the fix patch (7ed51e3a1)
> It works but I found the logs when stopping the video stream
> ...
> [  527.733851] amd_isp_capture amd_isp_capture: fail to disable stream
> ...
> [  528.237828] amd_isp_capture amd_isp_capture: fail to stop steam
> ...
> The isp4 tries to stop the stream but it fails to write the data to
> the firmware.
> I think the ISP stops working before the ISP stops the video stream.
> So, it cannot write the data to the firmware.
> 
> 2.  fix patch (7ed51e3a1) + ISP4 runtime PM (my work)
> This also works.
> The ISP4 complains the error about "static void
> isp4if_dealloc_fw_gpumem(struct isp4_interface *ispif)" when suspend.
> 
> (This only happens when suspending the system)
> [  223.085419] WARNING: drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:517
> at amdgpu_bo_free_kernel+0xe9/0x100 [amdgpu], CPU#30:
> kworker/u130:4/794
> [  223.085552] Modules linked in: uinput(E) rfcomm(E) snd_seq_dummy(E)
> snd_hrtimer(E) hid_sensor_gyro_3d(E) hid_sensor_trigger(E)
> hid_sensor_iio_common(E) industrialio_triggered_buffer(E) kfifo_buf(E)
> industrialio(E) hid_sensor_hub(E) sunrpc(E) nf_conntrack_netbios_ns(E)
> nf_conntrack_broadcast(E) nft_fib_inet(E) nft_fib_ipv4(E)
> 
> <snip>
> 
> [  223.085591] CPU: 30 UID: 0 PID: 794 Comm: kworker/u130:4 Tainted: G
>         W   E       6.19.0-rc5+ #92 PREEMPT(lazy)
> [  223.085592] Tainted: [W]=WARN, [E]=UNSIGNED_MODULE
> [  223.085592] Hardware name: HP HP ZBook Ultra G1a 14 inch Mobile
> Workstation PC/8D01, BIOS X89 Ver. 01.01.00 01/16/2025
> [  223.085593] Workqueue: async async_run_entry_fn
> [  223.085594] RIP: 0010:amdgpu_bo_free_kernel+0xe9/0x100 [amdgpu]
> [  223.085725] Code: f4 ff ff 4d 85 e4 74 08 49 c7 04 24 00 00 00 00
> 48 85 ed 74 08 48 c7 45 00 00 00 00 00 5b 5d 41 5c 41 5d 41 5e c3 cc
> cc cc cc <0f> 0b e9 42 ff ff ff 3d 00 fe ff ff 0f 85 a1 34 95 00 eb bd
> 0f 1f
> [  223.085725] RSP: 0018:ffffcd80413e7ae0 EFLAGS: 00010202
> [  223.085726] RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000080800077
> [  223.085726] RDX: ffff8b3719f6eef0 RSI: ffff8b3719f6eee8 RDI: ffff8b3719f6eef8
> [  223.085727] RBP: ffff8b3719f6eee0 R08: 0000000000000000 R09: ffffffffc0558135
> [  223.085727] R10: ffff8b3719f6e7a0 R11: ffffef494467db80 R12: ffff8b370ef86220
> [  223.085727] R13: ffff8b3718003c00 R14: ffff8b372a60ee00 R15: ffff8b370ef86220
> [  223.085728] FS:  0000000000000000(0000) GS:ffff8b5649311000(0000)
> knlGS:0000000000000000
> [  223.085728] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  223.085729] CR2: 0000563d26ad9000 CR3: 00000001ace2c000 CR4: 0000000000f50ef0
> [  223.085730] PKRU: 55555554
> [  223.085730] Call Trace:
> [  223.085730]  <TASK>
> [  223.085731]  isp4if_dealloc_fw_gpumem+0xcd/0x100 [amd_capture]
> [  223.085733]  isp4if_stop+0x59/0x70 [amd_capture]
> [  223.085735]  isp4sd_pwroff_and_deinit.isra.0+0x99/0x160 [amd_capture]
> [  223.085737]  isp4sd_stop_stream+0xbe/0x250 [amd_capture]
> [  223.085740]  v4l2_subdev_disable_streams+0x1ad/0x390 [videodev]
> [  223.085747]  ? dc_dmub_srv_wait_for_idle+0x50/0x150 [amdgpu]
> [  223.085914]  isp4_suspend+0x2d/0x80 [amd_capture]
> [  223.085916]  genpd_runtime_suspend+0xe7/0x300
> [  223.085917]  ? __pfx_isp_suspend_device+0x10/0x10 [amdgpu]
> [  223.086069]  pm_runtime_force_suspend+0x71/0x110
> [  223.086070]  ? __pfx_genpd_runtime_suspend+0x10/0x10
> [  223.086071]  device_for_each_child+0x71/0xb0
> [  223.086073]  isp_v4_1_1_hw_suspend+0x22/0x40 [amdgpu]
> [  223.086223]  ? amdgpu_dpm_gfx_state_change+0x49/0x60 [amdgpu]
> [  223.086399]  amdgpu_ip_block_suspend+0x27/0x50 [amdgpu]
> [  223.086522]  amdgpu_device_ip_suspend_phase2+0x13c/0x3d0 [amdgpu]
> [  223.086647]  amdgpu_device_suspend+0x161/0x240 [amdgpu]
> 
> 
> Both solutions work but need to be integrated.
> ISP4 already enables the runtime PM so the power settings can be moved
> to the runtime PM and the s_power callback can be dropped. The runtime
> PM can be used to power on and power off the ISP4 for the typical
> operations, for example, the user turns on the camera and shutdowns
> the camera. If the ISP4 media device manages the power status using
> the runtime PM, the video stream can be gracefully stopped to ensure
> no error events are propagated to the user apps. Moreover, it resolves
> the suspend/resume issues and the runtime PM will manage the
> suspend/resume status.
> 

Thank you, Kate, for verifying and debugging. Yes, s_power is now 
obsolete and should be replaced by runtime PM which is also suggested by 
Sakari in one of his comments. In our initial version, support for 
camera streaming suspend and resume wasn't included, so if you suspend 
the laptop while the camera is running, it won't continue to run after 
resuming. However, it's easy to fix by simply reopening the camera app. 
Therefore, the stream disabling or stopping failure after resume is 
expected and harmless. On the other hand, the isp4if_dealloc_fw_gpumem 
error was unexpected, as we hadn't encountered it before. Once we've 
switched from s_power to runtime PM, we'll test again on 6.19-RC5 as you.

-- 
Regards,
Bin

Powered by blists - more mailing lists