[PATCH 13/21] drm/msm/dpu: Support quad pipe in header files

[Dmitry Baryshkov]

On Thu, 29 Aug 2024 at 13:20, Jun Nie <jun.nie at linaro.org> wrote:
>
> Support 4 pipes and their configs at most. They are for 2 SSPP
> and their multi-rect mode. Because one SSPP can co-work with
> 2 mixer at most, 2 pair of mixer are needed for 2 SSPP in quad-
> pipe case. So 2 mixer configs are needed in quad-pipe case.

As you wrote this is based (depends?) on the virtual planes, then you
know that the code already uses either one or two SSPP blocks to drive
one sw_pipe. I'm not sure what do you mean by "2 mixer configs". There
are 4 LMs and 4 mixer configurations in the quad-pipe case. The commit
message is thus misleading.

>
> Signed-off-by: Jun Nie <jun.nie at linaro.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h |  2 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 11 ++++++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   | 30 +++++++++++++++++++++--------
>  3 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> index a2eff36a2224c..424725303ccad 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -32,7 +32,7 @@
>  #define DPU_MAX_PLANES                 4
>  #endif
>
> -#define PIPES_PER_STAGE                        2
> +#define PIPES_PER_STAGE                        4
>  #ifndef DPU_MAX_DE_CURVES
>  #define DPU_MAX_DE_CURVES              3
>  #endif
> 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..ae6beff2c294b 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,20 @@ struct dpu_hw_pixel_ext {
>   *             such as decimation, flip etc to program this field
>   * @dest_rect: destination ROI.
>   * @rotation: simplified drm rotation hint
> + * @visible: mark this cfg is valid

So is it valid or visible?

> + * @mxcfg_id: mixer config ID for left or right half screen.
> + *             We have single SSPP, dual SSPP, single SSPP+multi_rect or dual
> + *             SSPP+multi_rect case. mxcfg_id mark current pipe will use
> + *             which mixer cfg. The first mxcfg is for the left half of screen,
> + *             the 2nd mxcfg is for the right half screen. The heading cfg may
> + *             be skipped by pipe with the first mxcfg_id = 1 if the plane is
> + *             only displayed in the right side, thus SSPP goes to later mixers.

too long description for an unreadable name.

>   */
>  struct dpu_sw_pipe_cfg {
>         struct drm_rect src_rect;
>         struct drm_rect dst_rect;
> -       unsigned int rotation;
> +       unsigned int rotation, mxcfg_id;
> +       bool visible;
>  };
>
>  /**
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index e225d5baceb09..9e79cf9eba264 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -14,14 +14,30 @@
>  #include "dpu_hw_mdss.h"
>  #include "dpu_hw_sspp.h"
>
> +/**
> + * Max number of mixer configs. Because we support 4 pipes at most,
> + * the 4 pipes are with 2 SSPP and their multi-rect mode. While one

Or 4 SSPPs. Or 3 SSPPs. Or even a single SSPP if it doesn't cover the
whole screen.

I'm really sorry to say, but I can not understand this text.

> + * SSPP can co-work with 2 mixer at most, then 2 pair of mixer are
> + * needed for 2 SSPP in quad-pipe case. Thus 2 mixer configs are
> + * needed in quad-pipe case.
> + */
> +#define MIX_CFGS_IN_CRTC 2
> +
>  /**
>   * struct dpu_plane_state: Define dpu extension of drm plane state object
>   * @base:      base drm plane state object
>   * @aspace:    pointer to address space for input/output buffers
> - * @pipe:      software pipe description
> - * @r_pipe:    software pipe description of the second pipe
> - * @pipe_cfg:  software pipe configuration
> - * @r_pipe_cfg:        software pipe configuration for the second pipe
> + * @pipe:      software pipe description. Some or all of fields in array can

array has elements, not fields.

> + *             be in use per topology. The heading fields are used first,
> + *             and the later fields is invalid if visible field of pipe_cfg
> + *             is not set. For example, the visible fields of pipe_cfg are set
> + *             in the first 2 pipe_cfg fields, and the mxcfg_id for them are
> + *             0 and 1. That means the first pipe is for left half screen and
> + *             the 2nd pipe is for right half. The visible field of the 3rd
> + *             pipe_cfg is not set, which means the 3rd and 4th pipe are not
> + *             in use.

NAK. A single LM pair might already need two sw pipes.
After reading the comment I have doubts that you understand what the
code is currently doing.

> + * @pipe_cfg:  software pipe configuration. The 4 fields are for SSPP and their
> +               parallel rect as above pipes.
>   * @stage:     assigned by crtc blender
>   * @needs_qos_remap: qos remap settings need to be updated
>   * @multirect_index: index of the rectangle of SSPP
> @@ -34,10 +50,8 @@
>  struct dpu_plane_state {
>         struct drm_plane_state base;
>         struct msm_gem_address_space *aspace;
> -       struct dpu_sw_pipe pipe;
> -       struct dpu_sw_pipe r_pipe;
> -       struct dpu_sw_pipe_cfg pipe_cfg;
> -       struct dpu_sw_pipe_cfg r_pipe_cfg;
> +       struct dpu_sw_pipe pipe[PIPES_PER_STAGE];
> +       struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE];
>         enum dpu_stage stage;
>         bool needs_qos_remap;
>         bool pending;
>
> --
> 2.34.1
>


-- 
With best wishes
Dmitry