diff mbox series

[v4,1/3] drm/uapi: Add USB connector type

Message ID 20210120170033.38468-2-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show
Series Generic USB Display driver | expand

Commit Message

Noralf Trønnes Jan. 20, 2021, 5 p.m. UTC
Add a connector type for USB connected display panels.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 include/uapi/drm/drm_mode.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Daniel Vetter Jan. 20, 2021, 5:42 p.m. UTC | #1
On Wed, Jan 20, 2021 at 6:10 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>
> Add a connector type for USB connected display panels.
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  include/uapi/drm/drm_mode.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index fed66a03c7ae..33024cc5d26e 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -367,6 +367,7 @@ enum drm_mode_subconnector {
>  #define DRM_MODE_CONNECTOR_DPI         17
>  #define DRM_MODE_CONNECTOR_WRITEBACK   18
>  #define DRM_MODE_CONNECTOR_SPI         19
> +#define DRM_MODE_CONNECTOR_USB         20

Beware, new connector types have in the past resulted in userspace
burning&crashing. Maybe it's become better ...

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>  /**
>   * struct drm_mode_get_connector - Get connector metadata.
> --
> 2.23.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Thomas Zimmermann Jan. 21, 2021, 7:45 a.m. UTC | #2
Hi Noralf,

glad to hear from you! Welcome back!

Am 20.01.21 um 18:42 schrieb Daniel Vetter:
> On Wed, Jan 20, 2021 at 6:10 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>>
>> Add a connector type for USB connected display panels.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   include/uapi/drm/drm_mode.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index fed66a03c7ae..33024cc5d26e 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -367,6 +367,7 @@ enum drm_mode_subconnector {
>>   #define DRM_MODE_CONNECTOR_DPI         17
>>   #define DRM_MODE_CONNECTOR_WRITEBACK   18
>>   #define DRM_MODE_CONNECTOR_SPI         19
>> +#define DRM_MODE_CONNECTOR_USB         20

I would not call it USB. I could imagine that at some point a generic 
USB protocol could serve simple displays (i.e. in the sense of USB HID 
or data or imaging). (Maybe Thunderbold already counts.) Anyway, USB 
should be reserved for this case.

Best regards
Thomas

> 
> Beware, new connector types have in the past resulted in userspace
> burning&crashing. Maybe it's become better ...
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>>   /**
>>    * struct drm_mode_get_connector - Get connector metadata.
>> --
>> 2.23.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
>
Daniel Vetter Jan. 21, 2021, 8:27 a.m. UTC | #3
On Thu, Jan 21, 2021 at 8:45 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi Noralf,
>
> glad to hear from you! Welcome back!
>
> Am 20.01.21 um 18:42 schrieb Daniel Vetter:
> > On Wed, Jan 20, 2021 at 6:10 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> >>
> >> Add a connector type for USB connected display panels.
> >>
> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >> ---
> >>   include/uapi/drm/drm_mode.h | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> >> index fed66a03c7ae..33024cc5d26e 100644
> >> --- a/include/uapi/drm/drm_mode.h
> >> +++ b/include/uapi/drm/drm_mode.h
> >> @@ -367,6 +367,7 @@ enum drm_mode_subconnector {
> >>   #define DRM_MODE_CONNECTOR_DPI         17
> >>   #define DRM_MODE_CONNECTOR_WRITEBACK   18
> >>   #define DRM_MODE_CONNECTOR_SPI         19
> >> +#define DRM_MODE_CONNECTOR_USB         20
>
> I would not call it USB. I could imagine that at some point a generic
> USB protocol could serve simple displays (i.e. in the sense of USB HID
> or data or imaging). (Maybe Thunderbold already counts.) Anyway, USB
> should be reserved for this case.

We end up calling those DisplayPort, since that's what's being
transported over thunderbolt or usb-C. So the usb connector would be
called usb-C. I think the reason we don't do fancy connector names is
that adding them is a bit a pain. Plus drm/i915 specifically has some
very quirky connector enumerating that doesn't match much with reality
unfortunately anyway :-/
-Daniel

>
> Best regards
> Thomas
>
> >
> > Beware, new connector types have in the past resulted in userspace
> > burning&crashing. Maybe it's become better ...
> >
> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>
> >>   /**
> >>    * struct drm_mode_get_connector - Get connector metadata.
> >> --
> >> 2.23.0
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
Simon Ser Jan. 21, 2021, 8:40 a.m. UTC | #4
On Thursday, January 21st, 2021 at 9:27 AM, Daniel Vetter <daniel@ffwll.ch> wrote:

> > I would not call it USB. I could imagine that at some point a generic
> > USB protocol could serve simple displays (i.e. in the sense of USB HID
> > or data or imaging). (Maybe Thunderbold already counts.) Anyway, USB
> > should be reserved for this case.
>
> We end up calling those DisplayPort, since that's what's being
> transported over thunderbolt or usb-C. So the usb connector would be
> called usb-C. I think the reason we don't do fancy connector names is
> that adding them is a bit a pain. Plus drm/i915 specifically has some
> very quirky connector enumerating that doesn't match much with reality
> unfortunately anyway :-/

Maybe could add "USB-C" to the list of subconnector types and use that?
Thomas Zimmermann Jan. 21, 2021, 10:01 a.m. UTC | #5
Hi

Am 21.01.21 um 09:27 schrieb Daniel Vetter:
> On Thu, Jan 21, 2021 at 8:45 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi Noralf,
>>
>> glad to hear from you! Welcome back!
>>
>> Am 20.01.21 um 18:42 schrieb Daniel Vetter:
>>> On Wed, Jan 20, 2021 at 6:10 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>>>>
>>>> Add a connector type for USB connected display panels.
>>>>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>> ---
>>>>    include/uapi/drm/drm_mode.h | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>>>> index fed66a03c7ae..33024cc5d26e 100644
>>>> --- a/include/uapi/drm/drm_mode.h
>>>> +++ b/include/uapi/drm/drm_mode.h
>>>> @@ -367,6 +367,7 @@ enum drm_mode_subconnector {
>>>>    #define DRM_MODE_CONNECTOR_DPI         17
>>>>    #define DRM_MODE_CONNECTOR_WRITEBACK   18
>>>>    #define DRM_MODE_CONNECTOR_SPI         19
>>>> +#define DRM_MODE_CONNECTOR_USB         20
>>
>> I would not call it USB. I could imagine that at some point a generic
>> USB protocol could serve simple displays (i.e. in the sense of USB HID
>> or data or imaging). (Maybe Thunderbold already counts.) Anyway, USB
>> should be reserved for this case.
> 
> We end up calling those DisplayPort, since that's what's being
> transported over thunderbolt or usb-C. So the usb connector would be
> called usb-C. I think the reason we don't do fancy connector names is
> that adding them is a bit a pain. Plus drm/i915 specifically has some
> very quirky connector enumerating that doesn't match much with reality
> unfortunately anyway :-/

In the case of the other USB drivers, IIRC we use the connector type 
that is at the output (i.e., HDMI in the case of udl). I think we should 
do the same here. Or use 'Unknown'.

Best regards
Thomas

> -Daniel
> 
>>
>> Best regards
>> Thomas
>>
>>>
>>> Beware, new connector types have in the past resulted in userspace
>>> burning&crashing. Maybe it's become better ...
>>>
>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>
>>>>    /**
>>>>     * struct drm_mode_get_connector - Get connector metadata.
>>>> --
>>>> 2.23.0
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> 
>
Noralf Trønnes Jan. 21, 2021, 6:07 p.m. UTC | #6
Den 21.01.2021 11.01, skrev Thomas Zimmermann:
> Hi
> 
> Am 21.01.21 um 09:27 schrieb Daniel Vetter:
>> On Thu, Jan 21, 2021 at 8:45 AM Thomas Zimmermann
>> <tzimmermann@suse.de> wrote:
>>>
>>> Hi Noralf,
>>>
>>> glad to hear from you! Welcome back!

Thanks Thomas!

>>>
>>> Am 20.01.21 um 18:42 schrieb Daniel Vetter:
>>>> On Wed, Jan 20, 2021 at 6:10 PM Noralf Trønnes <noralf@tronnes.org>
>>>> wrote:
>>>>>
>>>>> Add a connector type for USB connected display panels.
>>>>>
>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>> ---

I have forgotten to update drm_connector_enum_list which maps type to name.

>>>>>    include/uapi/drm/drm_mode.h | 1 +
>>>>>    1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>>>>> index fed66a03c7ae..33024cc5d26e 100644
>>>>> --- a/include/uapi/drm/drm_mode.h
>>>>> +++ b/include/uapi/drm/drm_mode.h
>>>>> @@ -367,6 +367,7 @@ enum drm_mode_subconnector {
>>>>>    #define DRM_MODE_CONNECTOR_DPI         17
>>>>>    #define DRM_MODE_CONNECTOR_WRITEBACK   18
>>>>>    #define DRM_MODE_CONNECTOR_SPI         19
>>>>> +#define DRM_MODE_CONNECTOR_USB         20
>>>
>>> I would not call it USB. I could imagine that at some point a generic
>>> USB protocol could serve simple displays (i.e. in the sense of USB HID
>>> or data or imaging). (Maybe Thunderbold already counts.) Anyway, USB
>>> should be reserved for this case.
>>
>> We end up calling those DisplayPort, since that's what's being
>> transported over thunderbolt or usb-C. So the usb connector would be
>> called usb-C. I think the reason we don't do fancy connector names is
>> that adding them is a bit a pain. Plus drm/i915 specifically has some
>> very quirky connector enumerating that doesn't match much with reality
>> unfortunately anyway :-/
> 
> In the case of the other USB drivers, IIRC we use the connector type
> that is at the output (i.e., HDMI in the case of udl). I think we should
> do the same here. Or use 'Unknown'.
> 

There are 2 DRM USB drivers and they use:
- udl: DRM_MODE_CONNECTOR_DVII
- gm12u320: DRM_MODE_CONNECTOR_VGA

gm12u320 is a mini projector so it doesn't actually have a VGA
connector. I have never seen a udl device but I assume it has a DVII
connector?

For display adapters it makes sense to use the connector on the adapter
as the reported connector, but for display panels that don't have any
connector except for the cable that is connected to the hosts USB
connector, why can't it be called a USB connector? That's the connector
the user sees.

Ofc as Daniel mentions it's a downside that userspace doesn't know about
the connector type, and who knows when it will updated (if I don't do it).
Weston will name it: "UNNAMED-%d"
Mutter: "Unknown%d-%d"
X: "Unknown%d-%d"

Sam and Laurent has discussed adding a PANEL connector type instead of
adding more connector types for panel connectors. I think that would
have been a better choice instead of the SPI connector type that I added
in 2019. But I think PANEL was meant for panels connected to an internal
connector.

Here's my protocol connector types and how it's mapped to DRM:

#define GUD_CONNECTOR_TYPE_PANEL		0
#define GUD_CONNECTOR_TYPE_VGA			1
#define GUD_CONNECTOR_TYPE_COMPOSITE		2
#define GUD_CONNECTOR_TYPE_SVIDEO		3
#define GUD_CONNECTOR_TYPE_COMPONENT		4
#define GUD_CONNECTOR_TYPE_DVI			5
#define GUD_CONNECTOR_TYPE_DISPLAYPORT		6
#define GUD_CONNECTOR_TYPE_HDMI			7

static int gud_gadget_ctrl_get_connector(struct gud_gadget *gdg,
unsigned int index,
					 struct gud_connector_descriptor_req *desc)
{
...
	gconn = &gdg->connectors[index];

	switch (gconn->connector->connector_type) {
	case DRM_MODE_CONNECTOR_VGA:
		desc->connector_type = GUD_CONNECTOR_TYPE_VGA;
		break;
	case DRM_MODE_CONNECTOR_DVII:
		fallthrough;
	case DRM_MODE_CONNECTOR_DVID:
		fallthrough;
	case DRM_MODE_CONNECTOR_DVIA:
		desc->connector_type = GUD_CONNECTOR_TYPE_DVI;
		break;
	case DRM_MODE_CONNECTOR_Composite:
		desc->connector_type = GUD_CONNECTOR_TYPE_COMPOSITE;
		break;
	case DRM_MODE_CONNECTOR_SVIDEO:
		desc->connector_type = GUD_CONNECTOR_TYPE_SVIDEO;
		break;
	case DRM_MODE_CONNECTOR_Component:
		desc->connector_type = GUD_CONNECTOR_TYPE_COMPONENT;
		break;
	case DRM_MODE_CONNECTOR_DisplayPort:
		desc->connector_type = GUD_CONNECTOR_TYPE_DISPLAYPORT;
		break;
	case DRM_MODE_CONNECTOR_HDMIA:
		fallthrough;
	case DRM_MODE_CONNECTOR_HDMIB:
		desc->connector_type = GUD_CONNECTOR_TYPE_HDMI;
		break;
	default:
		desc->connector_type = GUD_CONNECTOR_TYPE_PANEL;
		break;
	};


int gud_connector_create(struct gud_device *gdrm, unsigned int index)
{
...
	switch (desc.connector_type) {
	case GUD_CONNECTOR_TYPE_PANEL:
		connector_type = DRM_MODE_CONNECTOR_USB;
		break;
	case GUD_CONNECTOR_TYPE_VGA:
		connector_type = DRM_MODE_CONNECTOR_VGA;
		break;
	case GUD_CONNECTOR_TYPE_DVI:
		connector_type = DRM_MODE_CONNECTOR_DVID;
		break;
	case GUD_CONNECTOR_TYPE_COMPOSITE:
		connector_type = DRM_MODE_CONNECTOR_Composite;
		break;
	case GUD_CONNECTOR_TYPE_SVIDEO:
		connector_type = DRM_MODE_CONNECTOR_SVIDEO;
		break;
	case GUD_CONNECTOR_TYPE_COMPONENT:
		connector_type = DRM_MODE_CONNECTOR_Component;
		break;
	case GUD_CONNECTOR_TYPE_DISPLAYPORT:
		connector_type = DRM_MODE_CONNECTOR_DisplayPort;
		break;
	case GUD_CONNECTOR_TYPE_HDMI:
		connector_type = DRM_MODE_CONNECTOR_HDMIA;
		break;
	default: /* future types */
		connector_type = DRM_MODE_CONNECTOR_USB;
		break;
	};

