Re: [PATCH] drm: add drm device name

[Jani Nikula <jani.nikula@linux.intel.com>]

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