[PATCH v3 20/27] drm/msm/dpu: add dpu_hw_pipe_cfg to dpu_plane_state
Abhinav Kumar
quic_abhinavk at quicinc.com
Mon Feb 6 19:07:57 UTC 2023
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
> Now as all accesses to pipe_cfg and pstate have been cleaned, re-add
> struct dpu_hw_pipe_cfg back to dpu_plane_state, so that
> dpu_plane_atomic_check() and dpu_plane_atomic_update() do not have a
> chance to disagree about src/dst rectangles (currently
> dpu_plane_atomic_check() uses unclipped rectangles, while
> dpu_plane_atomic_update() uses clipped rectangles calculated by
> drm_atomic_helper_check_plane_state()).
>
The title of the patch should now say "add dpu_hw_sspp_cfg"
I have a question on the commit text, why does it say "re-add" and not
"add".
dpu_hw_pipe_cfg/dpu_hw_sspp_cfg was not a part of dpu_plane_state even
before and I dont recall it was removed in this series and then added back.
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 64 ++++++++++-------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 2 +
> 2 files changed, 30 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 09a3fde1c910..ecf5402ab61a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -951,7 +951,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
> struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> const struct drm_crtc_state *crtc_state = NULL;
> const struct dpu_format *fmt;
> - struct drm_rect src, dst, fb_rect = { 0 };
> + struct dpu_hw_sspp_cfg *pipe_cfg = &pstate->pipe_cfg;
> + struct drm_rect fb_rect = { 0 };
> uint32_t min_src_size, max_linewidth;
> unsigned int rotation;
> uint32_t supported_rotations;
> @@ -984,12 +985,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
> return -EINVAL;
> }
>
> - src.x1 = new_plane_state->src_x >> 16;
> - src.y1 = new_plane_state->src_y >> 16;
> - src.x2 = src.x1 + (new_plane_state->src_w >> 16);
> - src.y2 = src.y1 + (new_plane_state->src_h >> 16);
> + pipe_cfg->src_rect = new_plane_state->src;
>
> - dst = drm_plane_state_dest(new_plane_state);
> + /* state->src is 16.16, src_rect is not */
> + pipe_cfg->src_rect.x1 >>= 16;
> + pipe_cfg->src_rect.x2 >>= 16;
> + pipe_cfg->src_rect.y1 >>= 16;
> + pipe_cfg->src_rect.y2 >>= 16;
> +
> + pipe_cfg->dst_rect = new_plane_state->dst;
>
> fb_rect.x2 = new_plane_state->fb->width;
> fb_rect.y2 = new_plane_state->fb->height;
> @@ -1008,30 +1012,30 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
> return -EINVAL;
>
> /* check src bounds */
> - } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) {
> + } else if (!dpu_plane_validate_src(&pipe_cfg->src_rect, &fb_rect, min_src_size)) {
> DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
> - DRM_RECT_ARG(&src));
> + DRM_RECT_ARG(&pipe_cfg->src_rect));
> return -E2BIG;
>
> /* valid yuv image */
> } else if (DPU_FORMAT_IS_YUV(fmt) &&
> - (src.x1 & 0x1 || src.y1 & 0x1 ||
> - drm_rect_width(&src) & 0x1 ||
> - drm_rect_height(&src) & 0x1)) {
> + (pipe_cfg->src_rect.x1 & 0x1 || pipe_cfg->src_rect.y1 & 0x1 ||
> + drm_rect_width(&pipe_cfg->src_rect) & 0x1 ||
> + drm_rect_height(&pipe_cfg->src_rect) & 0x1)) {
> DPU_DEBUG_PLANE(pdpu, "invalid yuv source " DRM_RECT_FMT "\n",
> - DRM_RECT_ARG(&src));
> + DRM_RECT_ARG(&pipe_cfg->src_rect));
> return -EINVAL;
>
> /* min dst support */
> - } else if (drm_rect_width(&dst) < 0x1 || drm_rect_height(&dst) < 0x1) {
> + } else if (drm_rect_width(&pipe_cfg->dst_rect) < 0x1 || drm_rect_height(&pipe_cfg->dst_rect) < 0x1) {
> DPU_DEBUG_PLANE(pdpu, "invalid dest rect " DRM_RECT_FMT "\n",
> - DRM_RECT_ARG(&dst));
> + DRM_RECT_ARG(&pipe_cfg->dst_rect));
> return -EINVAL;
>
> /* check decimated source width */
> - } else if (drm_rect_width(&src) > max_linewidth) {
> + } else if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
> DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
> - DRM_RECT_ARG(&src), max_linewidth);
> + DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
> return -E2BIG;
> }
>
> @@ -1045,7 +1049,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>
> if ((pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION)) &&
> (rotation & DRM_MODE_ROTATE_90)) {
> - ret = dpu_plane_check_inline_rotation(pdpu, sblk, src, fmt);
> + ret = dpu_plane_check_inline_rotation(pdpu, sblk, pipe_cfg->src_rect, fmt);
> if (ret)
> return ret;
> }
> @@ -1120,9 +1124,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
> bool is_rt_pipe;
> const struct dpu_format *fmt =
> to_dpu_format(msm_framebuffer_format(fb));
> - struct dpu_hw_sspp_cfg pipe_cfg;
> -
> - memset(&pipe_cfg, 0, sizeof(struct dpu_hw_sspp_cfg));
> + struct dpu_hw_sspp_cfg *pipe_cfg = &pstate->pipe_cfg;
>
> _dpu_plane_set_scanout(plane, pstate, fb);
>
> @@ -1139,16 +1141,6 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
> crtc->base.id, DRM_RECT_ARG(&state->dst),
> (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt));
>
> - pipe_cfg.src_rect = state->src;
> -
> - /* state->src is 16.16, src_rect is not */
> - pipe_cfg.src_rect.x1 >>= 16;
> - pipe_cfg.src_rect.x2 >>= 16;
> - pipe_cfg.src_rect.y1 >>= 16;
> - pipe_cfg.src_rect.y2 >>= 16;
> -
> - pipe_cfg.dst_rect = state->dst;
> -
> /* override for color fill */
> if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
> /* skip remaining processing on color fill */
> @@ -1157,10 +1149,10 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>
> if (pipe->sspp->ops.setup_rects) {
> pipe->sspp->ops.setup_rects(pipe,
> - &pipe_cfg);
> + pipe_cfg);
> }
>
> - _dpu_plane_setup_scaler(pipe, fmt, false, &pipe_cfg, pstate->rotation);
> + _dpu_plane_setup_scaler(pipe, fmt, false, pipe_cfg, pstate->rotation);
>
> if (pipe->sspp->ops.setup_multirect)
> pipe->sspp->ops.setup_multirect(
> @@ -1201,12 +1193,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
> }
> }
>
> - _dpu_plane_set_qos_lut(plane, pipe, fmt, &pipe_cfg);
> + _dpu_plane_set_qos_lut(plane, pipe, fmt, &pstate->pipe_cfg);
> _dpu_plane_set_danger_lut(plane, pipe, fmt);
>
> if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> _dpu_plane_set_qos_ctrl(plane, pipe, true, DPU_PLANE_QOS_PANIC_CTRL);
> - _dpu_plane_set_ot_limit(plane, pipe, crtc, &pipe_cfg);
> + _dpu_plane_set_ot_limit(plane, pipe, crtc, &pstate->pipe_cfg);
> }
>
> if (pstate->needs_qos_remap) {
> @@ -1214,9 +1206,9 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
> _dpu_plane_set_qos_remap(plane, pipe);
> }
>
> - pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, &crtc->mode, &pipe_cfg);
> + pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, &crtc->mode, &pstate->pipe_cfg);
>
> - pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, &pipe_cfg);
> + pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, &pstate->pipe_cfg);
> }
>
> static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index a08b0539513b..079dad83eb37 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -19,6 +19,7 @@
> * @base: base drm plane state object
> * @aspace: pointer to address space for input/output buffers
> * @pipe: software pipe description
> + * @pipe_cfg: software pipe configuration
> * @stage: assigned by crtc blender
> * @needs_qos_remap: qos remap settings need to be updated
> * @multirect_index: index of the rectangle of SSPP
> @@ -33,6 +34,7 @@ struct dpu_plane_state {
> struct drm_plane_state base;
> struct msm_gem_address_space *aspace;
> struct dpu_sw_pipe pipe;
> + struct dpu_hw_sspp_cfg pipe_cfg;
> enum dpu_stage stage;
> bool needs_qos_remap;
> bool pending;
More information about the dri-devel
mailing list