[Intel-gfx] [PATCH 1/4] drm/i915: Make atomic use in-flight state for CRTC active value

Matt Roper matthew.d.roper at intel.com
Wed Apr 8 18:56:51 PDT 2015


Our atomic plane code currently uses intel_crtc->active to determine
how/when to update some derived state values.  This works fine for pure
plane updates at the moment since the CRTC state itself isn't changed as
part of the operation.  However as we convert more of our driver
internals over to atomic modesetting, we need to look at whether the
CRTC will be active at the *end* of the atomic transaction (which may
not match the currently committed state).

The in-flight value we want to use is generally in a crtc_state object
associated with our top-level atomic transaction.  However there are a
few cases where this isn't the case:

 * While using transitional atomic helpers (as we are at the moment),
   SetPlane() calls will operate on orphaned plane states that aren't
   part of a top-level atomic transaction.  In this case, we're not
   touching the CRTC state, so it's fine to use the already-committed
   value from crtc->state.

 * While updating properties of a disabled plane, we'll have a top-level
   atomic state, but it may not contain the CRTC state we're looking
   for.  Once again, this means we're not actually touching any CRTC
   state so it's safe to use the value from crtc->state directly.

Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c | 11 +++--
 drivers/gpu/drm/i915/intel_display.c      | 73 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h          |  3 ++
 drivers/gpu/drm/i915/intel_sprite.c       | 16 +++++--
 4 files changed, 93 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 976b891..90c4a82 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -111,12 +111,17 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 {
 	struct drm_crtc *crtc = state->crtc;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *intel_crtc_state;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct intel_plane_state *intel_state = to_intel_plane_state(state);
+	bool active;
 
 	crtc = crtc ? crtc : plane->crtc;
 	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
@@ -143,10 +148,8 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
 	intel_state->clip.x1 = 0;
 	intel_state->clip.y1 = 0;
-	intel_state->clip.x2 =
-		intel_crtc->active ? intel_crtc->config->pipe_src_w : 0;
-	intel_state->clip.y2 =
-		intel_crtc->active ? intel_crtc->config->pipe_src_h : 0;
+	intel_state->clip.x2 = active ? intel_crtc_state->pipe_src_w : 0;
+	intel_state->clip.y2 = active ? intel_crtc_state->pipe_src_h : 0;
 
 	/*
 	 * Disabling a plane is always okay; we just need to update
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7bfe2af..88b0f69 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12562,6 +12562,53 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 	}
 }
 
+/**
+ * intel_crtc_state_for_plane - Obtain CRTC state for a plane
+ * @pstate: plane state to lookup corresponding crtc state for
+ *
+ * When working with a top-level atomic transaction (drm_atomic_state),
+ * a CRTC state should be present that corresponds to the provided
+ * plane state; this function provides a quick way to fetch that
+ * CRTC state.  In cases where we have a plane state unassociated with any
+ * top-level atomic transaction (e.g., while using the transitional atomic
+ * helpers), the current CRTC state from crtc->state will be returned
+ * instead.
+ */
+struct intel_crtc_state *
+intel_crtc_state_for_plane(struct intel_plane_state *pstate)
+{
+	struct drm_atomic_state *state = pstate->base.state;
+	struct intel_plane *plane = to_intel_plane(pstate->base.plane);
+	struct drm_crtc *crtc = intel_get_crtc_for_pipe(pstate->base.plane->dev,
+							plane->pipe);
+	int i;
+
+	/*
+	 * While using transitional plane helpers, we may not have a top-level
+	 * atomic transaction.  Of course that also means that we're not
+	 * actually touching CRTC state, so just return the currently
+	 * committed state.
+	 */
+	if (!state)
+		return to_intel_crtc_state(crtc->state);
+
+	for (i = 0; i < state->dev->mode_config.num_crtc; i++) {
+		if (!state->crtcs[i])
+			continue;
+
+		if (to_intel_crtc(state->crtcs[i])->pipe == plane->pipe)
+			return to_intel_crtc_state(state->crtc_states[i]);
+	}
+
+	/*
+	 * We may have a plane state without a corresponding CRTC state if
+	 * we're updating a property of a disabled plane.  Again, just using
+	 * the already-committed state for this plane's CRTC should be fine
+	 * since we're not actually touching the CRTC here.
+	 */
+	return to_intel_crtc_state(crtc->state);
+}
+
 static int
 intel_check_primary_plane(struct drm_plane *plane,
 			  struct intel_plane_state *state)
@@ -12570,15 +12617,20 @@ intel_check_primary_plane(struct drm_plane *plane,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc = state->base.crtc;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *intel_crtc_state;
 	struct drm_framebuffer *fb = state->base.fb;
 	struct drm_rect *dest = &state->dst;
 	struct drm_rect *src = &state->src;
 	const struct drm_rect *clip = &state->clip;
+	bool active;
 	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
 
+	intel_crtc_state = intel_crtc_state_for_plane(state);
+	active = intel_crtc_state->base.enable;
+
 	ret = drm_plane_helper_check_update(plane, crtc, fb,
 					    src, dest, clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
@@ -12587,7 +12639,7 @@ intel_check_primary_plane(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
-	if (intel_crtc->active) {
+	if (active) {
 		intel_crtc->atomic.wait_for_flips = true;
 
 		/*
@@ -12638,11 +12690,16 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *intel_crtc_state;
 	struct drm_rect *src = &state->src;
+	bool active;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
 
+	intel_crtc_state = intel_crtc_state_for_plane(state);
+	active = intel_crtc_state->base.enable;
+
 	plane->fb = fb;
 	crtc->x = src->x1 >> 16;
 	crtc->y = src->y1 >> 16;
@@ -12854,12 +12911,17 @@ intel_check_cursor_plane(struct drm_plane *plane,
 	const struct drm_rect *clip = &state->clip;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *intel_crtc_state;
 	unsigned stride;
+	bool active;
 	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
 
+	intel_crtc_state = intel_crtc_state_for_plane(state);
+	active = intel_crtc_state->base.enable;
+
 	ret = drm_plane_helper_check_update(plane, crtc, fb,
 					    src, dest, clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
@@ -12892,7 +12954,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 	}
 
 finish:
-	if (intel_crtc->active) {
+	if (active) {
 		if (plane->state->crtc_w != state->base.crtc_w)
 			intel_crtc->atomic.update_wm = true;
 
@@ -12910,12 +12972,17 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 	struct drm_crtc *crtc = state->base.crtc;
 	struct drm_device *dev = plane->dev;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *intel_crtc_state;
 	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
 	uint32_t addr;
+	bool active;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
 
+	intel_crtc_state = intel_crtc_state_for_plane(state);
+	active = intel_crtc_state->base.enable;
+
 	plane->fb = state->base.fb;
 	crtc->cursor_x = state->base.crtc_x;
 	crtc->cursor_y = state->base.crtc_y;
@@ -12934,7 +13001,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 	intel_crtc->cursor_bo = obj;
 update:
 
-	if (intel_crtc->active)
+	if (active)
 		intel_crtc_update_cursor(crtc, state->visible);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index efa53d5..71e0152 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -998,6 +998,9 @@ intel_rotation_90_or_270(unsigned int rotation)
 
 bool intel_wm_need_update(struct drm_plane *plane,
 			  struct drm_plane_state *state);
+struct intel_crtc_state *
+intel_crtc_state_for_plane(struct intel_plane_state *pstate);
+
 
 /* shared dpll functions */
 struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a4c0a04..2016e78 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -864,6 +864,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 			 struct intel_plane_state *state)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
+	struct intel_crtc_state *intel_crtc_state;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = state->base.fb;
 	int crtc_x, crtc_y;
@@ -875,9 +876,13 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	int hscale, vscale;
 	int max_scale, min_scale;
 	int pixel_size;
+	bool active;
 
 	intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
 
+	intel_crtc_state = intel_crtc_state_for_plane(state);
+	active = intel_crtc_state->base.enable;
+
 	if (!fb) {
 		state->visible = false;
 		goto finish;
@@ -1024,9 +1029,9 @@ finish:
 	 */
 	state->hides_primary = fb != NULL && drm_rect_equals(dst, clip) &&
 		!colorkey_enabled(intel_plane);
-	WARN_ON(state->hides_primary && !state->visible && intel_crtc->active);
+	WARN_ON(state->hides_primary && !state->visible && active);
 
-	if (intel_crtc->active) {
+	if (active) {
 		if (intel_crtc->primary_enabled == state->hides_primary)
 			intel_crtc->atomic.wait_for_flips = true;
 
@@ -1062,18 +1067,23 @@ intel_commit_sprite_plane(struct drm_plane *plane,
 {
 	struct drm_crtc *crtc = state->base.crtc;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *intel_crtc_state;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = state->base.fb;
 	int crtc_x, crtc_y;
 	unsigned int crtc_w, crtc_h;
 	uint32_t src_x, src_y, src_w, src_h;
+	bool active;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
 
+	intel_crtc_state = intel_crtc_state_for_plane(state);
+	active = intel_crtc_state->base.enable;
+
 	plane->fb = fb;
 
-	if (intel_crtc->active) {
+	if (active) {
 		intel_crtc->primary_enabled = !state->hides_primary;
 
 		if (state->visible) {
-- 
1.8.5.1



More information about the Intel-gfx mailing list