Noralf.

> Best regards
> Thomas
> 
>> -Daniel
>>
>>>
>>> Best regards
>>> Thomas
>>>
>>>>
>>>> Beware, new connector types have in the past resulted in userspace
>>>> burning&crashing. Maybe it's become better ...
>>>>
>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>
>>>>>    /**
>>>>>     * struct drm_mode_get_connector - Get connector metadata.
>>>>> -- 
>>>>> 2.23.0
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>
>>>>
>>>>
>>>
>>> -- 
>>> Thomas Zimmermann
>>> Graphics Driver Developer
>>> SUSE Software Solutions Germany GmbH
>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>> (HRB 36809, AG Nürnberg)
>>> Geschäftsführer: Felix Imendörffer
>>>
>>
>>
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Thomas Zimmermann Jan. 22, 2021, 7:54 a.m. UTC | #7
Hi

Am 21.01.21 um 19:07 schrieb Noralf Trønnes:
> 
> 
> Den 21.01.2021 11.01, skrev Thomas Zimmermann:
>> Hi
>>
>> Am 21.01.21 um 09:27 schrieb Daniel Vetter:
>>> On Thu, Jan 21, 2021 at 8:45 AM Thomas Zimmermann
>>> <tzimmermann@suse.de> wrote:
>>>>
>>>> Hi Noralf,
>>>>
>>>> glad to hear from you! Welcome back!
> 
> Thanks Thomas!
> 
>>>>
>>>> Am 20.01.21 um 18:42 schrieb Daniel Vetter:
>>>>> On Wed, Jan 20, 2021 at 6:10 PM Noralf Trønnes <noralf@tronnes.org>
>>>>> wrote:
>>>>>>
>>>>>> Add a connector type for USB connected display panels.
>>>>>>
>>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>>> ---
> 
> I have forgotten to update drm_connector_enum_list which maps type to name.
> 
>>>>>>     include/uapi/drm/drm_mode.h | 1 +
>>>>>>     1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>>>>>> index fed66a03c7ae..33024cc5d26e 100644
>>>>>> --- a/include/uapi/drm/drm_mode.h
>>>>>> +++ b/include/uapi/drm/drm_mode.h
>>>>>> @@ -367,6 +367,7 @@ enum drm_mode_subconnector {
>>>>>>     #define DRM_MODE_CONNECTOR_DPI         17
>>>>>>     #define DRM_MODE_CONNECTOR_WRITEBACK   18
>>>>>>     #define DRM_MODE_CONNECTOR_SPI         19
>>>>>> +#define DRM_MODE_CONNECTOR_USB         20
>>>>
>>>> I would not call it USB. I could imagine that at some point a generic
>>>> USB protocol could serve simple displays (i.e. in the sense of USB HID
>>>> or data or imaging). (Maybe Thunderbold already counts.) Anyway, USB
>>>> should be reserved for this case.
>>>
>>> We end up calling those DisplayPort, since that's what's being
>>> transported over thunderbolt or usb-C. So the usb connector would be
>>> called usb-C. I think the reason we don't do fancy connector names is
>>> that adding them is a bit a pain. Plus drm/i915 specifically has some
>>> very quirky connector enumerating that doesn't match much with reality
>>> unfortunately anyway :-/
>>
>> In the case of the other USB drivers, IIRC we use the connector type
>> that is at the output (i.e., HDMI in the case of udl). I think we should
>> do the same here. Or use 'Unknown'.
>>
> 
> There are 2 DRM USB drivers and they use:
> - udl: DRM_MODE_CONNECTOR_DVII

Mine has plain old VGA. Maybe we should change generally this to Unknown.

> - gm12u320: DRM_MODE_CONNECTOR_VGA
> 
> gm12u320 is a mini projector so it doesn't actually have a VGA
> connector. I have never seen a udl device but I assume it has a DVII
> connector?
> 
> For display adapters it makes sense to use the connector on the adapter
> as the reported connector, but for display panels that don't have any
> connector except for the cable that is connected to the hosts USB
> connector, why can't it be called a USB connector? That's the connector
> the user sees.

It's not the relevant connector for the display output. USB is the bus 
system. (Making your argument in terms of discrete GPUs, the connector 
would always be PCI then.)

> 
> Ofc as Daniel mentions it's a downside that userspace doesn't know about
> the connector type, and who knows when it will updated (if I don't do it).
> Weston will name it: "UNNAMED-%d"
> Mutter: "Unknown%d-%d"
> X: "Unknown%d-%d"
> 
> Sam and Laurent has discussed adding a PANEL connector type instead of
> adding more connector types for panel connectors. I think that would
> have been a better choice instead of the SPI connector type that I added
> in 2019. But I think PANEL was meant for panels connected to an internal
> connector.
> 
> Here's my protocol connector types and how it's mapped to DRM:
> 
> #define GUD_CONNECTOR_TYPE_PANEL		0
> #define GUD_CONNECTOR_TYPE_VGA			1
> #define GUD_CONNECTOR_TYPE_COMPOSITE		2
> #define GUD_CONNECTOR_TYPE_SVIDEO		3
> #define GUD_CONNECTOR_TYPE_COMPONENT		4
> #define GUD_CONNECTOR_TYPE_DVI			5
> #define GUD_CONNECTOR_TYPE_DISPLAYPORT		6
> #define GUD_CONNECTOR_TYPE_HDMI			7
> 
> static int gud_gadget_ctrl_get_connector(struct gud_gadget *gdg,
> unsigned int index,
> 					 struct gud_connector_descriptor_req *desc)
> {
> ...
> 	gconn = &gdg->connectors[index];
> 
> 	switch (gconn->connector->connector_type) {
> 	case DRM_MODE_CONNECTOR_VGA:
> 		desc->connector_type = GUD_CONNECTOR_TYPE_VGA;
> 		break;
> 	case DRM_MODE_CONNECTOR_DVII:
> 		fallthrough;
> 	case DRM_MODE_CONNECTOR_DVID:
> 		fallthrough;
> 	case DRM_MODE_CONNECTOR_DVIA:
> 		desc->connector_type = GUD_CONNECTOR_TYPE_DVI;
> 		break;
> 	case DRM_MODE_CONNECTOR_Composite:
> 		desc->connector_type = GUD_CONNECTOR_TYPE_COMPOSITE;
> 		break;
> 	case DRM_MODE_CONNECTOR_SVIDEO:
> 		desc->connector_type = GUD_CONNECTOR_TYPE_SVIDEO;
> 		break;
> 	case DRM_MODE_CONNECTOR_Component:
> 		desc->connector_type = GUD_CONNECTOR_TYPE_COMPONENT;
> 		break;
> 	case DRM_MODE_CONNECTOR_DisplayPort:
> 		desc->connector_type = GUD_CONNECTOR_TYPE_DISPLAYPORT;
> 		break;
> 	case DRM_MODE_CONNECTOR_HDMIA:
> 		fallthrough;
> 	case DRM_MODE_CONNECTOR_HDMIB:
> 		desc->connector_type = GUD_CONNECTOR_TYPE_HDMI;
> 		break;
> 	default:
> 		desc->connector_type = GUD_CONNECTOR_TYPE_PANEL;
> 		break;
> 	};
> 
> 
> int gud_connector_create(struct gud_device *gdrm, unsigned int index)
> {
> ...
> 	switch (desc.connector_type) {
> 	case GUD_CONNECTOR_TYPE_PANEL:
> 		connector_type = DRM_MODE_CONNECTOR_USB;
> 		break;
> 	case GUD_CONNECTOR_TYPE_VGA:
> 		connector_type = DRM_MODE_CONNECTOR_VGA;
> 		break;
> 	case GUD_CONNECTOR_TYPE_DVI:
> 		connector_type = DRM_MODE_CONNECTOR_DVID;
> 		break;
> 	case GUD_CONNECTOR_TYPE_COMPOSITE:
> 		connector_type = DRM_MODE_CONNECTOR_Composite;
> 		break;
> 	case GUD_CONNECTOR_TYPE_SVIDEO:
> 		connector_type = DRM_MODE_CONNECTOR_SVIDEO;
> 		break;
> 	case GUD_CONNECTOR_TYPE_COMPONENT:
> 		connector_type = DRM_MODE_CONNECTOR_Component;
> 		break;
> 	case GUD_CONNECTOR_TYPE_DISPLAYPORT:
> 		connector_type = DRM_MODE_CONNECTOR_DisplayPort;
> 		break;
> 	case GUD_CONNECTOR_TYPE_HDMI:
> 		connector_type = DRM_MODE_CONNECTOR_HDMIA;
> 		break;
> 	default: /* future types */
> 		connector_type = DRM_MODE_CONNECTOR_USB;

