[xf86-video-armada][PATCH] common: fix up output naming convention

[Stefan Agner]

On 06.09.2018 02:29, Russell King wrote:
> Hi Stefan,
> 
> On Wed, Sep 05, 2018 at 05:44:49PM -0700, Stefan Agner wrote:
>> Currently some output type names are missing. Also different naming
>> connventions are used than in the kernel and in other drivers such as
>> the modesetting driver (e.g. missing dash between connector type and
>> connector type id).
>>
>> Follow modesetting and use the same output names introduced in the
>> in the xserver git repo with commit 139e36dd5cba ("modesetting:
>> fix up output naming convention")
>>
>> This will break backwards compatibility with existing xorg.conf's that
>> reference output names, but the alternative is to create a separate
>> counting system, further disconnecting from the kernel names.
> 
> The convention that xf86-video-armada follows is from the i915 driver
> rather than modesetting or the kernel.

Hm I see, and it seems that Intel sticks to that naming still:
https://cgit.freedesktop.org/xorg/driver/xf86-video-intel/tree/src/sna/sna_display.c#n4769

However, since Vivante HW can run with armada or modesetting driver it
would be probably better if those two are in sync...

> 
> I know users of this driver have an xorg.conf with a section to
> configure (eg) display rotation or preferred mode, so this change will
> cause a user visible regression.
> 

Yes, and that is mentioned in the commit message. 

Apparently that was OK for modesetting...

To be honest, I do not care that much. I think being in sync with
modesetting would be advantageous, but if you feel it is not worth the
regression, I will send a patch just adding the new connector types.

> Also, you seem to be introducing a different coding style - nowhere in
> the file you are modifying are four spaces used for indentation - it is
> all tabs.  Also note the preference for named initialisers.

Depending on libdrm version not all preprocessor defines are present...
Do we have a minimum libdrm version we support?

Virtual/DSI got added 2.4.61
DPI got added 2.4.81

Alternatively, we could use ifdefs for the latest ones, e.g.:

#ifdef DRM_MODE_CONNECTOR_VIRTUAL
	[DRM_MODE_CONNECTOR_VIRTUAL]       = "Virtual",
#endif

--
Stefan

> 
> Thanks.
> 
>>
>> Signed-off-by: Stefan Agner <stefan at agner.ch>
>> ---
>>  src/common_drm_conn.c | 34 +++++++++++++++++++---------------
>>  1 file changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/common_drm_conn.c b/src/common_drm_conn.c
>> index 9f73d20..77ed96b 100644
>> --- a/src/common_drm_conn.c
>> +++ b/src/common_drm_conn.c
>> @@ -372,21 +372,25 @@ static const xf86OutputFuncsRec drm_output_funcs = {
>>  	.destroy = common_drm_conn_destroy,
>>  };
>>
>> -/* Convert libdrm's connector type to Xorg name */
>>  static const char *const output_names[] = {
>> -	[DRM_MODE_CONNECTOR_Unknown]     = "None",
>> -	[DRM_MODE_CONNECTOR_VGA]         = "VGA",
>> -	[DRM_MODE_CONNECTOR_DVII]        = "DVI",
>> -	[DRM_MODE_CONNECTOR_DVID]        = "DVI",
>> -	[DRM_MODE_CONNECTOR_DVIA]        = "DVI",
>> -	[DRM_MODE_CONNECTOR_Composite]   = "Composite",
>> -	[DRM_MODE_CONNECTOR_SVIDEO]      = "TV",
>> -	[DRM_MODE_CONNECTOR_LVDS]        = "LVDS",
>> -	[DRM_MODE_CONNECTOR_Component]   = "CTV",
>> -	[DRM_MODE_CONNECTOR_9PinDIN]     = "DIN",
>> -	[DRM_MODE_CONNECTOR_DisplayPort] = "DP",
>> -	[DRM_MODE_CONNECTOR_HDMIA]       = "HDMI",
>> -	[DRM_MODE_CONNECTOR_HDMIB]       = "HDMI",
>> +    "None",
>> +    "VGA",
>> +    "DVI-I",
>> +    "DVI-D",
>> +    "DVI-A",
>> +    "Composite",
>> +    "SVIDEO",
>> +    "LVDS",
>> +    "Component",
>> +    "DIN",
>> +    "DP",
>> +    "HDMI",
>> +    "HDMI-B",
>> +    "TV",
>> +    "eDP",
>> +    "Virtual",
>> +    "DSI",
>> +    "DPI",
>>  };
>>
>>  static const char *common_drm_output_name(uint32_t type)
>> @@ -436,7 +440,7 @@ void common_drm_conn_init(ScrnInfoPtr pScrn, uint32_t id)
>>  		return;
>>  	}
>>
>> -	snprintf(name, sizeof(name), "%s%d",
>> +	snprintf(name, sizeof(name), "%s-%d",
>>  		 common_drm_output_name(koutput->connector_type),
>>  		 koutput->connector_type_id);
>>
>> --
>> 2.18.0
>>