Re: [Intel-gfx] [RFC 2/3] drm: Register drmfs filesystem from drm init
On Thu, 2016-12-01 at 23:57 -0800, Daniel Vetter wrote: > On Thu, Dec 01, 2016 at 01:44:16PM +0530, swati.dhin...@intel.com wrote: > > From: Swati Dhingra > > > > During drm module initialization, drm_core_init initializes the drmfs > > filesystem and register this with kernel. A driver specific directory is > > created > > inside drmfs root, and dentry of this directory is saved for subsequent use > > by the driver (e.g. i915). The driver can then create files/directories > > inside > > this root directory directly. > > In case of i915 driver, the top directory is created at > > '/sys/kernel/drm/i915'. > > > > Signed-off-by: Sourab Gupta > > Signed-off-by: Swati Dhingra > > --- > > drivers/gpu/drm/drm_drv.c | 22 ++ > > include/drm/drm_drv.h | 3 +++ > > 2 files changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index 84fcfcb..ead360bd 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -688,6 +688,14 @@ int drm_dev_register(struct drm_device *dev, unsigned > > long flags) > > { > > int ret; > > > > +#ifdef CONFIG_DRMFS > > + dev->driver->drmfs_root = drmfs_create_dir(dev->driver->name, NULL); > > + if (IS_ERR(dev->driver->drmfs_root)) { > > + DRM_ERROR("Failed to get drmfs root dentry\n"); > > + return PTR_ERR(dev->driver->drmfs_root); > > + } > > +#endif > > Don't do #ifdef in the code, instead provide dummy static inline functions > that do nothing in headers when a feature is disabled. For an example see > CONFIG_DRM_FBDEV_EMULATION in drm_fb_helper.[hc]. > Sorry, Will take take of this going forward. > Also, drmfs here is seriously lacking documentation. E.g. where are we > supposed to put different things related to rendering, modesetting, and > all these issues? You need to add a section in drm-uabi.rst, write > kernel-doc + overview sections for all of this and pull it in. > -Daniel > Ok. Will work on adding requisite documentation for drmfs. Thanks, Sourab > > + > > mutex_lock(&drm_global_mutex); > > > > ret = drm_minor_register(dev, DRM_MINOR_CONTROL); > > @@ -758,6 +766,9 @@ void drm_dev_unregister(struct drm_device *dev) > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > > drm_minor_unregister(dev, DRM_MINOR_RENDER); > > drm_minor_unregister(dev, DRM_MINOR_CONTROL); > > +#ifdef CONFIG_DRMFS > > + drmfs_remove(dev->driver->drmfs_root); > > +#endif > > } > > EXPORT_SYMBOL(drm_dev_unregister); > > > > @@ -825,6 +836,9 @@ static void drm_core_exit(void) > > { > > unregister_chrdev(DRM_MAJOR, "drm"); > > debugfs_remove(drm_debugfs_root); > > +#ifdef CONFIG_DRMFS > > + drmfs_fini(); > > +#endif > > drm_sysfs_destroy(); > > idr_destroy(&drm_minors_idr); > > drm_connector_ida_destroy(); > > @@ -845,6 +859,14 @@ static int __init drm_core_init(void) > > goto error; > > } > > > > +#ifdef CONFIG_DRMFS > > + ret = drmfs_init(); > > + if (ret < 0) { > > + DRM_ERROR("Cannot create DRM FS: %d\n", ret); > > + goto error; > > + } > > +#endif > > + > > drm_debugfs_root = debugfs_create_dir("dri", NULL); > > if (!drm_debugfs_root) { > > ret = -ENOMEM; > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > > index aad8bba..34804de 100644 > > --- a/include/drm/drm_drv.h > > +++ b/include/drm/drm_drv.h > > @@ -403,6 +403,9 @@ struct drm_driver { > > > > /* List of devices hanging off this driver with stealth attach. */ > > struct list_head legacy_dev_list; > > + > > + /* drmfs parent directory dentry for this driver */ > > + struct dentry *drmfs_root; > > }; > > > > extern __printf(6, 7) > > -- > > 2.7.4 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/3] drm: Register drmfs filesystem from drm init
On Thu, Dec 01, 2016 at 01:44:16PM +0530, swati.dhin...@intel.com wrote: > From: Swati Dhingra > > During drm module initialization, drm_core_init initializes the drmfs > filesystem and register this with kernel. A driver specific directory is > created > inside drmfs root, and dentry of this directory is saved for subsequent use > by the driver (e.g. i915). The driver can then create files/directories inside > this root directory directly. > In case of i915 driver, the top directory is created at > '/sys/kernel/drm/i915'. > > Signed-off-by: Sourab Gupta > Signed-off-by: Swati Dhingra > --- > drivers/gpu/drm/drm_drv.c | 22 ++ > include/drm/drm_drv.h | 3 +++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 84fcfcb..ead360bd 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -688,6 +688,14 @@ int drm_dev_register(struct drm_device *dev, unsigned > long flags) > { > int ret; > > +#ifdef CONFIG_DRMFS > + dev->driver->drmfs_root = drmfs_create_dir(dev->driver->name, NULL); > + if (IS_ERR(dev->driver->drmfs_root)) { > + DRM_ERROR("Failed to get drmfs root dentry\n"); > + return PTR_ERR(dev->driver->drmfs_root); > + } > +#endif Don't do #ifdef in the code, instead provide dummy static inline functions that do nothing in headers when a feature is disabled. For an example see CONFIG_DRM_FBDEV_EMULATION in drm_fb_helper.[hc]. Also, drmfs here is seriously lacking documentation. E.g. where are we supposed to put different things related to rendering, modesetting, and all these issues? You need to add a section in drm-uabi.rst, write kernel-doc + overview sections for all of this and pull it in. -Daniel > + > mutex_lock(&drm_global_mutex); > > ret = drm_minor_register(dev, DRM_MINOR_CONTROL); > @@ -758,6 +766,9 @@ void drm_dev_unregister(struct drm_device *dev) > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > drm_minor_unregister(dev, DRM_MINOR_RENDER); > drm_minor_unregister(dev, DRM_MINOR_CONTROL); > +#ifdef CONFIG_DRMFS > + drmfs_remove(dev->driver->drmfs_root); > +#endif > } > EXPORT_SYMBOL(drm_dev_unregister); > > @@ -825,6 +836,9 @@ static void drm_core_exit(void) > { > unregister_chrdev(DRM_MAJOR, "drm"); > debugfs_remove(drm_debugfs_root); > +#ifdef CONFIG_DRMFS > + drmfs_fini(); > +#endif > drm_sysfs_destroy(); > idr_destroy(&drm_minors_idr); > drm_connector_ida_destroy(); > @@ -845,6 +859,14 @@ static int __init drm_core_init(void) > goto error; > } > > +#ifdef CONFIG_DRMFS > + ret = drmfs_init(); > + if (ret < 0) { > + DRM_ERROR("Cannot create DRM FS: %d\n", ret); > + goto error; > + } > +#endif > + > drm_debugfs_root = debugfs_create_dir("dri", NULL); > if (!drm_debugfs_root) { > ret = -ENOMEM; > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index aad8bba..34804de 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -403,6 +403,9 @@ struct drm_driver { > > /* List of devices hanging off this driver with stealth attach. */ > struct list_head legacy_dev_list; > + > + /* drmfs parent directory dentry for this driver */ > + struct dentry *drmfs_root; > }; > > extern __printf(6, 7) > -- > 2.7.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/3] drm: Register drmfs filesystem from drm init
On Thu, 2016-12-01 at 00:15 -0800, Chris Wilson wrote: > On Thu, Dec 01, 2016 at 12:32:32PM +0530, swati.dhin...@intel.com wrote: > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index 84fcfcb..ead360bd 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -688,6 +688,14 @@ int drm_dev_register(struct drm_device *dev, unsigned > > long flags) > > { > > int ret; > > > > +#ifdef CONFIG_DRMFS > > Rule of thumb: avoid #ifdeffry in the body of the code, use headers to > hide conditional compilation. Ok. Will do the requisite changes. > > > + dev->driver->drmfs_root = drmfs_create_dir(dev->driver->name, NULL); > > + if (IS_ERR(dev->driver->drmfs_root)) { > > + DRM_ERROR("Failed to get drmfs root dentry\n"); > > + return PTR_ERR(dev->driver->drmfs_root); > > + } > > Considering use of drmfs is optional, should an error here prevent the > driver from loading? > -Chris Ok. Will remove the return on the error here. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/3] drm: Register drmfs filesystem from drm init
On Thu, Dec 01, 2016 at 12:32:32PM +0530, swati.dhin...@intel.com wrote: > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 84fcfcb..ead360bd 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -688,6 +688,14 @@ int drm_dev_register(struct drm_device *dev, unsigned > long flags) > { > int ret; > > +#ifdef CONFIG_DRMFS Rule of thumb: avoid #ifdeffry in the body of the code, use headers to hide conditional compilation. > + dev->driver->drmfs_root = drmfs_create_dir(dev->driver->name, NULL); > + if (IS_ERR(dev->driver->drmfs_root)) { > + DRM_ERROR("Failed to get drmfs root dentry\n"); > + return PTR_ERR(dev->driver->drmfs_root); > + } Considering use of drmfs is optional, should an error here prevent the driver from loading? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 2/3] drm: Register drmfs filesystem from drm init
From: Swati Dhingra During drm module initialization, drm_core_init initializes the drmfs filesystem and register this with kernel. A driver specific directory is created inside drmfs root, and dentry of this directory is saved for subsequent use by the driver (e.g. i915). The driver can then create files/directories inside this root directory directly. In case of i915 driver, the top directory is created at '/sys/kernel/drm/i915'. Signed-off-by: Sourab Gupta Signed-off-by: Swati Dhingra --- drivers/gpu/drm/drm_drv.c | 22 ++ include/drm/drm_drv.h | 3 +++ 2 files changed, 25 insertions(+) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 84fcfcb..ead360bd 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -688,6 +688,14 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) { int ret; +#ifdef CONFIG_DRMFS + dev->driver->drmfs_root = drmfs_create_dir(dev->driver->name, NULL); + if (IS_ERR(dev->driver->drmfs_root)) { + DRM_ERROR("Failed to get drmfs root dentry\n"); + return PTR_ERR(dev->driver->drmfs_root); + } +#endif + mutex_lock(&drm_global_mutex); ret = drm_minor_register(dev, DRM_MINOR_CONTROL); @@ -758,6 +766,9 @@ void drm_dev_unregister(struct drm_device *dev) drm_minor_unregister(dev, DRM_MINOR_PRIMARY); drm_minor_unregister(dev, DRM_MINOR_RENDER); drm_minor_unregister(dev, DRM_MINOR_CONTROL); +#ifdef CONFIG_DRMFS + drmfs_remove(dev->driver->drmfs_root); +#endif } EXPORT_SYMBOL(drm_dev_unregister); @@ -825,6 +836,9 @@ static void drm_core_exit(void) { unregister_chrdev(DRM_MAJOR, "drm"); debugfs_remove(drm_debugfs_root); +#ifdef CONFIG_DRMFS + drmfs_fini(); +#endif drm_sysfs_destroy(); idr_destroy(&drm_minors_idr); drm_connector_ida_destroy(); @@ -845,6 +859,14 @@ static int __init drm_core_init(void) goto error; } +#ifdef CONFIG_DRMFS + ret = drmfs_init(); + if (ret < 0) { + DRM_ERROR("Cannot create DRM FS: %d\n", ret); + goto error; + } +#endif + drm_debugfs_root = debugfs_create_dir("dri", NULL); if (!drm_debugfs_root) { ret = -ENOMEM; diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index aad8bba..34804de 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -403,6 +403,9 @@ struct drm_driver { /* List of devices hanging off this driver with stealth attach. */ struct list_head legacy_dev_list; + + /* drmfs parent directory dentry for this driver */ + struct dentry *drmfs_root; }; extern __printf(6, 7) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 2/3] drm: Register drmfs filesystem from drm init
From: Swati Dhingra During drm module initialization, drm_core_init initializes the drmfs filesystem and register this with kernel. A driver specific directory is created inside drmfs root, and dentry of this directory is saved for subsequent use by the driver (e.g. i915). The driver can then create files/directories inside this root directory directly. In case of i915 driver, the top directory is created at '/sys/kernel/drm/i915'. Signed-off-by: Sourab Gupta Signed-off-by: Swati Dhingra --- drivers/gpu/drm/drm_drv.c | 22 ++ include/drm/drm_drv.h | 3 +++ 2 files changed, 25 insertions(+) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 84fcfcb..ead360bd 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -688,6 +688,14 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) { int ret; +#ifdef CONFIG_DRMFS + dev->driver->drmfs_root = drmfs_create_dir(dev->driver->name, NULL); + if (IS_ERR(dev->driver->drmfs_root)) { + DRM_ERROR("Failed to get drmfs root dentry\n"); + return PTR_ERR(dev->driver->drmfs_root); + } +#endif + mutex_lock(&drm_global_mutex); ret = drm_minor_register(dev, DRM_MINOR_CONTROL); @@ -758,6 +766,9 @@ void drm_dev_unregister(struct drm_device *dev) drm_minor_unregister(dev, DRM_MINOR_PRIMARY); drm_minor_unregister(dev, DRM_MINOR_RENDER); drm_minor_unregister(dev, DRM_MINOR_CONTROL); +#ifdef CONFIG_DRMFS + drmfs_remove(dev->driver->drmfs_root); +#endif } EXPORT_SYMBOL(drm_dev_unregister); @@ -825,6 +836,9 @@ static void drm_core_exit(void) { unregister_chrdev(DRM_MAJOR, "drm"); debugfs_remove(drm_debugfs_root); +#ifdef CONFIG_DRMFS + drmfs_fini(); +#endif drm_sysfs_destroy(); idr_destroy(&drm_minors_idr); drm_connector_ida_destroy(); @@ -845,6 +859,14 @@ static int __init drm_core_init(void) goto error; } +#ifdef CONFIG_DRMFS + ret = drmfs_init(); + if (ret < 0) { + DRM_ERROR("Cannot create DRM FS: %d\n", ret); + goto error; + } +#endif + drm_debugfs_root = debugfs_create_dir("dri", NULL); if (!drm_debugfs_root) { ret = -ENOMEM; diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index aad8bba..34804de 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -403,6 +403,9 @@ struct drm_driver { /* List of devices hanging off this driver with stealth attach. */ struct list_head legacy_dev_list; + + /* drmfs parent directory dentry for this driver */ + struct dentry *drmfs_root; }; extern __printf(6, 7) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx