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

Daniel Vetter
2024-08-28T14:00:55Z

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[] = {