Re: [PATCH] drm/amd/display: reset the workload type when using MALL

[Harry Wentland <harry.wentland@amd.com>]

On 2025-03-07 09:48, Alex Deucher wrote:
> On Thu, Mar 6, 2025 at 10:45 PM Kenneth Feng <kenneth.feng@amd.com> wrote:
>>
>> Reset the workload type when using MALL.
>> When there is no activity on the screen, dal requestes dmcub
>> to use MALL. However, gfx ring is not empty at the same time.
>> Currrently the workload type is set to 3D fullscreen when gfx
>> ring has jobs. No activity on the screen and the gfx ring empty
>> state can not be synchronized to each other. By removing the
>> 3D fullscreen workload when there is no activity on screen, the
>> event can be passed down to dmcub->pmfw, since pmfw only allows
>> MALL when the workload type setting is bootup default, then MALL
>> can be really used. And this does not impact the thread to detect
>> the ring jobs and can set back to the 3D fullscreen later.
>>
>> Signed-off-by: Kenneth Feng <kenneth.feng@amd.com>
>> ---
>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
>> index 36a830a7440f..154936166896 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
>> @@ -244,6 +244,8 @@ static void amdgpu_dm_crtc_vblank_control_worker(struct work_struct *work)
>>         struct vblank_control_work *vblank_work =
>>                 container_of(work, struct vblank_control_work, work);
>>         struct amdgpu_display_manager *dm = vblank_work->dm;
>> +       int r;
>> +       struct amdgpu_device *adev = drm_to_adev(dm->ddev);
>>
>>         mutex_lock(&dm->dc_lock);
>>
>> @@ -271,8 +273,14 @@ static void amdgpu_dm_crtc_vblank_control_worker(struct work_struct *work)
>>                         vblank_work->acrtc->dm_irq_params.allow_sr_entry);
>>         }
>>
>> -       if (dm->active_vblank_irq_count == 0)
>> +       if (dm->active_vblank_irq_count == 0) {
>> +               r = amdgpu_dpm_switch_power_profile(adev, PP_SMC_POWER_PROFILE_FULLSCREEN3D, false);
> 
> To keep this balanced, you should get a reference when we disable the
> optimizations.  Also if ROCm applications are running, the compute
> profile will be active so that should be decremented too or if VCN
> jobs are running the video profile may be active as well.  On the
> other hand, if users have apps running when the display is off, maybe
> they don't want the idle optimizations in the first place.
> Alternatively, we could have a suspend/resume workload profile
> function that sets a flag in sw_smu which sets the default workload
> profile and skips all updates to the workload profile while that flag
> is set.  The swsmu will still track the ref counts and then when we
> resume the workload profile handling, we can restore all of the
> workloads profiles based on the ref counts.
> 

I don't think it's a good idea for DM to explicitly switch
power profiles.

Generally the display driver interacts with PM in
two ways:
- indicate min clocks that are needed for real-time operation
  of DCN
- allow various state switches when the driver knows they
  won't impact real-time operation of DCN

Harry

> Alex
> 
> 
>> +               if (r)
>> +            dev_warn(adev->dev, "(%d) failed to disable fullscreen 3D  power profile mode\n",
>> +                                        r);
>> +
>>                 dc_allow_idle_optimizations(dm->dc, true);
>> +       }
>>
>>         mutex_unlock(&dm->dc_lock);
>>
>> --
>> 2.34.1
>>