Re: [Intel-gfx] [RESEND] drm/edid/firmware: stop using a throwaway platform device

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

On Wed, 16 Nov 2022, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 14.11.22 um 12:17 schrieb Jani Nikula:
>> We've used a temporary platform device for firmware EDID loading since
>> it was introduced in commit da0df92b5731 ("drm: allow loading an EDID as
>> firmware to override broken monitor"), but there's no explanation why.
>> 
>> Using a temporary device does not play well with CONFIG_FW_CACHE=y,
>> which caches firmware images (e.g. on suspend) so that drivers can
>> request firmware when the system is not ready for it, and return the
>> images from the cache (e.g. during resume). This works automatically for
>> regular devices, but obviously not for a temporarily created device.
>> 
>> Stop using the throwaway platform device, and use the drm device
>> instead.
>> 
>> Note that this may still be problematic for cases where the display was
>> plugged in during suspend, and the firmware wasn't loaded and therefore
>> not cached before suspend.
>> 
>> References: https://lore.kernel.org/r/20220727074152.43059-1-matthieu.charette@gmail.com
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2061
>> Reported-by: Matthieu CHARETTE <matthieu.charette@gmail.com>
>> Tested-by: Matthieu CHARETTE <matthieu.charette@gmail.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> I looked through request_firmware() but did not see any signs that it 
> somehow depends on a platform device. I assume that this might only 
> affect the device name in the error message.

Thanks, pushed to drm-misc-next.

Matthieu, thanks for you patience and the report as well!

BR,
Jani.


>
> Best regards
> Thomas
>
>> 
>> ---
>> 
>> Resend with a proper commit message; patch itself is unchanged.
>> ---
>>   drivers/gpu/drm/drm_edid_load.c | 13 +------------
>>   1 file changed, 1 insertion(+), 12 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
>> index ef4ab59d6935..5d9ef267ebb3 100644
>> --- a/drivers/gpu/drm/drm_edid_load.c
>> +++ b/drivers/gpu/drm/drm_edid_load.c
>> @@ -172,20 +172,9 @@ static const struct drm_edid *edid_load(struct drm_connector *connector, const c
>>   		fwdata = generic_edid[builtin];
>>   		fwsize = sizeof(generic_edid[builtin]);
>>   	} else {
>> -		struct platform_device *pdev;
>>   		int err;
>>   
>> -		pdev = platform_device_register_simple(connector->name, -1, NULL, 0);
>> -		if (IS_ERR(pdev)) {
>> -			drm_err(connector->dev,
>> -				"[CONNECTOR:%d:%s] Failed to register EDID firmware platform device for connector \"%s\"\n",
>> -				connector->base.id, connector->name,
>> -				connector->name);
>> -			return ERR_CAST(pdev);
>> -		}
>> -
>> -		err = request_firmware(&fw, name, &pdev->dev);
>> -		platform_device_unregister(pdev);
>> +		err = request_firmware(&fw, name, connector->dev->dev);
>>   		if (err) {
>>   			drm_err(connector->dev,
>>   				"[CONNECTOR:%d:%s] Requesting EDID firmware \"%s\" failed (err=%d)\n",

-- 
Jani Nikula, Intel Open Source Graphics Center