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

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

Hi Bin,

Could you please review the below patch that was submitted to address 
the automatic modprobe issue.

https://lore.kernel.org/all/20260318034842.1216536-1-pratap.nirujogi@amd.com/

Thanks,
Pratap

On 3/13/2026 7:49 AM, Du, Bin wrote:
> Hi Pratap,
> 
> FYI, the following patch may resolve the AMD ISP driver not auto-loading 
> issue (without using MODULE_SOFTDEP). Please double check.
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/ 
> amd/amdgpu/amdgpu_drv.c
> index 920595f0d22c..95d26f086d54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -3212,4 +3212,3 @@ 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 485ecdec9618..409c1ca5a5e1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/isp_v4_1_1.c
> @@ -118,6 +118,29 @@ static int isp_set_performance_state(struct 
> generic_pm_domain *genpd,
>          return 0;
>   }
> 
> +/*
> + * MFD core sets the parent GPU's ACPI companion (LNXVIDEO) on each
> + * child, giving them modalias "acpi:LNXVIDEO:" instead of
> + * "platform:<name>".  Clear it and retrigger the uevent so udev
> + * sees the correct platform modalias for module autoloading.
> + */
> +static int isp_clear_acpi_fwnode(struct device *dev, void *data)
> +{
> +       struct platform_device *pdev;
> +
> +       if (!dev->type || !dev->type->name ||
> +           strcmp(dev->type->name, "mfd_device"))
> +               return 0;
> +
> +       pdev = to_platform_device(dev);
> +       if (pdev->mfd_cell) {
> +               device_set_node(dev, NULL);
> +               kobject_uevent(&dev->kobj, KOBJ_ADD);
> +       }
> +
> +       return 0;
> +}
> +
>   static int isp_genpd_add_device(struct device *dev, void *data)
>   {
>          struct generic_pm_domain *gpd = data;
> @@ -375,6 +398,13 @@ static int isp_v4_1_1_hw_init(struct amdgpu_isp *isp)
>                  goto failure;
>          }
> 
> +       /*
> +        * MFD core sets the parent GPU's ACPI companion (LNXVIDEO) on
> +        * each child, giving them modalias "acpi:LNXVIDEO:" instead of
> +        * "platform:<name>". Clear it so udev matches by platform alias.
> +        */
> +       device_for_each_child(isp->parent, NULL, isp_clear_acpi_fwnode);
> +
>          return 0;
> 
>   failure:
> 
>> -----Original Message-----
>> From: Nirujogi, Pratap <Pratap.Nirujogi@amd.com>
>> Sent: Wednesday, March 11, 2026 11:29 PM
>> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Nirujogi, Pratap
>> <Pratap.Nirujogi@amd.com>; amd-gfx@lists.freedesktop.org; Deucher,
>> Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
>> <Mario.Limonciello@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>
>> Cc: rafael.j.wysocki@intel.com; Chan, Benjamin (Koon Pan)
>> <Benjamin.Chan@amd.com>; Du, Bin <Bin.Du@amd.com>; Li, King
>> <King.Li@amd.com>
>> Subject: Re: [PATCH v1] drm/amdgpu: Fix ISP regression issue in kernel 
>> v7.0
>>
>>
>>
>> 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;
>>>>>>       }
>>>>>
>>>>>
>>>>
>>>
> 
> Regards,
> Bin