[QXL PATCH 1/2] Make output name numbering 0-based

[Jonathon Jongsma]

The QXL driver names its outputs starting at 0 (e.g. Virtual-0,
Virtual-1, etc). This code was presumably copy/pasted from a different
driver, and is not necessary for the QXL driver. Other drivers simply
use the kernel connector_type_id which starts at 1. For example, the
modsetting driver changed from 0-based names to 1-based names for the
same reason in xserver commit 139e36dd.

This will help to make it easier to identify which xrandr outputs belong
to which drm connector without requiring as many driver-specific
special-cases.

This change might effect custom xorg configurations that references a
specific output name. But the same change was made in modesetting driver
despite that possibility.

Signed-off-by: Jonathon Jongsma <***@redhat.com>
---
src/qxl_drmmode.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qxl_drmmode.c b/src/qxl_drmmode.c
index a2f84b1..a814859 100644
--- a/src/qxl_drmmode.c
+++ b/src/qxl_drmmode.c
@@ -765,8 +765,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
}
}

- /* need to do smart conversion here for compat with non-kms ATI driver */
- snprintf(name, 32, "%s-%d", output_names[koutput->connector_type], koutput->connector_type_id - 1);
+ snprintf(name, 32, "%s-%d", output_names[koutput->connector_type], koutput->connector_type_id);


output = xf86OutputCreate (pScrn, &drmmode_output_funcs, name);

[Jonathon Jongsma]

The xrandr output name used by the QXL driver is based on the drm
connector type, but the names do not match the kernel names (see
/drivers/gpu/drm/drm_connector.c) or the modsetting driver names (see
hw/xfree86/drivers/modesetting/drmmode_display.c). Making these more
consistent will require less driver-specific special-case code if a user
wants to match an xrandr output to a drm connector.

Note that this patch should not actually change any behavior, since the
QXL driver only uses the 'Virtual' connector type, so this is done only
for consistency.

