All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Subject: [PATCH v2 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state()
Date: Wed,  1 Nov 2017 22:15:58 +0200	[thread overview]
Message-ID: <20171101201558.6059-1-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20171101182920.14386-5-ville.syrjala@linux.intel.com>

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_plane_helper_check_state() is supposed to do things the atomic way,
so it should not be inspecting crtc->enabled. Rather we should be
looking at crtc_state->enable.

We have a slight complication due to drm_plane_helper_check_update()
reusing drm_plane_helper_check_state() for non-atomic drivers. Thus
we'll have to pass the crtc_state in manally and construct a fake
crtc_state in drm_plane_helper_check_update().

v2: Fix the WARNs about plane_state->crtc matching crtc_state->crtc

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c            |  2 +-
 drivers/gpu/drm/arm/malidp_planes.c         |  3 +-
 drivers/gpu/drm/drm_plane_helper.c          | 51 ++++++++++++++++-------------
 drivers/gpu/drm/drm_simple_kms_helper.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c        |  2 ++
 drivers/gpu/drm/imx/ipuv3-plane.c           |  2 +-
 drivers/gpu/drm/mediatek/mtk_drm_plane.c    |  2 +-
 drivers/gpu/drm/meson/meson_plane.c         |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  4 +--
 drivers/gpu/drm/nouveau/nv50_display.c      |  6 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  2 +-
 drivers/gpu/drm/tegra/dc.c                  |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         |  2 +-
 drivers/gpu/drm/zte/zx_plane.c              |  4 +--
 include/drm/drm_plane_helper.h              |  3 +-
 15 files changed, 52 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 72b22b805412..14721723fa8a 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -252,7 +252,7 @@ static int hdlcd_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    false, true);
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 94e7e3fa3408..492d99dd55d4 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -150,7 +150,8 @@ static int malidp_se_check_scaling(struct malidp_plane *mp,
 
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, &clip, 0, INT_MAX, true, true);
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   0, INT_MAX, true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 759ed93f4ba8..eef0ffcd7ed5 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -101,7 +101,8 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
 
 /**
  * drm_plane_helper_check_state() - Check plane state for validity
- * @state: plane state to check
+ * @plane_state: plane state to check
+ * @crtc_state: crtc state to check
  * @clip: integer clipping coordinates
  * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
  * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
@@ -120,35 +121,37 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
  * RETURNS:
  * Zero if update appears valid, error code on failure
  */
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+				 const struct drm_crtc_state *crtc_state,
 				 const struct drm_rect *clip,
 				 int min_scale,
 				 int max_scale,
 				 bool can_position,
 				 bool can_update_disabled)
 {
-	struct drm_crtc *crtc = state->crtc;
-	struct drm_framebuffer *fb = state->fb;
-	struct drm_rect *src = &state->src;
-	struct drm_rect *dst = &state->dst;
-	unsigned int rotation = state->rotation;
+	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;
 	int hscale, vscale;
 
-	*src = drm_plane_state_src(state);
-	*dst = drm_plane_state_dest(state);
+	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
+
+	*src = drm_plane_state_src(plane_state);
+	*dst = drm_plane_state_dest(plane_state);
 
 	if (!fb) {
-		state->visible = false;
+		plane_state->visible = false;
 		return 0;
 	}
 
 	/* crtc should only be NULL when disabling (i.e., !fb) */
-	if (WARN_ON(!crtc)) {
-		state->visible = false;
+	if (WARN_ON(!plane_state->crtc)) {
+		plane_state->visible = false;
 		return 0;
 	}
 
-	if (!crtc->enabled && !can_update_disabled) {
+	if (!crtc_state->enable && !can_update_disabled) {
 		DRM_DEBUG_KMS("Cannot update plane of a disabled CRTC.\n");
 		return -EINVAL;
 	}
@@ -160,16 +163,16 @@ int drm_plane_helper_check_state(struct drm_plane_state *state,
 	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
 	if (hscale < 0 || vscale < 0) {
 		DRM_DEBUG_KMS("Invalid scaling of plane\n");
-		drm_rect_debug_print("src: ", &state->src, true);
-		drm_rect_debug_print("dst: ", &state->dst, false);
+		drm_rect_debug_print("src: ", &plane_state->src, true);
+		drm_rect_debug_print("dst: ", &plane_state->dst, false);
 		return -ERANGE;
 	}
 
-	state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+	plane_state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
 
 	drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
 
-	if (!state->visible)
+	if (!plane_state->visible)
 		/*
 		 * Plane isn't visible; some drivers can handle this
 		 * so we just return success here.  Drivers that can't
@@ -230,7 +233,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
 				  bool can_update_disabled,
 				  bool *visible)
 {
-	struct drm_plane_state state = {
+	struct drm_plane_state plane_state = {
 		.plane = plane,
 		.crtc = crtc,
 		.fb = fb,
@@ -245,18 +248,22 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
 		.rotation = rotation,
 		.visible = *visible,
 	};
+	struct drm_crtc_state crtc_state = {
+		.crtc = crtc,
+		.enable = crtc->enabled,
+	};
 	int ret;
 
-	ret = drm_plane_helper_check_state(&state, clip,
+	ret = drm_plane_helper_check_state(&plane_state, &crtc_state, clip,
 					   min_scale, max_scale,
 					   can_position,
 					   can_update_disabled);
 	if (ret)
 		return ret;
 
-	*src = state.src;
-	*dst = state.dst;
-	*visible = state.visible;
+	*src = plane_state.src;
+	*dst = plane_state.dst;
+	*visible = plane_state.visible;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index dc9fd109de14..d428c805025c 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -103,7 +103,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(plane_state, &clip,
+	ret = drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..4a459726b36b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9317,6 +9317,7 @@ static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 	int ret;
 
 	ret = drm_plane_helper_check_state(&plane_state->base,
+					   &crtc_state->base,
 					   &plane_state->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
@@ -12808,6 +12809,7 @@ intel_check_primary_plane(struct intel_plane *plane,
 	}
 
 	ret = drm_plane_helper_check_state(&state->base,
+					   &crtc_state->base,
 					   &state->clip,
 					   min_scale, max_scale,
 					   can_position, true);
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 247c60e6bed2..51b23ac175e3 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -342,7 +342,7 @@ static int ipu_plane_atomic_check(struct drm_plane *plane,
 	clip.y1 = 0;
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   can_position, true);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 6f121891430f..7ebb33657704 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -111,7 +111,7 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->mode.hdisplay;
 	clip.y2 = crtc_state->mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    true, true);
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 17e96fa47868..2a00b720c371 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -61,7 +61,7 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->mode.hdisplay;
 	clip.y2 = crtc_state->mode.vdisplay;
 
-	return drm_plane_helper_check_state(state, &clip,
+	return drm_plane_helper_check_state(state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    true, true);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 4b22ac3413a1..1c194a9453d9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -348,8 +348,8 @@ static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
 	min_scale = FRAC_16_16(1, 8);
 	max_scale = FRAC_16_16(8, 1);
 
-	ret = drm_plane_helper_check_state(state, &clip, min_scale,
-					   max_scale, true, true);
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   min_scale, max_scale, true, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index e4751f92b342..7f11a88d0593 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1143,7 +1143,8 @@ nv50_curs_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 {
 	int ret;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+					   &asyw->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   true, true);
@@ -1432,7 +1433,8 @@ nv50_base_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
 	if (!fb->format->depth)
 		return -EINVAL;
 
-	ret = drm_plane_helper_check_state(&asyw->state, &asyw->clip,
+	ret = drm_plane_helper_check_state(&asyw->state, &asyh->state,
+					   &asyw->clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 19128b4dea54..36d0f101e30d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -659,7 +659,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   min_scale, max_scale,
 					   true, true);
 	if (ret)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 24a5ef4f5bb8..e95d9e2a7f6c 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -491,8 +491,8 @@ static int tegra_plane_state_add(struct tegra_plane *plane,
 	clip.y2 = crtc_state->mode.vdisplay;
 
 	/* Check plane state for visibility and calculate clipping bounds */
-	err = drm_plane_helper_check_state(state, &clip, 0, INT_MAX,
-					   true, true);
+	err = drm_plane_helper_check_state(state, crtc_state, &clip,
+					   0, INT_MAX, true, true);
 	if (err < 0)
 		return err;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index efa41c086198..d95e7b1c3b11 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -454,7 +454,7 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 		clip.y2 = crtc_state->adjusted_mode.vdisplay;
 	}
 
-	ret = drm_plane_helper_check_state(state, &clip,
+	ret = drm_plane_helper_check_state(state, crtc_state, &clip,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   DRM_PLANE_HELPER_NO_SCALING,
 					   false, true);
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index 18e763493264..ee0002529b8f 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -80,7 +80,7 @@ static int zx_vl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, &clip,
+	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					    min_scale, max_scale,
 					    true, true);
 }
@@ -315,7 +315,7 @@ static int zx_gl_plane_atomic_check(struct drm_plane *plane,
 	clip.x2 = crtc_state->adjusted_mode.hdisplay;
 	clip.y2 = crtc_state->adjusted_mode.vdisplay;
 
-	return drm_plane_helper_check_state(plane_state, &clip,
+	return drm_plane_helper_check_state(plane_state, crtc_state, &clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    false, true);
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 7c8a00ceadb7..41b8309b0a57 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -38,7 +38,8 @@
  */
 #define DRM_PLANE_HELPER_NO_SCALING (1<<16)
 
-int drm_plane_helper_check_state(struct drm_plane_state *state,
+int drm_plane_helper_check_state(struct drm_plane_state *plane_state,
+				 const struct drm_crtc_state *crtc_state,
 				 const struct drm_rect *clip,
 				 int min_scale, int max_scale,
 				 bool can_position,
-- 
2.13.6

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2017-11-01 20:15 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-01 18:29 [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Ville Syrjala
2017-11-01 18:29 ` [PATCH 1/5] drm/vmwgfx: Remove bogus crtc coords vs fb size check Ville Syrjala
2017-11-02 10:04   ` Daniel Vetter
2017-11-23  9:54   ` Laurent Pinchart
2017-11-01 18:29 ` [PATCH 2/5] drm/vmwgfx: Use drm_plane_helper_check_state() Ville Syrjala
2017-11-02 10:06   ` Daniel Vetter
2017-11-23  9:54   ` Laurent Pinchart
2017-11-01 18:29 ` [PATCH 3/5] drm/vmwgfx: Try to fix plane clipping Ville Syrjala
2017-11-02 10:12   ` Daniel Vetter
2017-11-02 13:19     ` Ville Syrjälä
2017-11-06 18:04       ` [Intel-gfx] " Ville Syrjälä
2017-11-07 12:30         ` Daniel Vetter
2017-11-23  9:46         ` [Intel-gfx] " Laurent Pinchart
2017-11-01 18:29 ` [PATCH 4/5] drm: Check crtc_state->enable rather than crtc->enabled in drm_plane_helper_check_state() Ville Syrjala
2017-11-01 20:15   ` Ville Syrjala [this message]
2017-11-02 10:15     ` [PATCH v2 " Daniel Vetter
2017-11-23  9:52   ` [PATCH " Laurent Pinchart
2017-11-01 18:29 ` [PATCH 5/5] drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c Ville Syrjala
2017-11-01 20:16   ` [PATCH v2 " Ville Syrjala
2017-11-02 10:16     ` Daniel Vetter
2017-11-23  9:53   ` [PATCH " Laurent Pinchart
2017-11-01 19:46 ` ✗ Fi.CI.BAT: failure for drm: drm_plane_helper_check_state() related stuff Patchwork
2017-11-01 20:10   ` Ville Syrjälä
2017-11-01 21:02 ` ✓ Fi.CI.BAT: success for drm: drm_plane_helper_check_state() related stuff (rev3) Patchwork
2017-11-01 21:52 ` ✓ Fi.CI.IGT: " Patchwork
2017-11-10 21:26 ` [PATCH 0/5] drm: drm_plane_helper_check_state() related stuff Sinclair Yeh
2017-11-10 21:42   ` Ville Syrjälä
2017-11-20  7:34     ` Daniel Vetter
2017-11-20 17:32       ` Sinclair Yeh
2017-11-20 19:36         ` Ville Syrjälä
2017-11-23  9:56           ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171101201558.6059-1-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.