@@ -127,16 +127,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
if (!crtc)
return 0;
- /* FIXME: temporary hack necessary while we still use the plane update
- * helper. */
- if (state->state) {
- crtc_state =
- intel_atomic_get_crtc_state(state->state, intel_crtc);
- if (IS_ERR(crtc_state))
- return PTR_ERR(crtc_state);
- } else {
- crtc_state = intel_crtc->config;
- }
+ crtc_state = intel_crtc_state_for_plane(intel_state);
/*
* The original src/dest coordinates are stored in state->base, but
@@ -13034,6 +13034,46 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
return max_scale;
}
+/**
+ * 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);
+ struct intel_crtc_state *crtc_state;
+
+ /*
+ * 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.
+ *
+ * FIXME: Once our modeset code stops using transitional helpers
+ * internally we should add a WARN_ON() to this condition.
+ */
+ if (!state)
+ return to_intel_crtc_state(crtc->state);
+
+ crtc_state = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
+ if (WARN_ON(IS_ERR(crtc_state)))
+ crtc_state = to_intel_crtc_state(crtc->state);
+
+ return crtc_state;
+}
+
static int
intel_check_primary_plane(struct drm_plane *plane,
struct intel_plane_state *state)
@@ -13054,8 +13094,7 @@ intel_check_primary_plane(struct drm_plane *plane,
crtc = crtc ? crtc : plane->crtc;
intel_crtc = to_intel_crtc(crtc);
- crtc_state = state->base.state ?
- intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
+ crtc_state = intel_crtc_state_for_plane(state);
if (INTEL_INFO(dev)->gen >= 9) {
min_scale = 1;
@@ -13072,7 +13111,7 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret)
return ret;
- if (intel_crtc->active) {
+ if (crtc_state->base.active) {
struct intel_plane_state *old_state =
to_intel_plane_state(plane->state);
@@ -13366,6 +13405,7 @@ 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;
int ret;
@@ -13404,7 +13444,8 @@ intel_check_cursor_plane(struct drm_plane *plane,
}
finish:
- if (intel_crtc->active) {
+ intel_crtc_state = intel_crtc_state_for_plane(state);
+ if (intel_crtc_state->base.active) {
if (plane->state->crtc_w != state->base.crtc_w)
intel_crtc->atomic.update_wm = true;
@@ -1072,6 +1072,9 @@ void intel_create_rotation_property(struct drm_device *dev,
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);
@@ -753,8 +753,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
int ret;
intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
- crtc_state = state->base.state ?
- intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
+ crtc_state = intel_crtc_state_for_plane(state);
if (!fb) {
state->visible = false;
@@ -905,7 +904,7 @@ finish:
* If the sprite is completely covering the primary plane,
* we can disable the primary and save power.
*/
- if (intel_crtc->active) {
+ if (crtc_state->base.active) {
intel_crtc->atomic.fb_bits |=
INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
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 one exception to note is that when updating properties of a disabled plane (that is staying disabled), we'll have a top-level atomic state, but it may not contain the CRTC state we're looking for. This means we're not actually touching any CRTC state so it's safe to use the value from crtc->state directly. Note that we're only changing the 'check' side of updates to read out of in-flight state vs committed state; this takes care of making sure our derived state is updated as expected. The 'commit' code responsible for actually programming the hardware still looks at intel_crtc->active so that we won't try to write plane registers while the CRTC is disabled. v2: Rebasing and cleanup Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_atomic_plane.c | 11 +------ drivers/gpu/drm/i915/intel_display.c | 49 ++++++++++++++++++++++++++++--- drivers/gpu/drm/i915/intel_drv.h | 3 ++ drivers/gpu/drm/i915/intel_sprite.c | 5 ++-- 4 files changed, 51 insertions(+), 17 deletions(-)