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

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko