Hey On Fri, Dec 9, 2016 at 11:42 AM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > We thought that no userspace is using them, but oops libdrm is using > them to figure out whether a driver supports modesetting. Check out > drmCheckModesettingSupported but maybe don't because it's horrible and > totally runs counter to where we want to go with libdrm device > handling. The function looks in the device hierarchy for whether > controlD* exist using the following format string: > > /sys/bus/pci/devices/%04x:%02x:%02x.%d/drm/controlD%d > > The "/drm" subdirectory is the glue directory from the sysfs class > stuff, and the only way to get at it seems to through > kdev->kobj.parent (when kdev is represents e.g. the card0 chardev > instance in sysfs). Git grep says we're not the only ones touching > that, so I hope it's ok we dig into such internals - I couldn't find a > proper interface for getting at the glue directory. > > Quick git grep shows that at least -amdgpu, -ati and UXA in -intel are > using this. -modesetting and SNA in -intel do not, which is why this > didn't blow up earlier. > > Oh well, we need to keep it working, and the simplest way is to add a > symlink at the right place in debugfs from controlD* to card*. > > Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes") > Cc: Dave Airlie <airlied@xxxxxxxxx> > Reported-by: Alex Deucher <alexander.deucher@xxxxxxx> > Cc: Alex Deucher <alexander.deucher@xxxxxxx> > Cc: David Herrmann <dh.herrmann@xxxxxxxxx> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > --- > drivers/gpu/drm/drm_drv.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 4ec61ac27477..5baec7202558 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -650,6 +650,47 @@ void drm_dev_unref(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_dev_unref); > > +static int create_compat_control_link(struct drm_device *dev) > +{ > + struct drm_minor *minor; > + char *name; > + int ret; > + > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return 0; > + > + minor = *drm_minor_get_slot(dev, DRM_MINOR_PRIMARY); if (!minor) return -EINVAL; (I don't insist on that one, but see below) > + name = kasprintf(GFP_KERNEL, "controlD%d", minor->index); Wrong index, right? This would use '0' rather than '64'. Probably does not matter, though. > + if (!name) > + return -ENOMEM; > + > + ret = sysfs_create_link(minor->kdev->kobj.parent, > + &minor->kdev->kobj, > + name); > + > + kfree(name); > + > + return ret; > +} > + > +static void remove_compat_control_link(struct drm_device *dev) > +{ > + struct drm_minor *minor; > + char *name; > + > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return; > + > + minor = *drm_minor_get_slot(dev, DRM_MINOR_PRIMARY); if (!minor) return; I mean, our error-paths often coalesce multiple similar destructor calls, and expect the destructor calls. So no reason to assume the primary-node is initialized. In fact, our drm_dev_register() creates the primary node last, so if the render-node creation fails we will call here and NULL-deref. Otherwise looks good to me. Thanks David > + name = kasprintf(GFP_KERNEL, "controlD%d", minor->index); > + if (!name) > + return; > + > + sysfs_remove_link(minor->kdev->kobj.parent, name); > + > + kfree(name); > +} > + > /** > * drm_dev_register - Register DRM device > * @dev: Device to register > @@ -688,6 +729,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > if (ret) > goto err_minors; > > + ret = create_compat_control_link(dev); > + if (ret) > + goto err_minors; > + > if (dev->driver->load) { > ret = dev->driver->load(dev, flags); > if (ret) > @@ -701,6 +746,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > goto out_unlock; > > err_minors: > + remove_compat_control_link(dev); > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > drm_minor_unregister(dev, DRM_MINOR_RENDER); > drm_minor_unregister(dev, DRM_MINOR_CONTROL); > @@ -741,6 +787,7 @@ void drm_dev_unregister(struct drm_device *dev) > list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) > drm_legacy_rmmap(dev, r_list->map); > > + remove_compat_control_link(dev); > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > drm_minor_unregister(dev, DRM_MINOR_RENDER); > drm_minor_unregister(dev, DRM_MINOR_CONTROL); > -- > 2.10.2 > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel