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@xxxxxxx>> >> Sent: Friday, March 15, 2024 6:14 PM >> To: Ma, Le <_Le.Ma@amd.com_ <mailto:Le.Ma@xxxxxxx>>; _amd-gfx@lists.freedesktop.org_ > <mailto:amd-gfx@xxxxxxxxxxxxxxxxxxxxx> >> Cc: Zhang, Hawking <_Hawking.Zhang@amd.com_ <mailto:Hawking.Zhang@xxxxxxx>>; Song, Asher >> <_Asher.Song@amd.com_ <mailto:Asher.Song@xxxxxxx>>; Deucher, Alexander > <_Alexander.Deucher@amd.com_ <mailto:Alexander.Deucher@xxxxxxx>> >> 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@xxxxxxx>> >> > --- >> > 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;
