| Message ID | 20230203182132.1307834-25-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,01/27] drm/msm/dpu: rename struct dpu_hw_pipe(_cfg) to dpu_hw_sspp(_cfg) | expand |
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@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)? > + else > + dpu_plane_flush_csc(pdpu, &pstate->pipe); > > /* flag h/w flush complete */ > if (plane->state)
On Tue, 7 Feb 2023 at 22:05, Abhinav Kumar <quic_abhinavk@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@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)
On 2/7/2023 12:44 PM, Dmitry Baryshkov wrote: > On Tue, 7 Feb 2023 at 22:05, Abhinav Kumar <quic_abhinavk@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@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. Ah my bad. Only pipe.sspp is a pointer which you are already checking. In that case, Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > >> >>> + else >>> + dpu_plane_flush_csc(pdpu, &pstate->pipe); >>> >>> /* flag h/w flush complete */ >>> if (plane->state) > > >
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); - } + else + dpu_plane_flush_csc(pdpu, &pstate->pipe); /* flag h/w flush complete */ if (plane->state)
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@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 45 +++++++++++++---------- 1 file changed, 25 insertions(+), 20 deletions(-)