[Freedreno] [PATCH 1/3] drm/msm/dpu: move intf and wb assignment to dpu_encoder_setup_display()

[Dmitry Baryshkov]

On 15/06/2022 19:40, Abhinav Kumar wrote:
> 
> 
> On 6/15/2022 5:36 AM, Dmitry Baryshkov wrote:
>> On 14/06/2022 22:32, Abhinav Kumar wrote:
>>> intf and wb resources are not dependent on the rm global
>>> state so need not be allocated during 
>>> dpu_encoder_virt_atomic_mode_set().
>>>
>>> Move the allocation of intf and wb resources to 
>>> dpu_encoder_setup_display()
>>> so that we can utilize the hw caps even during atomic_check() phase.
>>>
>>> Since dpu_encoder_setup_display() already has protection against
>>> setting invalid intf_idx and wb_idx, these checks can now
>>> be dropped as well.
>>>
>>> Fixes: e02a559a720f ("make changes to dpu_encoder to support virtual 
>>> encoder")
>>> Signed-off-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>>> ---
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 
>>> +++++++------------------
>>>   1 file changed, 7 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> index 3a462e327e0e..e991d4ba8a40 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> @@ -1048,24 +1048,6 @@ static void 
>>> dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>>>           phys->hw_pp = dpu_enc->hw_pp[i];
>>>           phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
>>> -        if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
>>> -            phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, 
>>> phys->intf_idx);
>>> -
>>> -        if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
>>> -            phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx);
>>> -
>>> -        if (!phys->hw_intf && !phys->hw_wb) {
>>> -            DPU_ERROR_ENC(dpu_enc,
>>> -                      "no intf or wb block assigned at idx: %d\n", i);
>>> -            return;
>>> -        }
>>> -
>>> -        if (phys->hw_intf && phys->hw_wb) {
>>> -            DPU_ERROR_ENC(dpu_enc,
>>> -                    "invalid phys both intf and wb block at idx: 
>>> %d\n", i);
>>> -            return;
>>> -        }
>>
>> Please retain these checks in dpu_encoder_setup_display().
>> It checks that we really have got the intf or wb. For example one 
>> might have specified the INTF that leads to INTF_NONE interface. Or 
>> non-existing/not supported WB.
> 
> Right, so the reason I omitted that was dpu_encoder_setup_display() 
> already has these checks:
> 
> https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c#L2273 
> 
> 
> Please check lines 2273-2284.
> 
> Only if all those checks succeeded we call 
> dpu_encoder_virt_add_phys_encs which increments num_phys_encs.

As I wrote, it checks indices from phys_params, but not the acquired 
hardware instances.

> 
> Thats why I dropped those.
> 
> Let me know if you have more questions.
> 
>>
>>> -
>>>           phys->cached_mode = crtc_state->adjusted_mode;
>>>           if (phys->ops.atomic_mode_set)
>>>               phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
>>> @@ -2293,7 +2275,14 @@ static int dpu_encoder_setup_display(struct 
>>> dpu_encoder_virt *dpu_enc,
>>>           struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>>>           atomic_set(&phys->vsync_cnt, 0);
>>>           atomic_set(&phys->underrun_cnt, 0);
>>> +
>>> +        if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
>>> +            phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, 
>>> phys->intf_idx);
>>> +
>>> +        if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
>>> +            phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx);
>>>       }
>>> +
>>>       mutex_unlock(&dpu_enc->enc_lock);
>>>       return ret;
>>
>>


-- 
With best wishes
Dmitry