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

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3065 bytes --]

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.

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",

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]