dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes
@ 2023-09-14  5:06 Dmitry Baryshkov
  2023-09-14  5:06 ` [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state Dmitry Baryshkov
                   ` (11 more replies)
  0 siblings, 12 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:06 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

As promised in the basic wide planes support ([1]) here comes a series
supporting 2*max_linewidth for all the planes.

Note: Unlike v1 and v2 this series finally includes support for
additional planes - having more planes than the number of SSPP blocks.

Note: this iteration features handling of rotation and reflection of the
wide plane. However rot90 is still not tested: it is enabled on sc7280
and it only supports UBWC (tiled) framebuffers, it was quite low on my
priority list.

[1] https://patchwork.freedesktop.org/series/99909/

Changes since v2:
- Dropped the encoder-related parts, leave all resource allocation as is
  (Abhinav)
- Significantly reworked the SSPP allocation code
- Added debugging code to dump RM state in dri/N/state

Changes since v1:
- Fixed build error due to me missing one of fixups, it was left
  uncommitted.
- Implementated proper handling of wide plane rotation & reflection.

Dmitry Baryshkov (12):
  drm/atomic-helper: split not-scaling part of
    drm_atomic_helper_check_plane_state
  drm/msm/dpu: add current resource allocation to dumped state
  drm/msm/dpu: take plane rotation into account for wide planes
  drm/msm/dpu: move pstate->pipe initialization to
    dpu_plane_atomic_check
  drm/msm/dpu: split dpu_plane_atomic_check()
  drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe()
  drm/msm/dpu: add support for virtual planes
  drm/msm/dpu: allow using two SSPP blocks for a single plane
  drm/msm/dpu: allow sharing SSPP between planes
  drm/msm/dpu: create additional virtual planes
  drm/msm/dpu: allow sharing of blending stages
  drm/msm/dpu: include SSPP allocation state into the dumped state

 drivers/gpu/drm/drm_atomic_helper.c         | 110 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    |  59 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |   2 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  26 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     |   6 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 671 ++++++++++++++++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  29 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      | 130 ++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h      |  36 ++
 include/drm/drm_atomic_helper.h             |   7 +
 10 files changed, 924 insertions(+), 152 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
@ 2023-09-14  5:06 ` Dmitry Baryshkov
  2024-02-14 18:30   ` Abhinav Kumar
  2024-02-14 18:37   ` Ville Syrjälä
  2023-09-14  5:06 ` [PATCH v3 02/12] drm/msm/dpu: add current resource allocation to dumped state Dmitry Baryshkov
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:06 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

The helper drm_atomic_helper_check_plane_state() runs several checks on
plane src and dst rectangles, including the check whether required
scaling fits into the required margins. The msm driver would benefit
from having a function that does all these checks except the scaling
one. Split them into a new helper called
drm_atomic_helper_check_plane_noscale().

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/drm_atomic_helper.c | 110 ++++++++++++++++++++++------
 include/drm/drm_atomic_helper.h     |   7 ++
 2 files changed, 96 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 292e38eb6218..2d7dd66181c9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
 EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
 
 /**
- * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
  * @plane_state: plane state to check
  * @crtc_state: CRTC state to check
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
  * @can_position: is it legal to position the plane such that it
  *                doesn't cover the entire CRTC?  This will generally
  *                only be false for primary planes.
@@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
  * RETURNS:
  * Zero if update appears valid, error code on failure
  */
-int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
-					const struct drm_crtc_state *crtc_state,
-					int min_scale,
-					int max_scale,
-					bool can_position,
-					bool can_update_disabled)
+int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
+					  const struct drm_crtc_state *crtc_state,
+					  bool can_position,
+					  bool can_update_disabled)
 {
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_rect *src = &plane_state->src;
 	struct drm_rect *dst = &plane_state->dst;
 	unsigned int rotation = plane_state->rotation;
 	struct drm_rect clip = {};
-	int hscale, vscale;
 
 	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
 
@@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
 
 	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
 
-	/* Check scaling */
-	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
-	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
-	if (hscale < 0 || vscale < 0) {
-		drm_dbg_kms(plane_state->plane->dev,
-			    "Invalid scaling of plane\n");
-		drm_rect_debug_print("src: ", &plane_state->src, true);
-		drm_rect_debug_print("dst: ", &plane_state->dst, false);
-		return -ERANGE;
-	}
-
 	if (crtc_state->enable)
 		drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
 
@@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
 
 	return 0;
 }
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
+
+/**
+ * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
+ * @plane_state: plane state to check
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ *
+ * Checks that a desired plane scale fits into the min_scale..max_scale
+ * boundaries.
+ * Drivers that provide their own plane handling rather than helper-provided
+ * implementations may still wish to call this function to avoid duplication of
+ * error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
+					int min_scale,
+					int max_scale)
+{
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_rect src;
+	struct drm_rect dst;
+	int hscale, vscale;
+
+	if (!plane_state->visible)
+		return 0;
+
+	src = drm_plane_state_src(plane_state);
+	dst = drm_plane_state_dest(plane_state);
+
+	drm_rect_rotate(&src, fb->width << 16, fb->height << 16, plane_state->rotation);
+
+	hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
+	vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale);
+	if (hscale < 0 || vscale < 0) {
+		drm_dbg_kms(plane_state->plane->dev,
+			    "Invalid scaling of plane\n");
+		drm_rect_debug_print("src: ", &plane_state->src, true);
+		drm_rect_debug_print("dst: ", &plane_state->dst, false);
+		return -ERANGE;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_scale);
+
+/**
+ * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * @plane_state: plane state to check
+ * @crtc_state: CRTC state to check
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ * @can_position: is it legal to position the plane such that it
+ *                doesn't cover the entire CRTC?  This will generally
+ *                only be false for primary planes.
+ * @can_update_disabled: can the plane be updated while the CRTC
+ *                       is disabled?
+ *
+ * Checks that a desired plane update is valid, and updates various
+ * bits of derived state (clipped coordinates etc.). Drivers that provide
+ * their own plane handling rather than helper-provided implementations may
+ * still wish to call this function to avoid duplication of error checking
+ * code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+					const struct drm_crtc_state *crtc_state,
+					int min_scale,
+					int max_scale,
+					bool can_position,
+					bool can_update_disabled)
+{
+	int ret;
+
+	ret = drm_atomic_helper_check_plane_noscale(plane_state, crtc_state, can_position, can_update_disabled);
+	if (ret < 0)
+		return ret;
+
+	return drm_atomic_helper_check_plane_scale(plane_state, min_scale, max_scale);
+}
 EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
 
 /**
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 536a0b0091c3..32ac55aea94e 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -52,6 +52,13 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev,
 int
 drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
 					 struct drm_connector_state *conn_state);
+int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
+					  const struct drm_crtc_state *crtc_state,
+					  bool can_position,
+					  bool can_update_disabled);
+int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
+					int min_scale,
+					int max_scale);
 int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
 					const struct drm_crtc_state *crtc_state,
 					int min_scale,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 02/12] drm/msm/dpu: add current resource allocation to dumped state
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
  2023-09-14  5:06 ` [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state Dmitry Baryshkov
@ 2023-09-14  5:06 ` Dmitry Baryshkov
  2024-02-14 18:40   ` Abhinav Kumar
  2023-09-14  5:06 ` [PATCH v3 03/12] drm/msm/dpu: take plane rotation into account for wide planes Dmitry Baryshkov
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:06 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Provide atomic_print_state callback to the DPU's private object. This
way the debugfs/dri/0/state will also include RM's internal state.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  4 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  2 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c  | 48 +++++++++++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h  |  8 +++++
 4 files changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index ee84160592ce..172b64dc60e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -362,6 +362,7 @@ static void dpu_kms_global_destroy_state(struct drm_private_obj *obj,
 static const struct drm_private_state_funcs dpu_kms_global_state_funcs = {
 	.atomic_duplicate_state = dpu_kms_global_duplicate_state,
 	.atomic_destroy_state = dpu_kms_global_destroy_state,
+	.atomic_print_state = dpu_rm_print_state,
 };
 
 static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
@@ -375,6 +376,9 @@ static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
 	drm_atomic_private_obj_init(dpu_kms->dev, &dpu_kms->global_state,
 				    &state->base,
 				    &dpu_kms_global_state_funcs);
+
+	state->rm = &dpu_kms->rm;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index ed549f0f7c65..dd2be279b366 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -130,6 +130,8 @@ struct vsync_info {
 struct dpu_global_state {
 	struct drm_private_state base;
 
+	struct dpu_rm *rm;
+
 	uint32_t pingpong_to_enc_id[PINGPONG_MAX - PINGPONG_0];
 	uint32_t mixer_to_enc_id[LM_MAX - LM_0];
 	uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f9215643c71a..5e3442fb8678 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -652,3 +652,51 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
 
 	return num_blks;
 }
+
+void dpu_rm_print_state(struct drm_printer *p,
+			const struct drm_private_state *state)
+{
+	const struct dpu_global_state *global_state = to_dpu_global_state(state);
+	const struct dpu_rm *rm = global_state->rm;
+	int i;
+
+	drm_puts(p, "pingpong:");
+	for (i = 0; i < ARRAY_SIZE(global_state->pingpong_to_enc_id); i++)
+		if (rm->pingpong_blks[i])
+			drm_printf(p, " %d,", global_state->pingpong_to_enc_id[i]);
+		else
+			drm_puts(p, " -,");
+	drm_puts(p, "\n");
+
+	drm_puts(p, "mixer:");
+	for (i = 0; i < ARRAY_SIZE(global_state->mixer_to_enc_id); i++)
+		if (rm->mixer_blks[i])
+			drm_printf(p, " %d,", global_state->mixer_to_enc_id[i]);
+		else
+			drm_puts(p, " -,");
+	drm_puts(p, "\n");
+
+	drm_puts(p, "ctl:");
+	for (i = 0; i < ARRAY_SIZE(global_state->ctl_to_enc_id); i++)
+		if (rm->ctl_blks[i])
+			drm_printf(p, " %d,", global_state->ctl_to_enc_id[i]);
+		else
+			drm_puts(p, " -,");
+	drm_puts(p, "\n");
+
+	drm_puts(p, "dspp:");
+	for (i = 0; i < ARRAY_SIZE(global_state->dspp_to_enc_id); i++)
+		if (rm->dspp_blks[i])
+			drm_printf(p, " %d,", global_state->dspp_to_enc_id[i]);
+		else
+			drm_puts(p, " -,");
+	drm_puts(p, "\n");
+
+	drm_puts(p, "dsc:");
+	for (i = 0; i < ARRAY_SIZE(global_state->dsc_to_enc_id); i++)
+		if (rm->dsc_blks[i])
+			drm_printf(p, " %d,", global_state->dsc_to_enc_id[i]);
+		else
+			drm_puts(p, " -,");
+	drm_puts(p, "\n");
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 2b551566cbf4..913baca81a42 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -92,6 +92,14 @@ 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);
 
+/**
+ * dpu_rm_print_state - output the RM private state
+ * @p: DRM printer
+ * @state: private object state
+ */
+void dpu_rm_print_state(struct drm_printer *p,
+			const struct drm_private_state *state);
+
 /**
  * dpu_rm_get_intf - Return a struct dpu_hw_intf instance given it's index.
  * @rm: DPU Resource Manager handle
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 03/12] drm/msm/dpu: take plane rotation into account for wide planes
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
  2023-09-14  5:06 ` [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state Dmitry Baryshkov
  2023-09-14  5:06 ` [PATCH v3 02/12] drm/msm/dpu: add current resource allocation to dumped state Dmitry Baryshkov
@ 2023-09-14  5:06 ` Dmitry Baryshkov
  2024-02-14 19:07   ` Abhinav Kumar
  2023-09-14  5:06 ` [PATCH v3 04/12] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check Dmitry Baryshkov
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:06 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Take into account the plane rotation and flipping when calculating src
positions for the wide plane parts.

This is not an issue yet, because rotation is only supported for the
UBWC planes and wide UBWC planes are rejected anyway because in parallel
multirect case only the half of the usual width is supported for tiled
formats. However it's better to fix this now rather than stumbling upon
it later.

Fixes: 80e8ae3b38ab ("drm/msm/dpu: add support for wide planes")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++++++++++++++---------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c2aaaded07ed..67f9c2a62a17 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -827,16 +827,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
-	pipe_cfg->src_rect = new_plane_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 = new_plane_state->dst;
-
 	fb_rect.x2 = new_plane_state->fb->width;
 	fb_rect.y2 = new_plane_state->fb->height;
 
@@ -852,6 +842,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 
 	max_linewidth = pdpu->catalog->caps->max_linewidth;
 
+	/* state->src is 16.16, src_rect is not */
+	drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src);
+
+	pipe_cfg->dst_rect = new_plane_state->dst;
+
+	drm_rect_rotate(&pipe_cfg->src_rect,
+			new_plane_state->fb->width, new_plane_state->fb->height,
+			new_plane_state->rotation);
+
 	if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
 		/*
 		 * In parallel multirect case only the half of the usual width
@@ -899,6 +898,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 		r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
 	}
 
+	drm_rect_rotate_inv(&pipe_cfg->src_rect,
+			    new_plane_state->fb->width, new_plane_state->fb->height,
+			    new_plane_state->rotation);
+	if (r_pipe->sspp)
+		drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
+				    new_plane_state->fb->width, new_plane_state->fb->height,
+				    new_plane_state->rotation);
+
 	ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
 	if (ret)
 		return ret;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 04/12] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (2 preceding siblings ...)
  2023-09-14  5:06 ` [PATCH v3 03/12] drm/msm/dpu: take plane rotation into account for wide planes Dmitry Baryshkov
@ 2023-09-14  5:06 ` Dmitry Baryshkov
  2023-09-14  5:06 ` [PATCH v3 05/12] drm/msm/dpu: split dpu_plane_atomic_check() Dmitry Baryshkov
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:06 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

In preparation for virtualized planes support, move pstate->pipe
initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In
case of virtual planes the plane's pipe will not be known up to the
point of atomic_check() callback.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 26 +++++++++++------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 67f9c2a62a17..3a75c474c4cd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -785,6 +785,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	int ret = 0, min_scale;
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
 	struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
+	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
 	struct dpu_sw_pipe *pipe = &pstate->pipe;
 	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
 	const struct drm_crtc_state *crtc_state = NULL;
@@ -795,13 +796,22 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	uint32_t max_linewidth;
 	unsigned int rotation;
 	uint32_t supported_rotations;
-	const struct dpu_sspp_cfg *pipe_hw_caps = pstate->pipe.sspp->cap;
-	const struct dpu_sspp_sub_blks *sblk = pstate->pipe.sspp->cap->sblk;
+	const struct dpu_sspp_cfg *pipe_hw_caps;
+	const struct dpu_sspp_sub_blks *sblk;
 
 	if (new_plane_state->crtc)
 		crtc_state = drm_atomic_get_new_crtc_state(state,
 							   new_plane_state->crtc);
 
+	pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
+	r_pipe->sspp = NULL;
+
+	if (!pipe->sspp)
+		return -EINVAL;
+
+	pipe_hw_caps = pipe->sspp->cap;
+	sblk = pipe->sspp->cap->sblk;
+
 	min_scale = FRAC_16_16(1, sblk->maxupscale);
 	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
 						  min_scale,
@@ -818,7 +828,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
 	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
 	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-	r_pipe->sspp = NULL;
 
 	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
 	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
@@ -1302,7 +1311,6 @@ static void dpu_plane_reset(struct drm_plane *plane)
 {
 	struct dpu_plane *pdpu;
 	struct dpu_plane_state *pstate;
-	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
 
 	if (!plane) {
 		DPU_ERROR("invalid plane\n");
@@ -1324,16 +1332,6 @@ static void dpu_plane_reset(struct drm_plane *plane)
 		return;
 	}
 
-	/*
-	 * Set the SSPP here until we have proper virtualized DPU planes.
-	 * This is the place where the state is allocated, so fill it fully.
-	 */
-	pstate->pipe.sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
-	pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
-	pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-
-	pstate->r_pipe.sspp = NULL;
-
 	__drm_atomic_helper_plane_reset(plane, &pstate->base);
 }
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 05/12] drm/msm/dpu: split dpu_plane_atomic_check()
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (3 preceding siblings ...)
  2023-09-14  5:06 ` [PATCH v3 04/12] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check Dmitry Baryshkov
@ 2023-09-14  5:06 ` Dmitry Baryshkov
  2023-09-14  5:07 ` [PATCH v3 06/12] drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe() Dmitry Baryshkov
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:06 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Split dpu_plane_atomic_check() function into two pieces:

dpu_plane_atomic_check_nopipe() performing generic checks on the pstate,
without touching the associated pipe,

and

dpu_plane_atomic_check_pipes(), which takes into account used pipes.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 178 ++++++++++++++--------
 1 file changed, 112 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 3a75c474c4cd..4b1fbe9dbb3f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -777,46 +777,20 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
 	return 0;
 }
 
-static int dpu_plane_atomic_check(struct drm_plane *plane,
-				  struct drm_atomic_state *state)
+static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane,
+					 struct drm_plane_state *new_plane_state,
+					 const struct drm_crtc_state *crtc_state)
 {
-	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
-										 plane);
-	int ret = 0, min_scale;
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
 	struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
-	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
-	struct dpu_sw_pipe *pipe = &pstate->pipe;
-	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
-	const struct drm_crtc_state *crtc_state = NULL;
-	const struct dpu_format *fmt;
 	struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
 	struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
 	struct drm_rect fb_rect = { 0 };
 	uint32_t max_linewidth;
-	unsigned int rotation;
-	uint32_t supported_rotations;
-	const struct dpu_sspp_cfg *pipe_hw_caps;
-	const struct dpu_sspp_sub_blks *sblk;
+	int ret = 0;
 
-	if (new_plane_state->crtc)
-		crtc_state = drm_atomic_get_new_crtc_state(state,
-							   new_plane_state->crtc);
-
-	pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
-	r_pipe->sspp = NULL;
-
-	if (!pipe->sspp)
-		return -EINVAL;
-
-	pipe_hw_caps = pipe->sspp->cap;
-	sblk = pipe->sspp->cap->sblk;
-
-	min_scale = FRAC_16_16(1, sblk->maxupscale);
-	ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
-						  min_scale,
-						  sblk->maxdwnscale << 16,
-						  true, true);
+	ret = drm_atomic_helper_check_plane_noscale(new_plane_state, crtc_state,
+						    true, true);
 	if (ret) {
 		DPU_DEBUG_PLANE(pdpu, "Check plane state failed (%d)\n", ret);
 		return ret;
@@ -824,11 +798,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	if (!new_plane_state->visible)
 		return 0;
 
-	pipe->multirect_index = DPU_SSPP_RECT_SOLO;
-	pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
-	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-
 	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
 	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
 		DPU_ERROR("> %d plane stages assigned\n",
@@ -847,8 +816,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 		return -E2BIG;
 	}
 
-	fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
-
 	max_linewidth = pdpu->catalog->caps->max_linewidth;
 
 	/* state->src is 16.16, src_rect is not */
@@ -861,6 +828,77 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 			new_plane_state->rotation);
 
 	if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
+		if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) {
+			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
+					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
+			return -E2BIG;
+		}
+
+		*r_pipe_cfg = *pipe_cfg;
+		pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
+		pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
+		r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
+		r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
+	} else {
+		memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg));
+	}
+
+	drm_rect_rotate_inv(&pipe_cfg->src_rect,
+			    new_plane_state->fb->width, new_plane_state->fb->height,
+			    new_plane_state->rotation);
+	if (r_pipe_cfg->src_rect.x1 != 0)
+		drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
+				    new_plane_state->fb->width, new_plane_state->fb->height,
+				    new_plane_state->rotation);
+
+	pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state);
+
+	return 0;
+}
+
+static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
+					struct drm_atomic_state *state)
+{
+	struct drm_plane_state *new_plane_state =
+		drm_atomic_get_new_plane_state(state, plane);
+	int ret = 0, min_scale, max_scale;
+	struct dpu_plane *pdpu = to_dpu_plane(plane);
+	struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
+	struct dpu_sw_pipe *pipe = &pstate->pipe;
+	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
+	const struct dpu_format *fmt;
+	struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
+	struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
+	uint32_t max_linewidth;
+	unsigned int rotation;
+	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;
+
+	min_scale = FRAC_16_16(1, sblk->maxupscale);
+	max_scale = sblk->maxdwnscale << 16;
+
+	ret = drm_atomic_helper_check_plane_scale(new_plane_state, min_scale, max_scale);
+	if (ret)
+		return ret;
+
+	pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+	pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+	fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+
+	max_linewidth = pdpu->catalog->caps->max_linewidth;
+
+	ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
+	if (ret)
+		return ret;
+
+	if (r_pipe_cfg->src_rect.x1 != 0) {
 		/*
 		 * In parallel multirect case only the half of the usual width
 		 * is supported for tiled formats. If we are here, we know that
@@ -873,12 +911,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 			return -E2BIG;
 		}
 
-		if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) {
-			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
-					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
-			return -E2BIG;
-		}
-
 		if (drm_rect_width(&pipe_cfg->src_rect) != drm_rect_width(&pipe_cfg->dst_rect) ||
 		    drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect) ||
 		    (!test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features) &&
@@ -900,26 +932,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 		r_pipe->multirect_index = DPU_SSPP_RECT_1;
 		r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
 
-		*r_pipe_cfg = *pipe_cfg;
-		pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
-		pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
-		r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
-		r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
-	}
-
-	drm_rect_rotate_inv(&pipe_cfg->src_rect,
-			    new_plane_state->fb->width, new_plane_state->fb->height,
-			    new_plane_state->rotation);
-	if (r_pipe->sspp)
-		drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
-				    new_plane_state->fb->width, new_plane_state->fb->height,
-				    new_plane_state->rotation);
-
-	ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
-	if (ret)
-		return ret;
-
-	if (r_pipe->sspp) {
 		ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt);
 		if (ret)
 			return ret;
@@ -941,11 +953,45 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	}
 
 	pstate->rotation = rotation;
-	pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state);
 
 	return 0;
 }
 
+static int dpu_plane_atomic_check(struct drm_plane *plane,
+				  struct drm_atomic_state *state)
+{
+	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
+										 plane);
+	int ret = 0;
+	struct dpu_plane *pdpu = to_dpu_plane(plane);
+	struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
+	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
+	struct dpu_sw_pipe *pipe = &pstate->pipe;
+	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
+	const struct drm_crtc_state *crtc_state = NULL;
+
+	if (new_plane_state->crtc)
+		crtc_state = drm_atomic_get_new_crtc_state(state,
+							   new_plane_state->crtc);
+
+	if (pdpu->pipe != SSPP_NONE) {
+		pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
+		r_pipe->sspp = NULL;
+	}
+
+	if (!pipe->sspp)
+		return -EINVAL;
+
+	ret = dpu_plane_atomic_check_nopipe(plane, new_plane_state, crtc_state);
+	if (ret)
+		return ret;
+
+	if (!new_plane_state->visible)
+		return 0;
+
+	return dpu_plane_atomic_check_pipes(plane, state);
+}
+
 static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe *pipe)
 {
 	const struct dpu_format *format =
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 06/12] drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe()
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (4 preceding siblings ...)
  2023-09-14  5:06 ` [PATCH v3 05/12] drm/msm/dpu: split dpu_plane_atomic_check() Dmitry Baryshkov
@ 2023-09-14  5:07 ` Dmitry Baryshkov
  2023-09-14  5:07 ` [PATCH v3 07/12] drm/msm/dpu: add support for virtual planes Dmitry Baryshkov
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Move a call to dpu_plane_check_inline_rotation() to the
dpu_plane_atomic_check_pipe() function, so that the rot90 constraints
are checked for both pipes. Also move rotation field from struct
dpu_plane_state to struct dpu_sw_pipe_cfg.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h |  2 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 55 +++++++++++----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  2 -
 3 files changed, 31 insertions(+), 28 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 cbf4f95ff0fd..21b0c5a13ba8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -157,10 +157,12 @@ struct dpu_hw_pixel_ext {
  * @src_rect:  src ROI, caller takes into account the different operations
  *             such as decimation, flip etc to program this field
  * @dest_rect: destination ROI.
+ * @rotation: simplified drm rotation hint
  */
 struct dpu_sw_pipe_cfg {
 	struct drm_rect src_rect;
 	struct drm_rect dst_rect;
+	unsigned int rotation;
 };
 
 /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 4b1fbe9dbb3f..4cf69f93b44f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -527,8 +527,7 @@ static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct dpu_sw_pipe *pipe,
 
 static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe,
 		const struct dpu_format *fmt, bool color_fill,
-		struct dpu_sw_pipe_cfg *pipe_cfg,
-		unsigned int rotation)
+		struct dpu_sw_pipe_cfg *pipe_cfg)
 {
 	struct dpu_hw_sspp *pipe_hw = pipe->sspp;
 	const struct drm_format_info *info = drm_format_info(fmt->base.pixel_format);
@@ -551,7 +550,7 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe,
 			dst_height,
 			&scaler3_cfg, fmt,
 			info->hsub, info->vsub,
-			rotation);
+			pipe_cfg->rotation);
 
 	/* configure pixel extension based on scalar config */
 	_dpu_plane_setup_pixel_ext(&scaler3_cfg, &pixel_ext,
@@ -603,7 +602,7 @@ static void _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate,
 	if (pipe->sspp->ops.setup_rects)
 		pipe->sspp->ops.setup_rects(pipe, &pipe_cfg);
 
-	_dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg, pstate->rotation);
+	_dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg);
 }
 
 /**
@@ -703,12 +702,17 @@ static void dpu_plane_cleanup_fb(struct drm_plane *plane,
 }
 
 static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
-						const struct dpu_sspp_sub_blks *sblk,
-						struct drm_rect src, const struct dpu_format *fmt)
+					   struct dpu_sw_pipe *pipe,
+					   struct drm_rect src,
+					   const struct dpu_format *fmt)
 {
+	const struct dpu_sspp_sub_blks *sblk = pipe->sspp->cap->sblk;
 	size_t num_formats;
 	const u32 *supported_formats;
 
+	if (!test_bit(DPU_SSPP_INLINE_ROTATION, &pipe->sspp->cap->features))
+		return -EINVAL;
+
 	if (!sblk->rotation_cfg) {
 		DPU_ERROR("invalid rotation cfg\n");
 		return -EINVAL;
@@ -736,6 +740,7 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
 		const struct dpu_format *fmt)
 {
 	uint32_t min_src_size;
+	int ret;
 
 	min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
 
@@ -774,6 +779,12 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
 		return -EINVAL;
 	}
 
+	if (pipe_cfg->rotation & DRM_MODE_ROTATE_90) {
+		ret = dpu_plane_check_inline_rotation(pdpu, pipe, pipe_cfg->src_rect, fmt);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -870,7 +881,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 	struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
 	struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
 	uint32_t max_linewidth;
-	unsigned int rotation;
 	uint32_t supported_rotations;
 	const struct dpu_sspp_cfg *pipe_hw_caps;
 	const struct dpu_sspp_sub_blks *sblk;
@@ -894,6 +904,15 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 
 	max_linewidth = pdpu->catalog->caps->max_linewidth;
 
+	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);
 	if (ret)
 		return ret;
@@ -915,6 +934,7 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 		    drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect) ||
 		    (!test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features) &&
 		     !test_bit(DPU_SSPP_SMART_DMA_V2, &pipe->sspp->cap->features)) ||
+		    pipe_cfg->rotation & DRM_MODE_ROTATE_90 ||
 		    DPU_FORMAT_IS_YUV(fmt)) {
 			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, can't use split source\n",
 					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
@@ -937,23 +957,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 			return ret;
 	}
 
-	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;
-
-	rotation = drm_rotation_simplify(new_plane_state->rotation,
-					supported_rotations);
-
-	if ((pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION)) &&
-		(rotation & DRM_MODE_ROTATE_90)) {
-		ret = dpu_plane_check_inline_rotation(pdpu, sblk, pipe_cfg->src_rect, fmt);
-		if (ret)
-			return ret;
-	}
-
-	pstate->rotation = rotation;
-
 	return 0;
 }
 
@@ -1093,14 +1096,14 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
 				pipe_cfg);
 	}
 
-	_dpu_plane_setup_scaler(pipe, fmt, false, pipe_cfg, pstate->rotation);
+	_dpu_plane_setup_scaler(pipe, fmt, false, pipe_cfg);
 
 	if (pipe->sspp->ops.setup_multirect)
 		pipe->sspp->ops.setup_multirect(
 				pipe);
 
 	if (pipe->sspp->ops.setup_format) {
-		unsigned int rotation = pstate->rotation;
+		unsigned int rotation = pipe_cfg->rotation;
 
 		src_flags = 0x0;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index abd6b21a049b..a3ae45dc95d0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -30,7 +30,6 @@
  * @plane_fetch_bw: calculated BW per plane
  * @plane_clk: calculated clk per plane
  * @needs_dirtyfb: whether attached CRTC needs pixel data explicitly flushed
- * @rotation: simplified drm rotation hint
  */
 struct dpu_plane_state {
 	struct drm_plane_state base;
@@ -47,7 +46,6 @@ struct dpu_plane_state {
 	u64 plane_clk;
 
 	bool needs_dirtyfb;
-	unsigned int rotation;
 };
 
 #define to_dpu_plane_state(x) \
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 07/12] drm/msm/dpu: add support for virtual planes
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (5 preceding siblings ...)
  2023-09-14  5:07 ` [PATCH v3 06/12] drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe() Dmitry Baryshkov
@ 2023-09-14  5:07 ` Dmitry Baryshkov
  2023-09-14  5:07 ` [PATCH v3 08/12] drm/msm/dpu: allow using two SSPP blocks for a single plane Dmitry Baryshkov
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Only several SSPP blocks support such features as YUV output or scaling,
thus different DRM planes have different features.  Properly utilizing
all planes requires the attention of the compositor, who should
prefer simpler planes to YUV-supporting ones. Otherwise it is very easy
to end up in a situation when all featureful planes are already
allocated for simple windows, leaving no spare plane for YUV playback.

To solve this problem make all planes virtual. Each plane is registered
as if it supports all possible features, but then at the runtime during
the atomic_check phase the driver selects backing SSPP block for each
plane.

Note, this does not provide support for using two different SSPP blocks
for a single plane or using two rectangles of an SSPP to drive two
planes. Each plane still gets its own SSPP and can utilize either a solo
rectangle or both multirect rectangles depending on the resolution.

Note #2: By default support for virtual planes is turned off and the
driver still uses old code path with preallocated SSPP block for each
plane. To enable virtual planes, pass 'msm.dpu_use_virtual_planes=1'
kernel parameter.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  50 +++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |  10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h   |   4 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 228 +++++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  19 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c    |  74 +++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h    |  28 +++
 7 files changed, 384 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 8ce7586e2ddf..df4c2e503fa5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1179,6 +1179,49 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
 	return false;
 }
 
+static int dpu_crtc_reassign_planes(struct drm_crtc *crtc, struct drm_crtc_state *crtc_state)
+{
+	int total_planes = crtc->dev->mode_config.num_total_plane;
+	struct drm_atomic_state *state = crtc_state->state;
+	struct dpu_global_state *global_state;
+	struct drm_plane_state **states;
+	struct drm_plane *plane;
+	int ret;
+
+	global_state = dpu_kms_get_global_state(crtc_state->state);
+	if (IS_ERR(global_state))
+		return PTR_ERR(global_state);
+
+	dpu_rm_release_all_sspp(global_state, crtc);
+
+	if (!crtc_state->enable)
+		return 0;
+
+	states = kcalloc(total_planes, sizeof(*states), GFP_KERNEL);
+	if (!states)
+		return -ENOMEM;
+
+	drm_atomic_crtc_state_for_each_plane(plane, crtc_state) {
+		struct drm_plane_state *plane_state =
+			drm_atomic_get_plane_state(state, plane);
+
+		if (IS_ERR(plane_state)) {
+			ret = PTR_ERR(plane_state);
+			goto done;
+		}
+
+		states[plane_state->normalized_zpos] = plane_state;
+	}
+
+	ret = dpu_assign_plane_resources(global_state, state, crtc, states, total_planes);
+
+done:
+	kfree(states);
+	return ret;
+
+	return 0;
+}
+
 static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 		struct drm_atomic_state *state)
 {
@@ -1194,6 +1237,13 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 
 	bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
 
+	if (dpu_use_virtual_planes &&
+	    (crtc_state->planes_changed || crtc_state->zpos_changed)) {
+		rc = dpu_crtc_reassign_planes(crtc, crtc_state);
+		if (rc < 0)
+			return rc;
+	}
+
 	if (!crtc_state->enable || !drm_atomic_crtc_effectively_active(crtc_state)) {
 		DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip atomic_check\n",
 				crtc->base.id, crtc_state->enable,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 172b64dc60e6..5106abe366a3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -51,6 +51,9 @@
 #define DPU_DEBUGFS_DIR "msm_dpu"
 #define DPU_DEBUGFS_HWMASKNAME "hw_log_mask"
 
+bool dpu_use_virtual_planes = false;
+module_param(dpu_use_virtual_planes, bool, 0);
+
 static int dpu_kms_hw_init(struct msm_kms *kms);
 static void _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms);
 
@@ -772,8 +775,11 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 			  type, catalog->sspp[i].features,
 			  catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR));
 
-		plane = dpu_plane_init(dev, catalog->sspp[i].id, type,
-				       (1UL << max_crtc_count) - 1);
+		if (dpu_use_virtual_planes)
+			plane = dpu_plane_init_virtual(dev, type, (1UL << max_crtc_count) - 1);
+		else
+			plane = dpu_plane_init(dev, catalog->sspp[i].id, type,
+					       (1UL << max_crtc_count) - 1);
 		if (IS_ERR(plane)) {
 			DPU_ERROR("dpu_plane_init failed\n");
 			ret = PTR_ERR(plane);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index dd2be279b366..89947ba25230 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -63,6 +63,8 @@
 #define ktime_compare_safe(A, B) \
 	ktime_compare(ktime_sub((A), (B)), ktime_set(0, 0))
 
+extern bool dpu_use_virtual_planes;
+
 struct dpu_kms {
 	struct msm_kms base;
 	struct drm_device *dev;
@@ -137,6 +139,8 @@ struct dpu_global_state {
 	uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
 	uint32_t dspp_to_enc_id[DSPP_MAX - DSPP_0];
 	uint32_t dsc_to_enc_id[DSC_MAX - DSC_0];
+
+	uint32_t sspp_to_crtc_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 4cf69f93b44f..2ac6e1074c62 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -857,7 +857,7 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane,
 	drm_rect_rotate_inv(&pipe_cfg->src_rect,
 			    new_plane_state->fb->width, new_plane_state->fb->height,
 			    new_plane_state->rotation);
-	if (r_pipe_cfg->src_rect.x1 != 0)
+	if (drm_rect_width(&r_pipe_cfg->src_rect) != 0)
 		drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
 				    new_plane_state->fb->width, new_plane_state->fb->height,
 				    new_plane_state->rotation);
@@ -917,7 +917,7 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
-	if (r_pipe_cfg->src_rect.x1 != 0) {
+	if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
 		/*
 		 * In parallel multirect case only the half of the usual width
 		 * is supported for tiled formats. If we are here, we know that
@@ -995,6 +995,108 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	return dpu_plane_atomic_check_pipes(plane, state);
 }
 
+static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
+					  struct drm_atomic_state *state)
+{
+	struct drm_plane_state *plane_state =
+		drm_atomic_get_plane_state(state, plane);
+	struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state);
+	const struct dpu_format *format;
+	struct drm_crtc_state *crtc_state;
+	int ret;
+
+	if (plane_state->crtc)
+		crtc_state = drm_atomic_get_new_crtc_state(state,
+							   plane_state->crtc);
+
+	ret = dpu_plane_atomic_check_nopipe(plane, plane_state, crtc_state);
+	if (ret)
+		return ret;
+
+	if (!plane_state->visible) {
+		/*
+		 * resources are freed by dpu_crtc_assign_plane_resources(),
+		 * but clean them here.
+		 */
+		pstate->pipe.sspp = NULL;
+		pstate->r_pipe.sspp = NULL;
+
+		return 0;
+	}
+
+	format = to_dpu_format(msm_framebuffer_format(plane_state->fb));
+
+	/* force resource reallocation if the format of FB has changed */
+	if (pstate->saved_fmt != format) {
+		crtc_state->planes_changed = true;
+		pstate->saved_fmt = format;
+	}
+
+	return 0;
+}
+
+static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
+					      struct dpu_global_state *global_state,
+					      struct drm_atomic_state *state,
+					      struct drm_plane_state *plane_state)
+{
+	struct drm_plane *plane = plane_state->plane;
+	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
+	struct dpu_rm_sspp_requirements reqs;
+	struct dpu_plane_state *pstate;
+	struct dpu_sw_pipe *pipe;
+	struct dpu_sw_pipe *r_pipe;
+	const struct dpu_format *fmt;
+
+	pstate = to_dpu_plane_state(plane_state);
+	pipe = &pstate->pipe;
+	r_pipe = &pstate->r_pipe;
+
+	pipe->sspp = NULL;
+	r_pipe->sspp = NULL;
+
+	if (!plane_state->fb)
+		return -EINVAL;
+
+	fmt = to_dpu_format(msm_framebuffer_format(plane_state->fb));
+	reqs.yuv = DPU_FORMAT_IS_YUV(fmt);
+	reqs.scale = (plane_state->src_w >> 16 != plane_state->crtc_w) ||
+		(plane_state->src_h >> 16 != plane_state->crtc_h);
+
+	reqs.rot90 = drm_rotation_90_or_270(plane_state->rotation);
+
+	pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
+	if (!pipe->sspp)
+		return -ENODEV;
+
+	return dpu_plane_atomic_check_pipes(plane, state);
+}
+
+int dpu_assign_plane_resources(struct dpu_global_state *global_state,
+			       struct drm_atomic_state *state,
+			       struct drm_crtc *crtc,
+			       struct drm_plane_state **states,
+			       unsigned int num_planes)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_planes; i++) {
+		struct drm_plane_state *plane_state = states[i];
+
+		if (!plane_state ||
+		    !plane_state->visible)
+			continue;
+
+		ret = dpu_plane_virtual_assign_resources(crtc, global_state,
+							 state, plane_state);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
 static void dpu_plane_flush_csc(struct dpu_plane *pdpu, struct dpu_sw_pipe *pipe)
 {
 	const struct dpu_format *format =
@@ -1338,12 +1440,14 @@ static void dpu_plane_atomic_print_state(struct drm_printer *p,
 
 	drm_printf(p, "\tstage=%d\n", pstate->stage);
 
-	drm_printf(p, "\tsspp[0]=%s\n", pipe->sspp->cap->name);
-	drm_printf(p, "\tmultirect_mode[0]=%s\n", dpu_get_multirect_mode(pipe->multirect_mode));
-	drm_printf(p, "\tmultirect_index[0]=%s\n",
-		   dpu_get_multirect_index(pipe->multirect_index));
-	drm_printf(p, "\tsrc[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->src_rect));
-	drm_printf(p, "\tdst[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->dst_rect));
+	if (pipe->sspp) {
+		drm_printf(p, "\tsspp[0]=%s\n", pipe->sspp->cap->name);
+		drm_printf(p, "\tmultirect_mode[0]=%s\n", dpu_get_multirect_mode(pipe->multirect_mode));
+		drm_printf(p, "\tmultirect_index[0]=%s\n",
+			   dpu_get_multirect_index(pipe->multirect_index));
+		drm_printf(p, "\tsrc[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->src_rect));
+		drm_printf(p, "\tdst[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->dst_rect));
+	}
 
 	if (r_pipe->sspp) {
 		drm_printf(p, "\tsspp[1]=%s\n", r_pipe->sspp->cap->name);
@@ -1433,18 +1537,26 @@ static const struct drm_plane_helper_funcs dpu_plane_helper_funcs = {
 		.atomic_update = dpu_plane_atomic_update,
 };
 
+static const struct drm_plane_helper_funcs dpu_plane_virtual_helper_funcs = {
+	.prepare_fb = dpu_plane_prepare_fb,
+	.cleanup_fb = dpu_plane_cleanup_fb,
+	.atomic_check = dpu_plane_virtual_atomic_check,
+	.atomic_update = dpu_plane_atomic_update,
+};
+
 /* initialize plane */
-struct drm_plane *dpu_plane_init(struct drm_device *dev,
-		uint32_t pipe, enum drm_plane_type type,
-		unsigned long possible_crtcs)
+static struct drm_plane *dpu_plane_init_common(struct drm_device *dev,
+					       enum drm_plane_type type,
+					       unsigned long possible_crtcs,
+					       bool inline_rotation,
+					       const uint32_t *format_list,
+					       uint32_t num_formats,
+					       enum dpu_sspp pipe)
 {
 	struct drm_plane *plane = NULL;
-	const uint32_t *format_list;
 	struct dpu_plane *pdpu;
 	struct msm_drm_private *priv = dev->dev_private;
 	struct dpu_kms *kms = to_dpu_kms(priv->kms);
-	struct dpu_hw_sspp *pipe_hw;
-	uint32_t num_formats;
 	uint32_t supported_rotations;
 	int ret = -EINVAL;
 
@@ -1460,16 +1572,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 	plane = &pdpu->base;
 	pdpu->pipe = pipe;
 
-	/* initialize underlying h/w driver */
-	pipe_hw = dpu_rm_get_sspp(&kms->rm, pipe);
-	if (!pipe_hw || !pipe_hw->cap || !pipe_hw->cap->sblk) {
-		DPU_ERROR("[%u]SSPP is invalid\n", pipe);
-		goto clean_plane;
-	}
-
-	format_list = pipe_hw->cap->sblk->format_list;
-	num_formats = pipe_hw->cap->sblk->num_formats;
-
 	ret = drm_universal_plane_init(dev, plane, 0xff, &dpu_plane_funcs,
 				format_list, num_formats,
 				supported_format_modifiers, type, NULL);
@@ -1490,7 +1592,7 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 
 	supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0 | DRM_MODE_ROTATE_180;
 
-	if (pipe_hw->cap->features & BIT(DPU_SSPP_INLINE_ROTATION))
+	if (inline_rotation)
 		supported_rotations |= DRM_MODE_ROTATE_MASK;
 
 	drm_plane_create_rotation_property(plane,
@@ -1499,8 +1601,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 	drm_plane_enable_fb_damage_clips(plane);
 
 	/* success! finalize initialization */
-	drm_plane_helper_add(plane, &dpu_plane_helper_funcs);
-
 	mutex_init(&pdpu->lock);
 
 	DPU_DEBUG("%s created for pipe:%u id:%u\n", plane->name,
@@ -1511,3 +1611,77 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 	kfree(pdpu);
 	return ERR_PTR(ret);
 }
+
+struct drm_plane *dpu_plane_init(struct drm_device *dev,
+				 uint32_t pipe, enum drm_plane_type type,
+				 unsigned long possible_crtcs)
+{
+	struct drm_plane *plane = NULL;
+	struct msm_drm_private *priv = dev->dev_private;
+	struct dpu_kms *kms = to_dpu_kms(priv->kms);
+	struct dpu_hw_sspp *pipe_hw;
+
+	/* initialize underlying h/w driver */
+	pipe_hw = dpu_rm_get_sspp(&kms->rm, pipe);
+	if (!pipe_hw || !pipe_hw->cap || !pipe_hw->cap->sblk) {
+		DPU_ERROR("[%u]SSPP is invalid\n", pipe);
+		return ERR_PTR(-EINVAL);
+	}
+
+
+	plane = dpu_plane_init_common(dev, type, possible_crtcs,
+				      pipe_hw->cap->features & BIT(DPU_SSPP_INLINE_ROTATION),
+				      pipe_hw->cap->sblk->format_list,
+				      pipe_hw->cap->sblk->num_formats,
+				      pipe);
+	if (IS_ERR(plane))
+		return plane;
+
+	drm_plane_helper_add(plane, &dpu_plane_helper_funcs);
+
+	DPU_DEBUG("%s created for pipe:%u id:%u\n", plane->name,
+					pipe, plane->base.id);
+
+	return plane;
+}
+
+struct drm_plane *dpu_plane_init_virtual(struct drm_device *dev,
+					 enum drm_plane_type type,
+					 unsigned long possible_crtcs)
+{
+	struct drm_plane *plane = NULL;
+	struct msm_drm_private *priv = dev->dev_private;
+	struct dpu_kms *kms = to_dpu_kms(priv->kms);
+	bool has_inline_rotation = false;
+	const u32 *format_list = NULL;
+	u32 num_formats = 0;
+	int i;
+
+	/* Determine the largest configuration that we can implement */
+	for (i = 0; i < kms->catalog->sspp_count; i++) {
+		const struct dpu_sspp_cfg *cfg = &kms->catalog->sspp[i];
+
+		if (test_bit(DPU_SSPP_INLINE_ROTATION, &cfg->features))
+			has_inline_rotation = true;
+
+		if (!format_list ||
+		    cfg->features & DPU_SSPP_CSC_ANY) {
+			format_list = cfg->sblk->format_list;
+			num_formats = cfg->sblk->num_formats;
+		}
+	}
+
+	plane = dpu_plane_init_common(dev, type, possible_crtcs,
+				      has_inline_rotation,
+				      format_list,
+				      num_formats,
+				      SSPP_NONE);
+	if (IS_ERR(plane))
+		return plane;
+
+	drm_plane_helper_add(plane, &dpu_plane_virtual_helper_funcs);
+
+	DPU_DEBUG("%s created virtual id:%u\n", plane->name, plane->base.id);
+
+	return plane;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index a3ae45dc95d0..15f7d60d8b85 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -30,6 +30,7 @@
  * @plane_fetch_bw: calculated BW per plane
  * @plane_clk: calculated clk per plane
  * @needs_dirtyfb: whether attached CRTC needs pixel data explicitly flushed
+ * @saved_fmt: format used by the plane's FB, saved for for virtual plane support
  */
 struct dpu_plane_state {
 	struct drm_plane_state base;
@@ -46,6 +47,8 @@ struct dpu_plane_state {
 	u64 plane_clk;
 
 	bool needs_dirtyfb;
+
+	const struct dpu_format *saved_fmt;
 };
 
 #define to_dpu_plane_state(x) \
@@ -75,6 +78,16 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
 		uint32_t pipe, enum drm_plane_type type,
 		unsigned long possible_crtcs);
 
+/**
+ * dpu_plane_init_virtual - create new dpu virtualized plane
+ * @dev:   Pointer to DRM device
+ * @type:  Plane type - PRIMARY/OVERLAY/CURSOR
+ * @possible_crtcs: bitmask of crtc that can be attached to the given pipe
+ */
+struct drm_plane *dpu_plane_init_virtual(struct drm_device *dev,
+					 enum drm_plane_type type,
+					 unsigned long possible_crtcs);
+
 /**
  * dpu_plane_color_fill - enables color fill on plane
  * @plane:  Pointer to DRM plane object
@@ -91,4 +104,10 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable);
 static inline void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable) {}
 #endif
 
+int dpu_assign_plane_resources(struct dpu_global_state *global_state,
+			       struct drm_atomic_state *state,
+			       struct drm_crtc *crtc,
+			       struct drm_plane_state **states,
+			       unsigned int num_planes);
+
 #endif /* _DPU_PLANE_H_ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 5e3442fb8678..1b0166bc9bee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -593,6 +593,80 @@ int dpu_rm_reserve(
 	return ret;
 }
 
+struct dpu_hw_sspp *dpu_rm_reserve_sspp(struct dpu_rm *rm,
+					struct dpu_global_state *global_state,
+					struct drm_crtc *crtc,
+					struct dpu_rm_sspp_requirements *reqs)
+{
+	uint32_t crtc_id = crtc->base.id;
+	unsigned int weight, best_weght = UINT_MAX;
+	struct dpu_hw_sspp *hw_sspp;
+	unsigned long mask = 0;
+	int i, best_idx = -1;
+
+	/*
+	 * Don't take cursor feature into consideration until there is proper support for SSPP_CURSORn.
+	 */
+	mask |= BIT(DPU_SSPP_CURSOR);
+
+	if (reqs->scale)
+		mask |= DPU_SSPP_SCALER;
+
+	if (reqs->yuv)
+		mask |= DPU_SSPP_CSC_ANY;
+
+	if (reqs->rot90)
+		mask |= DPU_SSPP_INLINE_ROTATION;
+
+	for (i = 0; i < ARRAY_SIZE(rm->hw_sspp); i++) {
+		if (!rm->hw_sspp[i])
+			continue;
+
+		if (global_state->sspp_to_crtc_id[i])
+			continue;
+
+		hw_sspp = rm->hw_sspp[i];
+
+		/* skip incompatible planes */
+		if (reqs->scale && !(hw_sspp->cap->features & DPU_SSPP_SCALER))
+			continue;
+
+		if (reqs->yuv && !(hw_sspp->cap->features & DPU_SSPP_CSC_ANY))
+			continue;
+
+		if (reqs->rot90 && !(hw_sspp->cap->features & DPU_SSPP_INLINE_ROTATION))
+			continue;
+
+		/*
+		 * For non-yuv, non-scaled planes prefer simple (DMA or RGB)
+		 * plane, falling back to VIG only if there are no such planes.
+		 *
+		 * This way we'd leave VIG sspps to be later used for YUV formats.
+		 */
+		weight = hweight64(hw_sspp->cap->features & ~mask);
+		if (weight < best_weght) {
+			best_weght = weight;
+			best_idx = i;
+		}
+	}
+
+	if (best_idx < 0)
+		return NULL;
+
+	global_state->sspp_to_crtc_id[best_idx] = crtc_id;
+
+	return rm->hw_sspp[best_idx];
+}
+
+void dpu_rm_release_all_sspp(struct dpu_global_state *global_state,
+			     struct drm_crtc *crtc)
+{
+	uint32_t crtc_id = crtc->base.id;
+
+	_dpu_rm_clear_mapping(global_state->sspp_to_crtc_id,
+		ARRAY_SIZE(global_state->sspp_to_crtc_id), crtc_id);
+}
+
 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)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 913baca81a42..ae94848beb58 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -35,6 +35,12 @@ struct dpu_rm {
 	struct dpu_hw_sspp *hw_sspp[SSPP_MAX - SSPP_NONE];
 };
 
+struct dpu_rm_sspp_requirements {
+	bool yuv;
+	bool scale;
+	bool rot90;
+};
+
 /**
  * dpu_rm_init - Read hardware catalog and create reservation tracking objects
  *	for all HW blocks.
@@ -85,6 +91,28 @@ int dpu_rm_reserve(struct dpu_rm *rm,
 void dpu_rm_release(struct dpu_global_state *global_state,
 		struct drm_encoder *enc);
 
+/**
+ * dpu_rm_reserve_sspp - Reserve the required SSPP for the provided CRTC
+ * @rm: DPU Resource Manager handle
+ * @global_state: private global state
+ * @crtc: DRM CRTC handle
+ * @reqs: SSPP required features
+ */
+struct dpu_hw_sspp *dpu_rm_reserve_sspp(struct dpu_rm *rm,
+					struct dpu_global_state *global_state,
+					struct drm_crtc *crtc,
+					struct dpu_rm_sspp_requirements *reqs);
+
+/**
+ * dpu_rm_release_all_sspp - Given the CRTC, release all SSPP
+ *	blocks previously reserved for that use case.
+ * @rm: DPU Resource Manager handle
+ * @crtc: DRM CRTC handle
+ * @Return: 0 on Success otherwise -ERROR
+ */
+void dpu_rm_release_all_sspp(struct dpu_global_state *global_state,
+			     struct drm_crtc *crtc);
+
 /**
  * Get hw resources of the given type that are assigned to this encoder.
  */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 08/12] drm/msm/dpu: allow using two SSPP blocks for a single plane
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (6 preceding siblings ...)
  2023-09-14  5:07 ` [PATCH v3 07/12] drm/msm/dpu: add support for virtual planes Dmitry Baryshkov
@ 2023-09-14  5:07 ` Dmitry Baryshkov
  2023-09-14  5:07 ` [PATCH v3 09/12] drm/msm/dpu: allow sharing SSPP between planes Dmitry Baryshkov
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Virtual wide planes give high amount of flexibility, but it is not
always enough:

In parallel multirect case only the half of the usual width is supported
for tiled formats. Thus the whole width of two tiled multirect
rectangles can not be greater than max_linewidth, which is not enough
for some platforms/compositors.

Another example is as simple as wide YUV plane. YUV planes can not use
multirect, so currently they are limited to max_linewidth too.

Now that the planes are fully virtualized, add support for allocating
two SSPP blocks to drive a single DRM plane. This fixes both mentioned
cases and allows all planes to go up to 2*max_linewidth (at the cost of
making some of the planes unavailable to the user).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 170 ++++++++++++++++------
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |   8 +
 2 files changed, 130 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 2ac6e1074c62..9ffbcd91661a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -867,6 +867,28 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane,
 	return 0;
 }
 
+static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
+						   struct dpu_sw_pipe_cfg *pipe_cfg,
+						   const struct dpu_format *fmt,
+						   uint32_t max_linewidth)
+{
+	if (drm_rect_width(&pipe_cfg->src_rect) != drm_rect_width(&pipe_cfg->dst_rect) ||
+	    drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect))
+		return false;
+
+	if (pipe_cfg->rotation & DRM_MODE_ROTATE_90)
+		return false;
+
+	if (DPU_FORMAT_IS_YUV(fmt))
+		return false;
+
+	if (DPU_FORMAT_IS_UBWC(fmt) &&
+	    drm_rect_width(&pipe_cfg->src_rect) > max_linewidth / 2)
+		return false;
+
+	return true;
+}
+
 static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 					struct drm_atomic_state *state)
 {
@@ -880,7 +902,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 	const struct dpu_format *fmt;
 	struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
 	struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
-	uint32_t max_linewidth;
 	uint32_t supported_rotations;
 	const struct dpu_sspp_cfg *pipe_hw_caps;
 	const struct dpu_sspp_sub_blks *sblk;
@@ -895,15 +916,8 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
-	pipe->multirect_index = DPU_SSPP_RECT_SOLO;
-	pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
-	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-
 	fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
 
-	max_linewidth = pdpu->catalog->caps->max_linewidth;
-
 	supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
 
 	if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
@@ -918,40 +932,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 		return ret;
 
 	if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
-		/*
-		 * In parallel multirect case only the half of the usual width
-		 * is supported for tiled formats. If we are here, we know that
-		 * full width is more than max_linewidth, thus each rect is
-		 * wider than allowed.
-		 */
-		if (DPU_FORMAT_IS_UBWC(fmt)) {
-			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, tiled format\n",
-					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
-			return -E2BIG;
-		}
-
-		if (drm_rect_width(&pipe_cfg->src_rect) != drm_rect_width(&pipe_cfg->dst_rect) ||
-		    drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect) ||
-		    (!test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features) &&
-		     !test_bit(DPU_SSPP_SMART_DMA_V2, &pipe->sspp->cap->features)) ||
-		    pipe_cfg->rotation & DRM_MODE_ROTATE_90 ||
-		    DPU_FORMAT_IS_YUV(fmt)) {
-			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, can't use split source\n",
-					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
-			return -E2BIG;
-		}
-
-		/*
-		 * Use multirect for wide plane. We do not support dynamic
-		 * assignment of SSPPs, so we know the configuration.
-		 */
-		pipe->multirect_index = DPU_SSPP_RECT_0;
-		pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
-
-		r_pipe->sspp = pipe->sspp;
-		r_pipe->multirect_index = DPU_SSPP_RECT_1;
-		r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
-
 		ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt);
 		if (ret)
 			return ret;
@@ -971,16 +951,16 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
 	struct dpu_sw_pipe *pipe = &pstate->pipe;
 	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
+	struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
+	struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
 	const struct drm_crtc_state *crtc_state = NULL;
 
 	if (new_plane_state->crtc)
 		crtc_state = drm_atomic_get_new_crtc_state(state,
 							   new_plane_state->crtc);
 
-	if (pdpu->pipe != SSPP_NONE) {
-		pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
-		r_pipe->sspp = NULL;
-	}
+	pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
+	r_pipe->sspp = NULL;
 
 	if (!pipe->sspp)
 		return -EINVAL;
@@ -992,6 +972,51 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	if (!new_plane_state->visible)
 		return 0;
 
+	pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+	pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+	if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
+		uint32_t max_linewidth = dpu_kms->catalog->caps->max_linewidth;
+		const struct dpu_format *fmt;
+
+		fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+
+		/*
+		 * In parallel multirect case only the half of the usual width
+		 * is supported for tiled formats. If we are here, we know that
+		 * full width is more than max_linewidth, thus each rect is
+		 * wider than allowed.
+		 */
+		if (DPU_FORMAT_IS_UBWC(fmt)) {
+			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, tiled format\n",
+					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
+			return -E2BIG;
+		}
+
+		r_pipe->sspp = pipe->sspp;
+
+		if (!dpu_plane_is_multirect_parallel_capable(pipe, pipe_cfg, fmt, max_linewidth) ||
+		    !dpu_plane_is_multirect_parallel_capable(r_pipe, r_pipe_cfg, fmt, max_linewidth) ||
+		    !(test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features) ||
+		      test_bit(DPU_SSPP_SMART_DMA_V2, &pipe->sspp->cap->features))) {
+			DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u, can't use split source\n",
+					DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
+			return -E2BIG;
+		}
+
+		/*
+		 * Use multirect for wide plane. We do not support dynamic
+		 * assignment of SSPPs, so we know the configuration.
+		 */
+		pipe->multirect_index = DPU_SSPP_RECT_0;
+		pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+
+		r_pipe->multirect_index = DPU_SSPP_RECT_1;
+		r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+	}
+
 	return dpu_plane_atomic_check_pipes(plane, state);
 }
 
@@ -1026,10 +1051,18 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
 
 	format = to_dpu_format(msm_framebuffer_format(plane_state->fb));
 
-	/* force resource reallocation if the format of FB has changed */
-	if (pstate->saved_fmt != format) {
+	/* force resource reallocation if the format of FB or src/dst have changed */
+	if (pstate->saved_fmt != format ||
+	    pstate->saved_src_w != plane_state->src_w ||
+	    pstate->saved_src_h != plane_state->src_h ||
+	    pstate->saved_src_w != plane_state->src_w ||
+	    pstate->saved_crtc_h != plane_state->crtc_h) {
 		crtc_state->planes_changed = true;
 		pstate->saved_fmt = format;
+		pstate->saved_src_w = plane_state->src_w;
+		pstate->saved_src_h = plane_state->src_h;
+		pstate->saved_crtc_w = plane_state->crtc_w;
+		pstate->saved_crtc_h = plane_state->crtc_h;
 	}
 
 	return 0;
@@ -1046,11 +1079,16 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
 	struct dpu_plane_state *pstate;
 	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;
 	const struct dpu_format *fmt;
+	uint32_t max_linewidth;
 
 	pstate = to_dpu_plane_state(plane_state);
 	pipe = &pstate->pipe;
 	r_pipe = &pstate->r_pipe;
+	pipe_cfg = &pstate->pipe_cfg;
+	r_pipe_cfg = &pstate->r_pipe_cfg;
 
 	pipe->sspp = NULL;
 	r_pipe->sspp = NULL;
@@ -1065,10 +1103,46 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
 
 	reqs.rot90 = drm_rotation_90_or_270(plane_state->rotation);
 
+	max_linewidth = dpu_kms->catalog->caps->max_linewidth;
+
 	pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
 	if (!pipe->sspp)
 		return -ENODEV;
 
+	if (drm_rect_width(&r_pipe_cfg->src_rect) == 0) {
+		pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+		pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+		r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+		r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+		r_pipe->sspp = NULL;
+	} else {
+		if (dpu_plane_is_multirect_parallel_capable(pipe, pipe_cfg, fmt, max_linewidth) &&
+		    dpu_plane_is_multirect_parallel_capable(r_pipe, r_pipe_cfg, fmt, max_linewidth) &&
+		    (test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features) ||
+		     test_bit(DPU_SSPP_SMART_DMA_V2, &pipe->sspp->cap->features))) {
+			r_pipe->sspp = pipe->sspp;
+
+			pipe->multirect_index = DPU_SSPP_RECT_0;
+			pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+
+			r_pipe->multirect_index = DPU_SSPP_RECT_1;
+			r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+		} else {
+			/* multirect is not possible, use two SSPP blocks */
+			r_pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
+			if (!r_pipe->sspp)
+				return -ENODEV;
+
+			pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+			pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+			r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+			r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+		}
+	}
+
 	return dpu_plane_atomic_check_pipes(plane, state);
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 15f7d60d8b85..5522f9035d68 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -31,6 +31,10 @@
  * @plane_clk: calculated clk per plane
  * @needs_dirtyfb: whether attached CRTC needs pixel data explicitly flushed
  * @saved_fmt: format used by the plane's FB, saved for for virtual plane support
+ * @saved_src_w: cached value of plane's src_w, saved for for virtual plane support
+ * @saved_src_h: cached value of plane's src_h, saved for for virtual plane support
+ * @saved_crtc_w: cached value of plane's crtc_w, saved for for virtual plane support
+ * @saved_crtc_h: cached value of plane's crtc_h, saved for for virtual plane support
  */
 struct dpu_plane_state {
 	struct drm_plane_state base;
@@ -49,6 +53,10 @@ struct dpu_plane_state {
 	bool needs_dirtyfb;
 
 	const struct dpu_format *saved_fmt;
+	uint32_t saved_src_w;
+	uint32_t saved_src_h;
+	uint32_t saved_crtc_w;
+	uint32_t saved_crtc_h;
 };
 
 #define to_dpu_plane_state(x) \
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 09/12] drm/msm/dpu: allow sharing SSPP between planes
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (7 preceding siblings ...)
  2023-09-14  5:07 ` [PATCH v3 08/12] drm/msm/dpu: allow using two SSPP blocks for a single plane Dmitry Baryshkov
@ 2023-09-14  5:07 ` Dmitry Baryshkov
  2023-09-14  5:07 ` [PATCH v3 10/12] drm/msm/dpu: create additional virtual planes Dmitry Baryshkov
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Since SmartDMA planes provide two rectangles, it is possible to use them
to drive two different DRM planes, first plane getting the rect_0,
another one using rect_1 of the same SSPP. The sharing algorithm is
pretty simple, it requires that each of the planes can be driven by the
single rectangle and only consequetive planes are considered.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 128 +++++++++++++++++++---
 1 file changed, 112 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 9ffbcd91661a..61afd1cf033d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -867,10 +867,9 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane,
 	return 0;
 }
 
-static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
-						   struct dpu_sw_pipe_cfg *pipe_cfg,
-						   const struct dpu_format *fmt,
-						   uint32_t max_linewidth)
+static int dpu_plane_is_multirect_capable(struct dpu_sw_pipe *pipe,
+					  struct dpu_sw_pipe_cfg *pipe_cfg,
+					  const struct dpu_format *fmt)
 {
 	if (drm_rect_width(&pipe_cfg->src_rect) != drm_rect_width(&pipe_cfg->dst_rect) ||
 	    drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect))
@@ -882,6 +881,13 @@ static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
 	if (DPU_FORMAT_IS_YUV(fmt))
 		return false;
 
+	return true;
+}
+
+static int dpu_plane_is_parallel_capable(struct dpu_sw_pipe_cfg *pipe_cfg,
+					 const struct dpu_format *fmt,
+					 uint32_t max_linewidth)
+{
 	if (DPU_FORMAT_IS_UBWC(fmt) &&
 	    drm_rect_width(&pipe_cfg->src_rect) > max_linewidth / 2)
 		return false;
@@ -889,6 +895,82 @@ static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
 	return true;
 }
 
+static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
+						   struct dpu_sw_pipe_cfg *pipe_cfg,
+						   const struct dpu_format *fmt,
+						   uint32_t max_linewidth)
+{
+	return dpu_plane_is_multirect_capable(pipe, pipe_cfg, fmt) &&
+		dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth);
+}
+
+
+static int dpu_plane_try_multirect(struct dpu_plane_state *pstate,
+				   struct dpu_plane_state *prev_pstate,
+				   const struct dpu_format *fmt,
+				   uint32_t max_linewidth)
+{
+	struct dpu_sw_pipe *pipe = &pstate->pipe;
+	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
+	struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
+	struct dpu_sw_pipe *prev_pipe = &prev_pstate->pipe;
+	struct dpu_sw_pipe_cfg *prev_pipe_cfg = &prev_pstate->pipe_cfg;
+	const struct dpu_format *prev_fmt =
+		to_dpu_format(msm_framebuffer_format(prev_pstate->base.fb));
+	u16 max_tile_height = 1;
+
+	if (prev_pstate->r_pipe.sspp != NULL ||
+	    prev_pipe->multirect_mode != DPU_SSPP_MULTIRECT_NONE)
+		return false;
+
+	if (!dpu_plane_is_multirect_capable(pipe, pipe_cfg, fmt) ||
+	    !dpu_plane_is_multirect_capable(prev_pipe, prev_pipe_cfg, prev_fmt) ||
+	    !(test_bit(DPU_SSPP_SMART_DMA_V1, &prev_pipe->sspp->cap->features) ||
+	      test_bit(DPU_SSPP_SMART_DMA_V2, &prev_pipe->sspp->cap->features)))
+		return false;
+
+	if (DPU_FORMAT_IS_UBWC(fmt))
+		max_tile_height = max(max_tile_height, fmt->tile_height);
+
+	if (DPU_FORMAT_IS_UBWC(prev_fmt))
+		max_tile_height = max(max_tile_height, prev_fmt->tile_height);
+
+	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+	r_pipe->sspp = NULL;
+
+	if (dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth) &&
+	    dpu_plane_is_parallel_capable(prev_pipe_cfg, prev_fmt, max_linewidth) &&
+	    (pipe_cfg->dst_rect.x1 >= prev_pipe_cfg->dst_rect.x2 ||
+	     prev_pipe_cfg->dst_rect.x1 >= pipe_cfg->dst_rect.x2)) {
+		pipe->sspp = prev_pipe->sspp;
+
+		pipe->multirect_index = DPU_SSPP_RECT_1;
+		pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+
+		prev_pipe->multirect_index = DPU_SSPP_RECT_0;
+		prev_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+
+		return true;
+	}
+
+	if (pipe_cfg->dst_rect.y1 >= prev_pipe_cfg->dst_rect.y2 + 2 * max_tile_height ||
+	    prev_pipe_cfg->dst_rect.y1 >= pipe_cfg->dst_rect.y2 + 2 * max_tile_height) {
+		pipe->sspp = prev_pipe->sspp;
+
+		pipe->multirect_index = DPU_SSPP_RECT_1;
+		pipe->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
+
+		prev_pipe->multirect_index = DPU_SSPP_RECT_0;
+		prev_pipe->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
+
+		return true;
+	}
+
+	return false;
+}
+
 static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
 					struct drm_atomic_state *state)
 {
@@ -1071,12 +1153,13 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
 static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
 					      struct dpu_global_state *global_state,
 					      struct drm_atomic_state *state,
-					      struct drm_plane_state *plane_state)
+					      struct drm_plane_state *plane_state,
+					      struct drm_plane_state *prev_plane_state)
 {
 	struct drm_plane *plane = plane_state->plane;
 	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
 	struct dpu_rm_sspp_requirements reqs;
-	struct dpu_plane_state *pstate;
+	struct dpu_plane_state *pstate, *prev_pstate;
 	struct dpu_sw_pipe *pipe;
 	struct dpu_sw_pipe *r_pipe;
 	struct dpu_sw_pipe_cfg *pipe_cfg;
@@ -1085,6 +1168,7 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
 	uint32_t max_linewidth;
 
 	pstate = to_dpu_plane_state(plane_state);
+	prev_pstate = prev_plane_state ? to_dpu_plane_state(prev_plane_state) : NULL;
 	pipe = &pstate->pipe;
 	r_pipe = &pstate->r_pipe;
 	pipe_cfg = &pstate->pipe_cfg;
@@ -1105,19 +1189,27 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
 
 	max_linewidth = dpu_kms->catalog->caps->max_linewidth;
 
-	pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
-	if (!pipe->sspp)
-		return -ENODEV;
-
 	if (drm_rect_width(&r_pipe_cfg->src_rect) == 0) {
-		pipe->multirect_index = DPU_SSPP_RECT_SOLO;
-		pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+		if (!prev_pstate ||
+		    !dpu_plane_try_multirect(pstate, prev_pstate, fmt, max_linewidth)) {
+			pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
+			if (!pipe->sspp)
+				return -ENODEV;
 
-		r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
-		r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+			r_pipe->sspp = NULL;
+
+			pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+			pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+			r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+			r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+		}
 
-		r_pipe->sspp = NULL;
 	} else {
+		pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
+		if (!pipe->sspp)
+			return -ENODEV;
+
 		if (dpu_plane_is_multirect_parallel_capable(pipe, pipe_cfg, fmt, max_linewidth) &&
 		    dpu_plane_is_multirect_parallel_capable(r_pipe, r_pipe_cfg, fmt, max_linewidth) &&
 		    (test_bit(DPU_SSPP_SMART_DMA_V1, &pipe->sspp->cap->features) ||
@@ -1154,6 +1246,7 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
 {
 	unsigned int i;
 	int ret;
+	struct drm_plane_state *prev_plane_state = NULL;
 
 	for (i = 0; i < num_planes; i++) {
 		struct drm_plane_state *plane_state = states[i];
@@ -1163,9 +1256,12 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
 			continue;
 
 		ret = dpu_plane_virtual_assign_resources(crtc, global_state,
-							 state, plane_state);
+							 state, plane_state,
+							 prev_plane_state);
 		if (ret)
 			break;
+
+		prev_plane_state = plane_state;
 	}
 
 	return ret;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 10/12] drm/msm/dpu: create additional virtual planes
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (8 preceding siblings ...)
  2023-09-14  5:07 ` [PATCH v3 09/12] drm/msm/dpu: allow sharing SSPP between planes Dmitry Baryshkov
@ 2023-09-14  5:07 ` Dmitry Baryshkov
  2023-09-14  5:07 ` [PATCH v3 11/12] drm/msm/dpu: allow sharing of blending stages Dmitry Baryshkov
  2023-09-14  5:07 ` [PATCH v3 12/12] drm/msm/dpu: include SSPP allocation state into the dumped state Dmitry Baryshkov
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Since we have enabled sharing of SSPP blocks between two planes, it is
now possible to use twice as much planes as there are hardware SSPP
blocks. Create additional overlay planes.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 5106abe366a3..a6cd1346b298 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -792,6 +792,18 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 			primary_planes[primary_planes_idx++] = plane;
 	}
 
+	if (dpu_use_virtual_planes) {
+		for (i = 0; i < catalog->sspp_count; i++) {
+			plane = dpu_plane_init_virtual(dev, DRM_PLANE_TYPE_OVERLAY,
+						       (1UL << max_crtc_count) - 1);
+			if (IS_ERR(plane)) {
+				DPU_ERROR("dpu_plane_init failed\n");
+				ret = PTR_ERR(plane);
+				return ret;
+			}
+		}
+	}
+
 	max_crtc_count = min(max_crtc_count, primary_planes_idx);
 
 	/* Create one CRTC per encoder */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 11/12] drm/msm/dpu: allow sharing of blending stages
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (9 preceding siblings ...)
  2023-09-14  5:07 ` [PATCH v3 10/12] drm/msm/dpu: create additional virtual planes Dmitry Baryshkov
@ 2023-09-14  5:07 ` Dmitry Baryshkov
  2023-09-14  5:07 ` [PATCH v3 12/12] drm/msm/dpu: include SSPP allocation state into the dumped state Dmitry Baryshkov
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

It is possible to slightly bend the limitations of the HW blender. If
two rectangles are contiguous (like two rectangles of a single plane)
they can be blended using a single LM blending stage, allowing one to
blend more planes via a single LM.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  9 ++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 37 ++++++++++++++++++-----
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index df4c2e503fa5..4b5b2b7ed494 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -456,6 +456,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
 
 	uint32_t lm_idx;
 	bool bg_alpha_enable = false;
+	unsigned int stage_indices[DPU_STAGE_MAX] = {};
 	DECLARE_BITMAP(fetch_active, SSPP_MAX);
 
 	memset(fetch_active, 0, sizeof(fetch_active));
@@ -480,7 +481,9 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
 					   mixer, cstate->num_mixers,
 					   pstate->stage,
 					   format, fb ? fb->modifier : 0,
-					   &pstate->pipe, 0, stage_cfg);
+					   &pstate->pipe,
+					   stage_indices[pstate->stage]++,
+					   stage_cfg);
 
 		if (pstate->r_pipe.sspp) {
 			set_bit(pstate->r_pipe.sspp->idx, fetch_active);
@@ -488,7 +491,9 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
 						   mixer, cstate->num_mixers,
 						   pstate->stage,
 						   format, fb ? fb->modifier : 0,
-						   &pstate->r_pipe, 1, stage_cfg);
+						   &pstate->r_pipe,
+						   stage_indices[pstate->stage]++,
+						   stage_cfg);
 		}
 
 		/* blend config update */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 61afd1cf033d..e7a157feab22 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -809,13 +809,6 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane,
 	if (!new_plane_state->visible)
 		return 0;
 
-	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
-	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
-		DPU_ERROR("> %d plane stages assigned\n",
-			  pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
-		return -EINVAL;
-	}
-
 	fb_rect.x2 = new_plane_state->fb->width;
 	fb_rect.y2 = new_plane_state->fb->height;
 
@@ -952,6 +945,18 @@ static int dpu_plane_try_multirect(struct dpu_plane_state *pstate,
 		prev_pipe->multirect_index = DPU_SSPP_RECT_0;
 		prev_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
 
+		if (pipe_cfg->dst_rect.y1 == prev_pipe_cfg->dst_rect.y1 &&
+		    pipe_cfg->dst_rect.y2 == prev_pipe_cfg->dst_rect.y2 &&
+		    pipe_cfg->dst_rect.x1 == prev_pipe_cfg->dst_rect.x2) {
+			pstate->stage = prev_pstate->stage;
+		} else if (pipe_cfg->dst_rect.y1 == prev_pipe_cfg->dst_rect.y1 &&
+			   pipe_cfg->dst_rect.y2 == prev_pipe_cfg->dst_rect.y2 &&
+			   pipe_cfg->dst_rect.x2 == prev_pipe_cfg->dst_rect.x1) {
+			pstate->stage = prev_pstate->stage;
+			pipe->multirect_index = DPU_SSPP_RECT_0;
+			prev_pipe->multirect_index = DPU_SSPP_RECT_1;
+		}
+
 		return true;
 	}
 
@@ -1054,6 +1059,13 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 	if (!new_plane_state->visible)
 		return 0;
 
+	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
+	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
+		DPU_ERROR("> %d plane stages assigned\n",
+			  pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
+		return -EINVAL;
+	}
+
 	pipe->multirect_index = DPU_SSPP_RECT_SOLO;
 	pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
 	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
@@ -1189,6 +1201,11 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
 
 	max_linewidth = dpu_kms->catalog->caps->max_linewidth;
 
+	if (prev_pstate)
+		pstate->stage = prev_pstate->stage + 1;
+	else
+		pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
+
 	if (drm_rect_width(&r_pipe_cfg->src_rect) == 0) {
 		if (!prev_pstate ||
 		    !dpu_plane_try_multirect(pstate, prev_pstate, fmt, max_linewidth)) {
@@ -1235,6 +1252,12 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
 		}
 	}
 
+	if (pstate->stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
+		DPU_ERROR("> %d plane stages assigned\n",
+			  dpu_kms->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
+		return -EINVAL;
+	}
+
 	return dpu_plane_atomic_check_pipes(plane, state);
 }
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 12/12] drm/msm/dpu: include SSPP allocation state into the dumped state
  2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
                   ` (10 preceding siblings ...)
  2023-09-14  5:07 ` [PATCH v3 11/12] drm/msm/dpu: allow sharing of blending stages Dmitry Baryshkov
