[PATCH v2] drm: of: Add drm_of_lvds_get_data_mapping

[Sam Ravnborg]

Hi Marek,

On Mon, Oct 11, 2021 at 01:21:33PM +0200, Marek Vasut wrote:
> Add helper function to convert DT "data-mapping" property string value
> into media bus format value, and deduplicate the code in panel-lvds.c
> and lvds-codec.c .
> 
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Sam Ravnborg <sam at ravnborg.org>
> To: dri-devel at lists.freedesktop.org
> ---
> V2: Drop bogus semicolon
> ---
>  drivers/gpu/drm/bridge/lvds-codec.c | 21 ++++++------------
>  drivers/gpu/drm/drm_of.c            | 33 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/panel/panel-lvds.c  | 18 ++++------------
>  include/drm/drm_of.h                |  7 ++++++
>  4 files changed, 51 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c
> index ad460b96c0a3..f991842a161f 100644
> --- a/drivers/gpu/drm/bridge/lvds-codec.c
> +++ b/drivers/gpu/drm/bridge/lvds-codec.c
> @@ -14,6 +14,7 @@
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_bridge.h>
> +#include <drm/drm_of.h>
>  #include <drm/drm_panel.h>
>  
>  struct lvds_codec {
> @@ -118,7 +119,6 @@ static int lvds_codec_probe(struct platform_device *pdev)
>  	struct device_node *bus_node;
>  	struct drm_panel *panel;
>  	struct lvds_codec *lvds_codec;
> -	const char *mapping;
>  	int ret;
>  
>  	lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL);
> @@ -174,22 +174,15 @@ static int lvds_codec_probe(struct platform_device *pdev)
>  			return -ENXIO;
>  		}
>  
> -		ret = of_property_read_string(bus_node, "data-mapping",
> -					      &mapping);
> +		ret = drm_of_lvds_get_data_mapping(bus_node);
>  		of_node_put(bus_node);
> -		if (ret < 0) {
> +		if (ret == -ENODEV) {
>  			dev_warn(dev, "missing 'data-mapping' DT property\n");
> +		} else if (ret) {
> +			dev_err(dev, "invalid 'data-mapping' DT property\n");
> +			return ret;
>  		} else {
> -			if (!strcmp(mapping, "jeida-18")) {
> -				lvds_codec->bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG;
> -			} else if (!strcmp(mapping, "jeida-24")) {
> -				lvds_codec->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA;
> -			} else if (!strcmp(mapping, "vesa-24")) {
> -				lvds_codec->bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG;
> -			} else {
> -				dev_err(dev, "invalid 'data-mapping' DT property\n");
> -				return -EINVAL;
> -			}
> +			lvds_codec->bus_format = ret;
>  			lvds_codec->bridge.funcs = &funcs_decoder;
>  		}
>  	}
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index 37c34146eea8..a9217bc18e8f 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -402,3 +402,36 @@ int drm_of_lvds_get_dual_link_pixel_order(const struct device_node *port1,
>  		DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS;
>  }
>  EXPORT_SYMBOL_GPL(drm_of_lvds_get_dual_link_pixel_order);
> +
> +/**
> + * drm_of_lvds_get_data_mapping - Get LVDS data mapping
> + * @port: DT port node of the LVDS source or sink
> + *
> + * Convert DT "data-mapping" property string value into media bus format value.
> + *
> + * Return:
> + * MEDIA_BUS_FMT_RGB666_1X7X3_SPWG - data-mapping is "jeida-18"
> + * MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA - data-mapping is "jeida-24"
> + * MEDIA_BUS_FMT_RGB888_1X7X4_SPWG - data-mapping is "vesa-24"
> + * -EINVAL - the "data-mapping" property is unsupported
> + * -ENODEV - the "data-mapping" property is missing
> + */
I tried to look at the output of this - it looks bad.

See the chapter "Return values" here: https://docs.kernel.org/doc-guide/kernel-doc.html
Sorry for not catching this during the first review.

	Sam