[Intel-gfx] [RFC 05/15] drm/i915: Lookup CRTC for plane directly

Matt Roper matthew.d.roper at intel.com
Wed May 20 19:12:17 PDT 2015


Various places in the atomic plane code obtain the CRTC via
plane_state->crtc.  But plane_state->crtc is NULL when disabling the
plane, so the code will fall back to looking at the old CRTC value in
plane->crtc in that case.  This isn't going to work when we start
supporting non-blocking flips (since the legacy plane->crtc pointer will
already be updated to its new value before the asynchronous workqueue
task runs the plane commit function).  The code is also fragile in
general (we have to be careful when doing stuff like updating properties
on a disabled plane).  Since Intel hardware has a fixed plane to pipe
mapping, let's just lookup the CRTC via that fixed mapping and avoid
future mistakes.

Cc: Vivek Kasireddy <vivek.kasireddy at intel.com>
Reported-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c | 12 ++++++------
 drivers/gpu/drm/i915/intel_display.c      |  8 ++++----
 drivers/gpu/drm/i915/intel_drv.h          |  7 +++++++
 drivers/gpu/drm/i915/intel_sprite.c       |  4 ++--
 4 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 714ee24..21c808d 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -115,16 +115,16 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct intel_plane_state *intel_state = to_intel_plane_state(state);
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	/*
-	 * Both crtc and plane->crtc could be NULL if we're updating a
-	 * property while the plane is disabled.  We don't actually have
-	 * anything driver-specific we need to test in that case, so
-	 * just return success.
+	 * Both state->crtc and plane->state->crtc could be NULL if we're
+	 * updating a property while the plane is disabled.  We don't actually
+	 * have anything driver-specific we need to test in that case, so just
+	 * return success.
 	 */
-	if (!crtc)
+	if (!state->crtc && !plane->state->crtc)
 		return 0;
 
 	crtc_state = intel_crtc_state_for_plane(intel_state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1a7d2a9..f4398a7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13092,7 +13092,7 @@ intel_check_primary_plane(struct drm_plane *plane,
 	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
 	int ret;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 	crtc_state = intel_crtc_state_for_plane(state);
 
@@ -13174,7 +13174,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc;
 	struct drm_rect *src = &state->src;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	plane->fb = fb;
@@ -13409,7 +13409,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 	unsigned stride;
 	int ret;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	ret = drm_plane_helper_check_update(plane, crtc, fb,
@@ -13482,7 +13482,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
 	uint32_t addr;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	plane->fb = state->base.fb;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1e22ffe..ea67093 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -803,6 +803,13 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane)
 	return dev_priv->plane_to_crtc_mapping[plane];
 }
 
+static inline struct drm_crtc *
+intel_get_crtc_for_drm_plane(struct drm_plane *plane)
+{
+	struct drm_i915_private *dev_priv = to_i915(plane->dev);
+	return dev_priv->pipe_to_crtc_mapping[to_intel_plane(plane)->pipe];
+}
+
 struct intel_unpin_work {
 	struct work_struct work;
 	struct drm_crtc *crtc;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index bfe9213..2164e63 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -752,7 +752,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	int pixel_size;
 	int ret;
 
-	intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
+	intel_crtc = to_intel_crtc(intel_get_crtc_for_drm_plane(plane));
 	crtc_state = intel_crtc_state_for_plane(state);
 
 	if (!fb) {
@@ -942,7 +942,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
 	unsigned int crtc_w, crtc_h;
 	uint32_t src_x, src_y, src_w, src_h;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	plane->fb = fb;
-- 
1.8.5.1



More information about the Intel-gfx mailing list