Re: [PATCH 1/1] drm/amdgpu: drop setting buffer funcs in sdma442

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 3/15/2024 5:45 PM, Ma, Le wrote:
> [AMD Official Use Only - General]
> 
>  
>  
>> -----Original Message-----
>> From: Lazar, Lijo <_Lijo.Lazar@amd.com_ <mailto:Lijo.Lazar@amd.com>>
>> Sent: Friday, March 15, 2024 6:14 PM
>> To: Ma, Le <_Le.Ma@amd.com_ <mailto:Le.Ma@amd.com>>; _amd-gfx@lists.freedesktop.org_
> <mailto:amd-gfx@lists.freedesktop.org>
>> Cc: Zhang, Hawking <_Hawking.Zhang@amd.com_ <mailto:Hawking.Zhang@amd.com>>; Song, Asher
>> <_Asher.Song@amd.com_ <mailto:Asher.Song@amd.com>>; Deucher, Alexander
> <_Alexander.Deucher@amd.com_ <mailto:Alexander.Deucher@amd.com>>
>> Subject: Re: [PATCH 1/1] drm/amdgpu: drop setting buffer funcs in sdma442
>> 
>> 
>> 
>> On 3/15/2024 2:46 PM, Le Ma wrote:
>> > To fix the entity rq NULL issue. This setting has been moved to upper level.
>> >
>> 
>> Need to call amdgpu_ttm_set_buffer_funcs_status(adev, true/false) in
>> mode-2 reset handlers as well.
>  
> Thanks for pointing out this. I think we can make another separated
> patch to handle it for mode2 since this patch is for alignment purpose.
> Actually, the set_buffer_funcs will not be unset/set in reset case as
> the conditions below:
>  
> void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool
> enable)
> {
>         struct ttm_resource_manager *man =
> ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM);
>         uint64_t size;
>         int r;
>  
>         if (!adev->mman.initialized || amdgpu_in_reset(adev) ||
>             adev->mman.buffer_funcs_enabled == enable ||
> adev->gmc.is_app_apu)
>                 return;
>  
>  

Thanks for clarifying.

In this case, we don't require that since reset() condition is set. I
saw amdgpu_ttm_set_buffer_funcs_status(tmp_adev, true) getting called in
amdgpu_do_asic_reset(), thought it was affected by reset.

Thanks,
Lijo

>> 
>> Thanks,
>> Lijo
>> 
>> > Fixes b70438004a14 ("drm/amdgpu: move buffer funcs setting up a
>> > level")
>> >
>> > Signed-off-by: Le Ma <_le.ma@amd.com_ <mailto:le.ma@amd.com>>
>> > ---
>> >  drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 20 +-------------------
>> >  1 file changed, 1 insertion(+), 19 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>> > b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>> > index eaa4f5f49949..589a734982a7 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>> > @@ -431,16 +431,11 @@ static void sdma_v4_4_2_inst_gfx_stop(struct
>> amdgpu_device *adev,
>> >      struct amdgpu_ring *sdma[AMDGPU_MAX_SDMA_INSTANCES];
>> >      u32 doorbell_offset, doorbell;
>> >      u32 rb_cntl, ib_cntl;
>> > -   int i, unset = 0;
>> > +   int i;
>> >
>> >      for_each_inst(i, inst_mask) {
>> >              sdma[i] = &adev->sdma.instance[i].ring;
>> >
>> > -           if ((adev->mman.buffer_funcs_ring == sdma[i]) && unset != 1) {
>> > -                   amdgpu_ttm_set_buffer_funcs_status(adev, false);
>> > -                   unset = 1;
>> > -           }
>> > -
>> >              rb_cntl = RREG32_SDMA(i, regSDMA_GFX_RB_CNTL);
>> >              rb_cntl = REG_SET_FIELD(rb_cntl, SDMA_GFX_RB_CNTL,
>> RB_ENABLE, 0);
>> >              WREG32_SDMA(i, regSDMA_GFX_RB_CNTL, rb_cntl); @@ -
>> 490,17 +485,10 @@
>> > static void sdma_v4_4_2_inst_page_stop(struct amdgpu_device *adev,
>> >      struct amdgpu_ring *sdma[AMDGPU_MAX_SDMA_INSTANCES];
>> >      u32 rb_cntl, ib_cntl;
>> >      int i;
>> > -   bool unset = false;
>> >
>> >      for_each_inst(i, inst_mask) {
>> >              sdma[i] = &adev->sdma.instance[i].page;
>> >
>> > -           if ((adev->mman.buffer_funcs_ring == sdma[i]) &&
>> > -                   (!unset)) {
>> > -                   amdgpu_ttm_set_buffer_funcs_status(adev, false);
>> > -                   unset = true;
>> > -           }
>> > -
>> >              rb_cntl = RREG32_SDMA(i, regSDMA_PAGE_RB_CNTL);
>> >              rb_cntl = REG_SET_FIELD(rb_cntl, SDMA_PAGE_RB_CNTL,
>> >                                      RB_ENABLE, 0);
>> > @@ -950,13 +938,7 @@ static int sdma_v4_4_2_inst_start(struct
>> amdgpu_device *adev,
>> >                      r = amdgpu_ring_test_helper(page);
>> >                      if (r)
>> >                              return r;
>> > -
>> > -                   if (adev->mman.buffer_funcs_ring == page)
>> > -                           amdgpu_ttm_set_buffer_funcs_status(adev,
>> true);
>> >              }
>> > -
>> > -           if (adev->mman.buffer_funcs_ring == ring)
>> > -                   amdgpu_ttm_set_buffer_funcs_status(adev, true);
>> >      }
>> >
>> >      return r;