[PATCH v3 24/27] drm/msm/dpu: rework plane CSC setting

[Dmitry Baryshkov]

On Tue, 7 Feb 2023 at 22:05, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>
>
>
> On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
> > Rework the code flushing CSC settings for the plane. Separate out the
> > pipe and pipe_cfg as a preparation for r_pipe support.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 45 +++++++++++++----------
> >   1 file changed, 25 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index e69499490d39..05047192cb37 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -576,29 +576,18 @@ static const struct dpu_csc_cfg dpu_csc10_YUV2RGB_601L = {
> >       { 0x00, 0x3ff, 0x00, 0x3ff, 0x00, 0x3ff,},
> >   };
> >
> > -static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct dpu_plane *pdpu, const struct dpu_format *fmt)
> > +static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct dpu_sw_pipe *pipe, const struct dpu_format *fmt)
> >   {
> > -     struct dpu_plane_state *pstate = to_dpu_plane_state(pdpu->base.state);
> >       const struct dpu_csc_cfg *csc_ptr;
> >
> > -     if (!pdpu) {
> > -             DPU_ERROR("invalid plane\n");
> > -             return NULL;
> > -     }
> > -
> >       if (!DPU_FORMAT_IS_YUV(fmt))
> >               return NULL;
> >
> > -     if (BIT(DPU_SSPP_CSC_10BIT) & pstate->pipe.sspp->cap->features)
> > +     if (BIT(DPU_SSPP_CSC_10BIT) & pipe->sspp->cap->features)
> >               csc_ptr = &dpu_csc10_YUV2RGB_601L;
> >       else
> >               csc_ptr = &dpu_csc_YUV2RGB_601L;
> >
> > -     DPU_DEBUG_PLANE(pdpu, "using 0x%X 0x%X 0x%X...\n",
> > -                     csc_ptr->csc_mv[0],
> > -                     csc_ptr->csc_mv[1],
> > -                     csc_ptr->csc_mv[2]);
> > -
> >       return csc_ptr;
> >   }
> >
> > @@ -1049,6 +1038,27 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
> >       return 0;
> >   }
> >
> > +static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe *pipe)
> > +{
> > +     const struct dpu_format *format = to_dpu_format(msm_framebuffer_format(pdpu->base.state->fb));
> > +     const struct dpu_csc_cfg *csc_ptr;
> > +
> > +     if (!pipe->sspp || !pipe->sspp->ops.setup_csc)
> > +             return;
> > +
> > +     csc_ptr = _dpu_plane_get_csc(pipe, format);
> > +     if (!csc_ptr)
> > +             return;
> > +
> > +     DPU_DEBUG_PLANE(pdpu, "using 0x%X 0x%X 0x%X...\n",
> > +                     csc_ptr->csc_mv[0],
> > +                     csc_ptr->csc_mv[1],
> > +                     csc_ptr->csc_mv[2]);
> > +
> > +     pipe->sspp->ops.setup_csc(pipe->sspp, csc_ptr);
> > +
> > +}
> > +
> >   void dpu_plane_flush(struct drm_plane *plane)
> >   {
> >       struct dpu_plane *pdpu;
> > @@ -1072,13 +1082,8 @@ void dpu_plane_flush(struct drm_plane *plane)
> >       else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
> >               /* force 100% alpha */
> >               _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
> > -     else if (pstate->pipe.sspp && pstate->pipe.sspp->ops.setup_csc) {
> > -             const struct dpu_format *fmt = to_dpu_format(msm_framebuffer_format(plane->state->fb));
> > -             const struct dpu_csc_cfg *csc_ptr = _dpu_plane_get_csc(pdpu, fmt);
> > -
> > -             if (csc_ptr)
> > -                     pstate->pipe.sspp->ops.setup_csc(pstate->pipe.sspp, csc_ptr);
> > -     }
>
> Do we need to check for pipe being valid too (pstate->pipe) &&
> (pstate->pipe.sspp) && pstate->pipe.sspp->ops.setup_csc) ?
>
> Before moving the pipe_hw/sw pipe to the state the code used to check
> for pdpu->pipe_hw to be valid. Since dpu_plane_flush() can be called
> from other files too , dont we need to check for (pstate->pipe)?

pstate->pipe is not a pointer, there is nothing to check.

>
> > +     else
> > +             dpu_plane_flush_csc(pdpu, &pstate->pipe);
> >
> >       /* flag h/w flush complete */
> >       if (plane->state)



-- 
With best wishes
Dmitry