diff mbox

[08/16] drm: Use dev->name as fallback for dev->unique

Message ID 1466148814-8194-9-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter June 17, 2016, 7:33 a.m. UTC
Lots of arm drivers get this wrong and for most arm boards this is the
right thing actually. And anyway with most loaders you want to chase
sysfs links anyway to figure out which dri device you want.

This will fix dmesg noise for rockchip and sti.

Also add a fallback to driver->name for entirely virtual drivers like
vgem.

v2: Rebase on top of

commit e112e593b215c394c0303dbf0534db0928e87967
Author: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Date:   Fri Dec 11 11:20:28 2015 +0100

    drm: use dev_name as default unique name in drm_dev_alloc()

and simplify a bit. Plus add a comment.

Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Reported-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c   | 10 +++++-----
 drivers/gpu/drm/drm_ioctl.c |  8 +-------
 2 files changed, 6 insertions(+), 12 deletions(-)

Comments

Emil Velikov June 17, 2016, 10:25 p.m. UTC | #1
Hi Daniel,

On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Lots of arm drivers get this wrong and for most arm boards this is the
> right thing actually. And anyway with most loaders you want to chase
> sysfs links anyway to figure out which dri device you want.
>
> This will fix dmesg noise for rockchip and sti.
>
> Also add a fallback to driver->name for entirely virtual drivers like
> vgem.
>
> v2: Rebase on top of
>
> commit e112e593b215c394c0303dbf0534db0928e87967
> Author: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> Date:   Fri Dec 11 11:20:28 2015 +0100
>
>     drm: use dev_name as default unique name in drm_dev_alloc()
>
> and simplify a bit. Plus add a comment.
>
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Reported-by: Ilia Mirkin <imirkin@alum.mit.edu>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c   | 10 +++++-----
>  drivers/gpu/drm/drm_ioctl.c |  8 +-------
>  2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 10afa2539181..ecba2511ef5a 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -523,11 +523,11 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
>                 }
>         }
>
> -       if (parent) {
> -               ret = drm_dev_set_unique(dev, dev_name(parent));
> -               if (ret)
> -                       goto err_setunique;
> -       }
> +       /* Use the parent device name as DRM device unique identifier, but fall
> +        * back to the driver name for virtual devices like vgem. */
> +       ret = drm_dev_set_unique(dev, parent ? dev_name(parent) : driver->name);
> +       if (ret)
> +               goto err_setunique;
>
>         return dev;
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 11eda9050215..83892b6b1e0d 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -134,13 +134,7 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
>                         drm_unset_busid(dev, master);
>                         return ret;
>                 }
> -       } else {
> -               if (WARN(dev->unique == NULL,
> -                        "No drm_driver.set_busid() implementation provided by "
> -                        "%ps. Use drm_dev_set_unique() to set the unique "
> -                        "name explicitly.", dev->driver))
> -                       return -EINVAL;
> -
> +       } else if (dev->unique) {
With the drm_drv.c hunk in place this will always evaluate to true,
correct ? Hmmm strictly speaking it could be NULL since vgem/platform
devices call drm_dev_set_unique() and only sun4i checks if the
function has failed.

Is it worth dropping the check here or after (alongside) patch 10 ?

-Emil
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 10afa2539181..ecba2511ef5a 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -523,11 +523,11 @@  struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 		}
 	}
 
-	if (parent) {
-		ret = drm_dev_set_unique(dev, dev_name(parent));
-		if (ret)
-			goto err_setunique;
-	}
+	/* Use the parent device name as DRM device unique identifier, but fall
+	 * back to the driver name for virtual devices like vgem. */
+	ret = drm_dev_set_unique(dev, parent ? dev_name(parent) : driver->name);
+	if (ret)
+		goto err_setunique;
 
 	return dev;
 
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 11eda9050215..83892b6b1e0d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -134,13 +134,7 @@  static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 			drm_unset_busid(dev, master);
 			return ret;
 		}
-	} else {
-		if (WARN(dev->unique == NULL,
-			 "No drm_driver.set_busid() implementation provided by "
-			 "%ps. Use drm_dev_set_unique() to set the unique "
-			 "name explicitly.", dev->driver))
-			return -EINVAL;
-
+	} else if (dev->unique) {
 		master->unique = kstrdup(dev->unique, GFP_KERNEL);
 		if (master->unique)
 			master->unique_len = strlen(dev->unique);