[PATCH v3 04/11] drm/msm/dpu: drop separate dpu_core_perf_tune overrides

[Dmitry Baryshkov]

On 11/07/2023 05:31, Abhinav Kumar wrote:
> 
> 
> On 7/7/2023 12:39 PM, Dmitry Baryshkov wrote:
>> The values in struct dpu_core_perf_tune are fixed per the core perf
>> mode. Drop the 'tune' values and substitute them with known values when
>> performing perf management.
>>
>> Note: min_bus_vote was not used at all, so it is just silently dropped.
>>
> 
> Interesting ..... should bring this back properly. Will take it up.

Ack, thanks.

> 
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 29 ++++++++-----------
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  4 ---
>>   2 files changed, 12 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> index 05d340aa18c5..348550ac7e51 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> @@ -235,7 +235,7 @@ static int _dpu_core_perf_crtc_update_bus(struct 
>> dpu_kms *kms,
>>   {
>>       struct dpu_core_perf_params perf = { 0 };
>>       int i, ret = 0;
>> -    u64 avg_bw;
>> +    u32 avg_bw;
> 
> avg_bw seems unused in this patch, so unrelated change?
> 
>>       if (!kms->num_paths)
>>           return 0;
>> @@ -291,10 +291,16 @@ void dpu_core_perf_crtc_release_bw(struct 
>> drm_crtc *crtc)
>>   static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
>>   {
>> -    u64 clk_rate = kms->perf.perf_tune.min_core_clk;
>> +    u64 clk_rate;
>>       struct drm_crtc *crtc;
>>       struct dpu_crtc_state *dpu_cstate;
>> +    if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED)
>> +        return kms->perf.fix_core_clk_rate;
>> +
>> +    if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM)
>> +        return kms->perf.max_core_clk_rate;
>> +
>>       drm_for_each_crtc(crtc, kms->dev) {
>>           if (crtc->enabled) {
>>               dpu_cstate = to_dpu_crtc_state(crtc->state);
>> @@ -305,11 +311,6 @@ static u64 
>> _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
>>           }
>>       }
>> -    if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED)
>> -        clk_rate = kms->perf.fix_core_clk_rate;
>> -
>> -    DRM_DEBUG_ATOMIC("clk:%llu\n", clk_rate);
>> -
> 
> Why dont you move both FIXED and MINIMUM handling below instead of above.
> 
> So that they will just override the clk_rate and you can keep this 
> useful log here and it matches where the function is.

I can keep the log in the next version. The logic was quite simple: 
there is no need to loop over CRTCs if we know that we are overriding 
the value.

> 
> This chunk looks better that way.
> 
> <skipping the rest as it LGTM>

-- 
With best wishes
Dmitry