Signed-off-by: Jonathon Jongsma <***@redhat.com>
---
src/qxl_drmmode.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/qxl_drmmode.c b/src/qxl_drmmode.c
index a814859..0f95499 100644
--- a/src/qxl_drmmode.c
+++ b/src/qxl_drmmode.c
@@ -722,20 +722,22 @@ static int subpixel_conv_table[7] = { 0, SubPixelUnknown,

const char *output_names[] = { "None",
"VGA",
- "DVI",
- "DVI",
- "DVI",
+ "DVI-I",
+ "DVI-D",
+ "DVI-A",
"Composite",
- "S-video",
+ "SVIDEO",
"LVDS",
- "CTV",
+ "Component",
"DIN",
- "DisplayPort",
- "HDMI",
+ "DP",
"HDMI",
+ "HDMI-B",
"TV",
"eDP",
- "Virtual"
+ "Virtual",
+ "DSI",
+ "DPI",
};

static void

[Frediano Ziglio]

Typo in title "modesetting"

Post by Jonathon Jongsma
The xrandr output name used by the QXL driver is based on the drm
connector type, but the names do not match the kernel names (see
/drivers/gpu/drm/drm_connector.c) or the modsetting driver names (see

Similar here

Post by Jonathon Jongsma
hw/xfree86/drivers/modesetting/drmmode_display.c). Making these more
consistent will require less driver-specific special-case code if a user
wants to match an xrandr output to a drm connector.
Note that this patch should not actually change any behavior, since the
QXL driver only uses the 'Virtual' connector type, so this is done only
for consistency.
---
src/qxl_drmmode.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/qxl_drmmode.c b/src/qxl_drmmode.c
index a814859..0f95499 100644
--- a/src/qxl_drmmode.c
+++ b/src/qxl_drmmode.c
@@ -722,20 +722,22 @@ static int subpixel_conv_table[7] = { 0,
SubPixelUnknown,
const char *output_names[] = { "None",
"VGA",
- "DVI",
- "DVI",
- "DVI",
+ "DVI-I",
+ "DVI-D",
+ "DVI-A",
"Composite",
- "S-video",
+ "SVIDEO",
"LVDS",
- "CTV",
+ "Component",
"DIN",
- "DisplayPort",
- "HDMI",
+ "DP",
"HDMI",
+ "HDMI-B",
"TV",
"eDP",
- "Virtual"
+ "Virtual",
+ "DSI",
+ "DPI",
};
static void

Surely OT, I would change the indentation to

const char *output_names[] = {
"None",
"VGA",

and possibly have

static const char *const output_names[] = {


Otherwise,

Acked-by: Frediano Ziglio <***@redhat.com>

Frediano
_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-deve

[Frediano Ziglio]

I think you mean "Make output name numbering 1-based".

Post by Jonathon Jongsma
The QXL driver names its outputs starting at 0 (e.g. Virtual-0,
Virtual-1, etc). This code was presumably copy/pasted from a different
driver, and is not necessary for the QXL driver. Other drivers simply
use the kernel connector_type_id which starts at 1. For example, the
modsetting driver changed from 0-based names to 1-based names for the

typo: modesetting

Post by Jonathon Jongsma
same reason in xserver commit 139e36dd.
This will help to make it easier to identify which xrandr outputs belong
to which drm connector without requiring as many driver-specific
special-cases.
This change might effect custom xorg configurations that references a
specific output name. But the same change was made in modesetting driver
despite that possibility.
---
src/qxl_drmmode.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qxl_drmmode.c b/src/qxl_drmmode.c
index a2f84b1..a814859 100644
--- a/src/qxl_drmmode.c
+++ b/src/qxl_drmmode.c
@@ -765,8 +765,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int num)
}
}
- /* need to do smart conversion here for compat with non-kms ATI driver */
- snprintf(name, 32, "%s-%d", output_names[koutput->connector_type],
koutput->connector_type_id - 1);
+ snprintf(name, 32, "%s-%d", output_names[koutput->connector_type],
koutput->connector_type_id);
output = xf86OutputCreate (pScrn, &drmmode_output_funcs, name);

Otherwise looks good.

However is it possible that you have to handle both cases (0-index and
1-index) due to different packages installations. Is not guaranteed that
you'll have the new Xorg driver installed so this code instead of making
it easier will make more complicated.

Frediano
_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-dev

[Jonathon Jongsma]

Post by Frediano Ziglio
I think you mean "Make output name numbering 1-based".

Post by Jonathon Jongsma
The QXL driver names its outputs starting at 0 (e.g. Virtual-0,
Virtual-1, etc). This code was presumably copy/pasted from a
different
driver, and is not necessary for the QXL driver. Other drivers simply
use the kernel connector_type_id which starts at 1. For example, the
modsetting driver changed from 0-based names to 1-based names for the

typo: modesetting

Post by Jonathon Jongsma
same reason in xserver commit 139e36dd.
This will help to make it easier to identify which xrandr outputs belong
to which drm connector without requiring as many driver-specific
special-cases.
This change might effect custom xorg configurations that references a
specific output name. But the same change was made in modesetting driver
despite that possibility.
---
src/qxl_drmmode.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qxl_drmmode.c b/src/qxl_drmmode.c
index a2f84b1..a814859 100644
--- a/src/qxl_drmmode.c
+++ b/src/qxl_drmmode.c
@@ -765,8 +765,7 @@ drmmode_output_init(ScrnInfoPtr pScrn,
drmmode_ptr
drmmode, int num)
}
}
- /* need to do smart conversion here for compat with non-kms ATI driver */
- snprintf(name, 32, "%s-%d", output_names[koutput-

Post by Jonathon Jongsma
connector_type],

koutput->connector_type_id - 1);
+ snprintf(name, 32, "%s-%d", output_names[koutput-

Post by Jonathon Jongsma
connector_type],

koutput->connector_type_id);
output = xf86OutputCreate (pScrn, &drmmode_output_funcs, name);

Otherwise looks good.
However is it possible that you have to handle both cases (0-index and
1-index) due to different packages installations. Is not guaranteed that
you'll have the new Xorg driver installed so this code instead of making
it easier will make more complicated.

Yes, this change will theoretically make it slightly more complicated.
But the impact is pretty small I think. For example, in the case that
I'm working on right now (finding the xrandr output that matches a drm
output), the impact might be:

Without this patch:
* Loop through the xrandr outputs and see if any of them is named
'Virtual-0'.
* If there is such an output, increment the counter before comparing
against the drm output name

With this patch:
* The algorithm above still works.

If you instead chose to handle this case by looking exclusively at the
vendor_id/device_id of the device, then that does get more difficult
after this change. But it's pretty minor IMO.

Jonathon

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel