Re: [PATCH 1/6 v2] drm/omap: add new connector types

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

On Tuesday 09 May 2017 10:16:27 Tomi Valkeinen wrote:
> We have been using DRM_MODE_CONNECTOR_Unknown for many of our outputs
> because there has not been a proper connector type for them.
> 
> We now have connector type for DPI so let's take it into use. At the
> same time, add better connector types for the remaining outputs too.
> 
> This patch sets the following outputs to use the following connector
> types:
> 
> DPI -> DPI
> DBI -> DPI (MIPI DBI is very similar to DPI at the bus level)
> SDI -> LVDS (SDI, TI Flatlink 3G, is a type of LVDS)
> VENC -> SVIDEO (it could also be composite, but we don't have that
>         information here, so svideo should be quite good match)
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index e1f47f0b3ccf..16c537837742
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -214,6 +214,19 @@ static int get_connector_type(struct omap_dss_device
> *dssdev) return DRM_MODE_CONNECTOR_DVID;
>  	case OMAP_DISPLAY_TYPE_DSI:
>  		return DRM_MODE_CONNECTOR_DSI;
> +	case OMAP_DISPLAY_TYPE_DPI:
> +	case OMAP_DISPLAY_TYPE_DBI:
> +		return DRM_MODE_CONNECTOR_DPI;
> +	case OMAP_DISPLAY_TYPE_VENC:
> +		if (of_device_is_compatible(dssdev->dev->of_node,
> +		    "omapdss,svideo-connector"))
> +			return DRM_MODE_CONNECTOR_SVIDEO;
> +		if (of_device_is_compatible(dssdev->dev->of_node,
> +		    "omapdss,composite-video-connector"))
> +			return DRM_MODE_CONNECTOR_Composite;

Checking the compat string here feels like a bit of a hack to me. Wouldn't it 
be simpler and cleaner to add the connector type to the omap_dss_device 
structure ? That's more work though, so as a first step I think I could 
tolerate this hack if you really feel lazy ;-) Although, maybe we should just 
return SVIDEO unconditionally for VENC for now and fix that later.

> +		return DRM_MODE_CONNECTOR_Unknown;
> +	case OMAP_DISPLAY_TYPE_SDI:
> +		return DRM_MODE_CONNECTOR_LVDS;
>  	default:
>  		return DRM_MODE_CONNECTOR_Unknown;
>  	}

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel