[PATCH] drm: add drm device name

[PATCH] drm: add drm device name

[Jiang, Sonny]

Add DRM device name and use DRM_IOCTL_VERSION ioctl drmVersion::desc passing it to user space
instead of unused DRM driver name descriptor.

Change-Id: I809f6d3e057111417efbe8fa7cab8f0113ba4b21
Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
 drivers/gpu/drm/drm_drv.c                  | 17 +++++++++++++++++
 drivers/gpu/drm/drm_ioctl.c                |  2 +-
 include/drm/drm_device.h                   |  3 +++
 include/drm/drm_drv.h                      |  1 +
 5 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 67b09cb2a9e2..8f0971cea363 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2809,6 +2809,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	/* init the mode config */
 	drm_mode_config_init(adev->ddev);
 
+	drm_dev_set_name(adev->ddev, amdgpu_asic_name[adev->asic_type]);
+
 	r = amdgpu_device_ip_init(adev);
 	if (r) {
 		/* failed in exclusive mode due to timeout */
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 862621494a93..6c33879bb538 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -802,6 +802,7 @@ void drm_dev_fini(struct drm_device *dev)
 	mutex_destroy(&dev->struct_mutex);
 	drm_legacy_destroy_members(dev);
 	kfree(dev->unique);
+	kfree(dev->name);
 }
 EXPORT_SYMBOL(drm_dev_fini);
 
@@ -1078,6 +1079,22 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name)
 }
 EXPORT_SYMBOL(drm_dev_set_unique);
 
+/**
+ * drm_dev_set_name - Set the name of a DRM device
+ * @dev: device of which to set the name
+ * @name: name to be set
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_dev_set_name(struct drm_device *dev, const char *name)
+{
+	kfree(dev->name);
+	dev->name = kstrdup(name, GFP_KERNEL);
+
+	return dev->name ? 0 : -ENOMEM;
+}
+EXPORT_SYMBOL(drm_dev_set_name);
+
 /*
  * DRM Core
  * The DRM core module initializes all global DRM objects and makes them
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 2263e3ddd822..61f02965106b 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -506,7 +506,7 @@ int drm_version(struct drm_device *dev, void *data,
 				dev->driver->date);
 	if (!err)
 		err = drm_copy_field(version->desc, &version->desc_len,
-				dev->driver->desc);
+				dev->name);
 
 	return err;
 }
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 7f9ef709b2b6..e29912c484e4 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -123,6 +123,9 @@ struct drm_device {
 	/** @unique: Unique name of the device */
 	char *unique;
 
+	/** @name: device name */
+	char *name;
+
 	/**
 	 * @struct_mutex:
 	 *
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 68ca736c548d..f742e2bde467 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -798,6 +798,7 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
 
 
 int drm_dev_set_unique(struct drm_device *dev, const char *name);
+int drm_dev_set_name(struct drm_device *dev, const char *name);
 
 
 #endif
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 4019 bytes --]

+ dri-devel

On Tue, Sep 3, 2019 at 5:41 PM Jiang, Sonny <Sonny.Jiang-5C7GfCeVMHo@public.gmane.org> wrote:

> Add DRM device name and use DRM_IOCTL_VERSION ioctl drmVersion::desc
> passing it to user space
> instead of unused DRM driver name descriptor.
>
> Change-Id: I809f6d3e057111417efbe8fa7cab8f0113ba4b21
> Signed-off-by: Sonny Jiang <sonny.jiang-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
>  drivers/gpu/drm/drm_drv.c                  | 17 +++++++++++++++++
>  drivers/gpu/drm/drm_ioctl.c                |  2 +-
>  include/drm/drm_device.h                   |  3 +++
>  include/drm/drm_drv.h                      |  1 +
>  5 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 67b09cb2a9e2..8f0971cea363 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2809,6 +2809,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>         /* init the mode config */
>         drm_mode_config_init(adev->ddev);
>
> +       drm_dev_set_name(adev->ddev, amdgpu_asic_name[adev->asic_type]);
> +
>         r = amdgpu_device_ip_init(adev);
>         if (r) {
>                 /* failed in exclusive mode due to timeout */
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 862621494a93..6c33879bb538 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -802,6 +802,7 @@ void drm_dev_fini(struct drm_device *dev)
>         mutex_destroy(&dev->struct_mutex);
>         drm_legacy_destroy_members(dev);
>         kfree(dev->unique);
> +       kfree(dev->name);
>  }
>  EXPORT_SYMBOL(drm_dev_fini);
>
> @@ -1078,6 +1079,22 @@ int drm_dev_set_unique(struct drm_device *dev,
> const char *name)
>  }
>  EXPORT_SYMBOL(drm_dev_set_unique);
>
> +/**
> + * drm_dev_set_name - Set the name of a DRM device
> + * @dev: device of which to set the name
> + * @name: name to be set
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int drm_dev_set_name(struct drm_device *dev, const char *name)
> +{
> +       kfree(dev->name);
> +       dev->name = kstrdup(name, GFP_KERNEL);
> +
> +       return dev->name ? 0 : -ENOMEM;
> +}
> +EXPORT_SYMBOL(drm_dev_set_name);
> +
>  /*
>   * DRM Core
>   * The DRM core module initializes all global DRM objects and makes them
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 2263e3ddd822..61f02965106b 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -506,7 +506,7 @@ int drm_version(struct drm_device *dev, void *data,
>                                 dev->driver->date);
>         if (!err)
>                 err = drm_copy_field(version->desc, &version->desc_len,
> -                               dev->driver->desc);
> +                               dev->name);
>
>         return err;
>  }
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index 7f9ef709b2b6..e29912c484e4 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -123,6 +123,9 @@ struct drm_device {
>         /** @unique: Unique name of the device */
>         char *unique;
>
> +       /** @name: device name */
> +       char *name;
> +
>         /**
>          * @struct_mutex:
>          *
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 68ca736c548d..f742e2bde467 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -798,6 +798,7 @@ static inline bool drm_drv_uses_atomic_modeset(struct
> drm_device *dev)
>
>
>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
> +int drm_dev_set_name(struct drm_device *dev, const char *name);
>
>
>  #endif
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 5160 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Rob Clark]

On Fri, Sep 6, 2019 at 3:16 PM Marek Olšák <maraeo@gmail.com> wrote:
>
> + dri-devel
>
> On Tue, Sep 3, 2019 at 5:41 PM Jiang, Sonny <Sonny.Jiang@amd.com> wrote:
>>
>> Add DRM device name and use DRM_IOCTL_VERSION ioctl drmVersion::desc passing it to user space
>> instead of unused DRM driver name descriptor.
>>
>> Change-Id: I809f6d3e057111417efbe8fa7cab8f0113ba4b21
>> Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
>>  drivers/gpu/drm/drm_drv.c                  | 17 +++++++++++++++++
>>  drivers/gpu/drm/drm_ioctl.c                |  2 +-
>>  include/drm/drm_device.h                   |  3 +++
>>  include/drm/drm_drv.h                      |  1 +
>>  5 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 67b09cb2a9e2..8f0971cea363 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2809,6 +2809,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>         /* init the mode config */
>>         drm_mode_config_init(adev->ddev);
>>
>> +       drm_dev_set_name(adev->ddev, amdgpu_asic_name[adev->asic_type]);
>> +
>>         r = amdgpu_device_ip_init(adev);
>>         if (r) {
>>                 /* failed in exclusive mode due to timeout */
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 862621494a93..6c33879bb538 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -802,6 +802,7 @@ void drm_dev_fini(struct drm_device *dev)
>>         mutex_destroy(&dev->struct_mutex);
>>         drm_legacy_destroy_members(dev);
>>         kfree(dev->unique);
>> +       kfree(dev->name);
>>  }
>>  EXPORT_SYMBOL(drm_dev_fini);
>>
>> @@ -1078,6 +1079,22 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name)
>>  }
>>  EXPORT_SYMBOL(drm_dev_set_unique);
>>
>> +/**
>> + * drm_dev_set_name - Set the name of a DRM device
>> + * @dev: device of which to set the name
>> + * @name: name to be set
>> + *
>> + * Return: 0 on success or a negative error code on failure.
>> + */
>> +int drm_dev_set_name(struct drm_device *dev, const char *name)
>> +{
>> +       kfree(dev->name);
>> +       dev->name = kstrdup(name, GFP_KERNEL);
>> +
>> +       return dev->name ? 0 : -ENOMEM;
>> +}
>> +EXPORT_SYMBOL(drm_dev_set_name);
>> +
>>  /*
>>   * DRM Core
>>   * The DRM core module initializes all global DRM objects and makes them
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index 2263e3ddd822..61f02965106b 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -506,7 +506,7 @@ int drm_version(struct drm_device *dev, void *data,
>>                                 dev->driver->date);
>>         if (!err)
>>                 err = drm_copy_field(version->desc, &version->desc_len,
>> -                               dev->driver->desc);
>> +                               dev->name);

I suspect this needs to be something like dev->name ? dev->name :
dev->driver->desc

Or somewhere something needs to arrange for dev->name to default to
dev->driver->desc

And maybe this should be dev->desc instead of dev->name.. that at
least seems less confusing to me.

other than that, I don't see a big problem

BR,
-R

>>
>>         return err;
>>  }
>> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
>> index 7f9ef709b2b6..e29912c484e4 100644
>> --- a/include/drm/drm_device.h
>> +++ b/include/drm/drm_device.h
>> @@ -123,6 +123,9 @@ struct drm_device {
>>         /** @unique: Unique name of the device */
>>         char *unique;
>>
>> +       /** @name: device name */
>> +       char *name;
>> +
>>         /**
>>          * @struct_mutex:
>>          *
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index 68ca736c548d..f742e2bde467 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -798,6 +798,7 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
>>
>>
>>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
>> +int drm_dev_set_name(struct drm_device *dev, const char *name);
>>
>>
>>  #endif
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: add drm device name

[Daniel Vetter]

On Sat, Sep 7, 2019 at 3:18 AM Rob Clark <robdclark@gmail.com> wrote:
>
> On Fri, Sep 6, 2019 at 3:16 PM Marek Olšák <maraeo@gmail.com> wrote:
> >
> > + dri-devel
> >
> > On Tue, Sep 3, 2019 at 5:41 PM Jiang, Sonny <Sonny.Jiang@amd.com> wrote:
> >>
> >> Add DRM device name and use DRM_IOCTL_VERSION ioctl drmVersion::desc passing it to user space
> >> instead of unused DRM driver name descriptor.
> >>
> >> Change-Id: I809f6d3e057111417efbe8fa7cab8f0113ba4b21
> >> Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
> >>  drivers/gpu/drm/drm_drv.c                  | 17 +++++++++++++++++
> >>  drivers/gpu/drm/drm_ioctl.c                |  2 +-
> >>  include/drm/drm_device.h                   |  3 +++
> >>  include/drm/drm_drv.h                      |  1 +
> >>  5 files changed, 24 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index 67b09cb2a9e2..8f0971cea363 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -2809,6 +2809,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> >>         /* init the mode config */
> >>         drm_mode_config_init(adev->ddev);
> >>
> >> +       drm_dev_set_name(adev->ddev, amdgpu_asic_name[adev->asic_type]);
> >> +
> >>         r = amdgpu_device_ip_init(adev);
> >>         if (r) {
> >>                 /* failed in exclusive mode due to timeout */
> >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> >> index 862621494a93..6c33879bb538 100644
> >> --- a/drivers/gpu/drm/drm_drv.c
> >> +++ b/drivers/gpu/drm/drm_drv.c
> >> @@ -802,6 +802,7 @@ void drm_dev_fini(struct drm_device *dev)
> >>         mutex_destroy(&dev->struct_mutex);
> >>         drm_legacy_destroy_members(dev);
> >>         kfree(dev->unique);
> >> +       kfree(dev->name);
> >>  }
> >>  EXPORT_SYMBOL(drm_dev_fini);
> >>
> >> @@ -1078,6 +1079,22 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name)
> >>  }
> >>  EXPORT_SYMBOL(drm_dev_set_unique);
> >>
> >> +/**
> >> + * drm_dev_set_name - Set the name of a DRM device
> >> + * @dev: device of which to set the name
> >> + * @name: name to be set
> >> + *
> >> + * Return: 0 on success or a negative error code on failure.
> >> + */
> >> +int drm_dev_set_name(struct drm_device *dev, const char *name)
> >> +{
> >> +       kfree(dev->name);
> >> +       dev->name = kstrdup(name, GFP_KERNEL);
> >> +
> >> +       return dev->name ? 0 : -ENOMEM;
> >> +}
> >> +EXPORT_SYMBOL(drm_dev_set_name);
> >> +
> >>  /*
> >>   * DRM Core
> >>   * The DRM core module initializes all global DRM objects and makes them
> >> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> >> index 2263e3ddd822..61f02965106b 100644
> >> --- a/drivers/gpu/drm/drm_ioctl.c
> >> +++ b/drivers/gpu/drm/drm_ioctl.c
> >> @@ -506,7 +506,7 @@ int drm_version(struct drm_device *dev, void *data,
> >>                                 dev->driver->date);
> >>         if (!err)
> >>                 err = drm_copy_field(version->desc, &version->desc_len,
> >> -                               dev->driver->desc);
> >> +                               dev->name);
>
> I suspect this needs to be something like dev->name ? dev->name :
> dev->driver->desc
>
> Or somewhere something needs to arrange for dev->name to default to
> dev->driver->desc
>
> And maybe this should be dev->desc instead of dev->name.. that at
> least seems less confusing to me.
>
> other than that, I don't see a big problem

(recap from irc)

I thought we're using this as essentially an uapi identifier, so that
you know which kind of ioctl set a driver supports. Not so big deal on
pci, where we match against pci ids anyway, kinda bigger deal where
that's not around. Listing codenames and or something else that
changes all the time feels a bit silly for that. Imo if you just want
to expose this to userspace, stuff it into an amdgpu info/query ioctl.

So what do you need this for exactly, where's the userspace that needs this?
-Daniel

>
> BR,
> -R
>
> >>
> >>         return err;
> >>  }
> >> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> >> index 7f9ef709b2b6..e29912c484e4 100644
> >> --- a/include/drm/drm_device.h
> >> +++ b/include/drm/drm_device.h
> >> @@ -123,6 +123,9 @@ struct drm_device {
> >>         /** @unique: Unique name of the device */
> >>         char *unique;
> >>
> >> +       /** @name: device name */
> >> +       char *name;
> >> +
> >>         /**
> >>          * @struct_mutex:
> >>          *
> >> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> >> index 68ca736c548d..f742e2bde467 100644
> >> --- a/include/drm/drm_drv.h
> >> +++ b/include/drm/drm_drv.h
> >> @@ -798,6 +798,7 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
> >>
> >>
> >>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
> >> +int drm_dev_set_name(struct drm_device *dev, const char *name);
> >>
> >>
> >>  #endif
> >> --
> >> 2.17.1
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Jani Nikula]

On Sat, 07 Sep 2019, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sat, Sep 7, 2019 at 3:18 AM Rob Clark <robdclark@gmail.com> wrote:
>>
>> On Fri, Sep 6, 2019 at 3:16 PM Marek Olšák <maraeo@gmail.com> wrote:
>> >
>> > + dri-devel
>> >
>> > On Tue, Sep 3, 2019 at 5:41 PM Jiang, Sonny <Sonny.Jiang@amd.com> wrote:
>> >>
>> >> Add DRM device name and use DRM_IOCTL_VERSION ioctl drmVersion::desc passing it to user space
>> >> instead of unused DRM driver name descriptor.
>> >>
>> >> Change-Id: I809f6d3e057111417efbe8fa7cab8f0113ba4b21
>> >> Signed-off-by: Sonny Jiang <sonny.jiang@amd.com>
>> >> ---
>> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
>> >>  drivers/gpu/drm/drm_drv.c                  | 17 +++++++++++++++++
>> >>  drivers/gpu/drm/drm_ioctl.c                |  2 +-
>> >>  include/drm/drm_device.h                   |  3 +++
>> >>  include/drm/drm_drv.h                      |  1 +
>> >>  5 files changed, 24 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> index 67b09cb2a9e2..8f0971cea363 100644
>> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> @@ -2809,6 +2809,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>> >>         /* init the mode config */
>> >>         drm_mode_config_init(adev->ddev);
>> >>
>> >> +       drm_dev_set_name(adev->ddev, amdgpu_asic_name[adev->asic_type]);
>> >> +
>> >>         r = amdgpu_device_ip_init(adev);
>> >>         if (r) {
>> >>                 /* failed in exclusive mode due to timeout */
>> >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> >> index 862621494a93..6c33879bb538 100644
>> >> --- a/drivers/gpu/drm/drm_drv.c
>> >> +++ b/drivers/gpu/drm/drm_drv.c
>> >> @@ -802,6 +802,7 @@ void drm_dev_fini(struct drm_device *dev)
>> >>         mutex_destroy(&dev->struct_mutex);
>> >>         drm_legacy_destroy_members(dev);
>> >>         kfree(dev->unique);
>> >> +       kfree(dev->name);
>> >>  }
>> >>  EXPORT_SYMBOL(drm_dev_fini);
>> >>
>> >> @@ -1078,6 +1079,22 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name)
>> >>  }
>> >>  EXPORT_SYMBOL(drm_dev_set_unique);
>> >>
>> >> +/**
>> >> + * drm_dev_set_name - Set the name of a DRM device
>> >> + * @dev: device of which to set the name
>> >> + * @name: name to be set
>> >> + *
>> >> + * Return: 0 on success or a negative error code on failure.
>> >> + */
>> >> +int drm_dev_set_name(struct drm_device *dev, const char *name)
>> >> +{
>> >> +       kfree(dev->name);
>> >> +       dev->name = kstrdup(name, GFP_KERNEL);
>> >> +
>> >> +       return dev->name ? 0 : -ENOMEM;
>> >> +}
>> >> +EXPORT_SYMBOL(drm_dev_set_name);
>> >> +
>> >>  /*
>> >>   * DRM Core
>> >>   * The DRM core module initializes all global DRM objects and makes them
>> >> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> >> index 2263e3ddd822..61f02965106b 100644
>> >> --- a/drivers/gpu/drm/drm_ioctl.c
>> >> +++ b/drivers/gpu/drm/drm_ioctl.c
>> >> @@ -506,7 +506,7 @@ int drm_version(struct drm_device *dev, void *data,
>> >>                                 dev->driver->date);
>> >>         if (!err)
>> >>                 err = drm_copy_field(version->desc, &version->desc_len,
>> >> -                               dev->driver->desc);
>> >> +                               dev->name);
>>
>> I suspect this needs to be something like dev->name ? dev->name :
>> dev->driver->desc
>>
>> Or somewhere something needs to arrange for dev->name to default to
>> dev->driver->desc
>>
>> And maybe this should be dev->desc instead of dev->name.. that at
>> least seems less confusing to me.
>>
>> other than that, I don't see a big problem
>
> (recap from irc)
>
> I thought we're using this as essentially an uapi identifier, so that
> you know which kind of ioctl set a driver supports. Not so big deal on
> pci, where we match against pci ids anyway, kinda bigger deal where
> that's not around. Listing codenames and or something else that
> changes all the time feels a bit silly for that. Imo if you just want
> to expose this to userspace, stuff it into an amdgpu info/query ioctl.
>
> So what do you need this for exactly, where's the userspace that needs
> this?

Indeed; using this e.g. for changing userspace behaviour would seem
wrong.

BR,
Jani.


> -Daniel
>
>>
>> BR,
>> -R
>>
>> >>
>> >>         return err;
>> >>  }
>> >> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
>> >> index 7f9ef709b2b6..e29912c484e4 100644
>> >> --- a/include/drm/drm_device.h
>> >> +++ b/include/drm/drm_device.h
>> >> @@ -123,6 +123,9 @@ struct drm_device {
>> >>         /** @unique: Unique name of the device */
>> >>         char *unique;
>> >>
>> >> +       /** @name: device name */
>> >> +       char *name;
>> >> +
>> >>         /**
>> >>          * @struct_mutex:
>> >>          *
>> >> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> >> index 68ca736c548d..f742e2bde467 100644
>> >> --- a/include/drm/drm_drv.h
>> >> +++ b/include/drm/drm_drv.h
>> >> @@ -798,6 +798,7 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
>> >>
>> >>
>> >>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
>> >> +int drm_dev_set_name(struct drm_device *dev, const char *name);
>> >>
>> >>
>> >>  #endif
>> >> --
>> >> 2.17.1
>> >>
>> >> _______________________________________________
>> >> amd-gfx mailing list
>> >> amd-gfx@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> >
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: add drm device name

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 6704 bytes --]

The purpose is to get rid of all PCI ID tables for all drivers in
userspace. (or at least stop updating them)

Mesa common code and modesetting will use this.

Marek

On Sat, Sep 7, 2019 at 3:48 PM Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org> wrote:

> On Sat, Sep 7, 2019 at 3:18 AM Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> > On Fri, Sep 6, 2019 at 3:16 PM Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > >
> > > + dri-devel
> > >
> > > On Tue, Sep 3, 2019 at 5:41 PM Jiang, Sonny <Sonny.Jiang-5C7GfCeVMHo@public.gmane.org>
> wrote:
> > >>
> > >> Add DRM device name and use DRM_IOCTL_VERSION ioctl drmVersion::desc
> passing it to user space
> > >> instead of unused DRM driver name descriptor.
> > >>
> > >> Change-Id: I809f6d3e057111417efbe8fa7cab8f0113ba4b21
> > >> Signed-off-by: Sonny Jiang <sonny.jiang-5C7GfCeVMHo@public.gmane.org>
> > >> ---
> > >>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 ++
> > >>  drivers/gpu/drm/drm_drv.c                  | 17 +++++++++++++++++
> > >>  drivers/gpu/drm/drm_ioctl.c                |  2 +-
> > >>  include/drm/drm_device.h                   |  3 +++
> > >>  include/drm/drm_drv.h                      |  1 +
> > >>  5 files changed, 24 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > >> index 67b09cb2a9e2..8f0971cea363 100644
> > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > >> @@ -2809,6 +2809,8 @@ int amdgpu_device_init(struct amdgpu_device
> *adev,
> > >>         /* init the mode config */
> > >>         drm_mode_config_init(adev->ddev);
> > >>
> > >> +       drm_dev_set_name(adev->ddev,
> amdgpu_asic_name[adev->asic_type]);
> > >> +
> > >>         r = amdgpu_device_ip_init(adev);
> > >>         if (r) {
> > >>                 /* failed in exclusive mode due to timeout */
> > >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > >> index 862621494a93..6c33879bb538 100644
> > >> --- a/drivers/gpu/drm/drm_drv.c
> > >> +++ b/drivers/gpu/drm/drm_drv.c
> > >> @@ -802,6 +802,7 @@ void drm_dev_fini(struct drm_device *dev)
> > >>         mutex_destroy(&dev->struct_mutex);
> > >>         drm_legacy_destroy_members(dev);
> > >>         kfree(dev->unique);
> > >> +       kfree(dev->name);
> > >>  }
> > >>  EXPORT_SYMBOL(drm_dev_fini);
> > >>
> > >> @@ -1078,6 +1079,22 @@ int drm_dev_set_unique(struct drm_device *dev,
> const char *name)
> > >>  }
> > >>  EXPORT_SYMBOL(drm_dev_set_unique);
> > >>
> > >> +/**
> > >> + * drm_dev_set_name - Set the name of a DRM device
> > >> + * @dev: device of which to set the name
> > >> + * @name: name to be set
> > >> + *
> > >> + * Return: 0 on success or a negative error code on failure.
> > >> + */
> > >> +int drm_dev_set_name(struct drm_device *dev, const char *name)
> > >> +{
> > >> +       kfree(dev->name);
> > >> +       dev->name = kstrdup(name, GFP_KERNEL);
> > >> +
> > >> +       return dev->name ? 0 : -ENOMEM;
> > >> +}
> > >> +EXPORT_SYMBOL(drm_dev_set_name);
> > >> +
> > >>  /*
> > >>   * DRM Core
> > >>   * The DRM core module initializes all global DRM objects and makes
> them
> > >> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > >> index 2263e3ddd822..61f02965106b 100644
> > >> --- a/drivers/gpu/drm/drm_ioctl.c
> > >> +++ b/drivers/gpu/drm/drm_ioctl.c
> > >> @@ -506,7 +506,7 @@ int drm_version(struct drm_device *dev, void
> *data,
> > >>                                 dev->driver->date);
> > >>         if (!err)
> > >>                 err = drm_copy_field(version->desc,
> &version->desc_len,
> > >> -                               dev->driver->desc);
> > >> +                               dev->name);
> >
> > I suspect this needs to be something like dev->name ? dev->name :
> > dev->driver->desc
> >
> > Or somewhere something needs to arrange for dev->name to default to
> > dev->driver->desc
> >
> > And maybe this should be dev->desc instead of dev->name.. that at
> > least seems less confusing to me.
> >
> > other than that, I don't see a big problem
>
> (recap from irc)
>
> I thought we're using this as essentially an uapi identifier, so that
> you know which kind of ioctl set a driver supports. Not so big deal on
> pci, where we match against pci ids anyway, kinda bigger deal where
> that's not around. Listing codenames and or something else that
> changes all the time feels a bit silly for that. Imo if you just want
> to expose this to userspace, stuff it into an amdgpu info/query ioctl.
>
> So what do you need this for exactly, where's the userspace that needs
> this?
> -Daniel
>
> >
> > BR,
> > -R
> >
> > >>
> > >>         return err;
> > >>  }
> > >> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> > >> index 7f9ef709b2b6..e29912c484e4 100644
> > >> --- a/include/drm/drm_device.h
> > >> +++ b/include/drm/drm_device.h
> > >> @@ -123,6 +123,9 @@ struct drm_device {
> > >>         /** @unique: Unique name of the device */
> > >>         char *unique;
> > >>
> > >> +       /** @name: device name */
> > >> +       char *name;
> > >> +
> > >>         /**
> > >>          * @struct_mutex:
> > >>          *
> > >> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > >> index 68ca736c548d..f742e2bde467 100644
> > >> --- a/include/drm/drm_drv.h
> > >> +++ b/include/drm/drm_drv.h
> > >> @@ -798,6 +798,7 @@ static inline bool
> drm_drv_uses_atomic_modeset(struct drm_device *dev)
> > >>
> > >>
> > >>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
> > >> +int drm_dev_set_name(struct drm_device *dev, const char *name);
> > >>
> > >>
> > >>  #endif
> > >> --
> > >> 2.17.1
> > >>
> > >> _______________________________________________
> > >> amd-gfx mailing list
> > >> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>

[-- Attachment #1.2: Type: text/html, Size: 9761 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Jani Nikula]

On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
> The purpose is to get rid of all PCI ID tables for all drivers in
> userspace. (or at least stop updating them)
>
> Mesa common code and modesetting will use this.

I'd think this would warrant a high level description of what you want
to achieve in the commit message.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Christian König]

Am 17.09.19 um 07:47 schrieb Jani Nikula:
> On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
>> The purpose is to get rid of all PCI ID tables for all drivers in
>> userspace. (or at least stop updating them)
>>
>> Mesa common code and modesetting will use this.
> I'd think this would warrant a high level description of what you want
> to achieve in the commit message.

And maybe explicitly call it uapi_name or even uapi_driver_name.

Regards,
Christian.

>
> BR,
> Jani.
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Daniel Vetter]

On Tue, Sep 17, 2019 at 10:12 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 17.09.19 um 07:47 schrieb Jani Nikula:
> > On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
> >> The purpose is to get rid of all PCI ID tables for all drivers in
> >> userspace. (or at least stop updating them)
> >>
> >> Mesa common code and modesetting will use this.
> > I'd think this would warrant a high level description of what you want
> > to achieve in the commit message.
>
> And maybe explicitly call it uapi_name or even uapi_driver_name.

If it's uapi_name, then why do we need a new one for every generation?
Userspace drivers tend to span a lot more than just 1 generation. And
if you want to have per-generation data from the kernel to userspace,
then imo that's much better suited in some amdgpu ioctl, instead of
trying to encode that into the driver name.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: add drm device name

[Koenig, Christian]

Am 17.09.19 um 10:17 schrieb Daniel Vetter:
> On Tue, Sep 17, 2019 at 10:12 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 17.09.19 um 07:47 schrieb Jani Nikula:
>>> On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
>>>> The purpose is to get rid of all PCI ID tables for all drivers in
>>>> userspace. (or at least stop updating them)
>>>>
>>>> Mesa common code and modesetting will use this.
>>> I'd think this would warrant a high level description of what you want
>>> to achieve in the commit message.
>> And maybe explicitly call it uapi_name or even uapi_driver_name.
> If it's uapi_name, then why do we need a new one for every generation?
> Userspace drivers tend to span a lot more than just 1 generation. And
> if you want to have per-generation data from the kernel to userspace,
> then imo that's much better suited in some amdgpu ioctl, instead of
> trying to encode that into the driver name.

Well we already have an IOCTL for that, but I thought the intention here 
was to get rid of the PCI-ID tables in userspace to figure out which 
driver to load.

I mean it could be perfectly valid to not only match the kernel, but 
also the hardware generation for that.

Christian.

> -Daniel

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Michel Dänzer]

On 2019-09-17 10:23 a.m., Koenig, Christian wrote:
> Am 17.09.19 um 10:17 schrieb Daniel Vetter:
>> On Tue, Sep 17, 2019 at 10:12 AM Christian König
>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>> Am 17.09.19 um 07:47 schrieb Jani Nikula:
>>>> On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
>>>>> The purpose is to get rid of all PCI ID tables for all drivers in
>>>>> userspace. (or at least stop updating them)
>>>>>
>>>>> Mesa common code and modesetting will use this.
>>>> I'd think this would warrant a high level description of what you want
>>>> to achieve in the commit message.
>>> And maybe explicitly call it uapi_name or even uapi_driver_name.
>> If it's uapi_name, then why do we need a new one for every generation?
>> Userspace drivers tend to span a lot more than just 1 generation. And
>> if you want to have per-generation data from the kernel to userspace,
>> then imo that's much better suited in some amdgpu ioctl, instead of
>> trying to encode that into the driver name.
> 
> Well we already have an IOCTL for that, but I thought the intention here 
> was to get rid of the PCI-ID tables in userspace to figure out which 
> driver to load.

That's just unrealistic in general, I'm afraid. See e.g. the ongoing
transition from i965 to iris for recent Intel hardware. How is the
kernel supposed to know which driver is to be used?


For the Xorg modesetting driver, there's a simple solution, see
https://gitlab.freedesktop.org/xorg/xserver/merge_requests/278 .
Something similar can probably be done in Mesa as well.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Michel Dänzer]

On 2019-09-17 11:23 a.m., Michel Dänzer wrote:
> On 2019-09-17 10:23 a.m., Koenig, Christian wrote:
>> Am 17.09.19 um 10:17 schrieb Daniel Vetter:
>>> On Tue, Sep 17, 2019 at 10:12 AM Christian König
>>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>> Am 17.09.19 um 07:47 schrieb Jani Nikula:
>>>>> On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
>>>>>> The purpose is to get rid of all PCI ID tables for all drivers in
>>>>>> userspace. (or at least stop updating them)
>>>>>>
>>>>>> Mesa common code and modesetting will use this.
>>>>> I'd think this would warrant a high level description of what you want
>>>>> to achieve in the commit message.
>>>> And maybe explicitly call it uapi_name or even uapi_driver_name.
>>> If it's uapi_name, then why do we need a new one for every generation?
>>> Userspace drivers tend to span a lot more than just 1 generation. And
>>> if you want to have per-generation data from the kernel to userspace,
>>> then imo that's much better suited in some amdgpu ioctl, instead of
>>> trying to encode that into the driver name.
>>
>> Well we already have an IOCTL for that, but I thought the intention here 
>> was to get rid of the PCI-ID tables in userspace to figure out which 
>> driver to load.
> 
> That's just unrealistic in general, I'm afraid. See e.g. the ongoing
> transition from i965 to iris for recent Intel hardware. How is the
> kernel supposed to know which driver is to be used?
> 
> 
> For the Xorg modesetting driver, there's a simple solution, see
> https://gitlab.freedesktop.org/xorg/xserver/merge_requests/278 .
> Something similar can probably be done in Mesa as well.

Another possibility might be for Xorg to use
https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_query_driver.txt
to determine the driver name. Then only Mesa might need any HW specific
code.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: add drm device name

[Christian König]

Am 17.09.19 um 11:27 schrieb Michel Dänzer:
> On 2019-09-17 11:23 a.m., Michel Dänzer wrote:
>> On 2019-09-17 10:23 a.m., Koenig, Christian wrote:
>>> Am 17.09.19 um 10:17 schrieb Daniel Vetter:
>>>> On Tue, Sep 17, 2019 at 10:12 AM Christian König
>>>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>>> Am 17.09.19 um 07:47 schrieb Jani Nikula:
>>>>>> On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
>>>>>>> The purpose is to get rid of all PCI ID tables for all drivers in
>>>>>>> userspace. (or at least stop updating them)
>>>>>>>
>>>>>>> Mesa common code and modesetting will use this.
>>>>>> I'd think this would warrant a high level description of what you want
>>>>>> to achieve in the commit message.
>>>>> And maybe explicitly call it uapi_name or even uapi_driver_name.
>>>> If it's uapi_name, then why do we need a new one for every generation?
>>>> Userspace drivers tend to span a lot more than just 1 generation. And
>>>> if you want to have per-generation data from the kernel to userspace,
>>>> then imo that's much better suited in some amdgpu ioctl, instead of
>>>> trying to encode that into the driver name.
>>> Well we already have an IOCTL for that, but I thought the intention here
>>> was to get rid of the PCI-ID tables in userspace to figure out which
>>> driver to load.
>> That's just unrealistic in general, I'm afraid. See e.g. the ongoing
>> transition from i965 to iris for recent Intel hardware. How is the
>> kernel supposed to know which driver is to be used?

Well how is userspace currently handling that? The kernel should NOT say 
which driver to use in userspace, but rather which one is used in the 
kernel.

Mapping that information to an userspace driver still needs to be done 
somewhere else, but the difference is that you don't need to add all 
PCI-IDs twice.

Christian.

>>
>>
>> For the Xorg modesetting driver, there's a simple solution, see
>> https://gitlab.freedesktop.org/xorg/xserver/merge_requests/278 .
>> Something similar can probably be done in Mesa as well.
> Another possibility might be for Xorg to use
> https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_query_driver.txt
> to determine the driver name. Then only Mesa might need any HW specific
> code.
>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Michel Dänzer]

On 2019-09-17 1:20 p.m., Christian König wrote:
> Am 17.09.19 um 11:27 schrieb Michel Dänzer:
>> On 2019-09-17 11:23 a.m., Michel Dänzer wrote:
>>> On 2019-09-17 10:23 a.m., Koenig, Christian wrote:
>>>> Am 17.09.19 um 10:17 schrieb Daniel Vetter:
>>>>> On Tue, Sep 17, 2019 at 10:12 AM Christian König
>>>>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>>>> Am 17.09.19 um 07:47 schrieb Jani Nikula:
>>>>>>> On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
>>>>>>>> The purpose is to get rid of all PCI ID tables for all drivers in
>>>>>>>> userspace. (or at least stop updating them)
>>>>>>>>
>>>>>>>> Mesa common code and modesetting will use this.
>>>>>>> I'd think this would warrant a high level description of what you
>>>>>>> want
>>>>>>> to achieve in the commit message.
>>>>>> And maybe explicitly call it uapi_name or even uapi_driver_name.
>>>>> If it's uapi_name, then why do we need a new one for every generation?
>>>>> Userspace drivers tend to span a lot more than just 1 generation. And
>>>>> if you want to have per-generation data from the kernel to userspace,
>>>>> then imo that's much better suited in some amdgpu ioctl, instead of
>>>>> trying to encode that into the driver name.
>>>> Well we already have an IOCTL for that, but I thought the intention
>>>> here
>>>> was to get rid of the PCI-ID tables in userspace to figure out which
>>>> driver to load.
>>> That's just unrealistic in general, I'm afraid. See e.g. the ongoing
>>> transition from i965 to iris for recent Intel hardware. How is the
>>> kernel supposed to know which driver is to be used?
> 
> Well how is userspace currently handling that? The kernel should NOT say
> which driver to use in userspace, but rather which one is used in the
> kernel.

Would that really help though? E.g. the radeon kernel driver supports
radeon/r200/r300/r600/radeonsi DRI drivers, the i915 one i915/i965/iris
(and the amdgpu one radeonsi/amdgpu).

The HW generation identifier proposed in these patches might be useful,
but I suspect there'll always be cases where userspace needs to know
more precisely.


> Mapping that information to an userspace driver still needs to be done
> somewhere else, but the difference is that you don't need to add all
> PCI-IDs twice.

It should only really be necessary in Mesa.


On 2019-09-17 1:32 p.m., Daniel Vetter wrote:
> How are other compositors solving this? I don't expect they have a
> pciid table like modesetting copied to all of them ...

They don't need any of this. The Xorg modesetting driver only did for
determining the client driver name to advertise via the DRI2 extension.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: add drm device name

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 3613 bytes --]

drmVersion::name = amdgpu, radeon, intel, etc.
drmVersion::desc = vega10, vega12, vega20, ...

The common Mesa code will use name and desc to select the driver.

The AMD-specific Mesa code will use desc to identify the chip.

Mesa won't receive any PCI IDs for future chips.

Marek


On Tue, Sep 17, 2019 at 10:33 AM Michel Dänzer <michel-otUistvHUpPR7s880joybQ@public.gmane.org> wrote:

> On 2019-09-17 1:20 p.m., Christian König wrote:
> > Am 17.09.19 um 11:27 schrieb Michel Dänzer:
> >> On 2019-09-17 11:23 a.m., Michel Dänzer wrote:
> >>> On 2019-09-17 10:23 a.m., Koenig, Christian wrote:
> >>>> Am 17.09.19 um 10:17 schrieb Daniel Vetter:
> >>>>> On Tue, Sep 17, 2019 at 10:12 AM Christian König
> >>>>> <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>>>> Am 17.09.19 um 07:47 schrieb Jani Nikula:
> >>>>>>> On Mon, 16 Sep 2019, Marek Olšák <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>>>>>> The purpose is to get rid of all PCI ID tables for all drivers in
> >>>>>>>> userspace. (or at least stop updating them)
> >>>>>>>>
> >>>>>>>> Mesa common code and modesetting will use this.
> >>>>>>> I'd think this would warrant a high level description of what you
> >>>>>>> want
> >>>>>>> to achieve in the commit message.
> >>>>>> And maybe explicitly call it uapi_name or even uapi_driver_name.
> >>>>> If it's uapi_name, then why do we need a new one for every
> generation?
> >>>>> Userspace drivers tend to span a lot more than just 1 generation. And
> >>>>> if you want to have per-generation data from the kernel to userspace,
> >>>>> then imo that's much better suited in some amdgpu ioctl, instead of
> >>>>> trying to encode that into the driver name.
> >>>> Well we already have an IOCTL for that, but I thought the intention
> >>>> here
> >>>> was to get rid of the PCI-ID tables in userspace to figure out which
> >>>> driver to load.
> >>> That's just unrealistic in general, I'm afraid. See e.g. the ongoing
> >>> transition from i965 to iris for recent Intel hardware. How is the
> >>> kernel supposed to know which driver is to be used?
> >
> > Well how is userspace currently handling that? The kernel should NOT say
> > which driver to use in userspace, but rather which one is used in the
> > kernel.
>
> Would that really help though? E.g. the radeon kernel driver supports
> radeon/r200/r300/r600/radeonsi DRI drivers, the i915 one i915/i965/iris
> (and the amdgpu one radeonsi/amdgpu).
>
> The HW generation identifier proposed in these patches might be useful,
> but I suspect there'll always be cases where userspace needs to know
> more precisely.
>
>
> > Mapping that information to an userspace driver still needs to be done
> > somewhere else, but the difference is that you don't need to add all
> > PCI-IDs twice.
>
> It should only really be necessary in Mesa.
>
>
> On 2019-09-17 1:32 p.m., Daniel Vetter wrote:
> > How are other compositors solving this? I don't expect they have a
> > pciid table like modesetting copied to all of them ...
>
> They don't need any of this. The Xorg modesetting driver only did for
> determining the client driver name to advertise via the DRI2 extension.
>
>
> --
> Earthling Michel Dänzer               |               https://redhat.com
> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #1.2: Type: text/html, Size: 5171 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Michel Dänzer]

On 2019-09-18 1:41 a.m., Marek Olšák wrote:
> drmVersion::name = amdgpu, radeon, intel, etc.
> drmVersion::desc = vega10, vega12, vega20, ...
> 
> The common Mesa code will use name and desc to select the driver.

Like the Xorg modesetting driver, that code doesn't need this kernel
functionality or new PCI IDs. It can just select the current driver for
all devices which aren't supported by older drivers (which is a fixed
set at this point).


> The AMD-specific Mesa code will use desc to identify the chip.

Doesn't libdrm_amdgpu's struct amdgpu_gpu_info::family_id provide the
same information?


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 847 bytes --]

On Wed, Sep 18, 2019 at 10:03 AM Michel Dänzer <michel@daenzer.net> wrote:

> On 2019-09-18 1:41 a.m., Marek Olšák wrote:
> > drmVersion::name = amdgpu, radeon, intel, etc.
> > drmVersion::desc = vega10, vega12, vega20, ...
> >
> > The common Mesa code will use name and desc to select the driver.
>
> Like the Xorg modesetting driver, that code doesn't need this kernel
> functionality or new PCI IDs. It can just select the current driver for
> all devices which aren't supported by older drivers (which is a fixed
> set at this point).
>
>
> > The AMD-specific Mesa code will use desc to identify the chip.
>
> Doesn't libdrm_amdgpu's struct amdgpu_gpu_info::family_id provide the
> same information?
>

Not for the common code, though I guess common Mesa code could use the INFO
ioctl. Is that what you mean?

Marek

[-- Attachment #1.2: Type: text/html, Size: 1231 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: add drm device name

[Marek Olšák]

[-- Attachment #1.1: Type: text/plain, Size: 1013 bytes --]

Let's drop this patch. Mesa will use family_id.

Marek

On Wed, Sep 18, 2019 at 4:10 PM Marek Olšák <maraeo@gmail.com> wrote:

> On Wed, Sep 18, 2019 at 10:03 AM Michel Dänzer <michel@daenzer.net> wrote:
>
>> On 2019-09-18 1:41 a.m., Marek Olšák wrote:
>> > drmVersion::name = amdgpu, radeon, intel, etc.
>> > drmVersion::desc = vega10, vega12, vega20, ...
>> >
>> > The common Mesa code will use name and desc to select the driver.
>>
>> Like the Xorg modesetting driver, that code doesn't need this kernel
>> functionality or new PCI IDs. It can just select the current driver for
>> all devices which aren't supported by older drivers (which is a fixed
>> set at this point).
>>
>>
>> > The AMD-specific Mesa code will use desc to identify the chip.
>>
>> Doesn't libdrm_amdgpu's struct amdgpu_gpu_info::family_id provide the
>> same information?
>>
>
> Not for the common code, though I guess common Mesa code could use the
> INFO ioctl. Is that what you mean?
>
> Marek
>

[-- Attachment #1.2: Type: text/html, Size: 1699 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: add drm device name

[Daniel Vetter]

On Tue, Sep 17, 2019 at 11:27 AM Michel Dänzer <michel@daenzer.net> wrote:
>
> On 2019-09-17 11:23 a.m., Michel Dänzer wrote:
> > On 2019-09-17 10:23 a.m., Koenig, Christian wrote:
> >> Am 17.09.19 um 10:17 schrieb Daniel Vetter:
> >>> On Tue, Sep 17, 2019 at 10:12 AM Christian König
> >>> <ckoenig.leichtzumerken@gmail.com> wrote:
> >>>> Am 17.09.19 um 07:47 schrieb Jani Nikula:
> >>>>> On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:
> >>>>>> The purpose is to get rid of all PCI ID tables for all drivers in
> >>>>>> userspace. (or at least stop updating them)
> >>>>>>
> >>>>>> Mesa common code and modesetting will use this.
> >>>>> I'd think this would warrant a high level description of what you want
> >>>>> to achieve in the commit message.
> >>>> And maybe explicitly call it uapi_name or even uapi_driver_name.
> >>> If it's uapi_name, then why do we need a new one for every generation?
> >>> Userspace drivers tend to span a lot more than just 1 generation. And
> >>> if you want to have per-generation data from the kernel to userspace,
> >>> then imo that's much better suited in some amdgpu ioctl, instead of
> >>> trying to encode that into the driver name.
> >>
> >> Well we already have an IOCTL for that, but I thought the intention here
> >> was to get rid of the PCI-ID tables in userspace to figure out which
> >> driver to load.
> >
> > That's just unrealistic in general, I'm afraid. See e.g. the ongoing
> > transition from i965 to iris for recent Intel hardware. How is the
> > kernel supposed to know which driver is to be used?
> >
> >
> > For the Xorg modesetting driver, there's a simple solution, see
> > https://gitlab.freedesktop.org/xorg/xserver/merge_requests/278 .
> > Something similar can probably be done in Mesa as well.
>
> Another possibility might be for Xorg to use
> https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_query_driver.txt
> to determine the driver name. Then only Mesa might need any HW specific
> code.

How are other compositors solving this? I don't expect they have a
pciid table like modesetting copied to all of them ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: add drm device name

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 2797 bytes --]

On Tue, 17 Sep 2019 13:32:05 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Sep 17, 2019 at 11:27 AM Michel Dänzer <michel@daenzer.net> wrote:
> >
> > On 2019-09-17 11:23 a.m., Michel Dänzer wrote:  
> > > On 2019-09-17 10:23 a.m., Koenig, Christian wrote:  
> > >> Am 17.09.19 um 10:17 schrieb Daniel Vetter:  
> > >>> On Tue, Sep 17, 2019 at 10:12 AM Christian König
> > >>> <ckoenig.leichtzumerken@gmail.com> wrote:  
> > >>>> Am 17.09.19 um 07:47 schrieb Jani Nikula:  
> > >>>>> On Mon, 16 Sep 2019, Marek Olšák <maraeo@gmail.com> wrote:  
> > >>>>>> The purpose is to get rid of all PCI ID tables for all drivers in
> > >>>>>> userspace. (or at least stop updating them)
> > >>>>>>
> > >>>>>> Mesa common code and modesetting will use this.  
> > >>>>> I'd think this would warrant a high level description of what you want
> > >>>>> to achieve in the commit message.  
> > >>>> And maybe explicitly call it uapi_name or even uapi_driver_name.  
> > >>> If it's uapi_name, then why do we need a new one for every generation?
> > >>> Userspace drivers tend to span a lot more than just 1 generation. And
> > >>> if you want to have per-generation data from the kernel to userspace,
> > >>> then imo that's much better suited in some amdgpu ioctl, instead of
> > >>> trying to encode that into the driver name.  
> > >>
> > >> Well we already have an IOCTL for that, but I thought the intention here
> > >> was to get rid of the PCI-ID tables in userspace to figure out which
> > >> driver to load.  
> > >
> > > That's just unrealistic in general, I'm afraid. See e.g. the ongoing
> > > transition from i965 to iris for recent Intel hardware. How is the
> > > kernel supposed to know which driver is to be used?
> > >
> > >
> > > For the Xorg modesetting driver, there's a simple solution, see
> > > https://gitlab.freedesktop.org/xorg/xserver/merge_requests/278 .
> > > Something similar can probably be done in Mesa as well.  
> >
> > Another possibility might be for Xorg to use
> > https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_query_driver.txt
> > to determine the driver name. Then only Mesa might need any HW specific
> > code.  
> 
> How are other compositors solving this? I don't expect they have a
> pciid table like modesetting copied to all of them ...

Hi,

other compositors have no driver-specific code at all, so they do not
need to know what the driver in kernel or Mesa is, or what chip they
are running. They may report chip name and stuff in logs for the users'
amusement, but nothing more.

Even the NVIDIA proprietary driver detection happens through EGL
extension search: no-one else exposes EGLStreams.

Unless maybe if someone needs to quirk around driver bugs.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel