Re: [PATCH v1] i2c: designware: amdisp: Fix null pointer dereference in runtime resume

[Nirujogi, Pratap]

On 3/20/2026 3:59 AM, Andy Shevchenko wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Thu, Mar 19, 2026 at 05:09:55PM -0400, Nirujogi, Pratap wrote:
> > On 3/9/2026 9:29 PM, Mario Limonciello wrote:
> > > On 3/9/2026 5:00 PM, Pratap Nirujogi wrote:
>
> ...
>
> > > > Fixes: 02c057ddefef ("ACPI: video: Convert the driver to a platform one")
> > >
> > > Is this the right commit that introduced the race?  Did it change the
> > > timing?
> > >
> > > Or was the race always there and we just got lucky until that commit
> > > went in?
> > My apologies for mixing up this change along with other changes that were
> > needed to fix the AMD ISP driver regressions in v7.0.
> >
> > I investigated further and found that this issue was exposed by the below
> > commit.
> >
> > https://github.com/torvalds/linux/commit/38fa29b01a6a295aedb69d1bbdad70acd7d204c6
>
> Still it might be not a culprit as at that point the device should be in power
> on state.
>
> > However, the resume‑probe race condition existed even earlier and had not
> > surfaced because device initialization in resume is done only when init
> > handle is valid.
>
> That's what the initial cause, the AMD ISP driver initially relies on
> the broken (racy) behaviour. So, the original patch d6263c468a761 does not
> explain that bit. You need to go deeper and understand why the heck that
> condition is present there.
>
> > Here is some background and context on this change:
>
> Exactly, the below seems that you have nailed it down.
>
> > The amdisp i2c device requires ISP to be in power on state for probe to
> > succeed. To meet this requirement, we added this device, which is a amdgpu
> > MFD child to the genpd, to control isp power using runtime PM. The
> > pm_runtime_get_sync() call before probe triggers a PM resume, which powers
> > on the ISP and also invokes the amdisp i2c runtime resume before the probe
> > completes resulting in this race condition.
> >
> > To fix this issue properly, I’m considering this change. Please review and
> > share your feedback on whether this makes sense.
> >
> > - Call dev_pm_genpd_resume() to Power ON ISP before probe
> > - Call dev_pm_genpd_suspend() to Power OFF ISP after probe
> > - Call pm_runtime_enable() after probe is successful to take care of system
> > suspend/resume.
>
> I don't know the scope of dev_pm_genpd_*() usage, but the description of
> the proposed change sounds sane. And definitely it's much much better than
> previous attempt.
Thanks, Andy, for the feedback and comments. I’ll send out v2 with the 
proposed changes.

Thanks,
Pratap

> --
> With Best Regards,
> Andy Shevchenko