The more I look at it the more I think it should be 'Unknown' here.

BTW, can I try this out somehow? I do have an RPi3. Do I need a special 
disk image?

Best regards
Thomas

> 		break;
> 	};
> 
> Noralf.
> 
>> Best regards
>> Thomas
>>
>>> -Daniel
>>>
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>>
>>>>> Beware, new connector types have in the past resulted in userspace
>>>>> burning&crashing. Maybe it's become better ...
>>>>>
>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>
>>>>>>     /**
>>>>>>      * struct drm_mode_get_connector - Get connector metadata.
>>>>>> -- 
>>>>>> 2.23.0
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>>
>>>>>
>>>>
>>>> -- 
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>> (HRB 36809, AG Nürnberg)
>>>> Geschäftsführer: Felix Imendörffer
>>>>
>>>
>>>
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Thomas Zimmermann Jan. 22, 2021, 7:59 a.m. UTC | #8
Am 22.01.21 um 08:54 schrieb Thomas Zimmermann:
The more I look at it the more I think it should be 'Unknown' here.
> 
> BTW, can I try this out somehow? I do have an RPi3. Do I need a special 
> disk image?

Oh, I saw that wiki url now. I'll check this out.

> 
> Best regards
> Thomas
> 
>>         break;
>>     };
>>
>> Noralf.
>>
>>> Best regards
>>> Thomas
>>>
>>>> -Daniel
>>>>
>>>>>
>>>>> Best regards
>>>>> Thomas
>>>>>
>>>>>>
>>>>>> Beware, new connector types have in the past resulted in userspace
>>>>>> burning&crashing. Maybe it's become better ...
>>>>>>
>>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>>
>>>>>>>     /**
>>>>>>>      * struct drm_mode_get_connector - Get connector metadata.
>>>>>>> -- 
>>>>>>> 2.23.0
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> -- 
>>>>> Thomas Zimmermann
>>>>> Graphics Driver Developer
>>>>> SUSE Software Solutions Germany GmbH
>>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>>> (HRB 36809, AG Nürnberg)
>>>>> Geschäftsführer: Felix Imendörffer
>>>>>
>>>>
>>>>
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Noralf Trønnes Jan. 22, 2021, 11:44 a.m. UTC | #9
Den 22.01.2021 08.54, skrev Thomas Zimmermann:
> Hi
> 
> Am 21.01.21 um 19:07 schrieb Noralf Trønnes:
>>
>>
>> Den 21.01.2021 11.01, skrev Thomas Zimmermann:
>>> Hi
>>>
>>> Am 21.01.21 um 09:27 schrieb Daniel Vetter:
>>>> On Thu, Jan 21, 2021 at 8:45 AM Thomas Zimmermann
>>>> <tzimmermann@suse.de> wrote:
>>>>>
>>>>> Hi Noralf,
>>>>>
>>>>> glad to hear from you! Welcome back!
>>
>> Thanks Thomas!
>>
>>>>>
>>>>> Am 20.01.21 um 18:42 schrieb Daniel Vetter:
>>>>>> On Wed, Jan 20, 2021 at 6:10 PM Noralf Trønnes <noralf@tronnes.org>
>>>>>> wrote:
>>>>>>>
>>>>>>> Add a connector type for USB connected display panels.
>>>>>>>
>>>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>>>> ---
>>
>> I have forgotten to update drm_connector_enum_list which maps type to
>> name.
>>
>>>>>>>     include/uapi/drm/drm_mode.h | 1 +
>>>>>>>     1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/include/uapi/drm/drm_mode.h
>>>>>>> b/include/uapi/drm/drm_mode.h
>>>>>>> index fed66a03c7ae..33024cc5d26e 100644
>>>>>>> --- a/include/uapi/drm/drm_mode.h
>>>>>>> +++ b/include/uapi/drm/drm_mode.h
>>>>>>> @@ -367,6 +367,7 @@ enum drm_mode_subconnector {
>>>>>>>     #define DRM_MODE_CONNECTOR_DPI         17
>>>>>>>     #define DRM_MODE_CONNECTOR_WRITEBACK   18
>>>>>>>     #define DRM_MODE_CONNECTOR_SPI         19
>>>>>>> +#define DRM_MODE_CONNECTOR_USB         20
>>>>>
>>>>> I would not call it USB. I could imagine that at some point a generic
>>>>> USB protocol could serve simple displays (i.e. in the sense of USB HID
>>>>> or data or imaging). (Maybe Thunderbold already counts.) Anyway, USB
>>>>> should be reserved for this case.
>>>>
>>>> We end up calling those DisplayPort, since that's what's being
>>>> transported over thunderbolt or usb-C. So the usb connector would be
>>>> called usb-C. I think the reason we don't do fancy connector names is
>>>> that adding them is a bit a pain. Plus drm/i915 specifically has some
>>>> very quirky connector enumerating that doesn't match much with reality
>>>> unfortunately anyway :-/
>>>
>>> In the case of the other USB drivers, IIRC we use the connector type
>>> that is at the output (i.e., HDMI in the case of udl). I think we should
>>> do the same here. Or use 'Unknown'.
>>>
>>
>> There are 2 DRM USB drivers and they use:
>> - udl: DRM_MODE_CONNECTOR_DVII
> 
> Mine has plain old VGA.

Ok, maybe the Displaylink protocol doesn't provide info about the
connector type or if it does the driver doesn't know about it.

> Maybe we should change generally this to Unknown.
> 
>> - gm12u320: DRM_MODE_CONNECTOR_VGA
>>
>> gm12u320 is a mini projector so it doesn't actually have a VGA
>> connector. I have never seen a udl device but I assume it has a DVII
>> connector?
>>
>> For display adapters it makes sense to use the connector on the adapter
>> as the reported connector, but for display panels that don't have any
>> connector except for the cable that is connected to the hosts USB
>> connector, why can't it be called a USB connector? That's the connector
>> the user sees.
> 
> It's not the relevant connector for the display output. USB is the bus
> system. (Making your argument in terms of discrete GPUs, the connector
> would always be PCI then.)
> 

Yes strictly speaking USB is the bus and the connectors have other
names: USB (type)-A, USB-C etc., but I don't understand the problem
here. Why does it matter that it is a bus?

And wrt PCI it wouldn't be a PCI connector if the card has some other
connector for the display, but if it was possible to connect a display
directly to the PCI connector, then yes I would call that a PCI connector.

This begs the question: Why does the kernel provide info to userspace
about the connector type?

My take is that it is so the user can know which display is connected to
which port on the computer.

What's your opinion?

>>
>> Ofc as Daniel mentions it's a downside that userspace doesn't know about
>> the connector type, and who knows when it will updated (if I don't do
>> it).
>> Weston will name it: "UNNAMED-%d"
>> Mutter: "Unknown%d-%d"
>> X: "Unknown%d-%d"
>>
>> Sam and Laurent has discussed adding a PANEL connector type instead of
>> adding more connector types for panel connectors. I think that would
>> have been a better choice instead of the SPI connector type that I added
>> in 2019. But I think PANEL was meant for panels connected to an internal
>> connector.
>>
>> Here's my protocol connector types and how it's mapped to DRM:
>>
>> #define GUD_CONNECTOR_TYPE_PANEL        0
>> #define GUD_CONNECTOR_TYPE_VGA            1
>> #define GUD_CONNECTOR_TYPE_COMPOSITE        2
>> #define GUD_CONNECTOR_TYPE_SVIDEO        3
>> #define GUD_CONNECTOR_TYPE_COMPONENT        4
>> #define GUD_CONNECTOR_TYPE_DVI            5
>> #define GUD_CONNECTOR_TYPE_DISPLAYPORT        6
>> #define GUD_CONNECTOR_TYPE_HDMI            7
>>
>> static int gud_gadget_ctrl_get_connector(struct gud_gadget *gdg,
>> unsigned int index,
>>                      struct gud_connector_descriptor_req *desc)
>> {
>> ...
>>     gconn = &gdg->connectors[index];
>>
>>     switch (gconn->connector->connector_type) {
>>     case DRM_MODE_CONNECTOR_VGA:
>>         desc->connector_type = GUD_CONNECTOR_TYPE_VGA;
>>         break;
>>     case DRM_MODE_CONNECTOR_DVII:
>>         fallthrough;
>>     case DRM_MODE_CONNECTOR_DVID:
>>         fallthrough;
>>     case DRM_MODE_CONNECTOR_DVIA:
>>         desc->connector_type = GUD_CONNECTOR_TYPE_DVI;
>>         break;
>>     case DRM_MODE_CONNECTOR_Composite:
>>         desc->connector_type = GUD_CONNECTOR_TYPE_COMPOSITE;
>>         break;
>>     case DRM_MODE_CONNECTOR_SVIDEO:
>>         desc->connector_type = GUD_CONNECTOR_TYPE_SVIDEO;
>>         break;
>>     case DRM_MODE_CONNECTOR_Component:
>>         desc->connector_type = GUD_CONNECTOR_TYPE_COMPONENT;
>>         break;
>>     case DRM_MODE_CONNECTOR_DisplayPort:
>>         desc->connector_type = GUD_CONNECTOR_TYPE_DISPLAYPORT;
>>         break;
>>     case DRM_MODE_CONNECTOR_HDMIA:
>>         fallthrough;
>>     case DRM_MODE_CONNECTOR_HDMIB:
>>         desc->connector_type = GUD_CONNECTOR_TYPE_HDMI;
>>         break;
>>     default:
>>         desc->connector_type = GUD_CONNECTOR_TYPE_PANEL;
>>         break;
>>     };
>>
>>
>> int gud_connector_create(struct gud_device *gdrm, unsigned int index)
>> {
>> ...
>>     switch (desc.connector_type) {
>>     case GUD_CONNECTOR_TYPE_PANEL:
>>         connector_type = DRM_MODE_CONNECTOR_USB;
>>         break;
>>     case GUD_CONNECTOR_TYPE_VGA:
>>         connector_type = DRM_MODE_CONNECTOR_VGA;
>>         break;
>>     case GUD_CONNECTOR_TYPE_DVI:
>>         connector_type = DRM_MODE_CONNECTOR_DVID;
>>         break;
>>     case GUD_CONNECTOR_TYPE_COMPOSITE:
>>         connector_type = DRM_MODE_CONNECTOR_Composite;
>>         break;
>>     case GUD_CONNECTOR_TYPE_SVIDEO:
>>         connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>>         break;
>>     case GUD_CONNECTOR_TYPE_COMPONENT:
>>         connector_type = DRM_MODE_CONNECTOR_Component;
>>         break;
>>     case GUD_CONNECTOR_TYPE_DISPLAYPORT:
>>         connector_type = DRM_MODE_CONNECTOR_DisplayPort;
>>         break;
>>     case GUD_CONNECTOR_TYPE_HDMI:
>>         connector_type = DRM_MODE_CONNECTOR_HDMIA;
>>         break;
>>     default: /* future types */
>>         connector_type = DRM_MODE_CONNECTOR_USB;
> 
> The more I look at it the more I think it should be 'Unknown' here.
> 

