Re: [PATCH v1] drm/amdgpu: Fix ISP regression issue in kernel v7.0

["Nirujogi, Pratap" <pnirujog@amd.com>]

On 3/11/2026 1:26 AM, Mario Limonciello wrote:
> 
> 
> On 3/10/2026 5:52 PM, Nirujogi, Pratap wrote:
>>
>>
>> On 3/9/2026 5:58 PM, Mario Limonciello wrote:
>>>
>>>
>>> On 3/9/2026 4:50 PM, Pratap Nirujogi wrote:
>>>> Add NULL pointer checks for dev->type before accessing
>>>> dev->type->name in ISP genpd add/remove functions to
>>>> prevent kernel crashes. Also add MODULE_SOFTDEP to ensure
>>>> ISP driver dependencies are loaded in correct order.
>>>>
>>>> The regression was introduced in kernel v7.0 where MFD ISP
>>>> device enumeration doesn't complete by the time it is added
>>>> to gendp. The timing of ISP device enumeration has changed
>>>> because of the changes in registering the device sources in
>>>> the device hierarchy.
>>>
>>> It's a little bit pedantic; but I /think/ there are two different 
>>> problems here with two different root causes that both happened in 
>>> 7.0-rc.
>>>
>>> As a consequence I think you should have this split out as two 
>>> separate patches in a series linked to a Fixes tag with the reason 
>>> for each of them.
>>>
>> Hi Mario,
>>
>> Few things to clarify before I split into 2 patches and submit v2.
>>
>> - Yes, you are correct, the changes in this patch are not directly 
>> related to the Fixes tag 02c057ddefef mentioned. But since it has the 
>> dependency on the below patch for automatic modprobe of isp to work, I 
>> have used the same tag to cover the dependency. My apologies if this 
>> approach is incorrect and misleading.
>>
>> https://lore.kernel.org/all/5986516.DvuYhMxLoT@rafael.j.wysocki/
>>
>> - NULL dereferencing issue with dev->type observed on v7.0 is specific 
>> to this commit 057edc58aa5926d63840c7f30afe0953d3994fa3. As the wakeup 
>> sources are registered using physical device instead of ACPI device, 
>> wakeup source device (wakeup14) is added as the first child of AMDGPU 
>> device; and since its dev->type is not initialized properly it has 
>> resulted in segfault.
> 
> Sure this makes sense and is a no brainer to get in.  I would just send
> this one right now and we can keep noodling on MODULE_SOFTDEP (more
> comments below).
> 
thanks, I’ll go ahead and send this one out now and will submit the 
MODULE_SOFTDEP change separately after the root cause is identified ( 
more details below ).

>>
>> In 6.19-rc4 or earlier versions, this issue was not observed as the 
>> wakeup source device was never part of AMDGPU children list.
>>
>> For the changes in isp_v4_1_1.c, I will use Fixes tag 057edc58aa59 in v2.
>>
>> - MODULE_SOFTDEP change in amdgpu_drv.c is needed for automatic 
>> modprobe of isp (and other amdgpu mfd child devices) to work in v7.0. 
>> But couldn't identify the specific commit in v7.0 that is causing the 
>> issue. I can confirm it is not because of commit 057edc58aa59 as the 
>> automatic modprobe doesn't work even on reverting this commit. Can I 
>> submit this as the fix needed for isp probe to work in v7.0 without 
>> the fixes tag?
>>
> 
> MODULE_SOFTDEP is generally for ordering, but I don't think you have an
> issue with those modules loading before amdgpu do you?  I'm not really
> following why the modaliases stopped working and I'm a bit worried that
> it's papering over a more nuanced issue still.
> 
yes, this explicit load‑order dependency was not required earlier. I 
agree that the regression point is not clearly identified. I will 
identify the bisected commit and submit the patch later.

> To identify the root cause, it might be helpful to do a bisect, although
> it's a bit complicated.
> 1) At any point that has that platform driver conversion you need to
> either revert or add the change to auxillary
> 2) At any point that has 057edc58aa5926d63840c7f30afe0953d3994fa3 you
> need to apply the NULL pointer derf fix
> 3) At any point that has the changes in linux-media for API adjustments
> in 7.0 you'll need to pick what version of ISP series to apply.
> 
I tried #1, #2 and few other combinations, but none of them helped. I 
will investigate further to identify the commit causing the issue.
> 
>> Thanks,
>> Pratap
>>
>>>>
>>>> Co-developed-by: Bin Du <Bin.Du@amd.com>
>>>> Fixes: 02c057ddefef ("ACPI: video: Convert the driver to a platform 
>>>> one")
>>>> Signed-off-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
>>>>   drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c | 4 ++--
>>>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/ 
>>>> drm/amd/amdgpu/amdgpu_drv.c
>>>> index 95d26f086d545..920595f0d22ca 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> @@ -3212,3 +3212,4 @@ module_exit(amdgpu_exit);
>>>>   MODULE_AUTHOR(DRIVER_AUTHOR);
>>>>   MODULE_DESCRIPTION(DRIVER_DESC);
>>>>   MODULE_LICENSE("GPL and additional rights");
>>>> +MODULE_SOFTDEP("post: amd_isp4_capture i2c-designware-amdisp 
>>>> pinctrl- amdisp");
>>>> \ No newline at end of file
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c b/drivers/gpu/ 
>>>> drm/amd/amdgpu/isp_v4_1_1.c
>>>> index b3590b33cab9e..485ecdec96184 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c
>>>> @@ -129,7 +129,7 @@ static int isp_genpd_add_device(struct device 
>>>> *dev, void *data)
>>>>       if (!pdev)
>>>>           return -EINVAL;
>>>> -    if (!dev->type->name) {
>>>> +    if (!dev->type || !dev->type->name) {
>>>>           drm_dbg(&adev->ddev, "Invalid device type to add\n");
>>>>           goto exit;
>>>>       }
>>>> @@ -165,7 +165,7 @@ static int isp_genpd_remove_device(struct device 
>>>> *dev, void *data)
>>>>       if (!pdev)
>>>>           return -EINVAL;
>>>> -    if (!dev->type->name) {
>>>> +    if (!dev->type || !dev->type->name) {
>>>>           drm_dbg(&adev->ddev, "Invalid device type to remove\n");
>>>>           goto exit;
>>>>       }
>>>
>>>
>>
>