On Sat, Sep 7, 2019 at 3:18 AM Rob Clark <robdclark@xxxxxxxxx> wrote:
>
> On Fri, Sep 6, 2019 at 3:16 PM Marek Olšák <maraeo@xxxxxxxxx> wrote:
> >
> > + dri-devel
> >
> > On Tue, Sep 3, 2019 at 5:41 PM Jiang, Sonny <Sonny.Jiang@xxxxxxx> 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@xxxxxxx>
> >> ---
> >> 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@xxxxxxxxxxxxxxxxxxxxx
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