I don't understand this, how will that be better for the user?

> BTW, can I try this out somehow? I do have an RPi3. Do I need a special
> disk image?

The Pi3 doesn'have a USB device/otg connector so I haven't made an image
for that one. Only the Pi Zero, model A and Pi 4 have that.

The Pi2 and Pi3 have a USB hub on the soc's single USB port.

Noralf.

> 
> Best regards
> Thomas
> 
>>         break;
>>     };
>>
>> Noralf.
>>
>>> Best regards
>>> Thomas
>>>
>>>> -Daniel
>>>>
>>>>>
>>>>> Best regards
>>>>> Thomas
>>>>>
>>>>>>
>>>>>> Beware, new connector types have in the past resulted in userspace
>>>>>> burning&crashing. Maybe it's become better ...
>>>>>>
>>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>>
>>>>>>>     /**
>>>>>>>      * struct drm_mode_get_connector - Get connector metadata.
>>>>>>> -- 
>>>>>>> 2.23.0
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> -- 
>>>>> Thomas Zimmermann
>>>>> Graphics Driver Developer
>>>>> SUSE Software Solutions Germany GmbH
>>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>>> (HRB 36809, AG Nürnberg)
>>>>> Geschäftsführer: Felix Imendörffer
>>>>>
>>>>
>>>>
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
Thomas Zimmermann Jan. 22, 2021, 12:47 p.m. UTC | #10
Hi

Am 22.01.21 um 12:44 schrieb Noralf Trønnes:
> 
> And wrt PCI it wouldn't be a PCI connector if the card has some other
> connector for the display, but if it was possible to connect a display
> directly to the PCI connector, then yes I would call that a PCI connector.

You're not connecting a display to the computer. You're connecting an 
RPi and then connect the display to the RPi. The RPi acts like an 
external graphics card.

> This begs the question: Why does the kernel provide info to userspace
> about the connector type?
> 
> My take is that it is so the user can know which display is connected to
> which port on the computer.

This exactly illustrates the problem with the current naming. For a 
single output the distinction between bus and connector might be fuzzy. 
As soon as a connected SoC contains multiple connectors. The user then 
sees names such as card1-USB-0 and card1-USB-1, which makes no sense.

> 
> What's your opinion?
> 
>>>
>>> Ofc as Daniel mentions it's a downside that userspace doesn't know about
>>> the connector type, and who knows when it will updated (if I don't do
>>> it).
>>> Weston will name it: "UNNAMED-%d"
>>> Mutter: "Unknown%d-%d"
>>> X: "Unknown%d-%d"
>>>
>>> Sam and Laurent has discussed adding a PANEL connector type instead of
>>> adding more connector types for panel connectors. I think that would
>>> have been a better choice instead of the SPI connector type that I added
>>> in 2019. But I think PANEL was meant for panels connected to an internal
>>> connector.
>>>
>>> Here's my protocol connector types and how it's mapped to DRM:
>>>
>>> #define GUD_CONNECTOR_TYPE_PANEL        0
>>> #define GUD_CONNECTOR_TYPE_VGA            1
>>> #define GUD_CONNECTOR_TYPE_COMPOSITE        2
>>> #define GUD_CONNECTOR_TYPE_SVIDEO        3
>>> #define GUD_CONNECTOR_TYPE_COMPONENT        4
>>> #define GUD_CONNECTOR_TYPE_DVI            5
>>> #define GUD_CONNECTOR_TYPE_DISPLAYPORT        6
>>> #define GUD_CONNECTOR_TYPE_HDMI            7
>>>
>>> static int gud_gadget_ctrl_get_connector(struct gud_gadget *gdg,
>>> unsigned int index,
>>>                       struct gud_connector_descriptor_req *desc)
>>> {
>>> ...
>>>      gconn = &gdg->connectors[index];
>>>
>>>      switch (gconn->connector->connector_type) {
>>>      case DRM_MODE_CONNECTOR_VGA:
>>>          desc->connector_type = GUD_CONNECTOR_TYPE_VGA;
>>>          break;
>>>      case DRM_MODE_CONNECTOR_DVII:
>>>          fallthrough;
>>>      case DRM_MODE_CONNECTOR_DVID:
>>>          fallthrough;
>>>      case DRM_MODE_CONNECTOR_DVIA:
>>>          desc->connector_type = GUD_CONNECTOR_TYPE_DVI;
>>>          break;
>>>      case DRM_MODE_CONNECTOR_Composite:
>>>          desc->connector_type = GUD_CONNECTOR_TYPE_COMPOSITE;
>>>          break;
>>>      case DRM_MODE_CONNECTOR_SVIDEO:
>>>          desc->connector_type = GUD_CONNECTOR_TYPE_SVIDEO;
>>>          break;
>>>      case DRM_MODE_CONNECTOR_Component:
>>>          desc->connector_type = GUD_CONNECTOR_TYPE_COMPONENT;
>>>          break;
>>>      case DRM_MODE_CONNECTOR_DisplayPort:
>>>          desc->connector_type = GUD_CONNECTOR_TYPE_DISPLAYPORT;
>>>          break;
>>>      case DRM_MODE_CONNECTOR_HDMIA:
>>>          fallthrough;
>>>      case DRM_MODE_CONNECTOR_HDMIB:
>>>          desc->connector_type = GUD_CONNECTOR_TYPE_HDMI;
>>>          break;
>>>      default:
>>>          desc->connector_type = GUD_CONNECTOR_TYPE_PANEL;
>>>          break;
>>>      };
>>>
>>>
>>> int gud_connector_create(struct gud_device *gdrm, unsigned int index)
>>> {
>>> ...
>>>      switch (desc.connector_type) {
>>>      case GUD_CONNECTOR_TYPE_PANEL:
>>>          connector_type = DRM_MODE_CONNECTOR_USB;
>>>          break;
>>>      case GUD_CONNECTOR_TYPE_VGA:
>>>          connector_type = DRM_MODE_CONNECTOR_VGA;
>>>          break;
>>>      case GUD_CONNECTOR_TYPE_DVI:
>>>          connector_type = DRM_MODE_CONNECTOR_DVID;
>>>          break;
>>>      case GUD_CONNECTOR_TYPE_COMPOSITE:
>>>          connector_type = DRM_MODE_CONNECTOR_Composite;
>>>          break;
>>>      case GUD_CONNECTOR_TYPE_SVIDEO:
>>>          connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>>>          break;
>>>      case GUD_CONNECTOR_TYPE_COMPONENT:
>>>          connector_type = DRM_MODE_CONNECTOR_Component;
>>>          break;
>>>      case GUD_CONNECTOR_TYPE_DISPLAYPORT:
>>>          connector_type = DRM_MODE_CONNECTOR_DisplayPort;
>>>          break;
>>>      case GUD_CONNECTOR_TYPE_HDMI:
>>>          connector_type = DRM_MODE_CONNECTOR_HDMIA;
>>>          break;
>>>      default: /* future types */
>>>          connector_type = DRM_MODE_CONNECTOR_USB;
>>
>> The more I look at it the more I think it should be 'Unknown' here.
>>
> 
> I don't understand this, how will that be better for the user?

