On Wed, 16 Nov 2022, Thomas Zimmermann <tzimmermann@xxxxxxx> 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@xxxxxxxxx
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2061
>> Reported-by: Matthieu CHARETTE <matthieu.charette@xxxxxxxxx>
>> Tested-by: Matthieu CHARETTE <matthieu.charette@xxxxxxxxx>
>> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>
> Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>
> 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
