Set the mode_changed field on the crtc_states and use that instead.
Note that even though this patch doesn't completely replace the logic in
intel_modeset_affected_pipes(), that logic was never fully used to its
full extent. Since the commit mentioned below, modeset_pipes and
prepare_pipes would only contain at most the pipe for which the set_crtc
ioctl was called. We can grow back that logic when the time comes.
commit b6c5164d7bf624f3e1b750787ddb983150c5117c
Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
Date: Fri Apr 12 18:48:43 2013 +0200
drm/i915: Fixup Oops in the pipe config computation
---
drivers/gpu/drm/i915/i915_drv.h | 8 +
drivers/gpu/drm/i915/intel_display.c | 287 ++++++++++++++---------------------
2 files changed, 126 insertions(+), 169 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e12bc5a..08ea6d0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -272,6 +272,14 @@ enum hpd_pin {
(__i)++) \
if (connector)
+#define for_each_intel_crtc_in_state(state, crtc, crtc_state, __i) \
+ for ((__i) = 0; \
+ (crtc) = to_intel_crtc((state)->crtcs[__i]), \
+ (crtc_state) = to_intel_crtc_state((state)->crtc_states[__i]), \
+ (__i) < (state)->dev->mode_config.num_crtc; \
+ (__i)++) \
+ if (crtc_state)
+
struct drm_i915_private;
struct i915_mm_struct;
struct i915_mmu_object;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 232ef56..a255b24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5179,12 +5179,13 @@ static int intel_mode_max_pixclk(struct drm_atomic_state *state)
return max_pixclk;
}
-static int valleyview_modeset_global_pipes(struct drm_atomic_state *state,
- unsigned *prepare_pipes)
+static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->dev);
struct intel_crtc *intel_crtc;
+ struct intel_crtc_state *crtc_state;
int max_pixclk = intel_mode_max_pixclk(state);
+ int i;
if (max_pixclk < 0)
return max_pixclk;
@@ -5193,10 +5194,20 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state,
dev_priv->vlv_cdclk_freq)
return 0;
+ /* add all active pipes to the state */
+ for_each_intel_crtc(state->dev, intel_crtc) {
+ if (!intel_crtc->base.state->enable)
+ continue;
+
+ crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+ }
+
/* disable/enable all currently active pipes while we change cdclk */
- for_each_intel_crtc(state->dev, intel_crtc)
- if (intel_crtc->base.state->enable)
- *prepare_pipes |= (1 << intel_crtc->pipe);
+ for_each_intel_crtc_in_state(state, intel_crtc, crtc_state, i)
+ if (crtc_state->base.enable)
+ crtc_state->base.mode_changed = true;
return 0;
}
@@ -10879,94 +10890,6 @@ fail:
return ERR_PTR(ret);
}
-/* Computes which crtcs are affected and sets the relevant bits in the mask. For
- * simplicity we use the crtc's pipe number (because it's easier to obtain). */
-static void
-intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
- unsigned *prepare_pipes, unsigned *disable_pipes)
-{
- struct intel_crtc *intel_crtc;
- struct drm_device *dev = crtc->dev;
- struct intel_encoder *encoder;
- struct intel_connector *connector;
- struct drm_crtc *tmp_crtc;
-
- *disable_pipes = *modeset_pipes = *prepare_pipes = 0;
-
- /* Check which crtcs have changed outputs connected to them, these need
- * to be part of the prepare_pipes mask. We don't (yet) support global
- * modeset across multiple crtcs, so modeset_pipes will only have one
- * bit set at most. */
- for_each_intel_connector(dev, connector) {
- if (connector->base.encoder == &connector->new_encoder->base)
- continue;
-
- if (connector->base.encoder) {
- tmp_crtc = connector->base.encoder->crtc;
-
- *prepare_pipes |= 1 << to_intel_crtc(tmp_crtc)->pipe;
- }
-
- if (connector->new_encoder)
- *prepare_pipes |=
- 1 << connector->new_encoder->new_crtc->pipe;
- }
-
- for_each_intel_encoder(dev, encoder) {
- if (encoder->base.crtc == &encoder->new_crtc->base)
- continue;
-
- if (encoder->base.crtc) {
- tmp_crtc = encoder->base.crtc;
-
- *prepare_pipes |= 1 << to_intel_crtc(tmp_crtc)->pipe;
- }
-
- if (encoder->new_crtc)
- *prepare_pipes |= 1 << encoder->new_crtc->pipe;
- }
-
- /* Check for pipes that will be enabled/disabled ... */
- for_each_intel_crtc(dev, intel_crtc) {
- if (intel_crtc->base.state->enable == intel_crtc->new_enabled)
- continue;
-
- if (!intel_crtc->new_enabled)
- *disable_pipes |= 1 << intel_crtc->pipe;
- else
- *prepare_pipes |= 1 << intel_crtc->pipe;
- }
-
-
- /* set_mode is also used to update properties on life display pipes. */
- intel_crtc = to_intel_crtc(crtc);
- if (intel_crtc->new_enabled)
- *prepare_pipes |= 1 << intel_crtc->pipe;
-
- /*
- * For simplicity do a full modeset on any pipe where the output routing
- * changed. We could be more clever, but that would require us to be
- * more careful with calling the relevant encoder->mode_set functions.
- */
- if (*prepare_pipes)
- *modeset_pipes = *prepare_pipes;
-
- /* ... and mask these out. */
- *modeset_pipes &= ~(*disable_pipes);
- *prepare_pipes &= ~(*disable_pipes);
-
- /*
- * HACK: We don't (yet) fully support global modesets. intel_set_config
- * obies this rule, but the modeset restore mode of
- * intel_modeset_setup_hw_state does not.
- */
- *modeset_pipes &= 1 << intel_crtc->pipe;
- *prepare_pipes &= 1 << intel_crtc->pipe;
-
- DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
- *modeset_pipes, *prepare_pipes, *disable_pipes);
-}
-
static bool intel_crtc_in_use(struct drm_crtc *crtc)
{
struct drm_encoder *encoder;
@@ -10979,13 +10902,22 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)
return false;
}
+static bool
+needs_modeset(struct intel_crtc_state *state)
+{
+ return state->base.mode_changed || state->base.active_changed;
+}
+
static void
-intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
+intel_modeset_update_state(struct drm_atomic_state *state)
{
+ struct drm_device *dev = state->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *intel_encoder;
struct intel_crtc *intel_crtc;
+ struct intel_crtc_state *crtc_state;
struct drm_connector *connector;
+ int i;
intel_shared_dpll_commit(dev_priv);
@@ -10993,9 +10925,14 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
if (!intel_encoder->base.crtc)
continue;
- intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
+ for_each_intel_crtc_in_state(state, intel_crtc, crtc_state, i)
+ if (&intel_crtc->base == intel_encoder->base.crtc)
+ break;
+
+ if (&intel_crtc->base != intel_encoder->base.crtc)
+ continue;
- if (prepare_pipes & (1 << intel_crtc->pipe))
+ if (crtc_state->base.enable && needs_modeset(crtc_state))
intel_encoder->connectors_active = false;
}
@@ -11010,9 +10947,14 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
if (!connector->encoder || !connector->encoder->crtc)
continue;
- intel_crtc = to_intel_crtc(connector->encoder->crtc);
+ for_each_intel_crtc_in_state(state, intel_crtc, crtc_state, i)
+ if (&intel_crtc->base == connector->encoder->crtc)
+ break;
+
+ if (&intel_crtc->base != connector->encoder->crtc)
+ continue;
- if (prepare_pipes & (1 << intel_crtc->pipe)) {
+ if (crtc_state->base.enable && needs_modeset(crtc_state)) {
struct drm_property *dpms_property =
dev->mode_config.dpms_property;
@@ -11547,26 +11489,41 @@ static void update_scanline_offset(struct intel_crtc *crtc)
crtc->scanline_offset = 1;
}
+static void
+intel_atomic_modeset_compute_changed_flags(struct drm_atomic_state *state,
+ struct drm_crtc *modeset_crtc)
+{
+ struct intel_crtc_state *crtc_state;
+ struct intel_crtc *crtc;
+ int i;
+
+ for_each_intel_crtc_in_state(state, crtc, crtc_state, i) {
+ if (crtc_state->base.enable != crtc->base.state->enable)
+ crtc_state->base.mode_changed = true;
+
+ /* FIXME: move comment about changing properties in live pipes
+ * from affectec_pipes() here */
+ if (&crtc->base == modeset_crtc && crtc_state->base.enable)
+ crtc_state->base.mode_changed = true;
+ }
+}
+
static struct intel_crtc_state *
intel_modeset_compute_config(struct drm_crtc *crtc,
struct drm_display_mode *mode,
struct drm_framebuffer *fb,
- struct drm_atomic_state *state,
- unsigned *modeset_pipes,
- unsigned *prepare_pipes,
- unsigned *disable_pipes)
+ struct drm_atomic_state *state)
{
- struct drm_device *dev = crtc->dev;
- struct intel_crtc_state *pipe_config = NULL;
+ struct intel_crtc_state *crtc_state;
struct intel_crtc *intel_crtc;
int ret = 0;
+ int i;
ret = drm_atomic_add_affected_connectors(state, crtc);
if (ret)
return ERR_PTR(ret);
- intel_modeset_affected_pipes(crtc, modeset_pipes,
- prepare_pipes, disable_pipes);
+ intel_atomic_modeset_compute_changed_flags(state, crtc);
/*
* Note this needs changes when we start tracking multiple modes
@@ -11574,46 +11531,49 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
* (i.e. one pipe_config for each crtc) rather than just the one
* for this crtc.
*/
- for_each_intel_crtc_masked(dev, *modeset_pipes, intel_crtc) {
- /* FIXME: For now we still expect modeset_pipes has at most
- * one bit set. */
+ for_each_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
+ if (!crtc_state->base.enable || !needs_modeset(crtc_state))
+ continue;
+
+ /* FIXME: For now we still expect modeset on only one pipe. */
if (WARN_ON(&intel_crtc->base != crtc))
continue;
- pipe_config = intel_modeset_pipe_config(crtc, fb, mode, state);
- if (IS_ERR(pipe_config))
- return pipe_config;
+ crtc_state = intel_modeset_pipe_config(crtc, fb, mode, state);
+ if (IS_ERR(crtc_state))
+ return crtc_state;
- intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
+ intel_dump_pipe_config(to_intel_crtc(crtc), crtc_state,
"[modeset]");
}
return intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));;
}
-static int __intel_set_mode_setup_plls(struct drm_atomic_state *state,
- unsigned modeset_pipes,
- unsigned disable_pipes)
+static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- unsigned clear_pipes = modeset_pipes | disable_pipes;
+ unsigned clear_pipes = 0;
struct intel_crtc *intel_crtc;
+ struct intel_crtc_state *crtc_state;
int ret = 0;
+ int i;
if (!dev_priv->display.crtc_compute_clock)
return 0;
+ for_each_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
+ if (needs_modeset(crtc_state))
+ clear_pipes |= 1 << intel_crtc->pipe;
+ }
+
ret = intel_shared_dpll_start_config(dev_priv, clear_pipes);
if (ret)
goto done;
- for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
- struct intel_crtc_state *crtc_state =
- intel_atomic_get_crtc_state(state, intel_crtc);
-
- /* Modeset pipes should have a new state by now */
- if (WARN_ON(IS_ERR(crtc_state)))
+ for_each_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
+ if (!crtc_state->base.enable)
continue;
ret = dev_priv->display.crtc_compute_clock(intel_crtc,
@@ -11631,10 +11591,7 @@ done:
static int __intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
int x, int y, struct drm_framebuffer *fb,
- struct intel_crtc_state *pipe_config,
- unsigned modeset_pipes,
- unsigned prepare_pipes,
- unsigned disable_pipes)
+ struct intel_crtc_state *pipe_config)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -11642,7 +11599,9 @@ static int __intel_set_mode(struct drm_crtc *crtc,
struct drm_atomic_state *state = pipe_config->base.state;
struct intel_crtc_state *crtc_state_copy = NULL;
struct intel_crtc *intel_crtc;
+ struct intel_crtc_state *crtc_state;
int ret = 0;
+ int i;
saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
if (!saved_mode)
@@ -11664,23 +11623,22 @@ static int __intel_set_mode(struct drm_crtc *crtc,
* adjusted_mode bits in the crtc directly.
*/
if (IS_VALLEYVIEW(dev)) {
- ret = valleyview_modeset_global_pipes(state, &prepare_pipes);
+ ret = valleyview_modeset_global_pipes(state);
if (ret)
goto done;
-
- /* may have added more to prepare_pipes than we should */
- prepare_pipes &= ~disable_pipes;
}
- ret = __intel_set_mode_setup_plls(state, modeset_pipes, disable_pipes);
+ ret = __intel_set_mode_setup_plls(state);
if (ret)
goto done;
- for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
- intel_crtc_disable(&intel_crtc->base);
+ for_each_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
+ if (!needs_modeset(crtc_state))
+ continue;
- for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
- if (intel_crtc->base.state->enable)
+ if (!crtc_state->base.enable)
+ intel_crtc_disable(&intel_crtc->base);
+ else if (intel_crtc->base.state->enable)
dev_priv->display.crtc_disable(&intel_crtc->base);
}
@@ -11691,7 +11649,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
* pipes; here we assume a single modeset_pipe and only track the
* single crtc and mode.
*/
- if (modeset_pipes) {
+ if (pipe_config->base.enable && needs_modeset(pipe_config)) {
crtc->mode = *mode;
/* mode_set/enable/disable functions rely on a correct pipe
* config. */
@@ -11708,16 +11666,20 @@ static int __intel_set_mode(struct drm_crtc *crtc,
/* Only after disabling all output pipelines that will be changed can we
* update the the output configuration. */
- intel_modeset_update_state(dev, prepare_pipes);
+ intel_modeset_update_state(state);
modeset_update_crtc_power_domains(state);
- for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
- struct drm_plane *primary = intel_crtc->base.primary;
+ if (pipe_config->base.enable && needs_modeset(pipe_config)) {
+ struct drm_plane *primary;
int vdisplay, hdisplay;
+ intel_crtc = to_intel_crtc(crtc);
+ primary = intel_crtc->base.primary;
+
drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
- ret = primary->funcs->update_plane(primary, &intel_crtc->base,
+
+ ret = primary->funcs->update_plane(primary, crtc,
fb, 0, 0,
hdisplay, vdisplay,
x << 16, y << 16,
@@ -11725,7 +11687,10 @@ static int __intel_set_mode(struct drm_crtc *crtc,
}
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
- for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
+ for_each_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
+ if (!crtc_state->base.enable)
+ continue;
+
update_scanline_offset(intel_crtc);
dev_priv->display.crtc_enable(&intel_crtc->base);
@@ -11753,18 +11718,14 @@ done:
return ret;
}
-static int intel_set_mode_pipes(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
- int x, int y, struct drm_framebuffer *fb,
- struct intel_crtc_state *pipe_config,
- unsigned modeset_pipes,
- unsigned prepare_pipes,
- unsigned disable_pipes)
+static int intel_set_mode_with_config(struct drm_crtc *crtc,
+ struct drm_display_mode *mode,
+ int x, int y, struct drm_framebuffer *fb,
+ struct intel_crtc_state *pipe_config)
{
int ret;
- ret = __intel_set_mode(crtc, mode, x, y, fb, pipe_config, modeset_pipes,
- prepare_pipes, disable_pipes);
+ ret = __intel_set_mode(crtc, mode, x, y, fb, pipe_config);
if (ret == 0)
intel_modeset_check_state(crtc->dev);
@@ -11778,22 +11739,15 @@ static int intel_set_mode(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
struct intel_crtc_state *pipe_config;
- unsigned modeset_pipes, prepare_pipes, disable_pipes;
int ret = 0;
- pipe_config = intel_modeset_compute_config(crtc, mode, fb, state,
- &modeset_pipes,
- &prepare_pipes,
- &disable_pipes);
-
+ pipe_config = intel_modeset_compute_config(crtc, mode, fb, state);
if (IS_ERR(pipe_config)) {
ret = PTR_ERR(pipe_config);
goto out;
}
- ret = intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
- modeset_pipes, prepare_pipes,
- disable_pipes);
+ ret = intel_set_mode_with_config(crtc, mode, x, y, fb, pipe_config);
if (ret)
goto out;
@@ -12217,7 +12171,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
struct drm_atomic_state *state = NULL;
struct intel_set_config *config;
struct intel_crtc_state *pipe_config;
- unsigned modeset_pipes, prepare_pipes, disable_pipes;
int ret;
BUG_ON(!set);
@@ -12272,10 +12225,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
intel_set_config_compute_mode_changes(set, config);
pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
- set->fb, state,
- &modeset_pipes,
- &prepare_pipes,
- &disable_pipes);
+ set->fb, state);
if (IS_ERR(pipe_config)) {
ret = PTR_ERR(pipe_config);
goto fail;
@@ -12295,10 +12245,9 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
intel_update_pipe_size(to_intel_crtc(set->crtc));
if (config->mode_changed) {
- ret = intel_set_mode_pipes(set->crtc, set->mode,
- set->x, set->y, set->fb, pipe_config,
- modeset_pipes, prepare_pipes,
- disable_pipes);
+ ret = intel_set_mode_with_config(set->crtc, set->mode,
+ set->x, set->y, set->fb,
+ pipe_config);
} else if (config->fb_changed) {
struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
struct drm_plane *primary = set->crtc->primary;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