As I said before, the display is not connected via USB. The RPi (i.e., 
graphics card) is. The naming would be off.

Best regards
Thomas

> 
>> BTW, can I try this out somehow? I do have an RPi3. Do I need a special
>> disk image?
> 
> The Pi3 doesn'have a USB device/otg connector so I haven't made an image
> for that one. Only the Pi Zero, model A and Pi 4 have that.
> 
> The Pi2 and Pi3 have a USB hub on the soc's single USB port.
> 
> Noralf.
> 
>>
>> Best regards
>> Thomas
>>
>>>          break;
>>>      };
>>>
>>> Noralf.
>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>> -Daniel
>>>>>
>>>>>>
>>>>>> Best regards
>>>>>> Thomas
>>>>>>
>>>>>>>
>>>>>>> Beware, new connector types have in the past resulted in userspace
>>>>>>> burning&crashing. Maybe it's become better ...
>>>>>>>
>>>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>>>
>>>>>>>>      /**
>>>>>>>>       * struct drm_mode_get_connector - Get connector metadata.
>>>>>>>> -- 
>>>>>>>> 2.23.0
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> dri-devel mailing list
>>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> -- 
>>>>>> Thomas Zimmermann
>>>>>> Graphics Driver Developer
>>>>>> SUSE Software Solutions Germany GmbH
>>>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>>>> (HRB 36809, AG Nürnberg)
>>>>>> Geschäftsführer: Felix Imendörffer
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
Noralf Trønnes Jan. 22, 2021, 2:35 p.m. UTC | #11
Den 22.01.2021 13.47, skrev Thomas Zimmermann:
> Hi
> 
> Am 22.01.21 um 12:44 schrieb Noralf Trønnes:
>>
>> And wrt PCI it wouldn't be a PCI connector if the card has some other
>> connector for the display, but if it was possible to connect a display
>> directly to the PCI connector, then yes I would call that a PCI
>> connector.
> 
> You're not connecting a display to the computer. You're connecting an
> RPi and then connect the display to the RPi. The RPi acts like an
> external graphics card.
> 
>> This begs the question: Why does the kernel provide info to userspace
>> about the connector type?
>>
>> My take is that it is so the user can know which display is connected to
>> which port on the computer.
> 
> This exactly illustrates the problem with the current naming. For a
> single output the distinction between bus and connector might be fuzzy.
> As soon as a connected SoC contains multiple connectors. The user then
> sees names such as card1-USB-0 and card1-USB-1, which makes no sense.
> 

If you look at the code I pasted in, you'll see that the SoC connector
types are passed through to the host driver as-is unless they are panel
connectors like DSI/DPI, which will be interpreted as USB (the protocol
does support multiple connectors, but only one can be used at a time).

So for the Pi4 as a display adapter, the host will see card1-HDMI-0 and
card1-HDMI-1, the same is true for the composite output (if enabled) it
shows up as card1-Composite-0 (can't be enabled together with HDMI).

If the Pi4 is used together with a DSI connected touchscreen, it makes
sense to disable the SoC HDMI outputs and the host only will see the
board as card1-USB-0 (I haven't done this exercise yet since there's
problems with getting the official Pi touchscreen to work with vc4 on Pi4).

The USB connector type is most important for tiny displays that is
microcontroller based without Linux running. There are lots of tiny SPI
displays and I expect this market to shift towards USB because it much
easier to connect and the display will be useable on a desktop/server
computer as status displays perhaps. But embedded will also benefit from
having these displays USB interfaced.

Another use case I see is repurposing old Android tablets as USB
displays that can be connected to any computer and become a touchscreen.
In this case I also want the connector to be called card1-USB-0 (I
haven't done any work in this area, old Android is fbdev so some work is
needed for this to happen).

Noralf.

>>
>> What's your opinion?
>>
>>>>
>>>> Ofc as Daniel mentions it's a downside that userspace doesn't know
>>>> about
>>>> the connector type, and who knows when it will updated (if I don't do
>>>> it).
>>>> Weston will name it: "UNNAMED-%d"
>>>> Mutter: "Unknown%d-%d"
>>>> X: "Unknown%d-%d"
>>>>
>>>> Sam and Laurent has discussed adding a PANEL connector type instead of
>>>> adding more connector types for panel connectors. I think that would
>>>> have been a better choice instead of the SPI connector type that I
>>>> added
>>>> in 2019. But I think PANEL was meant for panels connected to an
>>>> internal
>>>> connector.
>>>>
>>>> Here's my protocol connector types and how it's mapped to DRM:
>>>>
>>>> #define GUD_CONNECTOR_TYPE_PANEL        0
>>>> #define GUD_CONNECTOR_TYPE_VGA            1
>>>> #define GUD_CONNECTOR_TYPE_COMPOSITE        2
>>>> #define GUD_CONNECTOR_TYPE_SVIDEO        3
>>>> #define GUD_CONNECTOR_TYPE_COMPONENT        4
>>>> #define GUD_CONNECTOR_TYPE_DVI            5
>>>> #define GUD_CONNECTOR_TYPE_DISPLAYPORT        6
>>>> #define GUD_CONNECTOR_TYPE_HDMI            7
>>>>
>>>> static int gud_gadget_ctrl_get_connector(struct gud_gadget *gdg,
>>>> unsigned int index,
>>>>                       struct gud_connector_descriptor_req *desc)
>>>> {
>>>> ...
>>>>      gconn = &gdg->connectors[index];
>>>>
>>>>      switch (gconn->connector->connector_type) {
>>>>      case DRM_MODE_CONNECTOR_VGA:
>>>>          desc->connector_type = GUD_CONNECTOR_TYPE_VGA;
>>>>          break;
>>>>      case DRM_MODE_CONNECTOR_DVII:
>>>>          fallthrough;
>>>>      case DRM_MODE_CONNECTOR_DVID:
>>>>          fallthrough;
>>>>      case DRM_MODE_CONNECTOR_DVIA:
>>>>          desc->connector_type = GUD_CONNECTOR_TYPE_DVI;
>>>>          break;
>>>>      case DRM_MODE_CONNECTOR_Composite:
>>>>          desc->connector_type = GUD_CONNECTOR_TYPE_COMPOSITE;
>>>>          break;
>>>>      case DRM_MODE_CONNECTOR_SVIDEO:
>>>>          desc->connector_type = GUD_CONNECTOR_TYPE_SVIDEO;
>>>>          break;
>>>>      case DRM_MODE_CONNECTOR_Component:
>>>>          desc->connector_type = GUD_CONNECTOR_TYPE_COMPONENT;
>>>>          break;
>>>>      case DRM_MODE_CONNECTOR_DisplayPort:
>>>>          desc->connector_type = GUD_CONNECTOR_TYPE_DISPLAYPORT;
>>>>          break;
>>>>      case DRM_MODE_CONNECTOR_HDMIA:
>>>>          fallthrough;
>>>>      case DRM_MODE_CONNECTOR_HDMIB:
>>>>          desc->connector_type = GUD_CONNECTOR_TYPE_HDMI;
>>>>          break;
>>>>      default:
>>>>          desc->connector_type = GUD_CONNECTOR_TYPE_PANEL;
>>>>          break;
>>>>      };
>>>>
>>>>
>>>> int gud_connector_create(struct gud_device *gdrm, unsigned int index)
>>>> {
>>>> ...
>>>>      switch (desc.connector_type) {
>>>>      case GUD_CONNECTOR_TYPE_PANEL:
>>>>          connector_type = DRM_MODE_CONNECTOR_USB;
>>>>          break;
>>>>      case GUD_CONNECTOR_TYPE_VGA:
>>>>          connector_type = DRM_MODE_CONNECTOR_VGA;
>>>>          break;
>>>>      case GUD_CONNECTOR_TYPE_DVI:
>>>>          connector_type = DRM_MODE_CONNECTOR_DVID;
>>>>          break;
>>>>      case GUD_CONNECTOR_TYPE_COMPOSITE:
>>>>          connector_type = DRM_MODE_CONNECTOR_Composite;
>>>>          break;
>>>>      case GUD_CONNECTOR_TYPE_SVIDEO:
>>>>          connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>>>>          break;
>>>>      case GUD_CONNECTOR_TYPE_COMPONENT:
>>>>          connector_type = DRM_MODE_CONNECTOR_Component;
>>>>          break;
>>>>      case GUD_CONNECTOR_TYPE_DISPLAYPORT:
>>>>          connector_type = DRM_MODE_CONNECTOR_DisplayPort;
>>>>          break;
>>>>      case GUD_CONNECTOR_TYPE_HDMI:
>>>>          connector_type = DRM_MODE_CONNECTOR_HDMIA;
>>>>          break;
>>>>      default: /* future types */
>>>>          connector_type = DRM_MODE_CONNECTOR_USB;
>>>
>>> The more I look at it the more I think it should be 'Unknown' here.
>>>
>>
>> I don't understand this, how will that be better for the user?
> 
> As I said before, the display is not connected via USB. The RPi (i.e.,
> graphics card) is. The naming would be off.
> 
> Best regards
> Thomas
> 
>>
>>> BTW, can I try this out somehow? I do have an RPi3. Do I need a special
>>> disk image?
>>
>> The Pi3 doesn'have a USB device/otg connector so I haven't made an image
>> for that one. Only the Pi Zero, model A and Pi 4 have that.
>>
>> The Pi2 and Pi3 have a USB hub on the soc's single USB port.
>>
>> Noralf.
>>
>>>
>>> Best regards
>>> Thomas
>>>
>>>>          break;
>>>>      };
>>>>
>>>> Noralf.
>>>>
>>>>> Best regards
>>>>> Thomas
>>>>>
>>>>>> -Daniel
>>>>>>
>>>>>>>
>>>>>>> Best regards
>>>>>>> Thomas
>>>>>>>
>>>>>>>>
>>>>>>>> Beware, new connector types have in the past resulted in userspace
>>>>>>>> burning&crashing. Maybe it's become better ...
>>>>>>>>
>>>>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>>>>
>>>>>>>>>      /**
>>>>>>>>>       * struct drm_mode_get_connector - Get connector metadata.
>>>>>>>>> -- 
>>>>>>>>> 2.23.0
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> dri-devel mailing list
>>>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> -- 
>>>>>>> Thomas Zimmermann
>>>>>>> Graphics Driver Developer
>>>>>>> SUSE Software Solutions Germany GmbH
>>>>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>>>>> (HRB 36809, AG Nürnberg)
>>>>>>> Geschäftsführer: Felix Imendörffer
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>
>>>
>
Thomas Zimmermann Jan. 22, 2021, 2:55 p.m. UTC | #12
Hi

Am 22.01.21 um 15:35 schrieb Noralf Trønnes:
> 
> 
> Den 22.01.2021 13.47, skrev Thomas Zimmermann:
>> Hi
>>
>> Am 22.01.21 um 12:44 schrieb Noralf Trønnes:
>>>
>>> And wrt PCI it wouldn't be a PCI connector if the card has some other
>>> connector for the display, but if it was possible to connect a display
>>> directly to the PCI connector, then yes I would call that a PCI
>>> connector.
>>
>> You're not connecting a display to the computer. You're connecting an
>> RPi and then connect the display to the RPi. The RPi acts like an
>> external graphics card.
>>
>>> This begs the question: Why does the kernel provide info to userspace
>>> about the connector type?
>>>
>>> My take is that it is so the user can know which display is connected to
>>> which port on the computer.
>>
>> This exactly illustrates the problem with the current naming. For a
>> single output the distinction between bus and connector might be fuzzy.
>> As soon as a connected SoC contains multiple connectors. The user then
>> sees names such as card1-USB-0 and card1-USB-1, which makes no sense.
>>
> 
> If you look at the code I pasted in, you'll see that the SoC connector
> types are passed through to the host driver as-is unless they are panel
> connectors like DSI/DPI, which will be interpreted as USB (the protocol
> does support multiple connectors, but only one can be used at a time).
> 
> So for the Pi4 as a display adapter, the host will see card1-HDMI-0 and
> card1-HDMI-1, the same is true for the composite output (if enabled) it
> shows up as card1-Composite-0 (can't be enabled together with HDMI).
> 
> If the Pi4 is used together with a DSI connected touchscreen, it makes
> sense to disable the SoC HDMI outputs and the host only will see the
> board as card1-USB-0 (I haven't done this exercise yet since there's
> problems with getting the official Pi touchscreen to work with vc4 on Pi4).

I saw that. I can even get your point about using USB for panel (still 
don't agree). But you're also using USB as default case.

Best regards
Thomas

> 
> The USB connector type is most important for tiny displays that is
> microcontroller based without Linux running. There are lots of tiny SPI
> displays and I expect this market to shift towards USB because it much
> easier to connect and the display will be useable on a desktop/server
> computer as status displays perhaps. But embedded will also benefit from
> having these displays USB interfaced.
> 
> Another use case I see is repurposing old Android tablets as USB
> displays that can be connected to any computer and become a touchscreen.
> In this case I also want the connector to be called card1-USB-0 (I
> haven't done any work in this area, old Android is fbdev so some work is
> needed for this to happen).
> 
> Noralf.
> 
>>>
>>> What's your opinion?
>>>
>>>>>
>>>>> Ofc as Daniel mentions it's a downside that userspace doesn't know
>>>>> about
>>>>> the connector type, and who knows when it will updated (if I don't do
>>>>> it).
>>>>> Weston will name it: "UNNAMED-%d"
>>>>> Mutter: "Unknown%d-%d"
>>>>> X: "Unknown%d-%d"
>>>>>
>>>>> Sam and Laurent has discussed adding a PANEL connector type instead of
>>>>> adding more connector types for panel connectors. I think that would
>>>>> have been a better choice instead of the SPI connector type that I
>>>>> added
>>>>> in 2019. But I think PANEL was meant for panels connected to an
>>>>> internal
>>>>> connector.
>>>>>
>>>>> Here's my protocol connector types and how it's mapped to DRM:
>>>>>
>>>>> #define GUD_CONNECTOR_TYPE_PANEL        0
>>>>> #define GUD_CONNECTOR_TYPE_VGA            1
>>>>> #define GUD_CONNECTOR_TYPE_COMPOSITE        2
>>>>> #define GUD_CONNECTOR_TYPE_SVIDEO        3
>>>>> #define GUD_CONNECTOR_TYPE_COMPONENT        4
>>>>> #define GUD_CONNECTOR_TYPE_DVI            5
>>>>> #define GUD_CONNECTOR_TYPE_DISPLAYPORT        6
>>>>> #define GUD_CONNECTOR_TYPE_HDMI            7
>>>>>
>>>>> static int gud_gadget_ctrl_get_connector(struct gud_gadget *gdg,
>>>>> unsigned int index,
>>>>>                        struct gud_connector_descriptor_req *desc)
>>>>> {
>>>>> ...
>>>>>       gconn = &gdg->connectors[index];
>>>>>
>>>>>       switch (gconn->connector->connector_type) {
>>>>>       case DRM_MODE_CONNECTOR_VGA:
>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_VGA;
>>>>>           break;
>>>>>       case DRM_MODE_CONNECTOR_DVII:
>>>>>           fallthrough;
>>>>>       case DRM_MODE_CONNECTOR_DVID:
>>>>>           fallthrough;
>>>>>       case DRM_MODE_CONNECTOR_DVIA:
>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_DVI;
>>>>>           break;
>>>>>       case DRM_MODE_CONNECTOR_Composite:
>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_COMPOSITE;
>>>>>           break;
>>>>>       case DRM_MODE_CONNECTOR_SVIDEO:
>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_SVIDEO;
>>>>>           break;
>>>>>       case DRM_MODE_CONNECTOR_Component:
>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_COMPONENT;
>>>>>           break;
>>>>>       case DRM_MODE_CONNECTOR_DisplayPort:
>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_DISPLAYPORT;
>>>>>           break;
>>>>>       case DRM_MODE_CONNECTOR_HDMIA:
>>>>>           fallthrough;
>>>>>       case DRM_MODE_CONNECTOR_HDMIB:
>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_HDMI;
>>>>>           break;
>>>>>       default:
>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_PANEL;
>>>>>           break;
>>>>>       };
>>>>>
>>>>>
>>>>> int gud_connector_create(struct gud_device *gdrm, unsigned int index)
>>>>> {
>>>>> ...
>>>>>       switch (desc.connector_type) {
>>>>>       case GUD_CONNECTOR_TYPE_PANEL:
>>>>>           connector_type = DRM_MODE_CONNECTOR_USB;
>>>>>           break;
>>>>>       case GUD_CONNECTOR_TYPE_VGA:
>>>>>           connector_type = DRM_MODE_CONNECTOR_VGA;
>>>>>           break;
>>>>>       case GUD_CONNECTOR_TYPE_DVI:
>>>>>           connector_type = DRM_MODE_CONNECTOR_DVID;
>>>>>           break;
>>>>>       case GUD_CONNECTOR_TYPE_COMPOSITE:
>>>>>           connector_type = DRM_MODE_CONNECTOR_Composite;
>>>>>           break;
>>>>>       case GUD_CONNECTOR_TYPE_SVIDEO:
>>>>>           connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>>>>>           break;
>>>>>       case GUD_CONNECTOR_TYPE_COMPONENT:
>>>>>           connector_type = DRM_MODE_CONNECTOR_Component;
>>>>>           break;
>>>>>       case GUD_CONNECTOR_TYPE_DISPLAYPORT:
>>>>>           connector_type = DRM_MODE_CONNECTOR_DisplayPort;
>>>>>           break;
>>>>>       case GUD_CONNECTOR_TYPE_HDMI:
>>>>>           connector_type = DRM_MODE_CONNECTOR_HDMIA;
>>>>>           break;
>>>>>       default: /* future types */
>>>>>           connector_type = DRM_MODE_CONNECTOR_USB;
>>>>
>>>> The more I look at it the more I think it should be 'Unknown' here.
>>>>
>>>
>>> I don't understand this, how will that be better for the user?
>>
>> As I said before, the display is not connected via USB. The RPi (i.e.,
>> graphics card) is. The naming would be off.
>>
>> Best regards
>> Thomas
>>
>>>
>>>> BTW, can I try this out somehow? I do have an RPi3. Do I need a special
>>>> disk image?
>>>
>>> The Pi3 doesn'have a USB device/otg connector so I haven't made an image
>>> for that one. Only the Pi Zero, model A and Pi 4 have that.
>>>
>>> The Pi2 and Pi3 have a USB hub on the soc's single USB port.
>>>
>>> Noralf.
>>>
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>>           break;
>>>>>       };
>>>>>
>>>>> Noralf.
>>>>>
>>>>>> Best regards
>>>>>> Thomas
>>>>>>
>>>>>>> -Daniel
>>>>>>>
>>>>>>>>
>>>>>>>> Best regards
>>>>>>>> Thomas
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Beware, new connector types have in the past resulted in userspace
>>>>>>>>> burning&crashing. Maybe it's become better ...
>>>>>>>>>
>>>>>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>>>>>
>>>>>>>>>>       /**
>>>>>>>>>>        * struct drm_mode_get_connector - Get connector metadata.
>>>>>>>>>> -- 
>>>>>>>>>> 2.23.0
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> dri-devel mailing list
>>>>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> Thomas Zimmermann
>>>>>>>> Graphics Driver Developer
>>>>>>>> SUSE Software Solutions Germany GmbH
>>>>>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>>>>>> (HRB 36809, AG Nürnberg)
>>>>>>>> Geschäftsführer: Felix Imendörffer
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Noralf Trønnes Jan. 22, 2021, 6:39 p.m. UTC | #13
Den 22.01.2021 15.55, skrev Thomas Zimmermann:
> Hi
> 
> Am 22.01.21 um 15:35 schrieb Noralf Trønnes:
>>
>>
>> Den 22.01.2021 13.47, skrev Thomas Zimmermann:
>>> Hi
>>>
>>> Am 22.01.21 um 12:44 schrieb Noralf Trønnes:
>>>>
>>>> And wrt PCI it wouldn't be a PCI connector if the card has some other
>>>> connector for the display, but if it was possible to connect a display
>>>> directly to the PCI connector, then yes I would call that a PCI
>>>> connector.
>>>
>>> You're not connecting a display to the computer. You're connecting an
>>> RPi and then connect the display to the RPi. The RPi acts like an
>>> external graphics card.
>>>
>>>> This begs the question: Why does the kernel provide info to userspace
>>>> about the connector type?
>>>>
>>>> My take is that it is so the user can know which display is
>>>> connected to
>>>> which port on the computer.
>>>
>>> This exactly illustrates the problem with the current naming. For a
>>> single output the distinction between bus and connector might be fuzzy.
>>> As soon as a connected SoC contains multiple connectors. The user then
>>> sees names such as card1-USB-0 and card1-USB-1, which makes no sense.
>>>
>>
>> If you look at the code I pasted in, you'll see that the SoC connector
>> types are passed through to the host driver as-is unless they are panel
>> connectors like DSI/DPI, which will be interpreted as USB (the protocol
>> does support multiple connectors, but only one can be used at a time).
>>
>> So for the Pi4 as a display adapter, the host will see card1-HDMI-0 and
>> card1-HDMI-1, the same is true for the composite output (if enabled) it
>> shows up as card1-Composite-0 (can't be enabled together with HDMI).
>>
>> If the Pi4 is used together with a DSI connected touchscreen, it makes
>> sense to disable the SoC HDMI outputs and the host only will see the
>> board as card1-USB-0 (I haven't done this exercise yet since there's
>> problems with getting the official Pi touchscreen to work with vc4 on
>> Pi4).
> 
> I saw that. I can even get your point about using USB for panel (still
> don't agree). But you're also using USB as default case.
> 

In the host driver the default case is to catch future additions to the
protocol. In this case the driver doesn't know the connector type, but
by using USB at least the user knows where to look.

On the gadget side these are the current types that will hit the default
case:

These are panels:
DRM_MODE_CONNECTOR_LVDS
DRM_MODE_CONNECTOR_eDP
DRM_MODE_CONNECTOR_DSI
DRM_MODE_CONNECTOR_DPI
DRM_MODE_CONNECTOR_SPI

This also seems to be used primarily by panels:
DRM_MODE_CONNECTOR_Unknown

Only used by i915, amd, radeon, very unlikely on a gadget:
DRM_MODE_CONNECTOR_9PinDIN

This one I should probably map to GUD_CONNECTOR_TYPE_COMPOSITE:
DRM_MODE_CONNECTOR_TV

Very unlikely on a gadget:
DRM_MODE_CONNECTOR_VIRTUAL

Future connector types will also be reported in the protocol as
GUD_CONNECTOR_TYPE_PANEL and thus on the host as DRM_MODE_CONNECTOR_USB.
The gadget driver will ofc be updated when new non-panel connector types
show up.

And this makes sense to me :-)

Giving the user a connector named "Unknown-1" does not make sense to me.

Noralf.


> Best regards
> Thomas
> 
>>
>> The USB connector type is most important for tiny displays that is
>> microcontroller based without Linux running. There are lots of tiny SPI
>> displays and I expect this market to shift towards USB because it much
>> easier to connect and the display will be useable on a desktop/server
>> computer as status displays perhaps. But embedded will also benefit from
>> having these displays USB interfaced.
>>
>> Another use case I see is repurposing old Android tablets as USB
>> displays that can be connected to any computer and become a touchscreen.
>> In this case I also want the connector to be called card1-USB-0 (I
>> haven't done any work in this area, old Android is fbdev so some work is
>> needed for this to happen).
>>
>> Noralf.
>>
>>>>
>>>> What's your opinion?
>>>>
>>>>>>
>>>>>> Ofc as Daniel mentions it's a downside that userspace doesn't know
>>>>>> about
>>>>>> the connector type, and who knows when it will updated (if I don't do
>>>>>> it).
>>>>>> Weston will name it: "UNNAMED-%d"
>>>>>> Mutter: "Unknown%d-%d"
>>>>>> X: "Unknown%d-%d"
>>>>>>
>>>>>> Sam and Laurent has discussed adding a PANEL connector type
>>>>>> instead of
>>>>>> adding more connector types for panel connectors. I think that would
>>>>>> have been a better choice instead of the SPI connector type that I
>>>>>> added
>>>>>> in 2019. But I think PANEL was meant for panels connected to an
>>>>>> internal
>>>>>> connector.
>>>>>>
>>>>>> Here's my protocol connector types and how it's mapped to DRM:
>>>>>>
>>>>>> #define GUD_CONNECTOR_TYPE_PANEL        0
>>>>>> #define GUD_CONNECTOR_TYPE_VGA            1
>>>>>> #define GUD_CONNECTOR_TYPE_COMPOSITE        2
>>>>>> #define GUD_CONNECTOR_TYPE_SVIDEO        3
>>>>>> #define GUD_CONNECTOR_TYPE_COMPONENT        4
>>>>>> #define GUD_CONNECTOR_TYPE_DVI            5
>>>>>> #define GUD_CONNECTOR_TYPE_DISPLAYPORT        6
>>>>>> #define GUD_CONNECTOR_TYPE_HDMI            7
>>>>>>
>>>>>> static int gud_gadget_ctrl_get_connector(struct gud_gadget *gdg,
>>>>>> unsigned int index,
>>>>>>                        struct gud_connector_descriptor_req *desc)
>>>>>> {
>>>>>> ...
>>>>>>       gconn = &gdg->connectors[index];
>>>>>>
>>>>>>       switch (gconn->connector->connector_type) {
>>>>>>       case DRM_MODE_CONNECTOR_VGA:
>>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_VGA;
>>>>>>           break;
>>>>>>       case DRM_MODE_CONNECTOR_DVII:
>>>>>>           fallthrough;
>>>>>>       case DRM_MODE_CONNECTOR_DVID:
>>>>>>           fallthrough;
>>>>>>       case DRM_MODE_CONNECTOR_DVIA:
>>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_DVI;
>>>>>>           break;
>>>>>>       case DRM_MODE_CONNECTOR_Composite:
>>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_COMPOSITE;
>>>>>>           break;
>>>>>>       case DRM_MODE_CONNECTOR_SVIDEO:
>>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_SVIDEO;
>>>>>>           break;
>>>>>>       case DRM_MODE_CONNECTOR_Component:
>>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_COMPONENT;
>>>>>>           break;
>>>>>>       case DRM_MODE_CONNECTOR_DisplayPort:
>>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_DISPLAYPORT;
>>>>>>           break;
>>>>>>       case DRM_MODE_CONNECTOR_HDMIA:
>>>>>>           fallthrough;
>>>>>>       case DRM_MODE_CONNECTOR_HDMIB:
>>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_HDMI;
>>>>>>           break;
>>>>>>       default:
>>>>>>           desc->connector_type = GUD_CONNECTOR_TYPE_PANEL;
>>>>>>           break;
>>>>>>       };
>>>>>>
>>>>>>
>>>>>> int gud_connector_create(struct gud_device *gdrm, unsigned int index)
>>>>>> {
>>>>>> ...
>>>>>>       switch (desc.connector_type) {
>>>>>>       case GUD_CONNECTOR_TYPE_PANEL:
>>>>>>           connector_type = DRM_MODE_CONNECTOR_USB;
>>>>>>           break;
>>>>>>       case GUD_CONNECTOR_TYPE_VGA:
>>>>>>           connector_type = DRM_MODE_CONNECTOR_VGA;
>>>>>>           break;
>>>>>>       case GUD_CONNECTOR_TYPE_DVI:
>>>>>>           connector_type = DRM_MODE_CONNECTOR_DVID;
>>>>>>           break;
>>>>>>       case GUD_CONNECTOR_TYPE_COMPOSITE:
>>>>>>           connector_type = DRM_MODE_CONNECTOR_Composite;
>>>>>>           break;
>>>>>>       case GUD_CONNECTOR_TYPE_SVIDEO:
>>>>>>           connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>>>>>>           break;
>>>>>>       case GUD_CONNECTOR_TYPE_COMPONENT:
>>>>>>           connector_type = DRM_MODE_CONNECTOR_Component;
>>>>>>           break;
>>>>>>       case GUD_CONNECTOR_TYPE_DISPLAYPORT:
>>>>>>           connector_type = DRM_MODE_CONNECTOR_DisplayPort;
>>>>>>           break;
>>>>>>       case GUD_CONNECTOR_TYPE_HDMI:
>>>>>>           connector_type = DRM_MODE_CONNECTOR_HDMIA;
>>>>>>           break;
>>>>>>       default: /* future types */
>>>>>>           connector_type = DRM_MODE_CONNECTOR_USB;
>>>>>
>>>>> The more I look at it the more I think it should be 'Unknown' here.
>>>>>
>>>>
>>>> I don't understand this, how will that be better for the user?
>>>
>>> As I said before, the display is not connected via USB. The RPi (i.e.,
>>> graphics card) is. The naming would be off.
>>>
>>> Best regards
>>> Thomas
>>>
>>>>
>>>>> BTW, can I try this out somehow? I do have an RPi3. Do I need a
>>>>> special
>>>>> disk image?
>>>>
>>>> The Pi3 doesn'have a USB device/otg connector so I haven't made an
>>>> image
>>>> for that one. Only the Pi Zero, model A and Pi 4 have that.
>>>>
>>>> The Pi2 and Pi3 have a USB hub on the soc's single USB port.
>>>>
>>>> Noralf.
>>>>
>>>>>
>>>>> Best regards
>>>>> Thomas
>>>>>
>>>>>>           break;
>>>>>>       };
>>>>>>
>>>>>> Noralf.
>>>>>>
>>>>>>> Best regards
>>>>>>> Thomas
>>>>>>>
>>>>>>>> -Daniel
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Best regards
>>>>>>>>> Thomas
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Beware, new connector types have in the past resulted in
>>>>>>>>>> userspace
>>>>>>>>>> burning&crashing. Maybe it's become better ...
>>>>>>>>>>
>>>>>>>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>>>>>>
>>>>>>>>>>>       /**
>>>>>>>>>>>        * struct drm_mode_get_connector - Get connector metadata.
>>>>>>>>>>> -- 
>>>>>>>>>>> 2.23.0
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> dri-devel mailing list
>>>>>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -- 
>>>>>>>>> Thomas Zimmermann
>>>>>>>>> Graphics Driver Developer
>>>>>>>>> SUSE Software Solutions Germany GmbH
>>>>>>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>>>>>>> (HRB 36809, AG Nürnberg)
>>>>>>>>> Geschäftsführer: Felix Imendörffer
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>
>>>>>
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
diff mbox series

Patch

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index fed66a03c7ae..33024cc5d26e 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -367,6 +367,7 @@  enum drm_mode_subconnector {
 #define DRM_MODE_CONNECTOR_DPI		17
 #define DRM_MODE_CONNECTOR_WRITEBACK	18
 #define DRM_MODE_CONNECTOR_SPI		19
+#define DRM_MODE_CONNECTOR_USB		20
 
 /**
  * struct drm_mode_get_connector - Get connector metadata.