[PATCH 1/2] drm/atomic: Add drm_crtc_state->active
Daniel Vetter
daniel.vetter at ffwll.ch
Thu Dec 18 08:08:51 PST 2014
This is the infrastructure for DPMS ported to the atomic world.
Fundamental changes compare to legacy DPMS are:
- No more per-connector dpms state, instead there's just one per each
display pipeline. So if you clone either you have to unclone first
if you only want to switch off one screen, or you just switch of
everything (like all desktops do). This massively reduces complexity
for cloning since now there's no more half-enabled cloned configs to
consider.
- Only on/off, dpms standby/suspend are as dead as real CRTs. Again
reduces complexity a lot.
Now especially for backwards compat the really important part for dpms
support is that dpms on always succeeds (except for hw death and
unplugged cables ofc). Which means everything that could fail (like
configuration checking, resources assignments and buffer management)
must be done irrespective from ->active. ->active is really only a
toggle to change the hardware state. More precisely:
- Drivers MUST NOT look at ->active in their ->atomic_check callbacks.
Changes to ->active MUST always suceed if nothing else changes.
- Drivers using the atomic helpers MUST NOT look at ->active anywhere,
period. The helpers will take care of calling the respective
enable/modeset/disable hooks as necessary. As before the helpers
will carefully keep track of the state and not call any hooks
unecessarily, so still no double-disables or enables like with crtc
helpers.
- ->mode_set hooks are only called when the mode or output
configuration changes, not for changes in ->active state.
- Drivers which reconstruct the state objects in their ->reset hooks
or through some other hw state readout infrastructure must ensure
that ->active reflects actual hw state.
This just implements the core bits and helper logic, a subsequent
patch will implement the helper code to implement legacy dpms with
this.
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 61 ++++++++++++++++++++++++++++++++++---
include/drm/drm_crtc.h | 10 ++++++
2 files changed, 66 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index f191da0e8a89..dd5a549dde2e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -379,6 +379,16 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
crtc->base.id);
crtc_state->mode_changed = true;
}
+
+ /*
+ * FIXME: This needs to be moved to the core checks once those
+ * landed.
+ */
+ if (crtc_state->active && !crtc_state->enable) {
+ DRM_DEBUG_KMS("[CRTC:%d] active without enabled\n",
+ crtc->base.id);
+ return -EINVAL;
+ }
}
for (i = 0; i < state->num_connector; i++) {
@@ -404,12 +414,28 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
crtc = state->crtcs[i];
crtc_state = state->crtc_states[i];
- if (!crtc || !crtc_state->mode_changed)
+ if (!crtc)
+ continue;
+
+ /*
+ * We must set ->active_changed after walking connectors for
+ * otherwise and an update that only changes active would result
+ * in a full modeset because update_connector_routing force
+ * that.
+ */
+ if (crtc->state->active != crtc_state->active) {
+ DRM_DEBUG_KMS("[CRTC:%d] active changed\n",
+ crtc->base.id);
+ crtc_state->active_changed = true;
+ }
+
+ if (!(crtc->state->mode_changed || crtc->state->active_changed))
continue;
- DRM_DEBUG_KMS("[CRTC:%d] needs full modeset, enable: %c\n",
+ DRM_DEBUG_KMS("[CRTC:%d] needs all connectors, enable: %c, active: %c\n",
crtc->base.id,
- crtc_state->enable ? 'y' : 'n');
+ crtc_state->enable ? 'y' : 'n',
+ crtc_state->active ? 'y' : 'n');
ret = drm_atomic_add_affected_connectors(state, crtc);
if (ret != 0)
@@ -545,6 +571,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
struct drm_connector *connector;
struct drm_encoder_helper_funcs *funcs;
struct drm_encoder *encoder;
+ struct drm_crtc_state *old_crtc_state;
old_conn_state = old_state->connector_states[i];
connector = old_state->connectors[i];
@@ -554,6 +581,11 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
if (!old_conn_state || !old_conn_state->crtc)
continue;
+ old_crtc_state = old_state->crtc_states[drm_crtc_index(old_conn_state->crtc)];
+
+ if (!old_crtc_state->active)
+ continue;
+
encoder = old_conn_state->best_encoder;
/* We shouldn't get this far if we didn't previously have
@@ -586,11 +618,17 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
for (i = 0; i < ncrtcs; i++) {
struct drm_crtc_helper_funcs *funcs;
struct drm_crtc *crtc;
+ struct drm_crtc_state *old_crtc_state;
crtc = old_state->crtcs[i];
+ old_crtc_state = old_state->crtc_states[i];
/* Shut down everything that needs a full modeset. */
- if (!crtc || !crtc->state->mode_changed)
+ if (!crtc ||
+ !(crtc->state->mode_changed || crtc->state->active_changed))
+ continue;
+
+ if (!old_crtc_state->active)
continue;
funcs = crtc->helper_private;
@@ -697,6 +735,9 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
mode = &new_crtc_state->mode;
adjusted_mode = &new_crtc_state->adjusted_mode;
+ if (!new_crtc_state->mode_changed)
+ continue;
+
/*
* Each encoder has at most one connector (since we always steal
* it away), so we won't call call mode_set hooks twice.
@@ -749,7 +790,11 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
crtc = old_state->crtcs[i];
/* Need to filter out CRTCs where only planes change. */
- if (!crtc || !crtc->state->mode_changed)
+ if (!crtc ||
+ !(crtc->state->mode_changed || crtc->state->active_changed))
+ continue;
+
+ if (!crtc->state->active)
continue;
funcs = crtc->helper_private;
@@ -768,6 +813,9 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
if (!connector || !connector->state->best_encoder)
continue;
+ if (!connector->state->crtc->state->active)
+ continue;
+
encoder = connector->state->best_encoder;
funcs = encoder->helper_private;
@@ -1518,6 +1566,7 @@ retry:
WARN_ON(set->num_connectors);
crtc_state->enable = false;
+ crtc_state->active = false;
ret = drm_atomic_set_crtc_for_plane(primary_state, NULL);
if (ret != 0)
@@ -1532,6 +1581,7 @@ retry:
WARN_ON(!set->num_connectors);
crtc_state->enable = true;
+ crtc_state->active = true;
drm_mode_copy(&crtc_state->mode, set->mode);
ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
@@ -1894,6 +1944,7 @@ drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc)
if (state) {
state->mode_changed = false;
+ state->active_changed = false;
state->planes_changed = false;
state->event = NULL;
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index ff83aefa2acc..560a1c63236d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -231,7 +231,9 @@ struct drm_atomic_state;
* struct drm_crtc_state - mutable CRTC state
* @crtc: backpointer to the CRTC
* @enable: whether the CRTC should be enabled, gates all other state
+ * @active: whether the CRTC is actively displaying (used for DPMS)
* @mode_changed: for use by helpers and drivers when computing state updates
+ * @active_changed: for use by helpers and drivers when computing state updates
* @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
* @last_vblank_count: for helpers and drivers to capture the vblank of the
* update to ensure framebuffer cleanup isn't done too early
@@ -241,15 +243,23 @@ struct drm_atomic_state;
* @event: optional pointer to a DRM event to signal upon completion of the
* state update
* @state: backpointer to global drm_atomic_state
+ *
+ * Note that the distinction between @enable and @active is rather subtile:
+ * Flipping @active while @enable is set without changing anything else may
+ * never return in a failure from the ->atomic_check callback. Userspace assumes
+ * that a DPMS On will always succeed. In other words: @enable controls resource
+ * assignment, @active controls the actual hardware state.
*/
struct drm_crtc_state {
struct drm_crtc *crtc;
bool enable;
+ bool active;
/* computed state bits used by helpers and drivers */
bool planes_changed : 1;
bool mode_changed : 1;
+ bool active_changed : 1;
/* attached planes bitmask:
* WARNING: transitional helpers do not maintain plane_mask so
--
2.1.3
More information about the dri-devel
mailing list