From eab3bbeffd152125ae0f90863b8e9bc8eef49423 Mon Sep 17 00:00:00 2001
From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu, 22 Jan 2015 16:36:21 +0100
Subject: drm/atomic: Add drm_crtc_state->active
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.
v2: Rebase on top of the drm ioctl work:
- Move crtc checks to the core check function.
- Also check for ->active_changed when deciding whether a modeset
might happen (for the ALLOW_MODESET mode).
- Expose the ->active state with an atomic prop.
v3: Review from Rob
- Spelling fix in comment.
- Extract needs_modeset helper to consolidate the ->mode_changed ||
->active_changed checks.
v4: Fixup fumble between crtc->state and crtc_state.
Cc: Rob Clark <robdclark@gmail.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Tested-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 54 +++++++++++++++++++++++++++++++++----
1 file changed, 49 insertions(+), 5 deletions(-)
(limited to 'drivers/gpu/drm/drm_atomic_helper.c')
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 541ba833ed36..3f17933b1d2c 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -330,6 +330,12 @@ mode_fixup(struct drm_atomic_state *state)
return 0;
}
+static bool
+needs_modeset(struct drm_crtc_state *state)
+{
+ return state->mode_changed || state->active_changed;
+}
+
/**
* drm_atomic_helper_check - validate state object for modeset changes
* @dev: DRM device
@@ -404,12 +410,27 @@ 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;
- DRM_DEBUG_KMS("[CRTC:%d] needs full modeset, enable: %c\n",
+ /*
+ * We must set ->active_changed after walking connectors for
+ * otherwise 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 (!needs_modeset(crtc_state))
+ continue;
+
+ 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 +566,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 +576,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 +613,16 @@ 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 || !needs_modeset(crtc->state))
+ continue;
+
+ if (!old_crtc_state->active)
continue;
funcs = crtc->helper_private;
@@ -697,6 +729,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 +784,10 @@ 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 || !needs_modeset(crtc->state))
+ continue;
+
+ if (!crtc->state->active)
continue;
funcs = crtc->helper_private;
@@ -768,6 +806,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 +1559,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 +1574,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 +1937,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;
}
--
cgit
From b486e0e6d599b9ca8667fb9a7d49b7383ee963c7 Mon Sep 17 00:00:00 2001
From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu, 22 Jan 2015 18:53:05 +0100
Subject: drm/atomic-helper: add connector->dpms() implementation
This builds on top of the crtc->active infrastructure to implement
legacy DPMS. My choice of semantics is somewhat arbitrary, but the
entire pipe is enabled as along as one output is still enabled.
Of course it also clamps everything that's not ON to OFF.
v2: Fix spelling in one comment.
v3: Don't do an async commit (Thierry)
v4: Dan Carpenter noticed missing error case handling.
Cc: Thierry Reding <thierry.reding@gmail.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Tested-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 77 +++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
(limited to 'drivers/gpu/drm/drm_atomic_helper.c')
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 3f17933b1d2c..6112ec261c3b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1886,6 +1886,83 @@ backoff:
}
EXPORT_SYMBOL(drm_atomic_helper_page_flip);
+/**
+ * drm_atomic_helper_connector_dpms() - connector dpms helper implementation
+ * @connector: affected connector
+ * @mode: DPMS mode
+ *
+ * This is the main helper function provided by the atomic helper framework for
+ * implementing the legacy DPMS connector interface. It computes the new desired
+ * ->active state for the corresponding CRTC (if the connector is enabled) and
+ * updates it.
+ */
+void drm_atomic_helper_connector_dpms(struct drm_connector *connector,
+ int mode)
+{
+ struct drm_mode_config *config = &connector->dev->mode_config;
+ struct drm_atomic_state *state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ struct drm_connector *tmp_connector;
+ int ret;
+ bool active = false;
+
+ if (mode != DRM_MODE_DPMS_ON)
+ mode = DRM_MODE_DPMS_OFF;
+
+ connector->dpms = mode;
+ crtc = connector->state->crtc;
+
+ if (!crtc)
+ return;
+
+ /* FIXME: ->dpms has no return value so can't forward the -ENOMEM. */
+ state = drm_atomic_state_alloc(connector->dev);
+ if (!state)
+ return;
+
+ state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+retry:
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return;
+
+ WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
+
+ list_for_each_entry(tmp_connector, &config->connector_list, head) {
+ if (connector->state->crtc != crtc)
+ continue;
+
+ if (connector->dpms == DRM_MODE_DPMS_ON) {
+ active = true;
+ break;
+ }
+ }
+ crtc_state->active = active;
+
+ ret = drm_atomic_commit(state);
+ if (ret != 0)
+ goto fail;
+
+ /* Driver takes ownership of state on successful async commit. */
+ return;
+fail:
+ if (ret == -EDEADLK)
+ goto backoff;
+
+ drm_atomic_state_free(state);
+
+ WARN(1, "Driver bug: Changing ->active failed with ret=%i\n", ret);
+
+ return;
+backoff:
+ drm_atomic_state_clear(state);
+ drm_atomic_legacy_backoff(state);
+
+ goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_dpms);
+
/**
* DOC: atomic state reset and initialization
*
--
cgit
From f02ad907cd9e7fe3a6405d2d005840912f1ed258 Mon Sep 17 00:00:00 2001
From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu, 22 Jan 2015 16:36:23 +0100
Subject: drm/atomic-helpers: Recover full cursor plane behaviour
Cursor plane updates have historically been fully async and mutliple
updates batched together for the next vsync. And userspace relies upon
that. Since implementing a full queue of async atomic updates is a bit
of work lets just recover the cursor specific behaviour with a hint
flag and some hacks to drop the vblank wait.
v2: Fix kerneldoc, reported by Wu Fengguang.
Reviewed-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 11 +++++++++++
1 file changed, 11 insertions(+)
(limited to 'drivers/gpu/drm/drm_atomic_helper.c')
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 6112ec261c3b..d0c3611402da 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -909,6 +909,11 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
if (!crtc->state->enable)
continue;
+ /* Legacy cursor ioctls are completely unsynced, and userspace
+ * relies on that (by doing tons of cursor updates). */
+ if (old_state->legacy_cursor_update)
+ continue;
+
if (!framebuffer_changed(dev, old_state, crtc))
continue;
@@ -1335,6 +1340,9 @@ retry:
if (ret != 0)
goto fail;
+ if (plane == crtc->cursor)
+ state->legacy_cursor_update = true;
+
/* Driver takes ownership of state on successful commit. */
return 0;
fail:
@@ -1410,6 +1418,9 @@ retry:
plane_state->src_h = 0;
plane_state->src_w = 0;
+ if (plane == plane->crtc->cursor)
+ state->legacy_cursor_update = true;
+
ret = drm_atomic_commit(state);
if (ret != 0)
goto fail;
--
cgit
From ee0a89cf3c2c550e6d877dda21dd2947afb90cb6 Mon Sep 17 00:00:00 2001
From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu, 22 Jan 2015 16:36:24 +0100
Subject: drm/atomic-helpers: Saner encoder/crtc callbacks
For historical reasons going all the way back to how the Xrandr code
was implemented the semantics of the callbacks used to enable/disable
crtcs and encoders are ... interesting.
But with atomic helpers all that complexity has been binned, with only
a well-defined on/off action left. Unfortunately the names stuck.
Let's fix that by adding enable/disable hooks every, make them the
preferred variant for atomic and update documentations.
Later on we add debug warnings when drivers have deprecated hooks. But
while everything is in-flight with lots of drivers converting to
atomic that's a bit too much - better wait for things to settle a bit
first.
v2: Fix kerneldoc, reported by Wu Fengguang.
Reviewed-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
(limited to 'drivers/gpu/drm/drm_atomic_helper.c')
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d0c3611402da..379b6c833bb2 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -599,7 +599,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
encoder->bridge->funcs->disable(encoder->bridge);
/* Right function depends upon target state. */
- if (connector->state->crtc)
+ if (connector->state->crtc && funcs->prepare)
funcs->prepare(encoder);
else if (funcs->disable)
funcs->disable(encoder);
@@ -628,7 +628,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
funcs = crtc->helper_private;
/* Right function depends upon target state. */
- if (crtc->state->enable)
+ if (crtc->state->enable && funcs->prepare)
funcs->prepare(crtc);
else if (funcs->disable)
funcs->disable(crtc);
@@ -792,8 +792,12 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
funcs = crtc->helper_private;
- if (crtc->state->enable)
- funcs->commit(crtc);
+ if (crtc->state->enable) {
+ if (funcs->enable)
+ funcs->enable(crtc);
+ else
+ funcs->commit(crtc);
+ }
}
for (i = 0; i < old_state->num_connector; i++) {
@@ -819,7 +823,10 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
if (encoder->bridge)
encoder->bridge->funcs->pre_enable(encoder->bridge);
- funcs->commit(encoder);
+ if (funcs->enable)
+ funcs->enable(encoder);
+ else
+ funcs->commit(encoder);
if (encoder->bridge)
encoder->bridge->funcs->enable(encoder->bridge);
--
cgit
From 95d6eb3b134e1826ed04cc92b224d93de13e281f Mon Sep 17 00:00:00 2001
From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu, 22 Jan 2015 16:36:25 +0100
Subject: drm/atomic-helper: debug output for modesets
With the combination of ->enable and ->active it's a bit complicated
to follow what exactly is going on sometimes within a full modeset.
Add debug output to make this all traceable.
Reviewed-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
(limited to 'drivers/gpu/drm/drm_atomic_helper.c')
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 379b6c833bb2..6efb94116237 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -591,6 +591,9 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
funcs = encoder->helper_private;
+ DRM_DEBUG_KMS("disabling [ENCODER:%d:%s]\n",
+ encoder->base.id, encoder->name);
+
/*
* Each encoder has at most one connector (since we always steal
* it away), so we won't call call disable hooks twice.
@@ -627,6 +630,10 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
funcs = crtc->helper_private;
+ DRM_DEBUG_KMS("disabling [CRTC:%d]\n",
+ crtc->base.id);
+
+
/* Right function depends upon target state. */
if (crtc->state->enable && funcs->prepare)
funcs->prepare(crtc);
@@ -707,8 +714,12 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
funcs = crtc->helper_private;
- if (crtc->state->enable)
+ if (crtc->state->enable) {
+ DRM_DEBUG_KMS("modeset on [CRTC:%d]\n",
+ crtc->base.id);
+
funcs->mode_set_nofb(crtc);
+ }
}
for (i = 0; i < old_state->num_connector; i++) {
@@ -732,6 +743,9 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
if (!new_crtc_state->mode_changed)
continue;
+ DRM_DEBUG_KMS("modeset on [ENCODER:%d:%s]\n",
+ encoder->base.id, encoder->name);
+
/*
* Each encoder has at most one connector (since we always steal
* it away), so we won't call call mode_set hooks twice.
@@ -793,6 +807,9 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
funcs = crtc->helper_private;
if (crtc->state->enable) {
+ DRM_DEBUG_KMS("enabling [CRTC:%d]\n",
+ crtc->base.id);
+
if (funcs->enable)
funcs->enable(crtc);
else
@@ -816,6 +833,9 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
encoder = connector->state->best_encoder;
funcs = encoder->helper_private;
+ DRM_DEBUG_KMS("enabling [ENCODER:%d:%s]\n",
+ encoder->base.id, encoder->name);
+
/*
* Each encoder has at most one connector (since we always steal
* it away), so we won't call call enable hooks twice.
--
cgit