[PATCH] drm: move debugfs init after dev->load()

[Daniel Vetter]

On Fri, May 30, 2014 at 12:37 AM, Rob Clark <robdclark at gmail.com> wrote:
> Avoids ugly hacks in drivers debugfs code, if it depends on
> dev->dev_private having already been initialized.
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>

So what I had in mind:
- stop using drm_platform_init, instead roll your own copy of
drm_get_platform_dev
- don't driver->bus, instead use Thierry's set_busid interface

Then as a second step you can stop calling your ->load callback and
instead insert stuff between drm_dev_alloc and drm_dev_register. We
have pretty much the same problem with sysfs as with debugfs, so your
patch here doesn't really solve all that much - ->load should be
called mostly before drm_dev_register sets up all the userspace
interfaces.

Long term I want to phase-out ->load except for legacy non-kms drivers.
-Daniel

> ---
> Some cleanup that Daniel Vetter wants to do may make this unnecessary
> in the future, but in order to unblock some msm patches that I'd like
> to send for 3.16, I propose this patch.  If no objections, I could
> include this in msm pull req.
>
>  drivers/gpu/drm/drm_debugfs.c | 15 +++++++++------
>  drivers/gpu/drm/drm_stub.c    | 24 ++++++++++++++++++------
>  include/drm/drmP.h            |  6 ++----
>  3 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index b4b51d4..231d1ab 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -134,23 +134,26 @@ EXPORT_SYMBOL(drm_debugfs_create_files);
>   * Initialize the DRI debugfs filesystem for a device
>   *
>   * \param dev DRM device
> - * \param minor device minor number
> - * \param root DRI debugfs dir entry.
>   *
>   * Create the DRI debugfs root entry "/sys/kernel/debug/dri", the device debugfs root entry
>   * "/sys/kernel/debug/dri/%minor%/", and each entry in debugfs_list as
>   * "/sys/kernel/debug/dri/%minor%/%name%".
>   */
> -int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> -                    struct dentry *root)
> +int drm_debugfs_init(struct drm_minor *minor)
>  {
> -       struct drm_device *dev = minor->dev;
> +       struct drm_device *dev;
> +       struct dentry *root = drm_debugfs_root;
>         char name[64];
>         int ret;
>
> +       if (!minor)
> +               return 0;
> +
> +       dev = minor->dev;
> +
>         INIT_LIST_HEAD(&minor->debugfs_list);
>         mutex_init(&minor->debugfs_lock);
> -       sprintf(name, "%d", minor_id);
> +       sprintf(name, "%d", minor->index);
>         minor->debugfs_root = debugfs_create_dir(name, root);
>         if (!minor->debugfs_root) {
>                 DRM_ERROR("Cannot create /sys/kernel/debug/dri/%s\n", name);
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 3727ac8..94e3742 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -328,12 +328,6 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
>
>         new_minor->index = minor_id;
>
> -       ret = drm_debugfs_init(new_minor, minor_id, drm_debugfs_root);
> -       if (ret) {
> -               DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> -               goto err_id;
> -       }
> -
>         ret = drm_sysfs_device_add(new_minor);
>         if (ret) {
>                 DRM_ERROR("DRM: Error sysfs_device_add.\n");
> @@ -723,6 +717,24 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>                         goto err_minors;
>         }
>
> +       ret = drm_debugfs_init(dev->control);
> +       if (ret) {
> +               DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> +               goto err_minors;
> +       }
> +
> +       ret = drm_debugfs_init(dev->render);
> +       if (ret) {
> +               DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> +               goto err_minors;
> +       }
> +
> +       ret = drm_debugfs_init(dev->primary);
> +       if (ret) {
> +               DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> +               goto err_minors;
> +       }
> +
>         /* setup grouping for legacy outputs */
>         if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>                 ret = drm_mode_group_init_legacy_group(dev,
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 12f10bc..76e6e03 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1409,8 +1409,7 @@ extern struct drm_local_map *drm_getsarea(struct drm_device *dev);
>
>                                 /* Debugfs support */
>  #if defined(CONFIG_DEBUG_FS)
> -extern int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> -                           struct dentry *root);
> +extern int drm_debugfs_init(struct drm_minor *minor);
>  extern int drm_debugfs_create_files(const struct drm_info_list *files,
>                                     int count, struct dentry *root,
>                                     struct drm_minor *minor);
> @@ -1418,8 +1417,7 @@ extern int drm_debugfs_remove_files(const struct drm_info_list *files,
>                                     int count, struct drm_minor *minor);
>  extern int drm_debugfs_cleanup(struct drm_minor *minor);
>  #else
> -static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> -                                  struct dentry *root)
> +static inline int drm_debugfs_init(struct drm_minor *minor)
>  {
>         return 0;
>  }
> --
> 1.9.3
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch