diff mbox

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

Message ID 1493288874-26654-2-git-send-email-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen April 27, 2017, 10:27 a.m. UTC
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 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 | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Laurent Pinchart May 8, 2017, 2:21 p.m. UTC | #1
Hi Tomi,

Thank you for the patch.

On Thursday 27 Apr 2017 13:27:49 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)

This looks OK to me.

> SDI -> LVDS (SDI is a type of LVDS)

If we're talking about this 
https://en.wikipedia.org/wiki/Serial_digital_interface SDI, it's not a type of 
LVDS at all. DRM_MODE_CONNECTOR_LVDS is interpreted as meaning that an LVDS 
display panel is attached to the device, and is likely not removable. I don't 
think it's a good match for SDI. We might need a new connector type.

> VENC -> SVIDEO (it could also be composite, but we don't have that
>         information here, so svideo should be quite good match)

Do we have the information anywhere ?

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
> b/drivers/gpu/drm/omapdrm/omap_drv.c index e1f47f0b3ccf..58c639e1e8e9
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -214,6 +214,13 @@ 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:
> +		return DRM_MODE_CONNECTOR_SVIDEO;
> +	case OMAP_DISPLAY_TYPE_SDI:
> +		return DRM_MODE_CONNECTOR_LVDS;
>  	default:
>  		return DRM_MODE_CONNECTOR_Unknown;
>  	}
Tomi Valkeinen May 8, 2017, 2:35 p.m. UTC | #2
On 08/05/17 17:21, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Thursday 27 Apr 2017 13:27:49 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)
> 
> This looks OK to me.
> 
>> SDI -> LVDS (SDI is a type of LVDS)
> 
> If we're talking about this 
> https://en.wikipedia.org/wiki/Serial_digital_interface SDI, it's not a type of 
> LVDS at all. DRM_MODE_CONNECTOR_LVDS is interpreted as meaning that an LVDS 
> display panel is attached to the device, and is likely not removable. I don't 
> think it's a good match for SDI. We might need a new connector type.

No, it's not that. It's Serial display interface. I think it's
FPDLink/FlatLink.

>> VENC -> SVIDEO (it could also be composite, but we don't have that
>>         information here, so svideo should be quite good match)
> 
> Do we have the information anywhere ?

Yes, in the dts. At some point we should use that data, but I didn't
want to start digging the encoder/connector properties here.

 Tomi
Laurent Pinchart May 8, 2017, 4:21 p.m. UTC | #3
Hi Tomi,

On Monday 08 May 2017 17:35:50 Tomi Valkeinen wrote:
> On 08/05/17 17:21, Laurent Pinchart wrote:
> > On Thursday 27 Apr 2017 13:27:49 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)
> > 
> > This looks OK to me.
> > 
> >> SDI -> LVDS (SDI is a type of LVDS)
> > 
> > If we're talking about this
> > https://en.wikipedia.org/wiki/Serial_digital_interface SDI, it's not a
> > type of LVDS at all. DRM_MODE_CONNECTOR_LVDS is interpreted as meaning
> > that an LVDS display panel is attached to the device, and is likely not
> > removable. I don't think it's a good match for SDI. We might need a new
> > connector type.
>
> No, it's not that. It's Serial display interface. I think it's
> FPDLink/FlatLink.

Ah that makes more sense. Could you maybe mention that in the commit message ?

> >> VENC -> SVIDEO (it could also be composite, but we don't have that
> >>         information here, so svideo should be quite good match)
> > 
> > Do we have the information anywhere ?
> 
> Yes, in the dts. At some point we should use that data, but I didn't
> want to start digging the encoder/connector properties here.

So we don't need to program the hardware differently for S-Video and Composite 
? OK, in that case, with a few words about SDI in the commit message,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tomi Valkeinen May 9, 2017, 6:57 a.m. UTC | #4
On 08/05/17 19:21, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Monday 08 May 2017 17:35:50 Tomi Valkeinen wrote:
>> On 08/05/17 17:21, Laurent Pinchart wrote:
>>> On Thursday 27 Apr 2017 13:27:49 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)
>>>
>>> This looks OK to me.
>>>
>>>> SDI -> LVDS (SDI is a type of LVDS)
>>>
>>> If we're talking about this
>>> https://en.wikipedia.org/wiki/Serial_digital_interface SDI, it's not a
>>> type of LVDS at all. DRM_MODE_CONNECTOR_LVDS is interpreted as meaning
>>> that an LVDS display panel is attached to the device, and is likely not
>>> removable. I don't think it's a good match for SDI. We might need a new
>>> connector type.
>>
>> No, it's not that. It's Serial display interface. I think it's
>> FPDLink/FlatLink.
> 
> Ah that makes more sense. Could you maybe mention that in the commit message ?

Sure.

>>>> VENC -> SVIDEO (it could also be composite, but we don't have that
>>>>         information here, so svideo should be quite good match)
>>>
>>> Do we have the information anywhere ?
>>
>> Yes, in the dts. At some point we should use that data, but I didn't
>> want to start digging the encoder/connector properties here.
> 
> So we don't need to program the hardware differently for S-Video and Composite 
> ? OK, in that case, with a few words about SDI in the commit message,

We do program the HW differently. connector-analog-tv's DT property has
the connector type in the compatible string. Looks like it tries to pass
this forward to DSS's VENC, but if I read it right, it's broken... No,
wait... that's for non-DT. Yeah, I'm confused on what the code tries to
do here =)

But my point was that svideo/composite is not visible at the moment from
omapdrm. That said, I think a simple check of the compatible property
between "omapdss,svideo-connector" and
"omapdss,composite-video-connector" should do it. Let's see...

 Tomi
diff mbox

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index e1f47f0b3ccf..58c639e1e8e9 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -214,6 +214,13 @@  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:
+		return DRM_MODE_CONNECTOR_SVIDEO;
+	case OMAP_DISPLAY_TYPE_SDI:
+		return DRM_MODE_CONNECTOR_LVDS;
 	default:
 		return DRM_MODE_CONNECTOR_Unknown;
 	}