AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Du, Bin" <bin.du@amd.com>
To: "Nirujogi, Pratap" <pnirujog@amd.com>,
	"Nirujogi, Pratap" <Pratap.Nirujogi@amd.com>,
	"Limonciello, Mario" <Mario.Limonciello@amd.com>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
	"Deucher, Alexander" <Alexander.Deucher@amd.com>,
	"Koenig, Christian" <Christian.Koenig@amd.com>
Cc: "rafael.j.wysocki@intel.com" <rafael.j.wysocki@intel.com>,
	"Chan, Benjamin (Koon Pan)" <Benjamin.Chan@amd.com>,
	"Li, King" <King.Li@amd.com>
Subject: Re: [PATCH v1] drm/amdgpu: Fix ISP regression issue in kernel v7.0
Date: Wed, 18 Mar 2026 14:21:32 +0800	[thread overview]
Message-ID: <55186bf9-5204-497e-8bb9-05c74074d080@amd.com> (raw)
In-Reply-To: <6d411b13-880e-4f7d-ad23-1c94073bb0a3@amd.com>

Many thanks, Pratap, after verification, your patch resolves the AMD ISP 
driver auto-load issue.

Hi Rafael,

Would you please check if Pratap's patch is acceptable since it does 
some modification to your merged patch originally from 
https://lore.kernel.org/all/5081593.31r3eYUQgx@rafael.j.wysocki/


On 3/18/2026 12:03 PM, Nirujogi, Pratap wrote:
> 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
> 

-- 
Regards,
Bin


  reply	other threads:[~2026-03-18  6:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 21:50 [PATCH v1] drm/amdgpu: Fix ISP regression issue in kernel v7.0 Pratap Nirujogi
2026-03-09 21:58 ` Mario Limonciello
2026-03-09 22:08   ` Nirujogi, Pratap
2026-03-10 22:52   ` Nirujogi, Pratap
2026-03-11  5:26     ` Mario Limonciello
2026-03-11 15:29       ` Nirujogi, Pratap
2026-03-13 11:49         ` Du, Bin
2026-03-18  4:03           ` Nirujogi, Pratap
2026-03-18  6:21             ` Du, Bin [this message]
2026-03-18  3:59         ` Nirujogi, Pratap
2026-03-09 22:05 ` Alex Deucher
2026-03-09 22:18   ` Nirujogi, Pratap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55186bf9-5204-497e-8bb9-05c74074d080@amd.com \
    --to=bin.du@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Benjamin.Chan@amd.com \
    --cc=Christian.Koenig@amd.com \
    --cc=King.Li@amd.com \
    --cc=Mario.Limonciello@amd.com \
    --cc=Pratap.Nirujogi@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=pnirujog@amd.com \
    --cc=rafael.j.wysocki@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox