drm/i915: stage modeset output changes
This is the core of the new modeset logic.
The current code which is based upon the crtc helper code first
updates all the link of the new display pipeline and then calls the
lower-level set_mode function to execute the required callbacks to get
there. The issue with this approach is that for disabling we need to
know the _current_ display pipe state, not the new one.
Hence we need to stage the new state of the display pipe and only
update it once we have disabled the current configuration and before we
start to update the hw registers with the new configuration.
This patch here just prepares the ground by switching the new output
state computation to these staging pointers. To make it clearer,
rename the old update_output_state function to stage_output_state.
A few peculiarities:
- We're also calling the set_mode function at various places to update
properties. Hence after a successfule modeset we need to stage the
current configuration (for otherwise we might fall back again). This
happens automatically because as part of the (successful) modeset we
need to copy the staged state to the real one. But for the hw
readout code we need to make sure that this happens, too.
- Teach the new staged output state computation code the required
smarts to handle the disabling of outputs. The current code handles
this in a special case, but to better handle global modeset changes
covering more than one crtc, we want to do this all in the same
low-level modeset code.
- The actual modeset code is still a bit ugly and wants to know the new
crtc->enabled state a bit early. Follow-on patches will clean that
up, for now we have to apply the staged output configuration early,
outside of the set_mode functions.
- Improve/add comments in stage_output_state.
Essentially all that is left to do now is move the disabling code into
set_mode and then move the staged state update code also into
set_mode, at the right place between disabling things and calling the
mode_set callbacks for the new configuration.
v2: Disabling a crtc works by passing in a NULL mode or fb, userspace
doesn't hand in the list of connectors. We therefore need to detect
this case manually and tear down all the output links.
v3: Properly update the output staging pointers after having read out
the hw state.
v4: Simplify the code, add more DRM_DEBUG_KMS output and check a few
assumptions with WARN_ON. Essentially all things that I've noticed
while debugging issues in other places of the code.
v4: Correctly disable the old set of connectors when enabling an
already enabled crtc on a new set of crtc. Reported by Paulo Zanoni.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a2ce117..5031e0c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6619,6 +6619,51 @@
}
}
+/**
+ * intel_modeset_update_staged_output_state
+ *
+ * Updates the staged output configuration state, e.g. after we've read out the
+ * current hw state.
+ */
+static void intel_modeset_update_staged_output_state(struct drm_device *dev)
+{
+ struct intel_encoder *encoder;
+ struct intel_connector *connector;
+
+ list_for_each_entry(connector, &dev->mode_config.connector_list,
+ base.head) {
+ connector->new_encoder =
+ to_intel_encoder(connector->base.encoder);
+ }
+
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list,
+ base.head) {
+ encoder->new_crtc =
+ to_intel_crtc(encoder->base.crtc);
+ }
+}
+
+/**
+ * intel_modeset_commit_output_state
+ *
+ * This function copies the stage display pipe configuration to the real one.
+ */
+static void intel_modeset_commit_output_state(struct drm_device *dev)
+{
+ struct intel_encoder *encoder;
+ struct intel_connector *connector;
+
+ list_for_each_entry(connector, &dev->mode_config.connector_list,
+ base.head) {
+ connector->base.encoder = &connector->new_encoder->base;
+ }
+
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list,
+ base.head) {
+ encoder->base.crtc = &encoder->new_crtc->base;
+ }
+}
+
bool intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
int x, int y, struct drm_framebuffer *old_fb)
@@ -6785,8 +6830,8 @@
struct intel_set_config *config)
{
struct drm_crtc *crtc;
- struct drm_encoder *encoder;
- struct drm_connector *connector;
+ struct intel_encoder *encoder;
+ struct intel_connector *connector;
int count;
count = 0;
@@ -6795,13 +6840,15 @@
}
count = 0;
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
- encoder->crtc = config->save_encoder_crtcs[count++];
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
+ encoder->new_crtc =
+ to_intel_crtc(config->save_encoder_crtcs[count++]);
}
count = 0;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
- connector->encoder = config->save_connector_encoders[count++];
+ list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) {
+ connector->new_encoder =
+ to_intel_encoder(config->save_connector_encoders[count++]);
}
}
@@ -6840,73 +6887,106 @@
}
static int
-intel_set_config_update_output_state(struct drm_device *dev,
- struct drm_mode_set *set,
- struct intel_set_config *config)
+intel_modeset_stage_output_state(struct drm_device *dev,
+ struct drm_mode_set *set,
+ struct intel_set_config *config)
{
struct drm_crtc *new_crtc;
- struct drm_encoder *new_encoder;
- struct drm_connector *connector;
+ struct intel_connector *connector;
+ struct intel_encoder *encoder;
int count, ro;
- /* a) traverse passed in connector list and get encoders for them */
+ /* The upper layers ensure that we either disabl a crtc or have a list
+ * of connectors. For paranoia, double-check this. */
+ WARN_ON(!set->fb && (set->num_connectors != 0));
+ WARN_ON(set->fb && (set->num_connectors == 0));
+
count = 0;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
- new_encoder = connector->encoder;
+ list_for_each_entry(connector, &dev->mode_config.connector_list,
+ base.head) {
+ /* Otherwise traverse passed in connector list and get encoders
+ * for them. */
for (ro = 0; ro < set->num_connectors; ro++) {
- if (set->connectors[ro] == connector) {
- new_encoder =
- &intel_attached_encoder(connector)->base;
+ if (set->connectors[ro] == &connector->base) {
+ connector->new_encoder = connector->encoder;
break;
}
}
- if (new_encoder != connector->encoder) {
+ /* If we disable the crtc, disable all its connectors. Also, if
+ * the connector is on the changing crtc but not on the new
+ * connector list, disable it. */
+ if ((!set->fb || ro == set->num_connectors) &&
+ connector->base.encoder &&
+ connector->base.encoder->crtc == set->crtc) {
+ connector->new_encoder = NULL;
+
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [NOCRTC]\n",
+ connector->base.base.id,
+ drm_get_connector_name(&connector->base));
+ }
+
+
+ if (&connector->new_encoder->base != connector->base.encoder) {
DRM_DEBUG_KMS("encoder changed, full mode switch\n");
config->mode_changed = true;
- /* If the encoder is reused for another connector, then
- * the appropriate crtc will be set later.
- */
- if (connector->encoder)
- connector->encoder->crtc = NULL;
- connector->encoder = new_encoder;
}
- }
+ /* Disable all disconnected encoders. */
+ if (connector->base.status == connector_status_disconnected)
+ connector->new_encoder = NULL;
+ }
+ /* connector->new_encoder is now updated for all connectors. */
+
+ /* Update crtc of enabled connectors. */
count = 0;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
- if (!connector->encoder)
+ list_for_each_entry(connector, &dev->mode_config.connector_list,
+ base.head) {
+ if (!connector->new_encoder)
continue;
- if (connector->encoder->crtc == set->crtc)
- new_crtc = NULL;
- else
- new_crtc = connector->encoder->crtc;
+ new_crtc = connector->new_encoder->base.crtc;
for (ro = 0; ro < set->num_connectors; ro++) {
- if (set->connectors[ro] == connector)
+ if (set->connectors[ro] == &connector->base)
new_crtc = set->crtc;
}
/* Make sure the new CRTC will work with the encoder */
- if (new_crtc &&
- !intel_encoder_crtc_ok(connector->encoder, new_crtc)) {
+ if (!intel_encoder_crtc_ok(&connector->new_encoder->base,
+ new_crtc)) {
return -EINVAL;
}
- if (new_crtc != connector->encoder->crtc) {
+ connector->encoder->new_crtc = to_intel_crtc(new_crtc);
+
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
+ connector->base.base.id,
+ drm_get_connector_name(&connector->base),
+ new_crtc->base.id);
+ }
+
+ /* Check for any encoders that needs to be disabled. */
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list,
+ base.head) {
+ list_for_each_entry(connector,
+ &dev->mode_config.connector_list,
+ base.head) {
+ if (connector->new_encoder == encoder) {
+ WARN_ON(!connector->new_encoder->new_crtc);
+
+ goto next_encoder;
+ }
+ }
+ encoder->new_crtc = NULL;
+next_encoder:
+ /* Only now check for crtc changes so we don't miss encoders
+ * that will be disabled. */
+ if (&encoder->new_crtc->base != encoder->base.crtc) {
DRM_DEBUG_KMS("crtc changed, full mode switch\n");
config->mode_changed = true;
- connector->encoder->crtc = new_crtc;
- }
- if (new_crtc) {
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
- connector->base.id, drm_get_connector_name(connector),
- new_crtc->base.id);
- } else {
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [NOCRTC]\n",
- connector->base.id, drm_get_connector_name(connector));
}
}
+ /* Now we've also updated encoder->new_crtc for all encoders. */
return 0;
}
@@ -6965,11 +7045,13 @@
* such cases. */
intel_set_config_compute_mode_changes(set, config);
- ret = intel_set_config_update_output_state(dev, set, config);
+ ret = intel_modeset_stage_output_state(dev, set, config);
if (ret)
goto fail;
if (config->mode_changed) {
+ intel_modeset_commit_output_state(dev);
+
set->crtc->enabled = drm_helper_crtc_in_use(set->crtc);
if (set->crtc->enabled) {
DRM_DEBUG_KMS("attempting to set mode from"
@@ -7015,6 +7097,8 @@
fail:
intel_set_config_restore_state(dev, config);
+ intel_modeset_commit_output_state(dev);
+
/* Try to restore the config */
if (config->mode_changed &&
!intel_set_mode(save_set.crtc, save_set.mode,
@@ -7888,6 +7972,8 @@
crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
intel_sanitize_crtc(crtc);
}
+
+ intel_modeset_update_staged_output_state(dev);
}
void intel_modeset_gem_init(struct drm_device *dev)