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

[Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>]

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@xxxxxxxxxx>
> > ---
> >   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