Re: [PATCH v2 10/14] drm/msm/dpu: Support quad-pipe in SSPP checking

From: Jun Nie
Date: Fri Oct 11 2024 - 03:56:13 EST

Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> 于2024年10月10日周四 21:20写道：
>
> On Wed, Oct 09, 2024 at 04:50:23PM GMT, Jun Nie wrote:
> > Move requreiment check to routine of every pipe check. Because there is
>
> s/Because there is/There will be/
>
> > multiple SSPPs for quad-pipe case in future.
> >
> > Signed-off-by: Jun Nie <jun.nie@xxxxxxxxxx>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 +
> > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 86 ++++++++++++++---------------
> > 2 files changed, 44 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > index fc54625ae5d4f..05b92ff7eb529 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> > @@ -143,11 +143,13 @@ struct dpu_hw_pixel_ext {
> > * such as decimation, flip etc to program this field
> > * @dest_rect: destination ROI.
> > * @rotation: simplified drm rotation hint
> > + * @valid: notify that this pipe and config is in use
>
> This is not related to code move, is it? And if it is, it should be
> described in the commit message.

Will move this part to plane splitting patch and add commit message
to address this valid flag.
>
> > */
> > struct dpu_sw_pipe_cfg {
> > struct drm_rect src_rect;
> > struct drm_rect dst_rect;
> > unsigned int rotation;
> > + bool valid;
> > };
> >
> > /**
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 9a8fbeec2e1e8..904ebec1c8a18 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -739,12 +739,40 @@ static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
> > static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
> > struct dpu_sw_pipe *pipe,
> > struct dpu_sw_pipe_cfg *pipe_cfg,
> > - const struct msm_format *fmt,
> > - const struct drm_display_mode *mode)
> > + const struct drm_display_mode *mode,
> > + struct drm_plane_state *new_plane_state)
> > {
> > uint32_t min_src_size;
> > struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
> > int ret;
> > + const struct msm_format *fmt;
> > + uint32_t supported_rotations;
> > + const struct dpu_sspp_cfg *pipe_hw_caps;
> > + const struct dpu_sspp_sub_blks *sblk;
> > +
> > + pipe_hw_caps = pipe->sspp->cap;
> > + sblk = pipe->sspp->cap->sblk;
> > +
> > + /*
> > + * We already have verified scaling against platform limitations.
> > + * Now check if the SSPP supports scaling at all.
> > + */
> > + if (!sblk->scaler_blk.len &&
> > + ((drm_rect_width(&new_plane_state->src) >> 16 !=
> > + drm_rect_width(&new_plane_state->dst)) ||
> > + (drm_rect_height(&new_plane_state->src) >> 16 !=
> > + drm_rect_height(&new_plane_state->dst))))
> > + return -ERANGE;
> > +
> > + fmt = msm_framebuffer_format(new_plane_state->fb);
> > +
> > + supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
> > +
> > + if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
> > + supported_rotations |= DRM_MODE_ROTATE_90;
> > +
> > + pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
> > + supported_rotations);
> >
> > min_src_size = MSM_FORMAT_IS_YUV(fmt) ? 2 : 1;
> >
> > @@ -920,49 +948,19 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
> > drm_atomic_get_new_plane_state(state, plane);
> > struct dpu_plane *pdpu = to_dpu_plane(plane);
> > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> > - const struct msm_format *fmt;
> > - struct dpu_sw_pipe *pipe = &pstate->pipe[0];
> > - struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
> > - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
> > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
> > - uint32_t supported_rotations;
> > - const struct dpu_sspp_cfg *pipe_hw_caps;
> > - const struct dpu_sspp_sub_blks *sblk;
> > - int ret = 0;
> > -
> > - pipe_hw_caps = pipe->sspp->cap;
> > - sblk = pipe->sspp->cap->sblk;
> > -
> > - /*
> > - * We already have verified scaling against platform limitations.
> > - * Now check if the SSPP supports scaling at all.
> > - */
> > - if (!sblk->scaler_blk.len &&
> > - ((drm_rect_width(&new_plane_state->src) >> 16 !=
> > - drm_rect_width(&new_plane_state->dst)) ||
> > - (drm_rect_height(&new_plane_state->src) >> 16 !=
> > - drm_rect_height(&new_plane_state->dst))))
> > - return -ERANGE;
> > -
> > - fmt = msm_framebuffer_format(new_plane_state->fb);
> > -
> > - supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
> > -
> > - if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
> > - supported_rotations |= DRM_MODE_ROTATE_90;
> > -
> > - pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
> > - supported_rotations);
> > - r_pipe_cfg->rotation = pipe_cfg->rotation;
> > -
> > - ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt,
> > - &crtc_state->adjusted_mode);
> > - if (ret)
> > - return ret;
> > + struct dpu_sw_pipe *pipe;
> > + struct dpu_sw_pipe_cfg *pipe_cfg;
> > + int ret = 0, i;
> >
> > - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
> > - ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt,
> > - &crtc_state->adjusted_mode);
> > + for (i = 0; i < PIPES_PER_PLANE; i++) {
> > + pipe = &pstate->pipe[i];
> > + pipe_cfg = &pstate->pipe_cfg[i];
> > + if (!pipe_cfg->valid || !pipe->sspp)
> > + break;
>
> And... this check broke display support at this point, didn't it? It's
> never set, so none of the pipes are going to be checked.

Yeah, no pipe is checked. It passes real test without the check luckily.
Will move plane splitting patch before this check patch.

- Jun

>
> > + DPU_DEBUG_PLANE(pdpu, "pipe %d is in use, validate it\n", i);
> > + ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
> > + &crtc_state->adjusted_mode,
> > + new_plane_state);
> > if (ret)
> > return ret;
> > }
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry