Re: [PATCH 1/1] drm/amdgpu: unify BO evicting method in amdgpu_ttm

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

On 10/7/2021 12:38 PM, Christian König wrote:
> Am 07.10.21 um 12:00 schrieb Nirmoy Das:
> > Unify BO evicting functionality for possible memory
> > types in amdgpu_ttm.c.
> >
> > Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  8 +++---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 30 ++++++++++++++++-----
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 23 ----------------
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 -
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 30 +++++++++++++++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
> >   6 files changed, 58 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > index 5497e2d31d1a..164d6a9e9fbb 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > @@ -1328,7 +1328,7 @@ static int amdgpu_debugfs_evict_vram(void 
> > *data, u64 *val)
> >           return r;
> >       }
> >   -    *val = amdgpu_bo_evict_vram(adev);
> > +    *val = amdgpu_ttm_evict_resources(adev, TTM_PL_VRAM);
> >         pm_runtime_mark_last_busy(dev->dev);
> >       pm_runtime_put_autosuspend(dev->dev);
> > @@ -1341,17 +1341,15 @@ static int amdgpu_debugfs_evict_gtt(void 
> > *data, u64 *val)
> >   {
> >       struct amdgpu_device *adev = (struct amdgpu_device *)data;
> >       struct drm_device *dev = adev_to_drm(adev);
> > -    struct ttm_resource_manager *man;
> >       int r;
> >         r = pm_runtime_get_sync(dev->dev);
> >       if (r < 0) {
> > -        pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > +        pm_runtime_put_autosuspend(dev->dev);
> >           return r;
> >       }
> >   -    man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
> > -    *val = ttm_resource_manager_evict_all(&adev->mman.bdev, man);
> > +    *val = amdgpu_ttm_evict_resources(adev, TTM_PL_TT);
> >         pm_runtime_mark_last_busy(dev->dev);
> >       pm_runtime_put_autosuspend(dev->dev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 57638fe9cfc2..032deca4cea2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -3880,6 +3880,25 @@ void amdgpu_device_fini_sw(struct 
> > amdgpu_device *adev)
> >     }
> >   +/**
> > + * amdgpu_device_evict_resources - evict device resources
> > + * @adev: amdgpu device object
> > + *
> > + * Evicts all ttm device resources(vram BOs, gart table) from the 
> > lru list
> > + * of the vram memory type. Mainly used for evicting device resources
> > + * at suspend time.
> > + *
> > + */
> > +void amdgpu_device_evict_resources(struct amdgpu_device *adev)
>
> Please add static here, apart from that the patch is Reviewed-by: 
> Christian König <christian.koenig@xxxxxxx>


Thanks, I will add that and push the commit.


Nirmoy

> Thanks,
> Christian.
>
> > +{
> > +    /* No need to evict vram on APUs for suspend to ram */
> > +    if (adev->in_s3 && (adev->flags & AMD_IS_APU))
> > +        return;
> > +
> > +    if (amdgpu_ttm_evict_resources(adev, TTM_PL_VRAM))
> > +        DRM_WARN("evicting device resources failed\n");
> > +
> > +}
> >     /*
> >    * Suspend & resume.
> > @@ -3920,17 +3939,16 @@ int amdgpu_device_suspend(struct drm_device 
> > *dev, bool fbcon)
> >       if (!adev->in_s0ix)
> >           amdgpu_amdkfd_suspend(adev, adev->in_runpm);
> >   -    /* evict vram memory */
> > -    amdgpu_bo_evict_vram(adev);
> > +    /* First evict vram memory */
> > +    amdgpu_device_evict_resources(adev);
> >         amdgpu_fence_driver_hw_fini(adev);
> >         amdgpu_device_ip_suspend_phase2(adev);
> > -    /* evict remaining vram memory
> > -     * This second call to evict vram is to evict the gart page table
> > -     * using the CPU.
> > +    /* This second call to evict device resources is to evict
> > +     * the gart page table using the CPU.
> >        */
> > -    amdgpu_bo_evict_vram(adev);
> > +    amdgpu_device_evict_resources(adev);
> >         return 0;
> >   }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > index 4ec904f36ceb..073ba2af0b9c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > @@ -1004,29 +1004,6 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
> >       }
> >   }
> >   -/**
> > - * amdgpu_bo_evict_vram - evict VRAM buffers
> > - * @adev: amdgpu device object
> > - *
> > - * Evicts all VRAM buffers on the lru list of the memory type.
> > - * Mainly used for evicting vram at suspend time.
> > - *
> > - * Returns:
> > - * 0 for success or a negative error code on failure.
> > - */
> > -int amdgpu_bo_evict_vram(struct amdgpu_device *adev)
> > -{
> > -    struct ttm_resource_manager *man;
> > -
> > -    if (adev->in_s3 && (adev->flags & AMD_IS_APU)) {
> > -        /* No need to evict vram on APUs for suspend to ram */
> > -        return 0;
> > -    }
> > -
> > -    man = ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM);
> > -    return ttm_resource_manager_evict_all(&adev->mman.bdev, man);
> > -}
> > -
> >   static const char *amdgpu_vram_names[] = {
> >       "UNKNOWN",
> >       "GDDR1",
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > index 8ff61bad4138..d787e0e89e0b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > @@ -305,7 +305,6 @@ int amdgpu_bo_pin(struct amdgpu_bo *bo, u32 domain);
> >   int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
> >                    u64 min_offset, u64 max_offset);
> >   void amdgpu_bo_unpin(struct amdgpu_bo *bo);
> > -int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
> >   int amdgpu_bo_init(struct amdgpu_device *adev);
> >   void amdgpu_bo_fini(struct amdgpu_device *adev);
> >   int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 
> > tiling_flags);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index e2896ac2c9ce..bd5dda8066fa 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -2034,6 +2034,36 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
> >       return r;
> >   }
> >   +/**
> > + * amdgpu_ttm_evict_resources - evict memory buffers
> > + * @adev: amdgpu device object
> > + * @mem_type: evicted BO's memory type
> > + *
> > + * Evicts all @mem_type buffers on the lru list of the memory type.
> > + *
> > + * Returns:
> > + * 0 for success or a negative error code on failure.
> > + */
> > +int amdgpu_ttm_evict_resources(struct amdgpu_device *adev, int 
> > mem_type)
> > +{
> > +    struct ttm_resource_manager *man;
> > +
> > +    switch (mem_type) {
> > +    case TTM_PL_VRAM:
> > +    case TTM_PL_TT:
> > +    case AMDGPU_PL_GWS:
> > +    case AMDGPU_PL_GDS:
> > +    case AMDGPU_PL_OA:
> > +        man = ttm_manager_type(&adev->mman.bdev, mem_type);
> > +        break;
> > +    default:
> > +        DRM_ERROR("Trying to evict invalid memory type\n");
> > +        return -EINVAL;
> > +    }
> > +
> > +    return ttm_resource_manager_evict_all(&adev->mman.bdev, man);
> > +}
> > +
> >   #if defined(CONFIG_DEBUG_FS)
> >     static int amdgpu_mm_vram_table_show(struct seq_file *m, void 
> > *unused)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > index e69f3e8e06e5..ba5c864b8de1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > @@ -190,6 +190,7 @@ bool amdgpu_ttm_tt_is_readonly(struct ttm_tt *ttm);
> >   uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct 
> > ttm_resource *mem);
> >   uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device *adev, struct 
> > ttm_tt *ttm,
> >                    struct ttm_resource *mem);
> > +int amdgpu_ttm_evict_resources(struct amdgpu_device *adev, int 
> > mem_type);
> >     void amdgpu_ttm_debugfs_init(struct amdgpu_device *adev);