diff mbox series

[libdrm,1/2] xf86drm: fallback to MODALIAS for OF less platform devices

Message ID 20190123104518.7332-1-emil.l.velikov@gmail.com (mailing list archive)
State New, archived
Headers show
Series [libdrm,1/2] xf86drm: fallback to MODALIAS for OF less platform devices | expand

Commit Message

Emil Velikov Jan. 23, 2019, 10:45 a.m. UTC
From: Emil Velikov <emil.velikov@collabora.com>

Some devices can lack OF data or it may not be available in the uevent
file. Fallback to the MODALIAS data in those cases.

We strip any leading "MODALIAS=.*:" thus the resulting information is
compatible with existing code in Mesa.

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 xf86drm.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 13 deletions(-)

Comments

Eric Engestrom Jan. 23, 2019, 11:04 a.m. UTC | #1
On Wednesday, 2019-01-23 10:45:17 +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
> 
> Some devices can lack OF data or it may not be available in the uevent
> file. Fallback to the MODALIAS data in those cases.
> 
> We strip any leading "MODALIAS=.*:" thus the resulting information is
> compatible with existing code in Mesa.
> 
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
>  xf86drm.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 10df682b..374734eb 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3511,15 +3511,28 @@ free_device:
>  static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr info)
>  {
>  #ifdef __linux__
> -    char path[PATH_MAX + 1], *name;
> +    char path[PATH_MAX + 1], *name, *foo;

I assume you didn't mean to send this patch yet? :P

>  
>      snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
>  
>      name = sysfs_uevent_get(path, "OF_FULLNAME");
> -    if (!name)
> -        return -ENOENT;
> +    foo = name;
> +    if (!name) {
> +        /* If the device lacks OF data, pick the MODALIAS info */
> +        name = sysfs_uevent_get(path, "MODALIAS");
> +        if (!name)
> +            return -ENOENT;
> +
> +        /* .. and strip the MODALIAS=[platform,usb...]: part. */
> +        foo = strrchr(name, ':');
> +        if (!foo) {
> +            free(name);
> +            return -ENOENT;
> +        }
> +        foo++;
> +    }
>  
> -    strncpy(info->fullname, name, DRM_PLATFORM_DEVICE_NAME_LEN);
> +    strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
>      info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
>      free(name);
>  
> @@ -3534,18 +3547,20 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
>                                        drmPlatformDeviceInfoPtr info)
>  {
>  #ifdef __linux__
> -    char path[PATH_MAX + 1], *value;
> +    char path[PATH_MAX + 1], *value, *foo;
>      unsigned int count, i;
>      int err;
>  
>      snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
>  
>      value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
> -    if (!value)
> -        return -ENOENT;
> -
> -    sscanf(value, "%u", &count);
> -    free(value);
> +    if (value) {
> +        sscanf(value, "%u", &count);
> +        free(value);
> +    } else {
> +        /* Assume one entry if the device lack OF data */
> +        count = 1;
> +    }
>  
>      info->compatible = calloc(count + 1, sizeof(*info->compatible));
>      if (!info->compatible)
> @@ -3553,12 +3568,26 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
>  
>      for (i = 0; i < count; i++) {
>          value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
> +        foo = value;
>          if (!value) {
> -            err = -ENOENT;
> -            goto free;
> +            /* If the device lacks OF data, pick the MODALIAS info */
> +            value = sysfs_uevent_get(path, "MODALIAS");
> +            if (!value) {
> +                err = -ENOENT;
> +                goto free;
> +            }
> +
> +            /* .. and strip the MODALIAS=[platform,usb...]: part. */
> +            foo = strrchr(value, ':');
> +            if (!foo) {
> +                free(value);
> +                return -ENOENT;
> +            }
> +            foo = strdup(foo + 1);
> +            free(value);
>          }
>  
> -        info->compatible[i] = value;
> +        info->compatible[i] = foo;
>      }
>  
>      return 0;
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Emil Velikov Jan. 23, 2019, 11:26 a.m. UTC | #2
On Wed, 23 Jan 2019 at 11:04, Eric Engestrom <eric.engestrom@intel.com> wrote:
>
> On Wednesday, 2019-01-23 10:45:17 +0000, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> >
> > Some devices can lack OF data or it may not be available in the uevent
> > file. Fallback to the MODALIAS data in those cases.
> >
> > We strip any leading "MODALIAS=.*:" thus the resulting information is
> > compatible with existing code in Mesa.
> >
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > ---
> >  xf86drm.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 42 insertions(+), 13 deletions(-)
> >
> > diff --git a/xf86drm.c b/xf86drm.c
> > index 10df682b..374734eb 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -3511,15 +3511,28 @@ free_device:
> >  static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr info)
> >  {
> >  #ifdef __linux__
> > -    char path[PATH_MAX + 1], *name;
> > +    char path[PATH_MAX + 1], *name, *foo;
>
> I assume you didn't mean to send this patch yet? :P
>
Thanks Eric, I intentionally sent it out. Mind was blank thinking for
a reasonable variable name :-\
Suggestions are more than welcome.

For reference with this patch drmdevice and other drmDevice API users list:
 - VGEM, needs "drm/vgem: Fix vgem_init to get drm device available."
- in v5.0 only :'-(
 - etnaviv, after "drm/etnaviv: remove the need for a gpu-subsystem DT
node" landed in v4.17/18

HTH
Emil
Emil Velikov Jan. 24, 2019, 2:42 p.m. UTC | #3
On Wed, 23 Jan 2019 at 11:26, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Wed, 23 Jan 2019 at 11:04, Eric Engestrom <eric.engestrom@intel.com> wrote:
> >
> > On Wednesday, 2019-01-23 10:45:17 +0000, Emil Velikov wrote:
> > > From: Emil Velikov <emil.velikov@collabora.com>
> > >
> > > Some devices can lack OF data or it may not be available in the uevent
> > > file. Fallback to the MODALIAS data in those cases.
> > >
> > > We strip any leading "MODALIAS=.*:" thus the resulting information is
> > > compatible with existing code in Mesa.
> > >
> > > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > > ---
> > >  xf86drm.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------------
> > >  1 file changed, 42 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/xf86drm.c b/xf86drm.c
> > > index 10df682b..374734eb 100644
> > > --- a/xf86drm.c
> > > +++ b/xf86drm.c
> > > @@ -3511,15 +3511,28 @@ free_device:
> > >  static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr info)
> > >  {
> > >  #ifdef __linux__
> > > -    char path[PATH_MAX + 1], *name;
> > > +    char path[PATH_MAX + 1], *name, *foo;
> >
> > I assume you didn't mean to send this patch yet? :P
> >
> Thanks Eric, I intentionally sent it out. Mind was blank thinking for
> a reasonable variable name :-\
> Suggestions are more than welcome.
>
> For reference with this patch drmdevice and other drmDevice API users list:
>  - VGEM, needs "drm/vgem: Fix vgem_init to get drm device available."
> - in v5.0 only :'-(
>  - etnaviv, after "drm/etnaviv: remove the need for a gpu-subsystem DT
> node" landed in v4.17/18
>
Christian can you please test that this patches brings etnaviv back to the list?
Above is a reasonable assumption, yet assumption never the less.

I've only tested VGEM.

Thanks
Emil
Lucas Stach Feb. 1, 2019, 2:15 p.m. UTC | #4
Hi Emil,

Am Donnerstag, den 24.01.2019, 14:42 +0000 schrieb Emil Velikov:
> > On Wed, 23 Jan 2019 at 11:26, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > 
> > On Wed, 23 Jan 2019 at 11:04, Eric Engestrom <eric.engestrom@intel.com> wrote:
> > > 
> > > On Wednesday, 2019-01-23 10:45:17 +0000, Emil Velikov wrote:
> > > > > > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > > 
> > > > Some devices can lack OF data or it may not be available in the uevent
> > > > file. Fallback to the MODALIAS data in those cases.
> > > > 
> > > > We strip any leading "MODALIAS=.*:" thus the resulting information is
> > > > compatible with existing code in Mesa.
> > > > 
> > > > > > > > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> > > > ---
> > > >  xf86drm.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------------
> > > >  1 file changed, 42 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/xf86drm.c b/xf86drm.c
> > > > index 10df682b..374734eb 100644
> > > > --- a/xf86drm.c
> > > > +++ b/xf86drm.c
> > > > @@ -3511,15 +3511,28 @@ free_device:
> > > >  static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr info)
> > > >  {
> > > >  #ifdef __linux__
> > > > -    char path[PATH_MAX + 1], *name;
> > > > +    char path[PATH_MAX + 1], *name, *foo;
> > > 
> > > I assume you didn't mean to send this patch yet? :P
> > > 
> > 
> > Thanks Eric, I intentionally sent it out. Mind was blank thinking for
> > a reasonable variable name :-\
> > Suggestions are more than welcome.
> > 
> > For reference with this patch drmdevice and other drmDevice API users list:
> >  - VGEM, needs "drm/vgem: Fix vgem_init to get drm device available."
> > - in v5.0 only :'-(
> >  - etnaviv, after "drm/etnaviv: remove the need for a gpu-subsystem DT
> > node" landed in v4.17/18
> > 
> 
> Christian can you please test that this patches brings etnaviv back to the list?
> Above is a reasonable assumption, yet assumption never the less.

I can confirm that with this patch applied
loader_open_render_node("etnaviv") works as intended.

Regards,
Lucas
Emil Velikov Feb. 4, 2019, 3:39 p.m. UTC | #5
On Fri, 1 Feb 2019 at 14:15, Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Emil,
>

> > > For reference with this patch drmdevice and other drmDevice API users list:
> > >  - VGEM, needs "drm/vgem: Fix vgem_init to get drm device available."
> > > - in v5.0 only :'-(
> > >  - etnaviv, after "drm/etnaviv: remove the need for a gpu-subsystem DT
> > > node" landed in v4.17/18
> > >
> >
> > Christian can you please test that this patches brings etnaviv back to the list?
> > Above is a reasonable assumption, yet assumption never the less.
>
> I can confirm that with this patch applied
> loader_open_render_node("etnaviv") works as intended.
>
Amazing, thank you Lucas.
Pushed to master.

-Emil
diff mbox series

Patch

diff --git a/xf86drm.c b/xf86drm.c
index 10df682b..374734eb 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3511,15 +3511,28 @@  free_device:
 static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr info)
 {
 #ifdef __linux__
-    char path[PATH_MAX + 1], *name;
+    char path[PATH_MAX + 1], *name, *foo;
 
     snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
 
     name = sysfs_uevent_get(path, "OF_FULLNAME");
-    if (!name)
-        return -ENOENT;
+    foo = name;
+    if (!name) {
+        /* If the device lacks OF data, pick the MODALIAS info */
+        name = sysfs_uevent_get(path, "MODALIAS");
+        if (!name)
+            return -ENOENT;
+
+        /* .. and strip the MODALIAS=[platform,usb...]: part. */
+        foo = strrchr(name, ':');
+        if (!foo) {
+            free(name);
+            return -ENOENT;
+        }
+        foo++;
+    }
 
-    strncpy(info->fullname, name, DRM_PLATFORM_DEVICE_NAME_LEN);
+    strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
     info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
     free(name);
 
@@ -3534,18 +3547,20 @@  static int drmParsePlatformDeviceInfo(int maj, int min,
                                       drmPlatformDeviceInfoPtr info)
 {
 #ifdef __linux__
-    char path[PATH_MAX + 1], *value;
+    char path[PATH_MAX + 1], *value, *foo;
     unsigned int count, i;
     int err;
 
     snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
 
     value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
-    if (!value)
-        return -ENOENT;
-
-    sscanf(value, "%u", &count);
-    free(value);
+    if (value) {
+        sscanf(value, "%u", &count);
+        free(value);
+    } else {
+        /* Assume one entry if the device lack OF data */
+        count = 1;
+    }
 
     info->compatible = calloc(count + 1, sizeof(*info->compatible));
     if (!info->compatible)
@@ -3553,12 +3568,26 @@  static int drmParsePlatformDeviceInfo(int maj, int min,
 
     for (i = 0; i < count; i++) {
         value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
+        foo = value;
         if (!value) {
-            err = -ENOENT;
-            goto free;
+            /* If the device lacks OF data, pick the MODALIAS info */
+            value = sysfs_uevent_get(path, "MODALIAS");
+            if (!value) {
+                err = -ENOENT;
+                goto free;
+            }
+
+            /* .. and strip the MODALIAS=[platform,usb...]: part. */
+            foo = strrchr(value, ':');
+            if (!foo) {
+                free(value);
+                return -ENOENT;
+            }
+            foo = strdup(foo + 1);
+            free(value);
         }
 
-        info->compatible[i] = value;
+        info->compatible[i] = foo;
     }
 
     return 0;