@ 2023-09-14  5:07 ` Dmitry Baryshkov
  11 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-09-14  5:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten
  Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

Make dpu_rm_print_state() also output the SSPP allocation state.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 1b0166bc9bee..e7c142bebab1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -773,4 +773,12 @@ void dpu_rm_print_state(struct drm_printer *p,
 		else
 			drm_puts(p, " -,");
 	drm_puts(p, "\n");
+
+	drm_puts(p, "sspp:");
+	for (i = 0; i < ARRAY_SIZE(global_state->sspp_to_crtc_id); i++)
+		if (rm->hw_sspp[i])
+			drm_printf(p, " %d,", global_state->sspp_to_crtc_id[i]);
+		else
+			drm_puts(p, " -,");
+	drm_puts(p, "\n");
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state
  2023-09-14  5:06 ` [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state Dmitry Baryshkov
@ 2024-02-14 18:30   ` Abhinav Kumar
  2024-02-14 18:37   ` Ville Syrjälä
  1 sibling, 0 replies; 22+ messages in thread
From: Abhinav Kumar @ 2024-02-14 18:30 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno



On 9/13/2023 10:06 PM, Dmitry Baryshkov wrote:
> The helper drm_atomic_helper_check_plane_state() runs several checks on
> plane src and dst rectangles, including the check whether required
> scaling fits into the required margins. The msm driver would benefit
> from having a function that does all these checks except the scaling
> one. Split them into a new helper called
> drm_atomic_helper_check_plane_noscale().
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/drm_atomic_helper.c | 110 ++++++++++++++++++++++------
>   include/drm/drm_atomic_helper.h     |   7 ++
>   2 files changed, 96 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 292e38eb6218..2d7dd66181c9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
>   EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
>   
>   /**
> - * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
>    * @plane_state: plane state to check
>    * @crtc_state: CRTC state to check
> - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
>    * @can_position: is it legal to position the plane such that it
>    *                doesn't cover the entire CRTC?  This will generally
>    *                only be false for primary planes.
> @@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
>    * RETURNS:
>    * Zero if update appears valid, error code on failure
>    */
> -int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> -					const struct drm_crtc_state *crtc_state,
> -					int min_scale,
> -					int max_scale,
> -					bool can_position,
> -					bool can_update_disabled)
> +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
> +					  const struct drm_crtc_state *crtc_state,
> +					  bool can_position,
> +					  bool can_update_disabled)
>   {
>   	struct drm_framebuffer *fb = plane_state->fb;
>   	struct drm_rect *src = &plane_state->src;
>   	struct drm_rect *dst = &plane_state->dst;
>   	unsigned int rotation = plane_state->rotation;
>   	struct drm_rect clip = {};
> -	int hscale, vscale;
>   
>   	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
>   
> @@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>   
>   	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
>   

Do we need to do the rotation even for noscale before validation?

> -	/* Check scaling */
> -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> -	if (hscale < 0 || vscale < 0) {
> -		drm_dbg_kms(plane_state->plane->dev,
> -			    "Invalid scaling of plane\n");
> -		drm_rect_debug_print("src: ", &plane_state->src, true);
> -		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> -		return -ERANGE;
> -	}
> -
>   	if (crtc_state->enable)
>   		drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
>   
> @@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>   
>   	return 0;
>   }
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
> +
> +/**
> + * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
> + * @plane_state: plane state to check
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + *
> + * Checks that a desired plane scale fits into the min_scale..max_scale
> + * boundaries.
> + * Drivers that provide their own plane handling rather than helper-provided
> + * implementations may still wish to call this function to avoid duplication of
> + * error checking code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> +					int min_scale,
> +					int max_scale)
> +{
> +	struct drm_framebuffer *fb = plane_state->fb;
> +	struct drm_rect src;
> +	struct drm_rect dst;
> +	int hscale, vscale;
> +
> +	if (!plane_state->visible)
> +		return 0;
> +
> +	src = drm_plane_state_src(plane_state);
> +	dst = drm_plane_state_dest(plane_state);
> +
> +	drm_rect_rotate(&src, fb->width << 16, fb->height << 16, plane_state->rotation);
> +

Does this need to be accompanied by a drm_rect_rotate_inv()?

> +	hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> +	vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale);
> +	if (hscale < 0 || vscale < 0) {
> +		drm_dbg_kms(plane_state->plane->dev,
> +			    "Invalid scaling of plane\n");
> +		drm_rect_debug_print("src: ", &plane_state->src, true);
> +		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> +		return -ERANGE;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_scale);
> +
> +/**
> + * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * @plane_state: plane state to check
> + * @crtc_state: CRTC state to check
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + * @can_position: is it legal to position the plane such that it
> + *                doesn't cover the entire CRTC?  This will generally
> + *                only be false for primary planes.
> + * @can_update_disabled: can the plane be updated while the CRTC
> + *                       is disabled?
> + *
> + * Checks that a desired plane update is valid, and updates various
> + * bits of derived state (clipped coordinates etc.). Drivers that provide
> + * their own plane handling rather than helper-provided implementations may
> + * still wish to call this function to avoid duplication of error checking
> + * code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> +					const struct drm_crtc_state *crtc_state,
> +					int min_scale,
> +					int max_scale,
> +					bool can_position,
> +					bool can_update_disabled)
> +{
> +	int ret;
> +
> +	ret = drm_atomic_helper_check_plane_noscale(plane_state, crtc_state, can_position, can_update_disabled);
> +	if (ret < 0)
> +		return ret;
> +
> +	return drm_atomic_helper_check_plane_scale(plane_state, min_scale, max_scale);
> +}
>   EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
>   
>   /**
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 536a0b0091c3..32ac55aea94e 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -52,6 +52,13 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev,
>   int
>   drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
>   					 struct drm_connector_state *conn_state);
> +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
> +					  const struct drm_crtc_state *crtc_state,
> +					  bool can_position,
> +					  bool can_update_disabled);
> +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> +					int min_scale,
> +					int max_scale);
>   int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>   					const struct drm_crtc_state *crtc_state,
>   					int min_scale,

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state
  2023-09-14  5:06 ` [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state Dmitry Baryshkov
  2024-02-14 18:30   ` Abhinav Kumar
@ 2024-02-14 18:37   ` Ville Syrjälä
  2024-02-14 18:47     ` Ville Syrjälä
  1 sibling, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2024-02-14 18:37 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, freedreno,
	linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> The helper drm_atomic_helper_check_plane_state() runs several checks on
> plane src and dst rectangles, including the check whether required
> scaling fits into the required margins. The msm driver would benefit
> from having a function that does all these checks except the scaling
> one. Split them into a new helper called
> drm_atomic_helper_check_plane_noscale().

What's the point in eliminating a nop scaling check?

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 110 ++++++++++++++++++++++------
>  include/drm/drm_atomic_helper.h     |   7 ++
>  2 files changed, 96 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 292e38eb6218..2d7dd66181c9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
>  EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
>  
>  /**
> - * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
>   * @plane_state: plane state to check
>   * @crtc_state: CRTC state to check
> - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
>   * @can_position: is it legal to position the plane such that it
>   *                doesn't cover the entire CRTC?  This will generally
>   *                only be false for primary planes.
> @@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
>   * RETURNS:
>   * Zero if update appears valid, error code on failure
>   */
> -int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> -					const struct drm_crtc_state *crtc_state,
> -					int min_scale,
> -					int max_scale,
> -					bool can_position,
> -					bool can_update_disabled)
> +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
> +					  const struct drm_crtc_state *crtc_state,
> +					  bool can_position,
> +					  bool can_update_disabled)
>  {
>  	struct drm_framebuffer *fb = plane_state->fb;
>  	struct drm_rect *src = &plane_state->src;
>  	struct drm_rect *dst = &plane_state->dst;
>  	unsigned int rotation = plane_state->rotation;
>  	struct drm_rect clip = {};
> -	int hscale, vscale;
>  
>  	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
>  
> @@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  
>  	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
>  
> -	/* Check scaling */
> -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> -	if (hscale < 0 || vscale < 0) {
> -		drm_dbg_kms(plane_state->plane->dev,
> -			    "Invalid scaling of plane\n");
> -		drm_rect_debug_print("src: ", &plane_state->src, true);
> -		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> -		return -ERANGE;
> -	}
> -
>  	if (crtc_state->enable)
>  		drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
>  
> @@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
> +
> +/**
> + * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
> + * @plane_state: plane state to check
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + *
> + * Checks that a desired plane scale fits into the min_scale..max_scale
> + * boundaries.
> + * Drivers that provide their own plane handling rather than helper-provided
> + * implementations may still wish to call this function to avoid duplication of
> + * error checking code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> +					int min_scale,
> +					int max_scale)
> +{
> +	struct drm_framebuffer *fb = plane_state->fb;
> +	struct drm_rect src;
> +	struct drm_rect dst;
> +	int hscale, vscale;
> +
> +	if (!plane_state->visible)
> +		return 0;
> +
> +	src = drm_plane_state_src(plane_state);
> +	dst = drm_plane_state_dest(plane_state);
> +
> +	drm_rect_rotate(&src, fb->width << 16, fb->height << 16, plane_state->rotation);
> +
> +	hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> +	vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale);
> +	if (hscale < 0 || vscale < 0) {
> +		drm_dbg_kms(plane_state->plane->dev,
> +			    "Invalid scaling of plane\n");
> +		drm_rect_debug_print("src: ", &plane_state->src, true);
> +		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> +		return -ERANGE;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_plane_scale);
> +
> +/**
> + * drm_atomic_helper_check_plane_state() - Check plane state for validity
> + * @plane_state: plane state to check
> + * @crtc_state: CRTC state to check
> + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> + * @can_position: is it legal to position the plane such that it
> + *                doesn't cover the entire CRTC?  This will generally
> + *                only be false for primary planes.
> + * @can_update_disabled: can the plane be updated while the CRTC
> + *                       is disabled?
> + *
> + * Checks that a desired plane update is valid, and updates various
> + * bits of derived state (clipped coordinates etc.). Drivers that provide
> + * their own plane handling rather than helper-provided implementations may
> + * still wish to call this function to avoid duplication of error checking
> + * code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> +					const struct drm_crtc_state *crtc_state,
> +					int min_scale,
> +					int max_scale,
> +					bool can_position,
> +					bool can_update_disabled)
> +{
> +	int ret;
> +
> +	ret = drm_atomic_helper_check_plane_noscale(plane_state, crtc_state, can_position, can_update_disabled);
> +	if (ret < 0)
> +		return ret;
> +
> +	return drm_atomic_helper_check_plane_scale(plane_state, min_scale, max_scale);
> +}
>  EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
>  
>  /**
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 536a0b0091c3..32ac55aea94e 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -52,6 +52,13 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev,
>  int
>  drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
>  					 struct drm_connector_state *conn_state);
> +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
> +					  const struct drm_crtc_state *crtc_state,
> +					  bool can_position,
> +					  bool can_update_disabled);
> +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> +					int min_scale,
> +					int max_scale);
>  int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  					const struct drm_crtc_state *crtc_state,
>  					int min_scale,
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 02/12] drm/msm/dpu: add current resource allocation to dumped state
  2023-09-14  5:06 ` [PATCH v3 02/12] drm/msm/dpu: add current resource allocation to dumped state Dmitry Baryshkov
@ 2024-02-14 18:40   ` Abhinav Kumar
  2024-02-14 19:18     ` Dmitry Baryshkov
  0 siblings, 1 reply; 22+ messages in thread
From: Abhinav Kumar @ 2024-02-14 18:40 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno



On 9/13/2023 10:06 PM, Dmitry Baryshkov wrote:
> Provide atomic_print_state callback to the DPU's private object. This
> way the debugfs/dri/0/state will also include RM's internal state.
> 

I like this idea !

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  4 +++
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  2 ++
>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c  | 48 +++++++++++++++++++++++++
>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h  |  8 +++++
>   4 files changed, 62 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index ee84160592ce..172b64dc60e6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -362,6 +362,7 @@ static void dpu_kms_global_destroy_state(struct drm_private_obj *obj,
>   static const struct drm_private_state_funcs dpu_kms_global_state_funcs = {
>   	.atomic_duplicate_state = dpu_kms_global_duplicate_state,
>   	.atomic_destroy_state = dpu_kms_global_destroy_state,
> +	.atomic_print_state = dpu_rm_print_state,
>   };
>   
>   static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
> @@ -375,6 +376,9 @@ static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
>   	drm_atomic_private_obj_init(dpu_kms->dev, &dpu_kms->global_state,
>   				    &state->base,
>   				    &dpu_kms_global_state_funcs);
> +
> +	state->rm = &dpu_kms->rm;
> +
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index ed549f0f7c65..dd2be279b366 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -130,6 +130,8 @@ struct vsync_info {
>   struct dpu_global_state {
>   	struct drm_private_state base;
>   
> +	struct dpu_rm *rm;
> +
>   	uint32_t pingpong_to_enc_id[PINGPONG_MAX - PINGPONG_0];
>   	uint32_t mixer_to_enc_id[LM_MAX - LM_0];
>   	uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index f9215643c71a..5e3442fb8678 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -652,3 +652,51 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
>   
>   	return num_blks;
>   }
> +
> +void dpu_rm_print_state(struct drm_printer *p,
> +			const struct drm_private_state *state)
> +{
> +	const struct dpu_global_state *global_state = to_dpu_global_state(state);
> +	const struct dpu_rm *rm = global_state->rm;
> +	int i;
> +
> +	drm_puts(p, "pingpong:");
> +	for (i = 0; i < ARRAY_SIZE(global_state->pingpong_to_enc_id); i++)
> +		if (rm->pingpong_blks[i])
> +			drm_printf(p, " %d,", global_state->pingpong_to_enc_id[i]);
> +		else
> +			drm_puts(p, " -,");
> +	drm_puts(p, "\n");
> +
> +	drm_puts(p, "mixer:");
> +	for (i = 0; i < ARRAY_SIZE(global_state->mixer_to_enc_id); i++)
> +		if (rm->mixer_blks[i])
> +			drm_printf(p, " %d,", global_state->mixer_to_enc_id[i]);
> +		else
> +			drm_puts(p, " -,");
> +	drm_puts(p, "\n");
> +
> +	drm_puts(p, "ctl:");
> +	for (i = 0; i < ARRAY_SIZE(global_state->ctl_to_enc_id); i++)
> +		if (rm->ctl_blks[i])
> +			drm_printf(p, " %d,", global_state->ctl_to_enc_id[i]);
> +		else
> +			drm_puts(p, " -,");
> +	drm_puts(p, "\n");
> +
> +	drm_puts(p, "dspp:");
> +	for (i = 0; i < ARRAY_SIZE(global_state->dspp_to_enc_id); i++)
> +		if (rm->dspp_blks[i])
> +			drm_printf(p, " %d,", global_state->dspp_to_enc_id[i]);
> +		else
> +			drm_puts(p, " -,");
> +	drm_puts(p, "\n");
> +
> +	drm_puts(p, "dsc:");
> +	for (i = 0; i < ARRAY_SIZE(global_state->dsc_to_enc_id); i++)
> +		if (rm->dsc_blks[i])
> +			drm_printf(p, " %d,", global_state->dsc_to_enc_id[i]);
> +		else
> +			drm_puts(p, " -,");
> +	drm_puts(p, "\n");
> +}

You also need to include cdm_to_enc_id now. But otherwise LGTM.

If you have run this before, do you have a sample output to share?


> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index 2b551566cbf4..913baca81a42 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -92,6 +92,14 @@ 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);
>   
> +/**
> + * dpu_rm_print_state - output the RM private state
> + * @p: DRM printer
> + * @state: private object state
> + */
> +void dpu_rm_print_state(struct drm_printer *p,
> +			const struct drm_private_state *state);
> +
>   /**
>    * dpu_rm_get_intf - Return a struct dpu_hw_intf instance given it's index.
>    * @rm: DPU Resource Manager handle

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state
  2024-02-14 18:37   ` Ville Syrjälä
@ 2024-02-14 18:47     ` Ville Syrjälä
  2024-02-14 19:17       ` Dmitry Baryshkov
  0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2024-02-14 18:47 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, freedreno,
	linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

On Wed, Feb 14, 2024 at 08:37:02PM +0200, Ville Syrjälä wrote:
> On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> > The helper drm_atomic_helper_check_plane_state() runs several checks on
> > plane src and dst rectangles, including the check whether required
> > scaling fits into the required margins. The msm driver would benefit
> > from having a function that does all these checks except the scaling
> > one. Split them into a new helper called
> > drm_atomic_helper_check_plane_noscale().
> 
> What's the point in eliminating a nop scaling check?

Actually, what are you even doing in there? Are you saying that
the hardware has absolutely no limits on how much it can scale
in either direction?

> 
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 110 ++++++++++++++++++++++------
> >  include/drm/drm_atomic_helper.h     |   7 ++
> >  2 files changed, 96 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 292e38eb6218..2d7dd66181c9 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
> >  EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
> >  
> >  /**
> > - * drm_atomic_helper_check_plane_state() - Check plane state for validity
> > + * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
> >   * @plane_state: plane state to check
> >   * @crtc_state: CRTC state to check
> > - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> >   * @can_position: is it legal to position the plane such that it
> >   *                doesn't cover the entire CRTC?  This will generally
> >   *                only be false for primary planes.
> > @@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
> >   * RETURNS:
> >   * Zero if update appears valid, error code on failure
> >   */
> > -int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > -					const struct drm_crtc_state *crtc_state,
> > -					int min_scale,
> > -					int max_scale,
> > -					bool can_position,
> > -					bool can_update_disabled)
> > +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
> > +					  const struct drm_crtc_state *crtc_state,
> > +					  bool can_position,
> > +					  bool can_update_disabled)
> >  {
> >  	struct drm_framebuffer *fb = plane_state->fb;
> >  	struct drm_rect *src = &plane_state->src;
> >  	struct drm_rect *dst = &plane_state->dst;
> >  	unsigned int rotation = plane_state->rotation;
> >  	struct drm_rect clip = {};
> > -	int hscale, vscale;
> >  
> >  	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> >  
> > @@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> >  
> >  	drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> >  
> > -	/* Check scaling */
> > -	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> > -	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> > -	if (hscale < 0 || vscale < 0) {
> > -		drm_dbg_kms(plane_state->plane->dev,
> > -			    "Invalid scaling of plane\n");
> > -		drm_rect_debug_print("src: ", &plane_state->src, true);
> > -		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> > -		return -ERANGE;
> > -	}
> > -
> >  	if (crtc_state->enable)
> >  		drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
> >  
> > @@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> >  
> >  	return 0;
> >  }
> > +EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
> > +
> > +/**
> > + * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
> > + * @plane_state: plane state to check
> > + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > + *
> > + * Checks that a desired plane scale fits into the min_scale..max_scale
> > + * boundaries.
> > + * Drivers that provide their own plane handling rather than helper-provided
> > + * implementations may still wish to call this function to avoid duplication of
> > + * error checking code.
> > + *
> > + * RETURNS:
> > + * Zero if update appears valid, error code on failure
> > + */
> > +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> > +					int min_scale,
> > +					int max_scale)
> > +{
> > +	struct drm_framebuffer *fb = plane_state->fb;
> > +	struct drm_rect src;
> > +	struct drm_rect dst;
> > +	int hscale, vscale;
> > +
> > +	if (!plane_state->visible)
> > +		return 0;
> > +
> > +	src = drm_plane_state_src(plane_state);
> > +	dst = drm_plane_state_dest(plane_state);
> > +
> > +	drm_rect_rotate(&src, fb->width << 16, fb->height << 16, plane_state->rotation);
> > +
> > +	hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> > +	vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale);
> > +	if (hscale < 0 || vscale < 0) {
> > +		drm_dbg_kms(plane_state->plane->dev,
> > +			    "Invalid scaling of plane\n");
> > +		drm_rect_debug_print("src: ", &plane_state->src, true);
> > +		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> > +		return -ERANGE;
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_check_plane_scale);
> > +
> > +/**
> > + * drm_atomic_helper_check_plane_state() - Check plane state for validity
> > + * @plane_state: plane state to check
> > + * @crtc_state: CRTC state to check
> > + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > + * @can_position: is it legal to position the plane such that it
> > + *                doesn't cover the entire CRTC?  This will generally
> > + *                only be false for primary planes.
> > + * @can_update_disabled: can the plane be updated while the CRTC
> > + *                       is disabled?
> > + *
> > + * Checks that a desired plane update is valid, and updates various
> > + * bits of derived state (clipped coordinates etc.). Drivers that provide
> > + * their own plane handling rather than helper-provided implementations may
> > + * still wish to call this function to avoid duplication of error checking
> > + * code.
> > + *
> > + * RETURNS:
> > + * Zero if update appears valid, error code on failure
> > + */
> > +int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > +					const struct drm_crtc_state *crtc_state,
> > +					int min_scale,
> > +					int max_scale,
> > +					bool can_position,
> > +					bool can_update_disabled)
> > +{
> > +	int ret;
> > +
> > +	ret = drm_atomic_helper_check_plane_noscale(plane_state, crtc_state, can_position, can_update_disabled);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return drm_atomic_helper_check_plane_scale(plane_state, min_scale, max_scale);
> > +}
> >  EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
> >  
> >  /**
> > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> > index 536a0b0091c3..32ac55aea94e 100644
> > --- a/include/drm/drm_atomic_helper.h
> > +++ b/include/drm/drm_atomic_helper.h
> > @@ -52,6 +52,13 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev,
> >  int
> >  drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
> >  					 struct drm_connector_state *conn_state);
> > +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
> > +					  const struct drm_crtc_state *crtc_state,
> > +					  bool can_position,
> > +					  bool can_update_disabled);
> > +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> > +					int min_scale,
> > +					int max_scale);
> >  int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> >  					const struct drm_crtc_state *crtc_state,
> >  					int min_scale,
> > -- 
> > 2.39.2
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 03/12] drm/msm/dpu: take plane rotation into account for wide planes
  2023-09-14  5:06 ` [PATCH v3 03/12] drm/msm/dpu: take plane rotation into account for wide planes Dmitry Baryshkov
@ 2024-02-14 19:07   ` Abhinav Kumar
  0 siblings, 0 replies; 22+ messages in thread
From: Abhinav Kumar @ 2024-02-14 19:07 UTC (permalink / raw)
  To: Dmitry Baryshkov, Rob Clark, Sean Paul, Marijn Suijten
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, Bjorn Andersson,
	linux-arm-msm, dri-devel, freedreno



On 9/13/2023 10:06 PM, Dmitry Baryshkov wrote:
> Take into account the plane rotation and flipping when calculating src
> positions for the wide plane parts.
> 
> This is not an issue yet, because rotation is only supported for the
> UBWC planes and wide UBWC planes are rejected anyway because in parallel
> multirect case only the half of the usual width is supported for tiled
> formats. However it's better to fix this now rather than stumbling upon
> it later.
> 
> Fixes: 80e8ae3b38ab ("drm/msm/dpu: add support for wide planes")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++++++++++++++---------
>   1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index c2aaaded07ed..67f9c2a62a17 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -827,16 +827,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   		return -EINVAL;
>   	}
>   
> -	pipe_cfg->src_rect = new_plane_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 = new_plane_state->dst;
> -

Why were these lines moved down?

So this change is doing two things:

1) Using drm_rect_fp_to_int() instead of the hand-rolled right shifting
2) Considering rotation for wide plane cases

Do we need to break this up into 2?

>   	fb_rect.x2 = new_plane_state->fb->width;
>   	fb_rect.y2 = new_plane_state->fb->height;
>   
> @@ -852,6 +842,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   
>   	max_linewidth = pdpu->catalog->caps->max_linewidth;
>   
> +	/* state->src is 16.16, src_rect is not */
> +	drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src);
> +
> +	pipe_cfg->dst_rect = new_plane_state->dst;
> +
> +	drm_rect_rotate(&pipe_cfg->src_rect,
> +			new_plane_state->fb->width, new_plane_state->fb->height,
> +			new_plane_state->rotation);
> +
>   	if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
>   		/*
>   		 * In parallel multirect case only the half of the usual width
> @@ -899,6 +898,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   		r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
>   	}
>   
> +	drm_rect_rotate_inv(&pipe_cfg->src_rect,
> +			    new_plane_state->fb->width, new_plane_state->fb->height,
> +			    new_plane_state->rotation);
> +	if (r_pipe->sspp)
> +		drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
> +				    new_plane_state->fb->width, new_plane_state->fb->height,
> +				    new_plane_state->rotation);
> +

iiuc, so we do

1) rotate() on full plane
2) rotate_inv() on left half-plane
3) rotate_inv() on right half-plane

If so, looks right to me.


>   	ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
>   	if (ret)
>   		return ret;

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state
  2024-02-14 18:47     ` Ville Syrjälä
@ 2024-02-14 19:17       ` Dmitry Baryshkov
  2024-02-14 19:39         ` Ville Syrjälä
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Baryshkov @ 2024-02-14 19:17 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, freedreno,
	linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

On Wed, 14 Feb 2024 at 20:47, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Wed, Feb 14, 2024 at 08:37:02PM +0200, Ville Syrjälä wrote:
> > On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> > > The helper drm_atomic_helper_check_plane_state() runs several checks on
> > > plane src and dst rectangles, including the check whether required
> > > scaling fits into the required margins. The msm driver would benefit
> > > from having a function that does all these checks except the scaling
> > > one. Split them into a new helper called
> > > drm_atomic_helper_check_plane_noscale().
> >
> > What's the point in eliminating a nop scaling check?
>
> Actually, what are you even doing in there? Are you saying that
> the hardware has absolutely no limits on how much it can scale
> in either direction?

No, I'm just saying that the scaling ability depends on the rotation
and other plane properties. So I had to separate the basic plane
checks and the scaling check.
Basic (noscale) plane check source and destination rectangles, etc.
After that the driver identifies possible hardware pipe usage and
after that it can perform a scaling check.

>
> >
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_helper.c | 110 ++++++++++++++++++++++------
> > >  include/drm/drm_atomic_helper.h     |   7 ++
> > >  2 files changed, 96 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 292e38eb6218..2d7dd66181c9 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
> > >  EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
> > >
> > >  /**
> > > - * drm_atomic_helper_check_plane_state() - Check plane state for validity
> > > + * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
> > >   * @plane_state: plane state to check
> > >   * @crtc_state: CRTC state to check
> > > - * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > > - * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > >   * @can_position: is it legal to position the plane such that it
> > >   *                doesn't cover the entire CRTC?  This will generally
> > >   *                only be false for primary planes.
> > > @@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
> > >   * RETURNS:
> > >   * Zero if update appears valid, error code on failure
> > >   */
> > > -int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > > -                                   const struct drm_crtc_state *crtc_state,
> > > -                                   int min_scale,
> > > -                                   int max_scale,
> > > -                                   bool can_position,
> > > -                                   bool can_update_disabled)
> > > +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
> > > +                                     const struct drm_crtc_state *crtc_state,
> > > +                                     bool can_position,
> > > +                                     bool can_update_disabled)
> > >  {
> > >     struct drm_framebuffer *fb = plane_state->fb;
> > >     struct drm_rect *src = &plane_state->src;
> > >     struct drm_rect *dst = &plane_state->dst;
> > >     unsigned int rotation = plane_state->rotation;
> > >     struct drm_rect clip = {};
> > > -   int hscale, vscale;
> > >
> > >     WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
> > >
> > > @@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > >
> > >     drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> > >
> > > -   /* Check scaling */
> > > -   hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> > > -   vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> > > -   if (hscale < 0 || vscale < 0) {
> > > -           drm_dbg_kms(plane_state->plane->dev,
> > > -                       "Invalid scaling of plane\n");
> > > -           drm_rect_debug_print("src: ", &plane_state->src, true);
> > > -           drm_rect_debug_print("dst: ", &plane_state->dst, false);
> > > -           return -ERANGE;
> > > -   }
> > > -
> > >     if (crtc_state->enable)
> > >             drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
> > >
> > > @@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > >
> > >     return 0;
> > >  }
> > > +EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
> > > +
> > > +/**
> > > + * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
> > > + * @plane_state: plane state to check
> > > + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > > + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > > + *
> > > + * Checks that a desired plane scale fits into the min_scale..max_scale
> > > + * boundaries.
> > > + * Drivers that provide their own plane handling rather than helper-provided
> > > + * implementations may still wish to call this function to avoid duplication of
> > > + * error checking code.
> > > + *
> > > + * RETURNS:
> > > + * Zero if update appears valid, error code on failure
> > > + */
> > > +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> > > +                                   int min_scale,
> > > +                                   int max_scale)
> > > +{
> > > +   struct drm_framebuffer *fb = plane_state->fb;
> > > +   struct drm_rect src;
> > > +   struct drm_rect dst;
> > > +   int hscale, vscale;
> > > +
> > > +   if (!plane_state->visible)
> > > +           return 0;
> > > +
> > > +   src = drm_plane_state_src(plane_state);
> > > +   dst = drm_plane_state_dest(plane_state);
> > > +
> > > +   drm_rect_rotate(&src, fb->width << 16, fb->height << 16, plane_state->rotation);
> > > +
> > > +   hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> > > +   vscale = drm_rect_calc_vscale(&src, &dst, min_scale, max_scale);
> > > +   if (hscale < 0 || vscale < 0) {
> > > +           drm_dbg_kms(plane_state->plane->dev,
> > > +                       "Invalid scaling of plane\n");
> > > +           drm_rect_debug_print("src: ", &plane_state->src, true);
> > > +           drm_rect_debug_print("dst: ", &plane_state->dst, false);
> > > +           return -ERANGE;
> > > +   }
> > > +
> > > +   return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_atomic_helper_check_plane_scale);
> > > +
> > > +/**
> > > + * drm_atomic_helper_check_plane_state() - Check plane state for validity
> > > + * @plane_state: plane state to check
> > > + * @crtc_state: CRTC state to check
> > > + * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > > + * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > > + * @can_position: is it legal to position the plane such that it
> > > + *                doesn't cover the entire CRTC?  This will generally
> > > + *                only be false for primary planes.
> > > + * @can_update_disabled: can the plane be updated while the CRTC
> > > + *                       is disabled?
> > > + *
> > > + * Checks that a desired plane update is valid, and updates various
> > > + * bits of derived state (clipped coordinates etc.). Drivers that provide
> > > + * their own plane handling rather than helper-provided implementations may
> > > + * still wish to call this function to avoid duplication of error checking
> > > + * code.
> > > + *
> > > + * RETURNS:
> > > + * Zero if update appears valid, error code on failure
> > > + */
> > > +int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > > +                                   const struct drm_crtc_state *crtc_state,
> > > +                                   int min_scale,
> > > +                                   int max_scale,
> > > +                                   bool can_position,
> > > +                                   bool can_update_disabled)
> > > +{
> > > +   int ret;
> > > +
> > > +   ret = drm_atomic_helper_check_plane_noscale(plane_state, crtc_state, can_position, can_update_disabled);
> > > +   if (ret < 0)
> > > +           return ret;
> > > +
> > > +   return drm_atomic_helper_check_plane_scale(plane_state, min_scale, max_scale);
> > > +}
> > >  EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
> > >
> > >  /**
> > > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> > > index 536a0b0091c3..32ac55aea94e 100644
> > > --- a/include/drm/drm_atomic_helper.h
> > > +++ b/include/drm/drm_atomic_helper.h
> > > @@ -52,6 +52,13 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev,
> > >  int
> > >  drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
> > >                                      struct drm_connector_state *conn_state);
> > > +int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
> > > +                                     const struct drm_crtc_state *crtc_state,
> > > +                                     bool can_position,
> > > +                                     bool can_update_disabled);
> > > +int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
> > > +                                   int min_scale,
> > > +                                   int max_scale);
> > >  int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> > >                                     const struct drm_crtc_state *crtc_state,
> > >                                     int min_scale,
> > > --
> > > 2.39.2
> >
> > --
> > Ville Syrjälä
> > Intel
>
> --
> Ville Syrjälä
> Intel



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 02/12] drm/msm/dpu: add current resource allocation to dumped state
  2024-02-14 18:40   ` Abhinav Kumar
@ 2024-02-14 19:18     ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2024-02-14 19:18 UTC (permalink / raw)
  To: Abhinav Kumar
  Cc: Rob Clark, Sean Paul, Marijn Suijten, Stephen Boyd, David Airlie,
	Daniel Vetter, Bjorn Andersson, linux-arm-msm, dri-devel,
	freedreno

On Wed, 14 Feb 2024 at 20:41, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 9/13/2023 10:06 PM, Dmitry Baryshkov wrote:
> > Provide atomic_print_state callback to the DPU's private object. This
> > way the debugfs/dri/0/state will also include RM's internal state.
> >
>
> I like this idea !
>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  4 +++
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  2 ++
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c  | 48 +++++++++++++++++++++++++
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h  |  8 +++++
> >   4 files changed, 62 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index ee84160592ce..172b64dc60e6 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -362,6 +362,7 @@ static void dpu_kms_global_destroy_state(struct drm_private_obj *obj,
> >   static const struct drm_private_state_funcs dpu_kms_global_state_funcs = {
> >       .atomic_duplicate_state = dpu_kms_global_duplicate_state,
> >       .atomic_destroy_state = dpu_kms_global_destroy_state,
> > +     .atomic_print_state = dpu_rm_print_state,
> >   };
> >
> >   static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
> > @@ -375,6 +376,9 @@ static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
> >       drm_atomic_private_obj_init(dpu_kms->dev, &dpu_kms->global_state,
> >                                   &state->base,
> >                                   &dpu_kms_global_state_funcs);
> > +
> > +     state->rm = &dpu_kms->rm;
> > +
> >       return 0;
> >   }
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > index ed549f0f7c65..dd2be279b366 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > @@ -130,6 +130,8 @@ struct vsync_info {
> >   struct dpu_global_state {
> >       struct drm_private_state base;
> >
> > +     struct dpu_rm *rm;
> > +
> >       uint32_t pingpong_to_enc_id[PINGPONG_MAX - PINGPONG_0];
> >       uint32_t mixer_to_enc_id[LM_MAX - LM_0];
> >       uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > index f9215643c71a..5e3442fb8678 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> > @@ -652,3 +652,51 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
> >
> >       return num_blks;
> >   }
> > +
> > +void dpu_rm_print_state(struct drm_printer *p,
> > +                     const struct drm_private_state *state)
> > +{
> > +     const struct dpu_global_state *global_state = to_dpu_global_state(state);
> > +     const struct dpu_rm *rm = global_state->rm;
> > +     int i;
> > +
> > +     drm_puts(p, "pingpong:");
> > +     for (i = 0; i < ARRAY_SIZE(global_state->pingpong_to_enc_id); i++)
> > +             if (rm->pingpong_blks[i])
> > +                     drm_printf(p, " %d,", global_state->pingpong_to_enc_id[i]);
> > +             else
> > +                     drm_puts(p, " -,");
> > +     drm_puts(p, "\n");
> > +
> > +     drm_puts(p, "mixer:");
> > +     for (i = 0; i < ARRAY_SIZE(global_state->mixer_to_enc_id); i++)
> > +             if (rm->mixer_blks[i])
> > +                     drm_printf(p, " %d,", global_state->mixer_to_enc_id[i]);
> > +             else
> > +                     drm_puts(p, " -,");
> > +     drm_puts(p, "\n");
> > +
> > +     drm_puts(p, "ctl:");
> > +     for (i = 0; i < ARRAY_SIZE(global_state->ctl_to_enc_id); i++)
> > +             if (rm->ctl_blks[i])
> > +                     drm_printf(p, " %d,", global_state->ctl_to_enc_id[i]);
> > +             else
> > +                     drm_puts(p, " -,");
> > +     drm_puts(p, "\n");
> > +
> > +     drm_puts(p, "dspp:");
> > +     for (i = 0; i < ARRAY_SIZE(global_state->dspp_to_enc_id); i++)
> > +             if (rm->dspp_blks[i])
> > +                     drm_printf(p, " %d,", global_state->dspp_to_enc_id[i]);
> > +             else
> > +                     drm_puts(p, " -,");
> > +     drm_puts(p, "\n");
> > +
> > +     drm_puts(p, "dsc:");
> > +     for (i = 0; i < ARRAY_SIZE(global_state->dsc_to_enc_id); i++)
> > +             if (rm->dsc_blks[i])
> > +                     drm_printf(p, " %d,", global_state->dsc_to_enc_id[i]);
> > +             else
> > +                     drm_puts(p, " -,");
> > +     drm_puts(p, "\n");
> > +}
>
> You also need to include cdm_to_enc_id now. But otherwise LGTM.
>
> If you have run this before, do you have a sample output to share?

No, I don't have a dump at hand. But I can post this patch separately,
including the CDM change.

>
>
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > index 2b551566cbf4..913baca81a42 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > @@ -92,6 +92,14 @@ 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);
> >
> > +/**
> > + * dpu_rm_print_state - output the RM private state
> > + * @p: DRM printer
> > + * @state: private object state
> > + */
> > +void dpu_rm_print_state(struct drm_printer *p,
> > +                     const struct drm_private_state *state);
> > +
> >   /**
> >    * dpu_rm_get_intf - Return a struct dpu_hw_intf instance given it's index.
> >    * @rm: DPU Resource Manager handle



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state
  2024-02-14 19:17       ` Dmitry Baryshkov
@ 2024-02-14 19:39         ` Ville Syrjälä
  2024-02-14 19:43           ` Dmitry Baryshkov
  0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2024-02-14 19:39 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, freedreno,
	linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

On Wed, Feb 14, 2024 at 09:17:06PM +0200, Dmitry Baryshkov wrote:
> On Wed, 14 Feb 2024 at 20:47, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> >
> > On Wed, Feb 14, 2024 at 08:37:02PM +0200, Ville Syrjälä wrote:
> > > On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> > > > The helper drm_atomic_helper_check_plane_state() runs several checks on
> > > > plane src and dst rectangles, including the check whether required
> > > > scaling fits into the required margins. The msm driver would benefit
> > > > from having a function that does all these checks except the scaling
> > > > one. Split them into a new helper called
> > > > drm_atomic_helper_check_plane_noscale().
> > >
> > > What's the point in eliminating a nop scaling check?
> >
> > Actually, what are you even doing in there? Are you saying that
> > the hardware has absolutely no limits on how much it can scale
> > in either direction?
> 
> No, I'm just saying that the scaling ability depends on the rotation
> and other plane properties. So I had to separate the basic plane
> checks and the scaling check.
> Basic (noscale) plane check source and destination rectangles, etc.
> After that the driver identifies possible hardware pipe usage and
> after that it can perform a scaling check.

Hmm. We have sport of similar situation in i915 where we pick a scaler
much later and so don't know the exact scaling limits at the time when
we do this check. But we opted to pass the lower/upper bounds of the
scaling limits instead. That will guarantee that at least completely
illegal values are rejected as early as possible, and so we don't have
to worry about running into them later on.

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state
  2024-02-14 19:39         ` Ville Syrjälä
@ 2024-02-14 19:43           ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2024-02-14 19:43 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Rob Clark, Sean Paul, Abhinav Kumar, Marijn Suijten, freedreno,
	linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd

On Wed, 14 Feb 2024 at 21:39, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> On Wed, Feb 14, 2024 at 09:17:06PM +0200, Dmitry Baryshkov wrote:
> > On Wed, 14 Feb 2024 at 20:47, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > On Wed, Feb 14, 2024 at 08:37:02PM +0200, Ville Syrjälä wrote:
> > > > On Thu, Sep 14, 2023 at 08:06:55AM +0300, Dmitry Baryshkov wrote:
> > > > > The helper drm_atomic_helper_check_plane_state() runs several checks on
> > > > > plane src and dst rectangles, including the check whether required
> > > > > scaling fits into the required margins. The msm driver would benefit
> > > > > from having a function that does all these checks except the scaling
> > > > > one. Split them into a new helper called
> > > > > drm_atomic_helper_check_plane_noscale().
> > > >
> > > > What's the point in eliminating a nop scaling check?
> > >
> > > Actually, what are you even doing in there? Are you saying that
> > > the hardware has absolutely no limits on how much it can scale
> > > in either direction?
> >
> > No, I'm just saying that the scaling ability depends on the rotation
> > and other plane properties. So I had to separate the basic plane
> > checks and the scaling check.
> > Basic (noscale) plane check source and destination rectangles, etc.
> > After that the driver identifies possible hardware pipe usage and
> > after that it can perform a scaling check.
>
> Hmm. We have sport of similar situation in i915 where we pick a scaler
> much later and so don't know the exact scaling limits at the time when
> we do this check. But we opted to pass the lower/upper bounds of the
> scaling limits instead. That will guarantee that at least completely
> illegal values are rejected as early as possible, and so we don't have
> to worry about running into them later on.

Ack, I'll follow this approach then.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2024-02-14 19:43 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-14  5:06 [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes Dmitry Baryshkov
2023-09-14  5:06 ` [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state Dmitry Baryshkov
2024-02-14 18:30   ` Abhinav Kumar
2024-02-14 18:37   ` Ville Syrjälä
2024-02-14 18:47     ` Ville Syrjälä
2024-02-14 19:17       ` Dmitry Baryshkov
2024-02-14 19:39         ` Ville Syrjälä
2024-02-14 19:43           ` Dmitry Baryshkov
2023-09-14  5:06 ` [PATCH v3 02/12] drm/msm/dpu: add current resource allocation to dumped state Dmitry Baryshkov
2024-02-14 18:40   ` Abhinav Kumar
2024-02-14 19:18     ` Dmitry Baryshkov
2023-09-14  5:06 ` [PATCH v3 03/12] drm/msm/dpu: take plane rotation into account for wide planes Dmitry Baryshkov
2024-02-14 19:07   ` Abhinav Kumar
2023-09-14  5:06 ` [PATCH v3 04/12] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check Dmitry Baryshkov
2023-09-14  5:06 ` [PATCH v3 05/12] drm/msm/dpu: split dpu_plane_atomic_check() Dmitry Baryshkov
2023-09-14  5:07 ` [PATCH v3 06/12] drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe() Dmitry Baryshkov
2023-09-14  5:07 ` [PATCH v3 07/12] drm/msm/dpu: add support for virtual planes Dmitry Baryshkov
2023-09-14  5:07 ` [PATCH v3 08/12] drm/msm/dpu: allow using two SSPP blocks for a single plane Dmitry Baryshkov
2023-09-14  5:07 ` [PATCH v3 09/12] drm/msm/dpu: allow sharing SSPP between planes Dmitry Baryshkov
2023-09-14  5:07 ` [PATCH v3 10/12] drm/msm/dpu: create additional virtual planes Dmitry Baryshkov
2023-09-14  5:07 ` [PATCH v3 11/12] drm/msm/dpu: allow sharing of blending stages Dmitry Baryshkov
2023-09-14  5:07 ` [PATCH v3 12/12] drm/msm/dpu: include SSPP allocation state into the dumped state Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).