Re: [PATCH 14/15] drm/amdgpu/ring: move debugfs init into core amdgpu debugfs

["Christian König" <christian.koenig@amd.com>]

Am 13.02.20 um 15:32 schrieb Alex Deucher:
> On Thu, Feb 13, 2020 at 5:17 AM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 13.02.20 um 10:54 schrieb Daniel Vetter:
>>> On Fri, Feb 07, 2020 at 02:50:57PM -0500, Alex Deucher wrote:
>>>> In order to remove the load and unload drm callbacks,
>>>> we need to reorder the init sequence to move all the drm
>>>> debugfs file handling.  Do this for rings.
>>>>
>>>> Acked-by: Christian König <christian.koenig@amd.com>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 23 ++++++++++++++++++++-
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c    | 15 +++-----------
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h    |  4 ++++
>>>>    3 files changed, 29 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>>>> index df3919ef886b..7379910790c9 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>>>> @@ -1218,7 +1218,7 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
>>>>
>>>>    int amdgpu_debugfs_init(struct amdgpu_device *adev)
>>>>    {
>>>> -    int r;
>>>> +    int r, i;
>>>>
>>>>       adev->debugfs_preempt =
>>>>               debugfs_create_file("amdgpu_preempt_ib", 0600,
>>>> @@ -1268,12 +1268,33 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
>>>>       }
>>>>    #endif
>>>>
>>>> +    for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>>>> +            struct amdgpu_ring *ring = adev->rings[i];
>>>> +
>>>> +            if (!ring)
>>>> +                    continue;
>>>> +
>>>> +            if (amdgpu_debugfs_ring_init(adev, ring)) {
>>>> +                    DRM_ERROR("Failed to register debugfs file for rings !\n");
>>>> +            }
>>>> +    }
>>>> +
>>>>       return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,
>>>>                                       ARRAY_SIZE(amdgpu_debugfs_list));
>>>>    }
>>>>
>>>>    void amdgpu_debugfs_fini(struct amdgpu_device *adev)
>>> btw debugfs_fini shouldn't be needed anymore, Greg KH removed this all.
>>> drm core removes all debugfs files recusrively for you, there should be 0
>>> need for debugfs cleanup.
>> Oh, yes please. Removing that was on my TODO list for an eternity as well.
>>
>>> Also at least my tree doesn't even have this, where does this apply to?
>> I would guess amd-staging-drm-next, but it might be that Alex is waiting
>> for the next rebase to land.
>>
> Patches are against my drm-next branch which is based on fdo drm-next.
> There are a number of files which the driver creates directly rather
> than through drm.

The last time I locked it I was about to completely nuke creating 
anything through DRM and just create it directly.

As Daniel wrote we also don't have to remove anything explicitly, that 
is done implicitly when the whole directory is removed.

Christian.

>
> Alex
>
>> Christian.
>>
>>> -Daniel
>>>
>>>>    {
>>>> +    int i;
>>>> +
>>>> +    for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>>>> +            struct amdgpu_ring *ring = adev->rings[i];
>>>> +
>>>> +            if (!ring)
>>>> +                    continue;
>>>> +
>>>> +            amdgpu_debugfs_ring_fini(ring);
>>>> +    }
>>>>       amdgpu_ttm_debugfs_fini(adev);
>>>>       debugfs_remove(adev->debugfs_preempt);
>>>>    }
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> index e5c83e164d82..539be138260e 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> @@ -48,9 +48,6 @@
>>>>     * wptr.  The GPU then starts fetching commands and executes
>>>>     * them until the pointers are equal again.
>>>>     */
>>>> -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
>>>> -                                struct amdgpu_ring *ring);
>>>> -static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring);
>>>>
>>>>    /**
>>>>     * amdgpu_ring_alloc - allocate space on the ring buffer
>>>> @@ -334,10 +331,6 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
>>>>       for (i = 0; i < DRM_SCHED_PRIORITY_MAX; ++i)
>>>>               atomic_set(&ring->num_jobs[i], 0);
>>>>
>>>> -    if (amdgpu_debugfs_ring_init(adev, ring)) {
>>>> -            DRM_ERROR("Failed to register debugfs file for rings !\n");
>>>> -    }
>>>> -
>>>>       return 0;
>>>>    }
>>>>
>>>> @@ -367,8 +360,6 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
>>>>                             &ring->gpu_addr,
>>>>                             (void **)&ring->ring);
>>>>
>>>> -    amdgpu_debugfs_ring_fini(ring);
>>>> -
>>>>       dma_fence_put(ring->vmid_wait);
>>>>       ring->vmid_wait = NULL;
>>>>       ring->me = 0;
>>>> @@ -485,8 +476,8 @@ static const struct file_operations amdgpu_debugfs_ring_fops = {
>>>>
>>>>    #endif
>>>>
>>>> -static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
>>>> -                                struct amdgpu_ring *ring)
>>>> +int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
>>>> +                         struct amdgpu_ring *ring)
>>>>    {
>>>>    #if defined(CONFIG_DEBUG_FS)
>>>>       struct drm_minor *minor = adev->ddev->primary;
>>>> @@ -507,7 +498,7 @@ static int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
>>>>       return 0;
>>>>    }
>>>>
>>>> -static void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring)
>>>> +void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring)
>>>>    {
>>>>    #if defined(CONFIG_DEBUG_FS)
>>>>       debugfs_remove(ring->ent);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> index 5134d0dd6dc2..0d098dafd23c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> @@ -329,4 +329,8 @@ static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>>>>
>>>>    int amdgpu_ring_test_helper(struct amdgpu_ring *ring);
>>>>
>>>> +int amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
>>>> +                         struct amdgpu_ring *ring);
>>>> +void amdgpu_debugfs_ring_fini(struct amdgpu_ring *ring);
>>>> +
>>>>    #endif
>>>> --
>>>> 2.24.1
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C0647e5ecad344fb4d19208d7b09198d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637172011693912378&amp;sdata=dBwdfJW2nPzwPZJYklNexgu0NWT3vGSOgbO79SLQ%2B74%3D&amp;reserved=0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx