Re: [PATCH] drm/edid/firmware: stop using throwaway platform device

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 11 Oct 2022, Matthieu CHARETTE <matthieu.charette@gmail.com> wrote:
> Currently the EDID is requested during the resume. But since it's
> requested too early, this means before the filesystem is mounted, the
> firmware request fails. This make the DRM driver crash when resuming.
> This kind of issue should be prevented by the firmware caching process
> which cache every firmware requested for the next resume. But since we
> are using a temporary device, the firmware isn't cached on suspend
> since the device doesn't work anymore.
> When using a non temporary device to get the EDID, the firmware will
> be cached on suspend for the next resume. So requesting the firmware
> during resume will succeed.
> But if the firmware has never been requested since the boot, this
> means that the monitor isn't plugged since the boot. The kernel will
> not be caching the EDID. So if we plug the monitor while the machine
> is suspended. The resume will fail to load the firmware. And the DRM
> driver will crash.
> So basically, your fix should solve the issue except for the case
> where the monitor hasn't been plugged since boot and is plugged while
> the machine is suspended.
> I hope I was clear. Tell me if I wasn't. I'm not really good at explaining.

That was a pretty good explanation. The only thing I'm missing is what
the failure mode is exactly when you claim the driver will crash. Why
would request_firmware() "crash" if called for the first time on the
resume path?

I'm not sure I care much about not being able to load the firmware EDID
in the suspend-plug-resume case (as this can be remedied with a
subsequent modeset), but obviously any errors need to be handled
gracefully, without crashing.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center