Re: [PATCH 12/25] drm/msm/dpu: inline _dpu_plane_set_scanout

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 07/05/2022 02:33, Abhinav Kumar wrote:
> 
> 
> On 2/9/2022 9:25 AM, Dmitry Baryshkov wrote:
>> In preparation to reworking dpu_plane_sspp_atomic_update() inline the
>> _dpu_plane_set_scanout() function.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 41 ++++++++++-------------
>>   1 file changed, 18 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index d029ce806039..3ce7dcc285e2 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -490,28 +490,6 @@ static void _dpu_plane_set_qos_remap(struct 
>> drm_plane *plane)
>>       dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
>>   }
>> -static void _dpu_plane_set_scanout(struct drm_plane *plane,
>> -        struct dpu_plane_state *pstate,
>> -        struct dpu_hw_pipe_cfg *pipe_cfg,
>> -        struct drm_framebuffer *fb)
>> -{
>> -    struct dpu_plane *pdpu = to_dpu_plane(plane);
>> -    struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>> -    struct msm_gem_address_space *aspace = kms->base.aspace;
>> -    int ret;
>> -
>> -    ret = dpu_format_populate_layout(aspace, fb, &pipe_cfg->layout);
>> -    if (ret == -EAGAIN)
>> -        DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
>> -    else if (ret)
>> -        DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
>> -    else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
>> -        trace_dpu_plane_set_scanout(&pstate->pipe,
>> -                        &pipe_cfg->layout);
>> -        pstate->pipe.sspp->ops.setup_sourceaddress(&pstate->pipe, 
>> pipe_cfg);
>> -    }
>> -}
>> -
>>   static void _dpu_plane_setup_scaler3(struct dpu_hw_pipe *pipe_hw,
>>           uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
>>           struct dpu_hw_scaler3_cfg *scale_cfg,
>> @@ -1074,10 +1052,27 @@ static void 
>> dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>>       const struct dpu_format *fmt =
>>           to_dpu_format(msm_framebuffer_format(fb));
>>       struct dpu_hw_pipe_cfg pipe_cfg;
>> +    struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
>> +    struct msm_gem_address_space *aspace = kms->base.aspace;
>> +    bool update_src_addr = true;
>> +    int ret;
>>       memset(&pipe_cfg, 0, sizeof(struct dpu_hw_pipe_cfg));
>> -    _dpu_plane_set_scanout(plane, pstate, &pipe_cfg, fb);
>> +    ret = dpu_format_populate_layout(aspace, fb, &pipe_cfg.layout);
>> +    if (ret == -EAGAIN) {
>> +        DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
>> +        update_src_addr = false;
>> +    } else if (ret) {
>> +        DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
>> +        update_src_addr = false;
>> +    }
>> +
> Do we need update_src_addr?
> 
> It seems we can just do
> 
> if (!ret &&  pipe->sspp->ops.setup_sourceaddress) {
>      .....
>      .....
> }

Ack, let's do it this way

> 
>> +    if (update_src_addr &&
>> +        pipe->sspp->ops.setup_sourceaddress) {
>> +        trace_dpu_plane_set_scanout(pipe, &pipe_cfg.layout);
>> +        pipe->sspp->ops.setup_sourceaddress(pipe, &pipe_cfg);
>> +    }
>>       pstate->pending = true;


-- 
With best wishes
Dmitry