Re: [PATCH 03/25] drm/msm/dpu: add support for SSPP allocation to RM

On 2/9/2022 9:24 AM, Dmitry Baryshkov wrote:

Add support for handling and allocting SSPP blocks through the resource manager. Handling code is not converted to use it though.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 10 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 81 +++++++++++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 6 ++ 5 files changed, 104 insertions(+), 12 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 1b18de957500..f805c30643b1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -388,6 +388,16 @@ struct dpu_hw_pipe { };

struct dpu_kms; +/**

• • to_dpu_hw_pipe - convert base object dpu_hw_base to container
• • @hw: Pointer to base hardware block
• • return: Pointer to hardware block container
• */

+static inline struct dpu_hw_pipe *to_dpu_hw_pipe(struct dpu_hw_blk *hw) +{

• return container_of(hw, struct dpu_hw_pipe, base);

+}

• /**
  • dpu_hw_sspp_init - initializes the sspp hw driver object.
  • Should be called once before accessing every pipe.

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 2d385b4b7f5e..824495ad2392 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -146,6 +146,7 @@ struct dpu_global_state { uint32_t ctl_to_enc_id[CTL_MAX - CTL_0]; uint32_t intf_to_enc_id[INTF_MAX - INTF_0]; uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];

• uint32_t pipe_to_plane_id[SSPP_MAX - SSPP_NONE]; };

  struct dpu_global_state

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index c04c3be16d85..146dbccd79cd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1235,8 +1235,6 @@ static void dpu_plane_destroy(struct drm_plane *plane) /* this will destroy the states as well */ drm_plane_cleanup(plane);

• dpu_hw_sspp_destroy(pdpu->pipe_hw);
  

• kfree(pdpu); } }

@@ -1445,14 +1443,13 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, pdpu->pipe = pipe;

/* initialize underlying h/w driver */

• pdpu->pipe_hw = dpu_hw_sspp_init(pipe, kms->mmio, kms->catalog);
• if (IS_ERR(pdpu->pipe_hw)) {

• DPU_ERROR("[%u]SSPP init failed\n", pipe);
  

• ret = PTR_ERR(pdpu->pipe_hw);
  

• if (!kms->rm.sspp_blks[pipe - SSPP_NONE]) goto clean_plane;

• } else if (!pdpu->pipe_hw->cap || !pdpu->pipe_hw->cap->sblk) {

• pdpu->pipe_hw = to_dpu_hw_pipe(kms->rm.sspp_blks[pipe - SSPP_NONE]);
• if (!pdpu->pipe_hw->cap || !pdpu->pipe_hw->cap->sblk) { DPU_ERROR("[%u]SSPP init returned invalid cfg\n", pipe);

• goto clean_sspp;
  

• goto clean_plane;
  

  }

  format_list = pdpu->pipe_hw->cap->sblk->format_list;

@@ -1462,7 +1459,7 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, format_list, num_formats, supported_format_modifiers, type, NULL); if (ret)

• goto clean_sspp;
  

• goto clean_plane;
  
  

  pdpu->catalog = kms->catalog;

@@ -1494,9 +1491,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, pipe, plane->base.id); return plane;

-clean_sspp:

• if (pdpu && pdpu->pipe_hw)

• dpu_hw_sspp_destroy(pdpu->pipe_hw);
  

  clean_plane: kfree(pdpu); return ERR_PTR(ret);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index f9c83d6e427a..21c9e513f1f6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -8,6 +8,7 @@ #include "dpu_hw_lm.h" #include "dpu_hw_ctl.h" #include "dpu_hw_pingpong.h" +#include "dpu_hw_sspp.h" #include "dpu_hw_intf.h" #include "dpu_hw_dspp.h" #include "dpu_hw_merge3d.h" @@ -35,6 +36,14 @@ int dpu_rm_destroy(struct dpu_rm *rm) { int i;

• for (i = 0; i < ARRAY_SIZE(rm->sspp_blks); i++) {

• struct dpu_hw_pipe *hw;
  

• if (rm->sspp_blks[i]) {
  

• 	hw = to_dpu_hw_pipe(rm->sspp_blks[i]);
  

• 	dpu_hw_sspp_destroy(hw);
  

• }
  

• } for (i = 0; i < ARRAY_SIZE(rm->pingpong_blks); i++) { struct dpu_hw_pingpong *hw;

@@ -166,6 +175,24 @@ int dpu_rm_init(struct dpu_rm *rm, rm->pingpong_blks[pp->id - PINGPONG_0] = &hw->base; }

• for (i = 0; i < cat->sspp_count; i++) {

• struct dpu_hw_pipe *hw;
  

• const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
  

• if (sspp->id <= SSPP_NONE || sspp->id >= SSPP_MAX) {
  

• 	DPU_ERROR("skip sspp %d with invalid id\n", sspp->id);
  

• 	continue;
  

• }
  

• hw = dpu_hw_sspp_init(sspp->id, mmio, cat);
  

• if (IS_ERR_OR_NULL(hw)) {
  

• 	rc = PTR_ERR(hw);
  

• 	DPU_ERROR("failed sspp object creation: err %d\n",
  

• 		rc);
  

• 	goto fail;
  

• }
  

• rm->sspp_blks[sspp->id - SSPP_NONE] = &hw->base;
  

• }
• for (i = 0; i < cat->intf_count; i++) { struct dpu_hw_intf *hw; const struct dpu_intf_cfg *intf = &cat->intf[i];

@@ -660,3 +687,57 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,

return num_blks; }

+enum dpu_sspp dpu_rm_get_sspp(struct dpu_rm *rm, struct dpu_global_state *global_state, uint32_t plane_id, bool yuv, bool scale) +{

Right now we have only yuv and scale as requirements for the incoming plane. But these requirements can expand.

Do you think a enum req_mask will be a good addition here? You can start with having 2 masks for YUV and Scale but atleast that way its expandable cleanly rather than just increasing function parameters here.

• int i;
• enum dpu_sspp pipe = SSPP_NONE;
• struct dpu_hw_pipe *pipe_hw;
• bool retry = false;

+retry_loop:

• for (i = 0; i < ARRAY_SIZE(rm->sspp_blks) && pipe == SSPP_NONE; i++) {

• if (!rm->sspp_blks[i])
  

• 	continue;
  

• if (reserved_by_other(global_state->pipe_to_plane_id, i, plane_id))
  

• 	continue;
  

• pipe_hw = to_dpu_hw_pipe(rm->sspp_blks[i]);
  

• /* skip incompatible planes */
  

• if (scale && !(pipe_hw->cap->features & DPU_SSPP_SCALER))
  

• 	continue;
  

• if (yuv && (!(pipe_hw->cap->features & DPU_SSPP_SCALER) ||
  

• 	    !(pipe_hw->cap->features & DPU_SSPP_CSC_ANY)))
  

• 	continue;
  

• /* For non-yuv, non-scaled planes try to find simple (DMA)
  

•  * plane, fallback to VIG on a second try.
  

•  *
  

•  * This way we'd leave VIG pipes to be later used for YUV formats.
  

•  */
  

this strategy should work. I just think we need to validate this quite a bit with some real world cases such as video playback with UI layers, opening and closing video players, browsers etc.

I am yet to review the usage of this in the next patches, but yes if even this wont kick in for small layers, we are protected.

• if (!scale && !yuv && !retry &&
  

•     (pipe_hw->cap->features & (DPU_SSPP_SCALER | DPU_SSPP_CSC_ANY)))
  

• 	continue;
  

• pipe = i + SSPP_NONE;
  

• };
• if (!scale && !yuv && !retry && pipe == SSPP_NONE) {

• retry = true;
  

• goto retry_loop;
  

• }
• if (pipe != SSPP_NONE)

• global_state->pipe_to_plane_id[pipe - SSPP_NONE] = plane_id;
  

• return pipe;

+}

+void dpu_rm_release_sspp(struct dpu_rm *rm, struct dpu_global_state *global_state, uint32_t plane_id) +{

• _dpu_rm_clear_mapping(global_state->pipe_to_plane_id,

• 	ARRAY_SIZE(global_state->pipe_to_plane_id), plane_id);
  

+} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h index 1f12c8d5b8aa..b759fe39f6d6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h @@ -16,6 +16,7 @@ struct dpu_global_state; /**

• struct dpu_rm - DPU dynamic hardware resource manager
• @pingpong_blks: array of pingpong hardware resources

• • @sspp_blks: array of sspp hardware resources
  • @mixer_blks: array of layer mixer hardware resources
  • @ctl_blks: array of ctl hardware resources
  • @intf_blks: array of intf hardware resources

@@ -25,6 +26,7 @@ struct dpu_global_state; */ struct dpu_rm { struct dpu_hw_blk *pingpong_blks[PINGPONG_MAX - PINGPONG_0];

• struct dpu_hw_blk *sspp_blks[SSPP_MAX - SSPP_NONE]; struct dpu_hw_blk *mixer_blks[LM_MAX - LM_0]; struct dpu_hw_blk *ctl_blks[CTL_MAX - CTL_0]; struct dpu_hw_blk *intf_blks[INTF_MAX - INTF_0];

@@ -88,5 +90,9 @@ void dpu_rm_release(struct dpu_global_state *global_state, int dpu_rm_get_assigned_resources(struct dpu_rm *rm, struct dpu_global_state *global_state, uint32_t enc_id, enum dpu_hw_blk_type type, struct dpu_hw_blk **blks, int blks_size);

+enum dpu_sspp dpu_rm_get_sspp(struct dpu_rm *rm, struct dpu_global_state *global_state, uint32_t plane_id, bool yuv, bool scale); +void dpu_rm_release_sspp(struct dpu_rm *rm, struct dpu_global_state *global_state, uint32_t plane_id);

• #endif /* __DPU_RM_H__ */