[PATCH 2/4] drm/i915: Lookup CRTC for plane directly

[Matt Roper <matthew.d.roper@intel.com>]

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@intel.com>
Reported-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@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 90c4a82..740d1a6 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -116,19 +116,19 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	struct intel_plane_state *intel_state = to_intel_plane_state(state);
 	bool active;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc_state = intel_crtc_state_for_plane(intel_state);
 	active = intel_crtc_state->base.enable;
 
 	/*
-	 * 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;
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 88b0f69..1cf91ad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12625,7 +12625,7 @@ intel_check_primary_plane(struct drm_plane *plane,
 	bool active;
 	int ret;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc_state = intel_crtc_state_for_plane(state);
@@ -12694,7 +12694,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	struct drm_rect *src = &state->src;
 	bool active;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc_state = intel_crtc_state_for_plane(state);
@@ -12916,7 +12916,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 	bool active;
 	int ret;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc_state = intel_crtc_state_for_plane(state);
@@ -12977,7 +12977,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 	uint32_t addr;
 	bool active;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc_state = intel_crtc_state_for_plane(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 71e0152..d5ea24f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -731,6 +731,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 2016e78..492abcd 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -878,7 +878,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	int pixel_size;
 	bool active;
 
-	intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
+	intel_crtc = to_intel_crtc(intel_get_crtc_for_drm_plane(plane));
 
 	intel_crtc_state = intel_crtc_state_for_plane(state);
 	active = intel_crtc_state->base.enable;
@@ -1075,7 +1075,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
 	uint32_t src_x, src_y, src_w, src_h;
 	bool active;
 
-	crtc = crtc ? crtc : plane->crtc;
+	crtc = intel_get_crtc_for_drm_plane(plane);
 	intel_crtc = to_intel_crtc(crtc);
 
 	intel_crtc_state = intel_crtc_state_for_plane(state);
-- 
1.8.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx