Re: [PATCH xserver resend] xf86: dri2: Use va_gl as vdpau_driver for Intel i965 GPUs

[Eric Anholt]

Hans de Goede  writes:

> The modesetting driver (which now often is used with Intel GPUs),
> relies on dri2_probe_driver_name() to get the dri and vdpau driver
> names, before this commit it would always assign the same name to
> the 2 names. But the vdpau driver for i965 GPUs should be va_gl
> (i915 does not support vdpau at all).
>
> This commit modifies the used lookup table and dri2_probe_driver_name()
> to set the vdpau_driver to va_gl for i965 GPUs, it leaves the 2
> names the same for all other GPUs.
>
> Note this commit adds a FIXME comment for a memory leak in
> dri2_probe_driver_name(), that leak was already present and fixing
> it falls outside of the scope of this commit.

It sounds from va_gl's page like it could be used not just on intel but
on AMD.  Since va_gl sounds like it's a generic fallback to layering
vaapi in between vdpau and hardware, could the vdpau wrapper lib be
modified to always try it as a fallback for no driver-specific lib being
present?  This seems to me like the right place to fix things: the X
server has only worse knowledge of what drivers your software has than
your software itself does.

Alternatively, if the vdpau wrapper lib is not modifiable, should we be
answering va_gl for basically everything where we don't know of an open
vdpau implementation?


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver resend] xf86: dri2: Use va_gl as vdpau_driver for Intel i965 GPUs

[Hans de Goede]

The modesetting driver (which now often is used with Intel GPUs),
relies on dri2_probe_driver_name() to get the dri and vdpau driver
names, before this commit it would always assign the same name to
the 2 names. But the vdpau driver for i965 GPUs should be va_gl
(i915 does not support vdpau at all).

This commit modifies the used lookup table and dri2_probe_driver_name()
to set the vdpau_driver to va_gl for i965 GPUs, it leaves the 2
names the same for all other GPUs.

Note this commit adds a FIXME comment for a memory leak in
dri2_probe_driver_name(), that leak was already present and fixing
it falls outside of the scope of this commit.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1413733
Cc: kwiz...@gmail.com
Signed-off-by: Hans de Goede 
---
 hw/xfree86/dri2/dri2.c  | 31 +++--
 hw/xfree86/dri2/pci_ids/pci_id_driver_map.h | 21 +--
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index f9f9859e1..53153383c 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -1437,14 +1437,18 @@ get_prime_id(void)
 
 #include "pci_ids/pci_id_driver_map.h"
 
-static char *
-dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr info)
+static void
+dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr info,
+   const char **dri_driver_ret,
+   const char **vdpau_driver_ret)
 {
 #ifdef WITH_LIBDRM
 int i, j;
-char *driver = NULL;
 drmDevicePtr dev;
 
+*dri_driver_ret = NULL;
+*vdpau_driver_ret = NULL;
+
 /* For non-PCI devices and drmGetDevice fail, just assume that
  * the 3D driver is named the same as the kernel driver. This is
  * currently true for vc4 and msm (freedreno).
@@ -1456,12 +1460,14 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
 xf86DrvMsg(pScreen->myNum, X_ERROR,
"[DRI2] Couldn't drmGetVersion() on non-PCI device, "
"no driver name found.\n");
-return NULL;
+return;
 }
 
-driver = strndup(version->name, version->name_len);
+/* FIXME this gets leaked */
+*dri_driver_ret = strndup(version->name, version->name_len);
+*vdpau_driver_ret = *dri_driver_ret;
 drmFreeVersion(version);
-return driver;
+return;
 }
 
 for (i = 0; driver_map[i].driver; i++) {
@@ -1469,13 +1475,15 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
 continue;
 
 if (driver_map[i].num_chips_ids == -1) {
- driver = strdup(driver_map[i].driver);
+ *dri_driver_ret = driver_map[i].driver;
+ *vdpau_driver_ret = driver_map[i].vdpau_driver;
  goto out;
 }
 
 for (j = 0; j < driver_map[i].num_chips_ids; j++) {
 if (driver_map[i].chip_ids[j] == dev->deviceinfo.pci->device_id) {
-driver = strdup(driver_map[i].driver);
+*dri_driver_ret = driver_map[i].driver;
+*vdpau_driver_ret = driver_map[i].vdpau_driver;
 goto out;
 }
 }
@@ -1487,9 +1495,9 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr 
info)
dev->deviceinfo.pci->vendor_id, dev->deviceinfo.pci->device_id);
 out:
 drmFreeDevice();
-return driver;
 #else
-return NULL;
+*dri_driver_ret = NULL;
+*vdpau_driver_ret = NULL;
 #endif
 }
 
@@ -1611,7 +1619,8 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
 if (info->driverName) {
 ds->driverNames[0] = info->driverName;
 } else {
-ds->driverNames[0] = ds->driverNames[1] = 
dri2_probe_driver_name(pScreen, info);
+dri2_probe_driver_name(pScreen, info,
+   >driverNames[0], >driverNames[1]);
 if (!ds->driverNames[0])
 return FALSE;
 }
diff --git a/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h 
b/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
index da7ea1c1e..7036d1003 100644
--- a/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
+++ b/hw/xfree86/dri2/pci_ids/pci_id_driver_map.h
@@ -66,21 +66,22 @@ static const int vmwgfx_chip_ids[] = {
 static const struct {
int vendor_id;
const char *driver;
+   const char *vdpau_driver;
const int *chip_ids;
int num_chips_ids;
 } driver_map[] = {
-   { 0x8086, "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids) },
-   { 0x8086, "i965", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
+   { 0x8086, "i915", "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids) },
+   { 0x8086, "i965", "va_gl", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
 #ifndef DRIVER_MAP_GALLIUM_ONLY
-   { 0x1002, "radeon", r100_chip_ids, ARRAY_SIZE(r100_chip_ids) },
-   { 0x1002, "r200", r200_chip_ids, ARRAY_SIZE(r200_chip_ids) },
+   { 0x1002, "radeon", "radeon", r100_chip_ids,