[Intel-gfx] [PATCH 1/2] drm/i915: Use enabled value from crtc_state rather than crtc (v2)
Matt Roper
matthew.d.roper at intel.com
Wed Feb 25 13:12:16 PST 2015
As vendors transition their drivers from legacy to atomic there's some
duplication of data between drm_crtc and drm_crtc_state (since
unconverted drivers likely won't have a state structure).
i915 is partially converted and does have a crtc->state structure, but
still uses direct crtc fields internally in many places, which causes
the two sets of data to get out of sync. As of commit
commit 31c946e85ce6b48ce0f25e3cdca8362e4fe8b300
Author: Daniel Vetter <daniel.vetter at ffwll.ch>
Date: Sun Feb 22 12:24:17 2015 +0100
drm: If available use atomic state in getcrtc ioctl
This way drivers fully converted to atomic don't need to update these
legacy state variables in their modeset code any more.
Reviewed-by: Rob Clark <robdclark at gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
the DRM core starts assuming that the presence of a ->state structure
implies that it should make use of the values stored there which, on
i915, leads to the core code using stale values for CRTC 'enabled'
status.
Let's switch over to using the state value of 'enable' internally rather
than using the drm_crtc field. This ensures that our driver internals
are working from the same data that the DRM core is, avoiding
mismatches.
This patch was generated with Coccinelle using the following semantic
patch:
<smpl>
@@
struct drm_crtc C;
struct drm_crtc *CP;
@@
(
- C.enabled
+ C.state->enable
|
- CP->enabled
+ CP->state->enable
)
// For assignments, we still update the legacy value as well as the state value
// so add an extra assignment statement for that.
@@
struct drm_crtc C;
struct drm_crtc *CP;
expression E;
@@
(
C.state->enable = E;
+ C.enabled = E;
|
CP->state->enable = E;
+ CP->enabled = E;
)
</smpl>
The crtc->mode and crtc->hwmode fields should probably be transitioned
over as well eventually, but we seem to do an okay job of keeping those
up-to-date already so I want to minimize the changes that will clash
with Ander's in-progress atomic work.
v2: Don't remove the assignments to the legacy value when we assign to
the state value. A second cocci stanza takes care of adding the
legacy assignment back where appropriate. (Daniel)
Cc: Daniel Vetter <daniel at ffwll.ch>
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 58 ++++++++++++++++++++----------------
drivers/gpu/drm/i915/intel_lvds.c | 2 +-
3 files changed, 34 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 07e257c..9baecb7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -791,7 +791,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
return -EINVAL;
}
- if (!crtc->enabled) {
+ if (!crtc->state->enable) {
DRM_DEBUG_KMS("crtc %d is disabled\n", pipe);
return -EBUSY;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9bc54cc..acbb362 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3077,7 +3077,7 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
{
- return crtc->base.enabled && crtc->active &&
+ return crtc->base.state->enable && crtc->active &&
crtc->config->has_pch_encoder;
}
@@ -4215,7 +4215,7 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc)
bool reenable_ips = false;
/* The clocks have to be on to load the palette. */
- if (!crtc->enabled || !intel_crtc->active)
+ if (!crtc->state->enable || !intel_crtc->active)
return;
if (!HAS_PCH_SPLIT(dev_priv->dev)) {
@@ -4328,7 +4328,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc->pipe;
- WARN_ON(!crtc->enabled);
+ WARN_ON(!crtc->state->enable);
if (intel_crtc->active)
return;
@@ -4436,7 +4436,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc->pipe;
- WARN_ON(!crtc->enabled);
+ WARN_ON(!crtc->state->enable);
if (intel_crtc->active)
return;
@@ -4783,7 +4783,7 @@ static void modeset_update_crtc_power_domains(struct drm_device *dev)
for_each_intel_crtc(dev, crtc) {
enum intel_display_power_domain domain;
- if (!crtc->base.enabled)
+ if (!crtc->base.state->enable)
continue;
pipe_domains[crtc->pipe] = get_crtc_power_domains(&crtc->base);
@@ -5004,7 +5004,7 @@ static void valleyview_modeset_global_pipes(struct drm_device *dev,
/* disable/enable all currently active pipes while we change cdclk */
for_each_intel_crtc(dev, intel_crtc)
- if (intel_crtc->base.enabled)
+ if (intel_crtc->base.state->enable)
*prepare_pipes |= (1 << intel_crtc->pipe);
}
@@ -5044,7 +5044,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
int pipe = intel_crtc->pipe;
bool is_dsi;
- WARN_ON(!crtc->enabled);
+ WARN_ON(!crtc->state->enable);
if (intel_crtc->active)
return;
@@ -5127,7 +5127,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc->pipe;
- WARN_ON(!crtc->enabled);
+ WARN_ON(!crtc->state->enable);
if (intel_crtc->active)
return;
@@ -5326,7 +5326,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
/* crtc should still be enabled when we disable it. */
- WARN_ON(!crtc->enabled);
+ WARN_ON(!crtc->state->enable);
dev_priv->display.crtc_disable(crtc);
dev_priv->display.off(crtc);
@@ -5404,7 +5404,8 @@ static void intel_connector_check_state(struct intel_connector *connector)
crtc = encoder->base.crtc;
- I915_STATE_WARN(!crtc->enabled, "crtc not enabled\n");
+ I915_STATE_WARN(!crtc->state->enable,
+ "crtc not enabled\n");
I915_STATE_WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
I915_STATE_WARN(pipe != to_intel_crtc(crtc)->pipe,
"encoder active on the wrong pipe\n");
@@ -8717,7 +8718,7 @@ retry:
i++;
if (!(encoder->possible_crtcs & (1 << i)))
continue;
- if (possible_crtc->enabled)
+ if (possible_crtc->state->enable)
continue;
/* This can occur when applying the pipe A quirk on resume. */
if (to_intel_crtc(possible_crtc)->new_enabled)
@@ -8785,7 +8786,7 @@ retry:
return true;
fail:
- intel_crtc->new_enabled = crtc->enabled;
+ intel_crtc->new_enabled = crtc->state->enable;
if (intel_crtc->new_enabled)
intel_crtc->new_config = intel_crtc->config;
else
@@ -9945,7 +9946,7 @@ static void intel_modeset_update_staged_output_state(struct drm_device *dev)
}
for_each_intel_crtc(dev, crtc) {
- crtc->new_enabled = crtc->base.enabled;
+ crtc->new_enabled = crtc->base.state->enable;
if (crtc->new_enabled)
crtc->new_config = crtc->config;
@@ -9975,6 +9976,7 @@ static void intel_modeset_commit_output_state(struct drm_device *dev)
}
for_each_intel_crtc(dev, crtc) {
+ crtc->base.state->enable = crtc->new_enabled;
crtc->base.enabled = crtc->new_enabled;
}
}
@@ -10386,7 +10388,7 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
/* Check for pipes that will be enabled/disabled ... */
for_each_intel_crtc(dev, intel_crtc) {
- if (intel_crtc->base.enabled == intel_crtc->new_enabled)
+ if (intel_crtc->base.state->enable == intel_crtc->new_enabled)
continue;
if (!intel_crtc->new_enabled)
@@ -10461,10 +10463,10 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
/* Double check state. */
for_each_intel_crtc(dev, intel_crtc) {
- WARN_ON(intel_crtc->base.enabled != intel_crtc_in_use(&intel_crtc->base));
+ WARN_ON(intel_crtc->base.state->enable != intel_crtc_in_use(&intel_crtc->base));
WARN_ON(intel_crtc->new_config &&
intel_crtc->new_config != intel_crtc->config);
- WARN_ON(intel_crtc->base.enabled != !!intel_crtc->new_config);
+ WARN_ON(intel_crtc->base.state->enable != !!intel_crtc->new_config);
}
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -10851,7 +10853,7 @@ check_crtc_state(struct drm_device *dev)
DRM_DEBUG_KMS("[CRTC:%d]\n",
crtc->base.base.id);
- I915_STATE_WARN(crtc->active && !crtc->base.enabled,
+ I915_STATE_WARN(crtc->active && !crtc->base.state->enable,
"active crtc, but not enabled in sw tracking\n");
for_each_intel_encoder(dev, encoder) {
@@ -10865,9 +10867,10 @@ check_crtc_state(struct drm_device *dev)
I915_STATE_WARN(active != crtc->active,
"crtc's computed active state doesn't match tracked active state "
"(expected %i, found %i)\n", active, crtc->active);
- I915_STATE_WARN(enabled != crtc->base.enabled,
+ I915_STATE_WARN(enabled != crtc->base.state->enable,
"crtc's computed enabled state doesn't match tracked enabled state "
- "(expected %i, found %i)\n", enabled, crtc->base.enabled);
+ "(expected %i, found %i)\n", enabled,
+ crtc->base.state->enable);
active = dev_priv->display.get_pipe_config(crtc,
&pipe_config);
@@ -10931,7 +10934,7 @@ check_shared_dpll_state(struct drm_device *dev)
pll->on, active);
for_each_intel_crtc(dev, crtc) {
- if (crtc->base.enabled && intel_crtc_to_shared_dpll(crtc) == pll)
+ if (crtc->base.state->enable && intel_crtc_to_shared_dpll(crtc) == pll)
enabled_crtcs++;
if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll)
active_crtcs++;
@@ -11117,7 +11120,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
intel_crtc_disable(&intel_crtc->base);
for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
- if (intel_crtc->base.enabled)
+ if (intel_crtc->base.state->enable)
dev_priv->display.crtc_disable(&intel_crtc->base);
}
@@ -11173,7 +11176,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
/* FIXME: add subpixel order */
done:
- if (ret && crtc->enabled)
+ if (ret && crtc->state->enable)
crtc->mode = *saved_mode;
kfree(saved_mode);
@@ -11269,7 +11272,7 @@ static int intel_set_config_save_state(struct drm_device *dev,
*/
count = 0;
for_each_crtc(dev, crtc) {
- config->save_crtc_enabled[count++] = crtc->enabled;
+ config->save_crtc_enabled[count++] = crtc->state->enable;
}
count = 0;
@@ -11503,7 +11506,7 @@ intel_modeset_stage_output_state(struct drm_device *dev,
}
}
- if (crtc->new_enabled != crtc->base.enabled) {
+ if (crtc->new_enabled != crtc->base.state->enable) {
DRM_DEBUG_KMS("crtc %sabled, full mode switch\n",
crtc->new_enabled ? "en" : "dis");
config->mode_changed = true;
@@ -13392,6 +13395,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
}
WARN_ON(crtc->active);
+ crtc->base.state->enable = false;
crtc->base.enabled = false;
}
@@ -13408,7 +13412,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
* have active connectors/encoders. */
intel_crtc_update_dpms(&crtc->base);
- if (crtc->active != crtc->base.enabled) {
+ if (crtc->active != crtc->base.state->enable) {
struct intel_encoder *encoder;
/* This can happen either due to bugs in the get_hw_state
@@ -13416,9 +13420,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
* pipe A quirk. */
DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was %s, now %s\n",
crtc->base.base.id,
- crtc->base.enabled ? "enabled" : "disabled",
+ crtc->base.state->enable ? "enabled" : "disabled",
crtc->active ? "enabled" : "disabled");
+ crtc->base.state->enable = crtc->active;
crtc->base.enabled = crtc->active;
/* Because we only establish the connector -> encoder ->
@@ -13555,6 +13560,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
crtc->active = dev_priv->display.get_pipe_config(crtc,
crtc->config);
+ crtc->base.state->enable = crtc->active;
crtc->base.enabled = crtc->active;
crtc->primary_enabled = primary_get_hw_state(crtc);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 071b96d..24e8730 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -509,7 +509,7 @@ static int intel_lvds_set_property(struct drm_connector *connector,
intel_connector->panel.fitting_mode = value;
crtc = intel_attached_encoder(connector)->base.crtc;
- if (crtc && crtc->enabled) {
+ if (crtc && crtc->state->enable) {
/*
* If the CRTC is enabled, the display will be changed
* according to the new panel fitting mode.
--
1.8.5.1
More information about the Intel-gfx
mailing list