Re: [PATCH 5/6] drm/ipuv3/parallel: convert to struct drm_edid

[Daniel Vetter <daniel.vetter@ffwll.ch>]

On Thu, Aug 22, 2024 at 08:42:51PM +0300, Jani Nikula wrote:
> Prefer the struct drm_edid based functions for storing the EDID and
> updating the connector.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/gpu/drm/imx/ipuv3/parallel-display.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3/parallel-display.c b/drivers/gpu/drm/imx/ipuv3/parallel-display.c
> index 55dedd73f528..91d7808a2d8d 100644
> --- a/drivers/gpu/drm/imx/ipuv3/parallel-display.c
> +++ b/drivers/gpu/drm/imx/ipuv3/parallel-display.c
> @@ -34,7 +34,7 @@ struct imx_parallel_display_encoder {
>  
>  struct imx_parallel_display {
>  	struct device *dev;
> -	void *edid;
> +	const struct drm_edid *drm_edid;
>  	u32 bus_format;
>  	u32 bus_flags;
>  	struct drm_display_mode mode;
> @@ -62,9 +62,9 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
>  	if (num_modes > 0)
>  		return num_modes;
>  
> -	if (imxpd->edid) {
> -		drm_connector_update_edid_property(connector, imxpd->edid);
> -		num_modes = drm_add_edid_modes(connector, imxpd->edid);
> +	if (imxpd->drm_edid) {
> +		drm_edid_connector_update(connector, imxpd->drm_edid);
> +		num_modes = drm_edid_connector_add_modes(connector);
>  	}
>  
>  	if (np) {
> @@ -331,7 +331,7 @@ static int imx_pd_probe(struct platform_device *pdev)
>  
>  	edidp = of_get_property(np, "edid", &edid_len);
>  	if (edidp)
> -		imxpd->edid = devm_kmemdup(dev, edidp, edid_len, GFP_KERNEL);
> +		imxpd->drm_edid = drm_edid_alloc(edidp, edid_len);

Randomly wondering whether drm_edid_alloc should at least sanity check the
edid size ... but for another time. On this:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  
>  	ret = of_property_read_string(np, "interface-pix-fmt", &fmt);
>  	if (!ret) {
> @@ -355,7 +355,11 @@ static int imx_pd_probe(struct platform_device *pdev)
>  
>  static void imx_pd_remove(struct platform_device *pdev)
>  {
> +	struct imx_parallel_display *imxpd = platform_get_drvdata(pdev);
> +
>  	component_del(&pdev->dev, &imx_pd_ops);
> +
> +	drm_edid_free(imxpd->drm_edid);
>  }
>  
>  static const struct of_device_id imx_pd_dt_ids[] = {
> -- 
> 2.39.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch