Re: [PATCH 1/1] drm/amdgpu: init debugfs drm driver callback

["Das, Nirmoy" <nirmoy.das@xxxxxxx>]

On 10/5/2021 2:01 PM, Christian König wrote:
> Am 05.10.21 um 13:58 schrieb Nirmoy Das:
> > drm_dev_register() will try to init driver's debugfs using
> > drm_driver.debugfs_init call back function. Use that callback
> > also for amdgpu to intialize debugfs.
>
> Mhm, why is that useful? We rather wanted to get rid of all this DRM 
> midlayering.


I was thinking of not calling further debugfs  APIs if we are unable to 
create the root dentry itself by adding another

patch in drm_debugfs_init(). But I agree with removing DRM midlayering, 
I  will then add a  IS_ERR(root) check in amdgpu_debugfs_init()


Nirmoy

> Christian.
>
> > Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 10 +++++-----
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h |  2 +-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     |  7 +++----
> >   3 files changed, 9 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > index 6611b3c7c149..3076742f8f85 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > @@ -1611,8 +1611,9 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_ib_preempt, NULL,
> >   DEFINE_DEBUGFS_ATTRIBUTE(fops_sclk_set, NULL,
> >               amdgpu_debugfs_sclk_set, "%llu\n");
> >   -int amdgpu_debugfs_init(struct amdgpu_device *adev)
> > +void amdgpu_debugfs_init(struct drm_minor *minor)
> >   {
> > +    struct amdgpu_device *adev = drm_to_adev(minor->dev);
> >       struct dentry *root = adev_to_drm(adev)->primary->debugfs_root;
> >       struct dentry *ent;
> >       int r, i;
> > @@ -1621,14 +1622,14 @@ int amdgpu_debugfs_init(struct amdgpu_device 
> > *adev)
> >                     &fops_ib_preempt);
> >       if (IS_ERR(ent)) {
> >           DRM_ERROR("unable to create amdgpu_preempt_ib debugsfs 
> > file\n");
> > -        return PTR_ERR(ent);
> > +        return;
> >       }
> >         ent = debugfs_create_file("amdgpu_force_sclk", 0200, root, adev,
> >                     &fops_sclk_set);
> >       if (IS_ERR(ent)) {
> >           DRM_ERROR("unable to create amdgpu_set_sclk debugsfs file\n");
> > -        return PTR_ERR(ent);
> > +        return;
> >       }
> >         /* Register debugfs entries for amdgpu_ttm */
> > @@ -1682,11 +1683,10 @@ int amdgpu_debugfs_init(struct amdgpu_device 
> > *adev)
> >       debugfs_create_blob("amdgpu_discovery", 0444, root,
> >                   &adev->debugfs_discovery_blob);
> >   -    return 0;
> >   }
> >     #else
> > -int amdgpu_debugfs_init(struct amdgpu_device *adev)
> > +void amdgpu_debugfs_init(struct drm_minor *minor)
> >   {
> >       return 0;
> >   }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> > index 371a6f0deb29..06b68e16e35d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.h
> > @@ -27,7 +27,7 @@
> >    */
> >     int amdgpu_debugfs_regs_init(struct amdgpu_device *adev);
> > -int amdgpu_debugfs_init(struct amdgpu_device *adev);
> > +void amdgpu_debugfs_init(struct drm_minor *minor);
> >   void amdgpu_debugfs_fini(struct amdgpu_device *adev);
> >   void amdgpu_debugfs_fence_init(struct amdgpu_device *adev);
> >   void amdgpu_debugfs_firmware_init(struct amdgpu_device *adev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index df83b1f438b6..ceda650895db 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -2012,10 +2012,6 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> >               drm_fbdev_generic_setup(adev_to_drm(adev), 32);
> >       }
> >   -    ret = amdgpu_debugfs_init(adev);
> > -    if (ret)
> > -        DRM_ERROR("Creating debugfs files failed (%d).\n", ret);
> > -
> >       return 0;
> >     err_pci:
> > @@ -2479,6 +2475,9 @@ static const struct drm_driver 
> > amdgpu_kms_driver = {
> >       .dumb_map_offset = amdgpu_mode_dumb_mmap,
> >       .fops = &amdgpu_driver_kms_fops,
> >       .release = &amdgpu_driver_release_kms,
> > +#if defined(CONFIG_DEBUG_FS)
> > +    .debugfs_init = amdgpu_debugfs_init,
> > +#endif
> >         .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> >       .prime_fd_to_handle = drm_gem_prime_fd_to_handle,