Discussion:
[QXL PATCH 0/2] Minor QXL patches
Jonathon Jongsma
2018-11-12 20:06:12 UTC
Permalink
These QXL patches were both reviewed and ACKed by Frediano, but
apparently neither of us has commit rights to the xf86-video-qxl
repository. I'm not sure if this is since the move to gitlab or if we
never had commit rights to this repository. In any case, it looks like
we'll need an Xorg developer to review and/or push the patches upstream.

Thanks,
Jonathon


Jonathon Jongsma (2):
Make output name numbering 0-based
Make output names match modesetting driver

src/qxl_drmmode.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
--
2.17.2

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: h
Jonathon Jongsma
2018-11-12 20:06:13 UTC
Permalink
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
modesetting driver changed from 1-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>
Acked-by: Frediano Ziglio <***@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);
--
2.17.2

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/l
Frediano Ziglio
2018-11-12 22:06:59 UTC
Permalink
Title should be "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
modesetting driver changed from 1-based names to 1-based names for the
maybe " ... from 0-based names to 1-based names ....", from 1 to 1 does
not change much.
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.
yes, acked.
Post by Jonathon Jongsma
---
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);
Frediano
_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/ma
Jonathon Jongsma
2018-11-13 17:49:58 UTC
Permalink
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
modesetting 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>
Acked-by: Frediano Ziglio <***@redhat.com>
---
Changes in v2:
- fix commit log -- not sure what went wrong there...

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);
--
2.17.2

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://list
Jonathon Jongsma
2018-11-12 20:06:14 UTC
Permalink
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 modesetting 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>
Acked-by: Frediano Ziglio <***@redhat.com>
---
src/qxl_drmmode.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/qxl_drmmode.c b/src/qxl_drmmode.c
index a814859..8ebc708 100644
--- a/src/qxl_drmmode.c
+++ b/src/qxl_drmmode.c
@@ -720,22 +720,25 @@ static int subpixel_conv_table[7] = { 0, SubPixelUnknown,
SubPixelVerticalBGR,
SubPixelNone };

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

static void
--
2.17.2

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