[PATCH V2] drm/amdgpu: Fix SDMA UTC_L1 handling during start/stop sequences

[Lazar, Lijo]

On 6/16/2025 8:54 PM, Zhang, Jesse(Jie) wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: Monday, June 16, 2025 11:05 PM
> To: Alex Deucher <alexdeucher at gmail.com>; Zhang, Jesse(Jie) <Jesse.Zhang at amd.com>
> Cc: amd-gfx at lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; Kim, Jonathan <Jonathan.Kim at amd.com>
> Subject: Re: [PATCH V2] drm/amdgpu: Fix SDMA UTC_L1 handling during start/stop sequences
> 
> 
> 
> On 6/16/2025 8:28 PM, Alex Deucher wrote:
>> On Mon, Jun 16, 2025 at 10:47 AM Jesse Zhang <jesse.zhang at amd.com> wrote:
>>>
>>> This commit makes two key fixes to SDMA v4.4.2 handling:
>>>
>>> 1. disable UTC_L1 in sdma_cntl register when stopping SDMA engines
>>>    by reading the current value before modifying UTC_L1_ENABLE bit.
>>>
>>> 2. Ensure UTC_L1_ENABLE is consistently managed by:
>>>    - Adding the missing register write when enabling UTC_L1 during start
>>>    - Keeping UTC_L1 enabled by default as per hardware requirements
>>>
>>> Fixes the commit 7f03b1d14d51 ("drm/amdgpu:
>>> Remove sdma halt/unhalt during frontdoor load") by maintaining proper
>>> SDMA controller state during initialization sequences.
>>
>> Proper fixes tag formatting:
>>
>> Fixes: 7f03b1d14d51 ("drm/amdgpu: Remove sdma halt/unhalt during
>> frontdoor load")
>>
> 
> The default reset value of that field is 1. Keeping SDMA unhalted but rings disabled shouldn't ideally be a problem - should be similar to the case when it's loaded also.
> 
> Yes, Lijo, I will remove the related description when submitting. This is our problem. We need a workaround to enable SDMA_CNTL::UTC_L1. Because when we call smu queue reset interface,
> it disables UTC_L1.
> 

Thanks for clarifying. While submitting, please update the description
to indicate that this is related to stop/restart of engines for an
engine reset.

Thanks,
Lijo

> Regards
> Jesse
> Thanks,
> Lijo
> 
>>>
>>> v2: Correct SDMA_CNTL setting (Philip)
>>>
>>> Suggested-by: Jonathan Kim <jonathan.kim at amd.com>
>>> Signed-off-by: Jesse Zhang <Jesse.Zhang at amd.com>
>>
>> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> 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 e6af50c947a1..52157acab55a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>>> @@ -492,7 +492,7 @@ 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;
>>> +       u32 rb_cntl, ib_cntl, sdma_cntl;
>>>         int i;
>>>
>>>         for_each_inst(i, inst_mask) { @@ -504,6 +504,9 @@ static void
>>> sdma_v4_4_2_inst_gfx_stop(struct amdgpu_device *adev,
>>>                 ib_cntl = RREG32_SDMA(i, regSDMA_GFX_IB_CNTL);
>>>                 ib_cntl = REG_SET_FIELD(ib_cntl, SDMA_GFX_IB_CNTL, IB_ENABLE, 0);
>>>                 WREG32_SDMA(i, regSDMA_GFX_IB_CNTL, ib_cntl);
>>> +               sdma_cntl = RREG32_SDMA(i, SDMA_CNTL);
>>> +               sdma_cntl = REG_SET_FIELD(sdma_cntl, SDMA_CNTL, UTC_L1_ENABLE, 0);
>>> +               WREG32_SDMA(i, regSDMA_CNTL, sdma_cntl);
>>>
>>>                 if (sdma[i]->use_doorbell) {
>>>                         doorbell = RREG32_SDMA(i,
>>> regSDMA_GFX_DOORBELL); @@ -997,6 +1000,7 @@ static int sdma_v4_4_2_inst_start(struct amdgpu_device *adev,
>>>                 /* set utc l1 enable flag always to 1 */
>>>                 temp = RREG32_SDMA(i, regSDMA_CNTL);
>>>                 temp = REG_SET_FIELD(temp, SDMA_CNTL, UTC_L1_ENABLE,
>>> 1);
>>> +               WREG32_SDMA(i, regSDMA_CNTL, temp);
>>>
>>>                 if (amdgpu_ip_version(adev, SDMA0_HWIP, 0) < IP_VERSION(4, 4, 5)) {
>>>                         /* enable context empty interrupt during
>>> initialization */
>>> --
>>> 2.34.1
>>>
>