* [PATCH v3 11/22] drm/i915: Use global atomic state for staged pll config, v2.
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 12/22] drm/i915: Use drm_atomic_helper_swap_state in intel_atomic_commit maarten.lankhorst
` (10 subsequent siblings)
11 siblings, 0 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Now that we can subclass drm_atomic_state we can also use it to keep
track of all the pll settings. atomic_state is a better place to hold
all shared state than keeping pll->new_config everywhere.
Changes since v1:
- Assert connection_mutex is held.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_atomic.c | 51 ++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 111 +++++++++++------------------------
drivers/gpu/drm/i915/intel_drv.h | 13 ++++
4 files changed, 97 insertions(+), 79 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 37e2312b547c..62e15a3d2984 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -319,7 +319,6 @@ struct intel_shared_dpll_config {
struct intel_shared_dpll {
struct intel_shared_dpll_config config;
- struct intel_shared_dpll_config *new_config;
int active; /* count of number of active CRTCs (i.e. DPMS on) */
bool on; /* is the PLL actually active? Disabled during modeset */
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 7ed8033aae60..6ab82dc56cea 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -421,3 +421,54 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
return 0;
}
+
+static void
+intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
+ struct intel_shared_dpll_config *shared_dpll)
+{
+ enum intel_dpll_id i;
+
+ /* Copy shared dpll state */
+ for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+ struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
+
+ shared_dpll[i] = pll->config;
+ }
+}
+
+struct intel_shared_dpll_config *
+intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s)
+{
+ struct intel_atomic_state *state = to_intel_atomic_state(s);
+
+ WARN_ON(!drm_modeset_is_locked(&s->dev->mode_config.connection_mutex));
+
+ if (!state->dpll_set) {
+ state->dpll_set = true;
+
+ intel_atomic_duplicate_dpll_state(to_i915(s->dev),
+ state->shared_dpll);
+ }
+
+ return state->shared_dpll;
+}
+
+struct drm_atomic_state *
+intel_atomic_state_alloc(struct drm_device *dev)
+{
+ struct intel_atomic_state *state = kzalloc(GFP_KERNEL, sizeof(*state));
+
+ if (!state || drm_atomic_state_init(dev, &state->base) < 0) {
+ kfree(state);
+ return NULL;
+ }
+
+ return &state->base;
+}
+
+void intel_atomic_state_clear(struct drm_atomic_state *s)
+{
+ struct intel_atomic_state *state = to_intel_atomic_state(s);
+ drm_atomic_state_default_clear(&state->base);
+ state->dpll_set = false;
+}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 652865e21bc5..adb19c3c9172 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4204,8 +4204,11 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
{
struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
struct intel_shared_dpll *pll;
+ struct intel_shared_dpll_config *shared_dpll;
enum intel_dpll_id i;
+ shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
+
if (HAS_PCH_IBX(dev_priv->dev)) {
/* Ironlake PCH has a fixed PLL->PCH pipe mapping. */
i = (enum intel_dpll_id) crtc->pipe;
@@ -4214,7 +4217,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n",
crtc->base.base.id, pll->name);
- WARN_ON(pll->new_config->crtc_mask);
+ WARN_ON(shared_dpll[i].crtc_mask);
goto found;
}
@@ -4234,7 +4237,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
pll = &dev_priv->shared_dplls[i];
DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n",
crtc->base.base.id, pll->name);
- WARN_ON(pll->new_config->crtc_mask);
+ WARN_ON(shared_dpll[i].crtc_mask);
goto found;
}
@@ -4243,15 +4246,15 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
pll = &dev_priv->shared_dplls[i];
/* Only want to check enabled timings first */
- if (pll->new_config->crtc_mask == 0)
+ if (shared_dpll[i].crtc_mask == 0)
continue;
if (memcmp(&crtc_state->dpll_hw_state,
- &pll->new_config->hw_state,
- sizeof(pll->new_config->hw_state)) == 0) {
+ &shared_dpll[i].hw_state,
+ sizeof(crtc_state->dpll_hw_state)) == 0) {
DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, ative %d)\n",
crtc->base.base.id, pll->name,
- pll->new_config->crtc_mask,
+ shared_dpll[i].crtc_mask,
pll->active);
goto found;
}
@@ -4260,7 +4263,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
/* Ok no matching timings, maybe there's a free one? */
for (i = 0; i < dev_priv->num_shared_dpll; i++) {
pll = &dev_priv->shared_dplls[i];
- if (pll->new_config->crtc_mask == 0) {
+ if (shared_dpll[i].crtc_mask == 0) {
DRM_DEBUG_KMS("CRTC:%d allocated %s\n",
crtc->base.base.id, pll->name);
goto found;
@@ -4270,83 +4273,33 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
return NULL;
found:
- if (pll->new_config->crtc_mask == 0)
- pll->new_config->hw_state = crtc_state->dpll_hw_state;
+ if (shared_dpll[i].crtc_mask == 0)
+ shared_dpll[i].hw_state =
+ crtc_state->dpll_hw_state;
crtc_state->shared_dpll = i;
DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
pipe_name(crtc->pipe));
- pll->new_config->crtc_mask |= 1 << crtc->pipe;
+ shared_dpll[i].crtc_mask |= 1 << crtc->pipe;
return pll;
}
-/**
- * intel_shared_dpll_start_config - start a new PLL staged config
- * @dev_priv: DRM device
- * @clear_pipes: mask of pipes that will have their PLLs freed
- *
- * Starts a new PLL staged config, copying the current config but
- * releasing the references of pipes specified in clear_pipes.
- */
-static int intel_shared_dpll_start_config(struct drm_i915_private *dev_priv,
- unsigned clear_pipes)
-{
- struct intel_shared_dpll *pll;
- enum intel_dpll_id i;
-
- for (i = 0; i < dev_priv->num_shared_dpll; i++) {
- pll = &dev_priv->shared_dplls[i];
-
- pll->new_config = kmemdup(&pll->config, sizeof pll->config,
- GFP_KERNEL);
- if (!pll->new_config)
- goto cleanup;
-
- pll->new_config->crtc_mask &= ~clear_pipes;
- }
-
- return 0;
-
-cleanup:
- while (--i >= 0) {
- pll = &dev_priv->shared_dplls[i];
- kfree(pll->new_config);
- pll->new_config = NULL;
- }
-
- return -ENOMEM;
-}
-
-static void intel_shared_dpll_commit(struct drm_i915_private *dev_priv)
+static void intel_shared_dpll_commit(struct drm_atomic_state *state)
{
+ struct drm_i915_private *dev_priv = to_i915(state->dev);
+ struct intel_shared_dpll_config *shared_dpll;
struct intel_shared_dpll *pll;
enum intel_dpll_id i;
- for (i = 0; i < dev_priv->num_shared_dpll; i++) {
- pll = &dev_priv->shared_dplls[i];
-
- WARN_ON(pll->new_config == &pll->config);
-
- pll->config = *pll->new_config;
- kfree(pll->new_config);
- pll->new_config = NULL;
- }
-}
-
-static void intel_shared_dpll_abort_config(struct drm_i915_private *dev_priv)
-{
- struct intel_shared_dpll *pll;
- enum intel_dpll_id i;
+ if (!to_intel_atomic_state(state)->dpll_set)
+ return;
+ shared_dpll = to_intel_atomic_state(state)->shared_dpll;
for (i = 0; i < dev_priv->num_shared_dpll; i++) {
pll = &dev_priv->shared_dplls[i];
-
- WARN_ON(pll->new_config == &pll->config);
-
- kfree(pll->new_config);
- pll->new_config = NULL;
+ pll->config = shared_dpll[i];
}
}
@@ -11552,13 +11505,12 @@ static void
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 drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
- intel_shared_dpll_commit(dev_priv);
+ intel_shared_dpll_commit(state);
drm_atomic_helper_swap_state(state->dev, state);
for_each_intel_encoder(dev, intel_encoder) {
@@ -12189,9 +12141,13 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
}
}
- ret = intel_shared_dpll_start_config(dev_priv, clear_pipes);
- if (ret)
- goto done;
+ if (clear_pipes) {
+ struct intel_shared_dpll_config *shared_dpll =
+ intel_atomic_get_shared_dpll_state(state);
+
+ for (i = 0; i < dev_priv->num_shared_dpll; i++)
+ shared_dpll[i].crtc_mask &= ~clear_pipes;
+ }
for_each_crtc_in_state(state, crtc, crtc_state, i) {
if (!needs_modeset(crtc_state) || !crtc_state->enable)
@@ -12202,13 +12158,10 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
ret = dev_priv->display.crtc_compute_clock(intel_crtc,
intel_crtc_state);
- if (ret) {
- intel_shared_dpll_abort_config(dev_priv);
- goto done;
- }
+ if (ret)
+ return ret;
}
-done:
return ret;
}
@@ -13900,6 +13853,8 @@ static const struct drm_mode_config_funcs intel_mode_funcs = {
.output_poll_changed = intel_fbdev_output_poll_changed,
.atomic_check = intel_atomic_check,
.atomic_commit = intel_atomic_commit,
+ .atomic_state_alloc = intel_atomic_state_alloc,
+ .atomic_state_clear = intel_atomic_state_clear,
};
/* Set up chip specific display functions */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d0d99669aa00..7012c67de8d5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -241,6 +241,13 @@ typedef struct dpll {
int p;
} intel_clock_t;
+struct intel_atomic_state {
+ struct drm_atomic_state base;
+
+ bool dpll_set;
+ struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
+};
+
struct intel_plane_state {
struct drm_plane_state base;
struct drm_rect src;
@@ -627,6 +634,7 @@ struct cxsr_latency {
unsigned long cursor_hpll_disable;
};
+#define to_intel_atomic_state(x) container_of(x, struct intel_atomic_state, base)
#define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
#define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, base)
#define to_intel_connector(x) container_of(x, struct intel_connector, base)
@@ -1402,6 +1410,11 @@ int intel_connector_atomic_get_property(struct drm_connector *connector,
struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
void intel_crtc_destroy_state(struct drm_crtc *crtc,
struct drm_crtc_state *state);
+struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
+void intel_atomic_state_clear(struct drm_atomic_state *);
+struct intel_shared_dpll_config *
+intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s);
+
static inline struct intel_crtc_state *
intel_atomic_get_crtc_state(struct drm_atomic_state *state,
struct intel_crtc *crtc)
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3 12/22] drm/i915: Use drm_atomic_helper_swap_state in intel_atomic_commit.
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 11/22] drm/i915: Use global atomic state for staged pll config, v2 maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 13/22] drm/i915: Swap planes on each crtc separately maarten.lankhorst
` (9 subsequent siblings)
11 siblings, 0 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
And update crtc->config to point to the new state. There is no point
in swapping only part of the state when the rest of the state
should be untouched.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_atomic.c | 44 ++++++++++---------------------------
1 file changed, 12 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 6ab82dc56cea..e93aade0f2dc 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -129,6 +129,8 @@ int intel_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state,
bool async)
{
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
int ret;
int i;
@@ -142,48 +144,26 @@ int intel_atomic_commit(struct drm_device *dev,
return ret;
/* Point of no return */
+ drm_atomic_helper_swap_state(dev, state);
+
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state);
+
+ if (INTEL_INFO(dev)->gen >= 9)
+ skl_detach_scalers(to_intel_crtc(crtc));
+ }
/*
* FIXME: The proper sequence here will eventually be:
*
- * drm_atomic_helper_swap_state(dev, state)
* drm_atomic_helper_commit_modeset_disables(dev, state);
* drm_atomic_helper_commit_planes(dev, state);
* drm_atomic_helper_commit_modeset_enables(dev, state);
- * drm_atomic_helper_wait_for_vblanks(dev, state);
- * drm_atomic_helper_cleanup_planes(dev, state);
- * drm_atomic_state_free(state);
*
- * once we have full atomic modeset. For now, just manually update
- * plane states to avoid clobbering good states with dummy states
- * while nuclear pageflipping.
+ * once we have full atomic modeset.
*/
- for (i = 0; i < dev->mode_config.num_total_plane; i++) {
- struct drm_plane *plane = state->planes[i];
-
- if (!plane)
- continue;
-
- plane->state->state = state;
- swap(state->plane_states[i], plane->state);
- plane->state->state = NULL;
- }
-
- /* swap crtc_scaler_state */
- for (i = 0; i < dev->mode_config.num_crtc; i++) {
- struct drm_crtc *crtc = state->crtcs[i];
- if (!crtc) {
- continue;
- }
-
- to_intel_crtc(crtc)->config->scaler_state =
- to_intel_crtc_state(state->crtc_states[i])->scaler_state;
-
- if (INTEL_INFO(dev)->gen >= 9)
- skl_detach_scalers(to_intel_crtc(crtc));
- }
-
drm_atomic_helper_commit_planes(dev, state);
+
drm_atomic_helper_wait_for_vblanks(dev, state);
drm_atomic_helper_cleanup_planes(dev, state);
drm_atomic_state_free(state);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3 13/22] drm/i915: Swap planes on each crtc separately.
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 11/22] drm/i915: Use global atomic state for staged pll config, v2 maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 12/22] drm/i915: Use drm_atomic_helper_swap_state in intel_atomic_commit maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-29 0:56 ` Matt Roper
2015-05-20 16:04 ` [PATCH v3 14/22] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config() maarten.lankhorst
` (8 subsequent siblings)
11 siblings, 1 reply; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Repeated calls to begin_crtc_commit can cause warnings like this:
[ 169.127746] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:616
[ 169.127835] in_atomic(): 0, irqs_disabled(): 1, pid: 1947, name: kms_flip
[ 169.127840] 3 locks held by kms_flip/1947:
[ 169.127843] #0: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffff814774bc>] __drm_modeset_lock_all+0x9c/0x130
[ 169.127860] #1: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffff814774cd>] __drm_modeset_lock_all+0xad/0x130
[ 169.127870] #2: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffff81477178>] drm_modeset_lock+0x38/0x110
[ 169.127879] irq event stamp: 665690
[ 169.127882] hardirqs last enabled at (665689): [<ffffffff817ffdb5>] _raw_spin_unlock_irqrestore+0x55/0x70
[ 169.127889] hardirqs last disabled at (665690): [<ffffffffc0197a23>] intel_pipe_update_start+0x113/0x5c0 [i915]
[ 169.127936] softirqs last enabled at (665470): [<ffffffff8108a766>] __do_softirq+0x236/0x650
[ 169.127942] softirqs last disabled at (665465): [<ffffffff8108ae75>] irq_exit+0xc5/0xd0
[ 169.127951] CPU: 1 PID: 1947 Comm: kms_flip Not tainted 4.1.0-rc4-patser+ #4039
[ 169.127954] Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014
[ 169.127957] ffff8800c49036f0 ffff8800cde5fa28 ffffffff817f6907 0000000080000001
[ 169.127964] 0000000000000000 ffff8800cde5fa58 ffffffff810aebed 0000000000000046
[ 169.127970] ffffffff81c5d518 0000000000000268 0000000000000000 ffff8800cde5fa88
[ 169.127981] Call Trace:
[ 169.127992] [<ffffffff817f6907>] dump_stack+0x4f/0x7b
[ 169.128001] [<ffffffff810aebed>] ___might_sleep+0x16d/0x270
[ 169.128008] [<ffffffff810aed38>] __might_sleep+0x48/0x90
[ 169.128017] [<ffffffff817fc359>] mutex_lock_nested+0x29/0x410
[ 169.128073] [<ffffffffc01635f0>] ? vgpu_write64+0x220/0x220 [i915]
[ 169.128138] [<ffffffffc017fddf>] ? ironlake_update_primary_plane+0x2ff/0x410 [i915]
[ 169.128198] [<ffffffffc0190e75>] intel_frontbuffer_flush+0x25/0x70 [i915]
[ 169.128253] [<ffffffffc01831ac>] intel_finish_crtc_commit+0x4c/0x180 [i915]
[ 169.128279] [<ffffffffc00784ac>] drm_atomic_helper_commit_planes+0x12c/0x240 [drm_kms_helper]
[ 169.128338] [<ffffffffc0184264>] __intel_set_mode+0x684/0x830 [i915]
[ 169.128378] [<ffffffffc018a84a>] intel_crtc_set_config+0x49a/0x620 [i915]
[ 169.128385] [<ffffffff817fdd39>] ? mutex_unlock+0x9/0x10
[ 169.128391] [<ffffffff81467b69>] drm_mode_set_config_internal+0x69/0x120
[ 169.128398] [<ffffffff8119b547>] ? might_fault+0x57/0xb0
[ 169.128403] [<ffffffff8146bf93>] drm_mode_setcrtc+0x253/0x620
[ 169.128409] [<ffffffff8145c600>] drm_ioctl+0x1a0/0x6a0
[ 169.128415] [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50
[ 169.128424] [<ffffffff811e9ab8>] do_vfs_ioctl+0x2f8/0x530
[ 169.128429] [<ffffffff810d0fcd>] ? trace_hardirqs_on+0xd/0x10
[ 169.128435] [<ffffffff812e7676>] ? selinux_file_ioctl+0x56/0x100
[ 169.128439] [<ffffffff811e9d71>] SyS_ioctl+0x81/0xa0
[ 169.128445] [<ffffffff81800697>] system_call_fastpath+0x12/0x6f
Solve it by using the newly introduced drm_atomic_helper_commit_planes_on_crtc.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_atomic.c | 13 +++----------
drivers/gpu/drm/i915/intel_display.c | 5 +++--
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index e93aade0f2dc..83078763ba14 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -151,18 +151,11 @@ int intel_atomic_commit(struct drm_device *dev,
if (INTEL_INFO(dev)->gen >= 9)
skl_detach_scalers(to_intel_crtc(crtc));
+
+ drm_atomic_helper_commit_planes_on_crtc(crtc_state);
}
- /*
- * FIXME: The proper sequence here will eventually be:
- *
- * drm_atomic_helper_commit_modeset_disables(dev, state);
- * drm_atomic_helper_commit_planes(dev, state);
- * drm_atomic_helper_commit_modeset_enables(dev, state);
- *
- * once we have full atomic modeset.
- */
- drm_atomic_helper_commit_planes(dev, state);
+ /* FIXME: This function should eventually call __intel_set_mode when needed */
drm_atomic_helper_wait_for_vblanks(dev, state);
drm_atomic_helper_cleanup_planes(dev, state);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index adb19c3c9172..81d5358efdde 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12225,10 +12225,10 @@ static int __intel_set_mode(struct drm_atomic_state *state)
modeset_update_crtc_power_domains(state);
- drm_atomic_helper_commit_planes(dev, state);
-
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ drm_atomic_helper_commit_planes_on_crtc(crtc_state);
+
if (!needs_modeset(crtc->state) || !crtc->state->active)
continue;
@@ -14572,6 +14572,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
struct intel_plane_state *plane_state;
memset(crtc->config, 0, sizeof(*crtc->config));
+ crtc->config->base.crtc = &crtc->base;
crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 13/22] drm/i915: Swap planes on each crtc separately.
2015-05-20 16:04 ` [PATCH v3 13/22] drm/i915: Swap planes on each crtc separately maarten.lankhorst
@ 2015-05-29 0:56 ` Matt Roper
0 siblings, 0 replies; 46+ messages in thread
From: Matt Roper @ 2015-05-29 0:56 UTC (permalink / raw)
To: maarten.lankhorst; +Cc: intel-gfx
On Wed, May 20, 2015 at 06:04:25PM +0200, maarten.lankhorst@linux.intel.com wrote:
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> Repeated calls to begin_crtc_commit can cause warnings like this:
> [ 169.127746] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:616
> [ 169.127835] in_atomic(): 0, irqs_disabled(): 1, pid: 1947, name: kms_flip
> [ 169.127840] 3 locks held by kms_flip/1947:
> [ 169.127843] #0: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffff814774bc>] __drm_modeset_lock_all+0x9c/0x130
> [ 169.127860] #1: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffff814774cd>] __drm_modeset_lock_all+0xad/0x130
> [ 169.127870] #2: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffff81477178>] drm_modeset_lock+0x38/0x110
> [ 169.127879] irq event stamp: 665690
> [ 169.127882] hardirqs last enabled at (665689): [<ffffffff817ffdb5>] _raw_spin_unlock_irqrestore+0x55/0x70
> [ 169.127889] hardirqs last disabled at (665690): [<ffffffffc0197a23>] intel_pipe_update_start+0x113/0x5c0 [i915]
> [ 169.127936] softirqs last enabled at (665470): [<ffffffff8108a766>] __do_softirq+0x236/0x650
> [ 169.127942] softirqs last disabled at (665465): [<ffffffff8108ae75>] irq_exit+0xc5/0xd0
> [ 169.127951] CPU: 1 PID: 1947 Comm: kms_flip Not tainted 4.1.0-rc4-patser+ #4039
> [ 169.127954] Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014
> [ 169.127957] ffff8800c49036f0 ffff8800cde5fa28 ffffffff817f6907 0000000080000001
> [ 169.127964] 0000000000000000 ffff8800cde5fa58 ffffffff810aebed 0000000000000046
> [ 169.127970] ffffffff81c5d518 0000000000000268 0000000000000000 ffff8800cde5fa88
> [ 169.127981] Call Trace:
> [ 169.127992] [<ffffffff817f6907>] dump_stack+0x4f/0x7b
> [ 169.128001] [<ffffffff810aebed>] ___might_sleep+0x16d/0x270
> [ 169.128008] [<ffffffff810aed38>] __might_sleep+0x48/0x90
> [ 169.128017] [<ffffffff817fc359>] mutex_lock_nested+0x29/0x410
> [ 169.128073] [<ffffffffc01635f0>] ? vgpu_write64+0x220/0x220 [i915]
> [ 169.128138] [<ffffffffc017fddf>] ? ironlake_update_primary_plane+0x2ff/0x410 [i915]
> [ 169.128198] [<ffffffffc0190e75>] intel_frontbuffer_flush+0x25/0x70 [i915]
> [ 169.128253] [<ffffffffc01831ac>] intel_finish_crtc_commit+0x4c/0x180 [i915]
> [ 169.128279] [<ffffffffc00784ac>] drm_atomic_helper_commit_planes+0x12c/0x240 [drm_kms_helper]
> [ 169.128338] [<ffffffffc0184264>] __intel_set_mode+0x684/0x830 [i915]
> [ 169.128378] [<ffffffffc018a84a>] intel_crtc_set_config+0x49a/0x620 [i915]
> [ 169.128385] [<ffffffff817fdd39>] ? mutex_unlock+0x9/0x10
> [ 169.128391] [<ffffffff81467b69>] drm_mode_set_config_internal+0x69/0x120
> [ 169.128398] [<ffffffff8119b547>] ? might_fault+0x57/0xb0
> [ 169.128403] [<ffffffff8146bf93>] drm_mode_setcrtc+0x253/0x620
> [ 169.128409] [<ffffffff8145c600>] drm_ioctl+0x1a0/0x6a0
> [ 169.128415] [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50
> [ 169.128424] [<ffffffff811e9ab8>] do_vfs_ioctl+0x2f8/0x530
> [ 169.128429] [<ffffffff810d0fcd>] ? trace_hardirqs_on+0xd/0x10
> [ 169.128435] [<ffffffff812e7676>] ? selinux_file_ioctl+0x56/0x100
> [ 169.128439] [<ffffffff811e9d71>] SyS_ioctl+0x81/0xa0
> [ 169.128445] [<ffffffff81800697>] system_call_fastpath+0x12/0x6f
>
> Solve it by using the newly introduced drm_atomic_helper_commit_planes_on_crtc.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
This commit looks good, but I think you might want to elaborate on the
commit message a bit for people not super familiar with the atomic work.
The problem here was that the drm_atomic_helper_commit_planes() helper
we were using was basically designed to do
begin_crtc_commit(crtc #1)
begin_crtc_commit(crtc #2)
...
commit all planes
finish_crtc_commit(crtc #1)
finish_crtc_commit(crtc #2)
The problem here is that since our hardware relies on vblank evasion,
our CRTC 'begin' function waits until we're out of the danger zone in
which register writes might wind up straddling the vblank, then disables
interrupts; our 'finish' function re-enables interrupts after the
registers have been written. The expectation is that the operations between
'begin' and 'end' must be performed without sleeping (since interrupts
are disabled) and should happen as quickly as possible. By clumping all
of the 'begin' calls together, we introducing a couple problems:
* Subsequent 'begin' invocations might sleep (which is illegal)
* The first 'begin' ensured that we were far enough from the vblank that
we could write our registers safely and ensure they all fell within
the same frame. Adding extra delay waiting for subsequent CRTC's
wasn't accounted for and could put us back into the 'danger zone' for
CRTC #1.
This commit solves the problem by using a new helper that allows an
order of operations like:
for each crtc {
begin_crtc_commit(crtc) // sleep (maybe), then disable interrupts
commit planes for this specific CRTC
end_crtc_commit(crtc) // reenable interrupts
}
so that sleeps will only be performed while interrupts are enabled and
we can be sure that registers for a CRTC will be written immediately
once we know we're in the safe zone.
The only other thing I noted is that the CRTC backpointer update in
intel_modeset_readout_hw_state() seems somewhat unrelated that might
have fit better in one of the earlier patches (but is necessary at this
point since otherwise the helper will dereference NULL). You might want
to make a quick note about that.
Matt
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 13 +++----------
> drivers/gpu/drm/i915/intel_display.c | 5 +++--
> 2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index e93aade0f2dc..83078763ba14 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -151,18 +151,11 @@ int intel_atomic_commit(struct drm_device *dev,
>
> if (INTEL_INFO(dev)->gen >= 9)
> skl_detach_scalers(to_intel_crtc(crtc));
> +
> + drm_atomic_helper_commit_planes_on_crtc(crtc_state);
> }
>
> - /*
> - * FIXME: The proper sequence here will eventually be:
> - *
> - * drm_atomic_helper_commit_modeset_disables(dev, state);
> - * drm_atomic_helper_commit_planes(dev, state);
> - * drm_atomic_helper_commit_modeset_enables(dev, state);
> - *
> - * once we have full atomic modeset.
> - */
> - drm_atomic_helper_commit_planes(dev, state);
> + /* FIXME: This function should eventually call __intel_set_mode when needed */
>
> drm_atomic_helper_wait_for_vblanks(dev, state);
> drm_atomic_helper_cleanup_planes(dev, state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index adb19c3c9172..81d5358efdde 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12225,10 +12225,10 @@ static int __intel_set_mode(struct drm_atomic_state *state)
>
> modeset_update_crtc_power_domains(state);
>
> - drm_atomic_helper_commit_planes(dev, state);
> -
> /* Now enable the clocks, plane, pipe, and connectors that we set up. */
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + drm_atomic_helper_commit_planes_on_crtc(crtc_state);
> +
> if (!needs_modeset(crtc->state) || !crtc->state->active)
> continue;
>
> @@ -14572,6 +14572,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> struct intel_plane_state *plane_state;
>
> memset(crtc->config, 0, sizeof(*crtc->config));
> + crtc->config->base.crtc = &crtc->base;
>
> crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
>
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 14/22] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config()
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (2 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 13/22] drm/i915: Swap planes on each crtc separately maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-29 0:55 ` Matt Roper
2015-05-20 16:04 ` [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct maarten.lankhorst
` (7 subsequent siblings)
11 siblings, 1 reply; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
It makes more sense there, since these are computation steps that can
fail.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 70 ++++++++++++++++++------------------
1 file changed, 35 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 81d5358efdde..e7aa8610cbdc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12085,37 +12085,6 @@ static void update_scanline_offset(struct intel_crtc *crtc)
crtc->scanline_offset = 1;
}
-static int
-intel_modeset_compute_config(struct drm_atomic_state *state)
-{
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- int ret, i;
-
- ret = drm_atomic_helper_check_modeset(state->dev, state);
- if (ret)
- return ret;
-
- for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!crtc_state->enable &&
- WARN_ON(crtc_state->active))
- crtc_state->active = false;
-
- if (!crtc_state->enable)
- continue;
-
- ret = intel_modeset_pipe_config(crtc, state);
- if (ret)
- return ret;
-
- intel_dump_pipe_config(to_intel_crtc(crtc),
- to_intel_crtc_state(crtc_state),
- "[modeset]");
- }
-
- return drm_atomic_helper_check_planes(state->dev, state);
-}
-
static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
@@ -12191,6 +12160,41 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
return 0;
}
+static int
+intel_modeset_compute_config(struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ int ret, i;
+
+ ret = drm_atomic_helper_check_modeset(state->dev, state);
+ if (ret)
+ return ret;
+
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ if (!crtc_state->enable &&
+ WARN_ON(crtc_state->active))
+ crtc_state->active = false;
+
+ if (!crtc_state->enable)
+ continue;
+
+ ret = intel_modeset_pipe_config(crtc, state);
+ if (ret)
+ return ret;
+
+ intel_dump_pipe_config(to_intel_crtc(crtc),
+ to_intel_crtc_state(crtc_state),
+ "[modeset]");
+ }
+
+ ret = drm_atomic_helper_check_planes(state->dev, state);
+ if (ret)
+ return ret;
+
+ return __intel_set_mode_checks(state);
+}
+
static int __intel_set_mode(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
@@ -12200,10 +12204,6 @@ static int __intel_set_mode(struct drm_atomic_state *state)
int ret = 0;
int i;
- ret = __intel_set_mode_checks(state);
- if (ret < 0)
- return ret;
-
ret = drm_atomic_helper_prepare_planes(dev, state);
if (ret)
return ret;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 14/22] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config()
2015-05-20 16:04 ` [PATCH v3 14/22] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config() maarten.lankhorst
@ 2015-05-29 0:55 ` Matt Roper
2015-06-01 6:31 ` Maarten Lankhorst
0 siblings, 1 reply; 46+ messages in thread
From: Matt Roper @ 2015-05-29 0:55 UTC (permalink / raw)
To: maarten.lankhorst; +Cc: Ander Conselvan de Oliveira, intel-gfx
On Wed, May 20, 2015 at 06:04:26PM +0200, maarten.lankhorst@linux.intel.com wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> It makes more sense there, since these are computation steps that can
> fail.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
I've noticed that a few of the patches in this series were originally
written by Ander, but seem to be missing his s-o-b line. I think you
generally want to just append your line after his in that case.
One other cosmetic note farther down.
> ---
> drivers/gpu/drm/i915/intel_display.c | 70 ++++++++++++++++++------------------
> 1 file changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 81d5358efdde..e7aa8610cbdc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12085,37 +12085,6 @@ static void update_scanline_offset(struct intel_crtc *crtc)
> crtc->scanline_offset = 1;
> }
>
> -static int
> -intel_modeset_compute_config(struct drm_atomic_state *state)
> -{
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *crtc_state;
> - int ret, i;
> -
> - ret = drm_atomic_helper_check_modeset(state->dev, state);
> - if (ret)
> - return ret;
> -
> - for_each_crtc_in_state(state, crtc, crtc_state, i) {
> - if (!crtc_state->enable &&
> - WARN_ON(crtc_state->active))
> - crtc_state->active = false;
> -
> - if (!crtc_state->enable)
> - continue;
> -
> - ret = intel_modeset_pipe_config(crtc, state);
> - if (ret)
> - return ret;
> -
> - intel_dump_pipe_config(to_intel_crtc(crtc),
> - to_intel_crtc_state(crtc_state),
> - "[modeset]");
> - }
> -
> - return drm_atomic_helper_check_planes(state->dev, state);
> -}
> -
> static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
> {
> struct drm_device *dev = state->dev;
> @@ -12191,6 +12160,41 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
> return 0;
> }
>
> +static int
> +intel_modeset_compute_config(struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> + int ret, i;
> +
> + ret = drm_atomic_helper_check_modeset(state->dev, state);
> + if (ret)
> + return ret;
> +
> + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + if (!crtc_state->enable &&
> + WARN_ON(crtc_state->active))
> + crtc_state->active = false;
> +
> + if (!crtc_state->enable)
> + continue;
> +
> + ret = intel_modeset_pipe_config(crtc, state);
> + if (ret)
> + return ret;
> +
> + intel_dump_pipe_config(to_intel_crtc(crtc),
> + to_intel_crtc_state(crtc_state),
> + "[modeset]");
> + }
> +
> + ret = drm_atomic_helper_check_planes(state->dev, state);
> + if (ret)
> + return ret;
> +
> + return __intel_set_mode_checks(state);
Just a cosmetic note, but maybe we should rename this function now?
It's not called from __intel_set_mode anymore and it isn't really
'checks' (but rather setup that we intend to be done during the check
phase), so the whole name seems a bit misleading now.
Matt
> +}
> +
> static int __intel_set_mode(struct drm_atomic_state *state)
> {
> struct drm_device *dev = state->dev;
> @@ -12200,10 +12204,6 @@ static int __intel_set_mode(struct drm_atomic_state *state)
> int ret = 0;
> int i;
>
> - ret = __intel_set_mode_checks(state);
> - if (ret < 0)
> - return ret;
> -
> ret = drm_atomic_helper_prepare_planes(dev, state);
> if (ret)
> return ret;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 14/22] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config()
2015-05-29 0:55 ` Matt Roper
@ 2015-06-01 6:31 ` Maarten Lankhorst
0 siblings, 0 replies; 46+ messages in thread
From: Maarten Lankhorst @ 2015-06-01 6:31 UTC (permalink / raw)
To: Matt Roper; +Cc: Ander Conselvan de Oliveira, intel-gfx
Op 29-05-15 om 02:55 schreef Matt Roper:
> On Wed, May 20, 2015 at 06:04:26PM +0200, maarten.lankhorst@linux.intel.com wrote:
>> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>>
>> It makes more sense there, since these are computation steps that can
>> fail.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> I've noticed that a few of the patches in this series were originally
> written by Ander, but seem to be missing his s-o-b line. I think you
> generally want to just append your line after his in that case.
>
> One other cosmetic note farther down.
>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 70 ++++++++++++++++++------------------
>> 1 file changed, 35 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 81d5358efdde..e7aa8610cbdc 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12085,37 +12085,6 @@ static void update_scanline_offset(struct intel_crtc *crtc)
>> crtc->scanline_offset = 1;
>> }
>>
>> -static int
>> -intel_modeset_compute_config(struct drm_atomic_state *state)
>> -{
>> - struct drm_crtc *crtc;
>> - struct drm_crtc_state *crtc_state;
>> - int ret, i;
>> -
>> - ret = drm_atomic_helper_check_modeset(state->dev, state);
>> - if (ret)
>> - return ret;
>> -
>> - for_each_crtc_in_state(state, crtc, crtc_state, i) {
>> - if (!crtc_state->enable &&
>> - WARN_ON(crtc_state->active))
>> - crtc_state->active = false;
>> -
>> - if (!crtc_state->enable)
>> - continue;
>> -
>> - ret = intel_modeset_pipe_config(crtc, state);
>> - if (ret)
>> - return ret;
>> -
>> - intel_dump_pipe_config(to_intel_crtc(crtc),
>> - to_intel_crtc_state(crtc_state),
>> - "[modeset]");
>> - }
>> -
>> - return drm_atomic_helper_check_planes(state->dev, state);
>> -}
>> -
>> static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
>> {
>> struct drm_device *dev = state->dev;
>> @@ -12191,6 +12160,41 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
>> return 0;
>> }
>>
>> +static int
>> +intel_modeset_compute_config(struct drm_atomic_state *state)
>> +{
>> + struct drm_crtc *crtc;
>> + struct drm_crtc_state *crtc_state;
>> + int ret, i;
>> +
>> + ret = drm_atomic_helper_check_modeset(state->dev, state);
>> + if (ret)
>> + return ret;
>> +
>> + for_each_crtc_in_state(state, crtc, crtc_state, i) {
>> + if (!crtc_state->enable &&
>> + WARN_ON(crtc_state->active))
>> + crtc_state->active = false;
>> +
>> + if (!crtc_state->enable)
>> + continue;
>> +
>> + ret = intel_modeset_pipe_config(crtc, state);
>> + if (ret)
>> + return ret;
>> +
>> + intel_dump_pipe_config(to_intel_crtc(crtc),
>> + to_intel_crtc_state(crtc_state),
>> + "[modeset]");
>> + }
>> +
>> + ret = drm_atomic_helper_check_planes(state->dev, state);
>> + if (ret)
>> + return ret;
>> +
>> + return __intel_set_mode_checks(state);
> Just a cosmetic note, but maybe we should rename this function now?
> It's not called from __intel_set_mode anymore and it isn't really
> 'checks' (but rather setup that we intend to be done during the check
> phase), so the whole name seems a bit misleading now.
>
Later on intel_modeset_compute_config gets renamed to intel_atomic_check,
and I conditionally run intel_set_mode_checks depending on whether a modeset
is requested or not. I guess __intel_set_mode_checks could be renamed to intel_modeset_checks.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (3 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 14/22] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config() maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-27 5:30 ` Maarten Lankhorst
2015-05-29 0:55 ` Matt Roper
2015-05-20 16:04 ` [PATCH v3 16/22] drm/i915: Implement intel_crtc_control using atomic state, v3 maarten.lankhorst
` (6 subsequent siblings)
11 siblings, 2 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
To make this work we load the new hardware state into the
atomic_state, then swap it with the sw state.
This lets us change the force restore path in setup_hw_state()
to use a single call to intel_mode_set() to restore all the
previous state.
As a nice bonus this kills off encoder->new_encoder,
connector->new_enabled and crtc->new_enabled. They were used only
to restore the state after a modeset.
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_atomic.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 377 ++++++++++++++++++++++-------------
drivers/gpu/drm/i915/intel_drv.h | 14 +-
3 files changed, 241 insertions(+), 152 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 83078763ba14..17bf9e80c557 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -395,7 +395,7 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
return 0;
}
-static void
+void
intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
struct intel_shared_dpll_config *shared_dpll)
{
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e7aa8610cbdc..a42e7d7bf86b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9639,7 +9639,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
retry:
ret = drm_modeset_lock(&config->connection_mutex, ctx);
if (ret)
- goto fail_unlock;
+ goto fail;
/*
* Algorithm gets a little messy:
@@ -9657,10 +9657,10 @@ retry:
ret = drm_modeset_lock(&crtc->mutex, ctx);
if (ret)
- goto fail_unlock;
+ goto fail;
ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
if (ret)
- goto fail_unlock;
+ goto fail;
old->dpms_mode = connector->dpms;
old->load_detect_temp = false;
@@ -9679,9 +9679,6 @@ retry:
continue;
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)
- continue;
crtc = possible_crtc;
break;
@@ -9692,20 +9689,17 @@ retry:
*/
if (!crtc) {
DRM_DEBUG_KMS("no pipe available for load-detect\n");
- goto fail_unlock;
+ goto fail;
}
ret = drm_modeset_lock(&crtc->mutex, ctx);
if (ret)
- goto fail_unlock;
+ goto fail;
ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
if (ret)
- goto fail_unlock;
- intel_encoder->new_crtc = to_intel_crtc(crtc);
- to_intel_connector(connector)->new_encoder = intel_encoder;
+ goto fail;
intel_crtc = to_intel_crtc(crtc);
- intel_crtc->new_enabled = true;
old->dpms_mode = connector->dpms;
old->load_detect_temp = true;
old->release_fb = NULL;
@@ -9773,9 +9767,7 @@ retry:
intel_wait_for_vblank(dev, intel_crtc->pipe);
return true;
- fail:
- intel_crtc->new_enabled = crtc->state->enable;
-fail_unlock:
+fail:
drm_atomic_state_free(state);
state = NULL;
@@ -9821,10 +9813,6 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
if (IS_ERR(crtc_state))
goto fail;
- to_intel_connector(connector)->new_encoder = NULL;
- intel_encoder->new_crtc = NULL;
- intel_crtc->new_enabled = false;
-
connector_state->best_encoder = NULL;
connector_state->crtc = NULL;
@@ -10969,33 +10957,6 @@ static const struct drm_crtc_helper_funcs intel_helper_funcs = {
.atomic_flush = intel_finish_crtc_commit,
};
-/**
- * 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_crtc *crtc;
- struct intel_encoder *encoder;
- struct intel_connector *connector;
-
- for_each_intel_connector(dev, connector) {
- connector->new_encoder =
- to_intel_encoder(connector->base.encoder);
- }
-
- for_each_intel_encoder(dev, encoder) {
- encoder->new_crtc =
- to_intel_crtc(encoder->base.crtc);
- }
-
- for_each_intel_crtc(dev, crtc) {
- crtc->new_enabled = crtc->base.state->enable;
- }
-}
-
/* Transitional helper to copy current connector/encoder state to
* connector->state. This is needed so that code that is partially
* converted to atomic does the right thing.
@@ -11526,7 +11487,6 @@ intel_modeset_update_state(struct drm_atomic_state *state)
}
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
- intel_modeset_update_staged_output_state(state->dev);
/* Double check state. */
for_each_crtc(dev, crtc) {
@@ -11832,11 +11792,14 @@ check_connector_state(struct drm_device *dev)
struct intel_connector *connector;
for_each_intel_connector(dev, connector) {
+ struct drm_encoder *encoder = connector->base.encoder;
+ struct drm_connector_state *state = connector->base.state;
+
/* This also checks the encoder/connector hw state with the
* ->get_hw_state callbacks. */
intel_connector_check_state(connector);
- I915_STATE_WARN(&connector->new_encoder->base != connector->base.encoder,
+ I915_STATE_WARN(state->best_encoder != encoder,
"connector's staged encoder doesn't match current encoder\n");
}
}
@@ -11856,8 +11819,6 @@ check_encoder_state(struct drm_device *dev)
encoder->base.base.id,
encoder->base.name);
- I915_STATE_WARN(&encoder->new_crtc->base != encoder->base.crtc,
- "encoder's stage crtc doesn't match current crtc\n");
I915_STATE_WARN(encoder->connectors_active && !encoder->base.crtc,
"encoder's active_connectors set, but no crtc\n");
@@ -11867,6 +11828,9 @@ check_encoder_state(struct drm_device *dev)
enabled = true;
if (connector->base.dpms != DRM_MODE_DPMS_OFF)
active = true;
+
+ I915_STATE_WARN(connector->base.state->crtc != encoder->base.crtc,
+ "encoder's stage crtc doesn't match current crtc\n");
}
/*
* for MST connectors if we unplug the connector is gone
@@ -12296,11 +12260,11 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
* need to copy the staged config to the atomic state, otherwise the
* mode set will just reapply the state the HW is already in. */
for_each_intel_encoder(dev, encoder) {
- if (&encoder->new_crtc->base != crtc)
+ if (encoder->base.crtc != crtc)
continue;
for_each_intel_connector(dev, connector) {
- if (connector->new_encoder != encoder)
+ if (connector->base.state->best_encoder != &encoder->base)
continue;
connector_state = drm_atomic_get_connector_state(state, &connector->base);
@@ -12313,14 +12277,10 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
}
connector_state->crtc = crtc;
- connector_state->best_encoder = &encoder->base;
}
}
for_each_intel_crtc(dev, intel_crtc) {
- if (intel_crtc->new_enabled == intel_crtc->base.enabled)
- continue;
-
crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
if (IS_ERR(crtc_state)) {
DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
@@ -12329,9 +12289,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
continue;
}
- crtc_state->base.active = crtc_state->base.enable =
- intel_crtc->new_enabled;
-
if (&intel_crtc->base == crtc)
drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
}
@@ -14552,99 +14509,224 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
- if (!crtc->active)
+ if (!crtc->base.enabled)
return false;
return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
}
-static void intel_modeset_readout_hw_state(struct drm_device *dev)
+static int readout_hw_crtc_state(struct drm_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- enum pipe pipe;
- struct intel_crtc *crtc;
- struct intel_encoder *encoder;
- struct intel_connector *connector;
- int i;
+ struct drm_i915_private *dev_priv = to_i915(state->dev);
+ struct intel_crtc_state *crtc_state;
+ struct drm_plane *primary = crtc->base.primary;
+ struct drm_plane_state *drm_plane_state;
+ struct intel_plane_state *plane_state;
+ int ret;
- for_each_intel_crtc(dev, crtc) {
- struct drm_plane *primary = crtc->base.primary;
- struct intel_plane_state *plane_state;
+ crtc_state = intel_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
- memset(crtc->config, 0, sizeof(*crtc->config));
- crtc->config->base.crtc = &crtc->base;
+ ret = drm_atomic_add_affected_planes(state, &crtc->base);
+ if (ret)
+ return ret;
- crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
+ memset(crtc_state, 0, sizeof(*crtc_state));
+ crtc_state->base.crtc = &crtc->base;
+ crtc_state->base.state = state;
- crtc->active = dev_priv->display.get_pipe_config(crtc,
- crtc->config);
+ crtc_state->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
- crtc->base.state->enable = crtc->active;
- crtc->base.state->active = crtc->active;
- crtc->base.enabled = crtc->active;
+ crtc_state->base.enable = crtc_state->base.active =
+ crtc->base.enabled = dev_priv->display.get_pipe_config(crtc, crtc_state);
- plane_state = to_intel_plane_state(primary->state);
- plane_state->visible = primary_get_hw_state(crtc);
+ /* update transitional state */
+ crtc->active = crtc_state->base.active;
+ crtc->config = crtc_state;
- DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
- crtc->base.base.id,
- crtc->active ? "enabled" : "disabled");
- }
+ drm_plane_state = drm_atomic_get_plane_state(state, primary);
+ if (IS_ERR(drm_plane_state))
+ return PTR_ERR(drm_plane_state);
+
+ plane_state = to_intel_plane_state(drm_plane_state);
+ plane_state->visible = primary_get_hw_state(crtc);
+
+ if (plane_state->visible)
+ crtc_state->base.plane_mask |= 1 << drm_plane_index(primary);
+ else
+ crtc_state->base.plane_mask &= ~(1 << drm_plane_index(primary));
+
+ DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
+ crtc->base.base.id,
+ crtc_state->base.active ? "enabled" : "disabled");
+
+ return 0;
+}
+
+static int readout_hw_pll_state(struct drm_atomic_state *state)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->dev);
+ struct intel_shared_dpll_config *shared_dpll;
+ struct intel_crtc *crtc;
+ struct intel_crtc_state *crtc_state;
+ int i;
+ shared_dpll = intel_atomic_get_shared_dpll_state(state);
for (i = 0; i < dev_priv->num_shared_dpll; i++) {
struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
pll->on = pll->get_hw_state(dev_priv, pll,
- &pll->config.hw_state);
+ &shared_dpll[i].hw_state);
+
pll->active = 0;
- pll->config.crtc_mask = 0;
- for_each_intel_crtc(dev, crtc) {
- if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) {
+ shared_dpll[i].crtc_mask = 0;
+
+ for_each_intel_crtc(state->dev, crtc) {
+ crtc_state = intel_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ if (crtc_state->base.active &&
+ crtc_state->shared_dpll == i) {
pll->active++;
- pll->config.crtc_mask |= 1 << crtc->pipe;
+ shared_dpll[i].crtc_mask |=
+ 1 << crtc->pipe;
}
}
DRM_DEBUG_KMS("%s hw state readout: crtc_mask 0x%08x, on %i\n",
- pll->name, pll->config.crtc_mask, pll->on);
+ pll->name, shared_dpll[i].crtc_mask,
+ pll->on);
- if (pll->config.crtc_mask)
+ if (shared_dpll[i].crtc_mask)
intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
}
- for_each_intel_encoder(dev, encoder) {
- pipe = 0;
+ return 0;
+}
- if (encoder->get_hw_state(encoder, &pipe)) {
- crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
- encoder->base.crtc = &crtc->base;
- encoder->get_config(encoder, crtc->config);
- } else {
- encoder->base.crtc = NULL;
- }
+static struct drm_connector_state *
+get_connector_state_for_encoder(struct drm_atomic_state *state,
+ struct intel_encoder *encoder)
+{
+ struct drm_connector *connector;
+ struct drm_connector_state *connector_state;
+ int i;
- encoder->connectors_active = false;
- DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
- encoder->base.base.id,
- encoder->base.name,
- encoder->base.crtc ? "enabled" : "disabled",
- pipe_name(pipe));
- }
+ for_each_connector_in_state(state, connector, connector_state, i)
+ if (connector_state->best_encoder == &encoder->base)
+ return connector_state;
+
+ return NULL;
+}
+
+static int readout_hw_connector_encoder_state(struct drm_atomic_state *state)
+{
+ struct drm_device *dev = state->dev;
+ struct drm_i915_private *dev_priv = to_i915(state->dev);
+ struct intel_crtc *crtc;
+ struct drm_crtc_state *drm_crtc_state;
+ struct intel_crtc_state *crtc_state;
+ struct intel_encoder *encoder;
+ struct intel_connector *connector;
+ struct drm_connector_state *connector_state;
+ enum pipe pipe;
for_each_intel_connector(dev, connector) {
+ connector_state =
+ drm_atomic_get_connector_state(state, &connector->base);
+ if (IS_ERR(connector_state))
+ return PTR_ERR(connector_state);
+
if (connector->get_hw_state(connector)) {
connector->base.dpms = DRM_MODE_DPMS_ON;
- connector->encoder->connectors_active = true;
connector->base.encoder = &connector->encoder->base;
} else {
connector->base.dpms = DRM_MODE_DPMS_OFF;
connector->base.encoder = NULL;
}
+
+ /* We'll update the crtc field when reading encoder state */
+ connector_state->crtc = NULL;
+
+ connector_state->best_encoder = connector->base.encoder;
+
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] hw state readout: %s\n",
connector->base.base.id,
connector->base.name,
connector->base.encoder ? "enabled" : "disabled");
}
+
+ for_each_intel_encoder(dev, encoder) {
+ pipe = 0;
+
+ connector_state =
+ get_connector_state_for_encoder(state, encoder);
+
+ encoder->connectors_active = !!connector_state;
+
+ if (encoder->get_hw_state(encoder, &pipe)) {
+ encoder->base.crtc =
+ dev_priv->pipe_to_crtc_mapping[pipe];
+ crtc = to_intel_crtc(encoder->base.crtc);
+
+ drm_crtc_state =
+ state->crtc_states[drm_crtc_index(&crtc->base)];
+ crtc_state = to_intel_crtc_state(drm_crtc_state);
+
+ encoder->get_config(encoder, crtc_state);
+
+ if (connector_state)
+ connector_state->crtc = &crtc->base;
+ } else {
+ encoder->base.crtc = NULL;
+ }
+
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
+ encoder->base.base.id,
+ encoder->base.name,
+ encoder->base.crtc ? "enabled" : "disabled",
+ pipe_name(pipe));
+ }
+
+ return 0;
+}
+
+static struct drm_atomic_state *
+intel_modeset_readout_hw_state(struct drm_device *dev)
+{
+ struct intel_crtc *crtc;
+ int ret = 0;
+
+ struct drm_atomic_state *state;
+
+ state = drm_atomic_state_alloc(dev);
+ if (!state)
+ return ERR_PTR(-ENOMEM);
+
+ state->acquire_ctx = dev->mode_config.acquire_ctx;
+
+ for_each_intel_crtc(dev, crtc) {
+ ret = readout_hw_crtc_state(state, crtc);
+ if (ret)
+ goto err_free;
+ }
+
+ ret = readout_hw_pll_state(state);
+ if (ret)
+ goto err_free;
+
+ ret = readout_hw_connector_encoder_state(state);
+ if (ret)
+ goto err_free;
+
+ return state;
+
+err_free:
+ drm_atomic_state_free(state);
+ return ERR_PTR(ret);
}
/* Scan out the current hw modeset state, sanitizes it and maps it into the drm
@@ -14653,37 +14735,57 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
bool force_restore)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- enum pipe pipe;
- struct intel_crtc *crtc;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
struct intel_encoder *encoder;
+ struct drm_atomic_state *state;
+ struct intel_shared_dpll_config shared_dplls[I915_NUM_PLLS];
int i;
- intel_modeset_readout_hw_state(dev);
-
- /*
- * Now that we have the config, copy it to each CRTC struct
- * Note that this could go away if we move to using crtc_config
- * checking everywhere.
- */
- for_each_intel_crtc(dev, crtc) {
- if (crtc->active && i915.fastboot) {
- intel_mode_from_pipe_config(&crtc->base.mode,
- crtc->config);
- DRM_DEBUG_KMS("[CRTC:%d] found active mode: ",
- crtc->base.base.id);
- drm_mode_debug_printmodeline(&crtc->base.mode);
- }
+ state = intel_modeset_readout_hw_state(dev);
+ if (IS_ERR(state)) {
+ DRM_ERROR("Failed to read out hw state\n");
+ return;
}
+ drm_atomic_helper_swap_state(dev, state);
+
+ /* swap sw/hw dpll state */
+ intel_atomic_duplicate_dpll_state(dev_priv, shared_dplls);
+ intel_shared_dpll_commit(state);
+ memcpy(to_intel_atomic_state(state)->shared_dpll,
+ shared_dplls, sizeof(*shared_dplls) * dev_priv->num_shared_dpll);
+
/* HW state is read out, now we need to sanitize this mess. */
for_each_intel_encoder(dev, encoder) {
intel_sanitize_encoder(encoder);
}
- for_each_pipe(dev_priv, pipe) {
- crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
- intel_sanitize_crtc(crtc);
- intel_dump_pipe_config(crtc, crtc->config,
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+ /* prevent unnneeded restores with force_restore */
+ crtc_state->active_changed =
+ crtc_state->mode_changed =
+ crtc_state->planes_changed = false;
+
+ if (crtc->enabled) {
+ intel_mode_from_pipe_config(&crtc->state->mode,
+ to_intel_crtc_state(crtc->state));
+
+ drm_mode_copy(&crtc->mode, &crtc->state->mode);
+ drm_mode_copy(&crtc->hwmode,
+ &crtc->state->adjusted_mode);
+ }
+
+ intel_sanitize_crtc(intel_crtc);
+
+ /*
+ * sanitize_crtc may have forced an update of crtc->state,
+ * so reload in intel_dump_pipe_config
+ */
+ intel_dump_pipe_config(intel_crtc,
+ to_intel_crtc_state(crtc->state),
"[setup_hw_state]");
}
@@ -14707,20 +14809,17 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
ilk_wm_get_hw_state(dev);
if (force_restore) {
- i915_redisable_vga(dev);
+ int ret;
- /*
- * We need to use raw interfaces for restoring state to avoid
- * checking (bogus) intermediate states.
- */
- for_each_pipe(dev_priv, pipe) {
- struct drm_crtc *crtc =
- dev_priv->pipe_to_crtc_mapping[pipe];
+ i915_redisable_vga(dev);
- intel_crtc_restore_mode(crtc);
+ ret = intel_set_mode(state);
+ if (ret) {
+ DRM_ERROR("Failed to restore previous mode\n");
+ drm_atomic_state_free(state);
}
} else {
- intel_modeset_update_staged_output_state(dev);
+ drm_atomic_state_free(state);
}
intel_modeset_check_state(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7012c67de8d5..65f5f3d41b5a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -130,11 +130,6 @@ struct intel_fbdev {
struct intel_encoder {
struct drm_encoder base;
- /*
- * The new crtc this encoder will be driven from. Only differs from
- * base->crtc while a modeset is in progress.
- */
- struct intel_crtc *new_crtc;
enum intel_output_type type;
unsigned int cloneable;
@@ -195,12 +190,6 @@ struct intel_connector {
*/
struct intel_encoder *encoder;
- /*
- * The new encoder this connector will be driven. Only differs from
- * encoder while a modeset is in progress.
- */
- struct intel_encoder *new_encoder;
-
/* Reads out the current hw, returning true if the connector is enabled
* and active (i.e. dpms ON state). */
bool (*get_hw_state)(struct intel_connector *);
@@ -534,7 +523,6 @@ struct intel_crtc {
struct intel_initial_plane_config plane_config;
struct intel_crtc_state *config;
- bool new_enabled;
/* reset counter value when the last flip was submitted */
unsigned int reset_counter;
@@ -1414,6 +1402,8 @@ struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
void intel_atomic_state_clear(struct drm_atomic_state *);
struct intel_shared_dpll_config *
intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s);
+void intel_atomic_duplicate_dpll_state(struct drm_i915_private *,
+ struct intel_shared_dpll_config *);
static inline struct intel_crtc_state *
intel_atomic_get_crtc_state(struct drm_atomic_state *state,
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct
2015-05-20 16:04 ` [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct maarten.lankhorst
@ 2015-05-27 5:30 ` Maarten Lankhorst
2015-05-27 11:26 ` Daniel Vetter
2015-05-29 0:55 ` Matt Roper
1 sibling, 1 reply; 46+ messages in thread
From: Maarten Lankhorst @ 2015-05-27 5:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira, Daniel Stone
Hey,
Op 20-05-15 om 18:04 schreef maarten.lankhorst@linux.intel.com:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> To make this work we load the new hardware state into the
> atomic_state, then swap it with the sw state.
>
> This lets us change the force restore path in setup_hw_state()
> to use a single call to intel_mode_set() to restore all the
> previous state.
>
> As a nice bonus this kills off encoder->new_encoder,
> connector->new_enabled and crtc->new_enabled. They were used only
> to restore the state after a modeset.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 377 ++++++++++++++++++++++-------------
> drivers/gpu/drm/i915/intel_drv.h | 14 +-
> 3 files changed, 241 insertions(+), 152 deletions(-)
>
Small fix up. This patch may force the modeset code to no longer do a modeset on boot, causing a black screen
on Daniel Stone's machine. Fixup below disables all crtc's for the !i915.fastboot case.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 13b09efeb2a0..b70e3890e2bf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14138,7 +14138,7 @@ void intel_modeset_init(struct drm_device *dev)
intel_fbc_disable(dev);
drm_modeset_lock_all(dev);
- intel_modeset_setup_hw_state(dev, false);
+ intel_modeset_setup_hw_state(dev, !i915.fastboot);
drm_modeset_unlock_all(dev);
for_each_intel_crtc(dev, crtc) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct
2015-05-27 5:30 ` Maarten Lankhorst
@ 2015-05-27 11:26 ` Daniel Vetter
0 siblings, 0 replies; 46+ messages in thread
From: Daniel Vetter @ 2015-05-27 11:26 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Ander Conselvan de Oliveira, intel-gfx, Daniel Stone
On Wed, May 27, 2015 at 07:30:01AM +0200, Maarten Lankhorst wrote:
> Hey,
>
> Op 20-05-15 om 18:04 schreef maarten.lankhorst@linux.intel.com:
> > From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >
> > To make this work we load the new hardware state into the
> > atomic_state, then swap it with the sw state.
> >
> > This lets us change the force restore path in setup_hw_state()
> > to use a single call to intel_mode_set() to restore all the
> > previous state.
> >
> > As a nice bonus this kills off encoder->new_encoder,
> > connector->new_enabled and crtc->new_enabled. They were used only
> > to restore the state after a modeset.
> >
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_atomic.c | 2 +-
> > drivers/gpu/drm/i915/intel_display.c | 377 ++++++++++++++++++++++-------------
> > drivers/gpu/drm/i915/intel_drv.h | 14 +-
> > 3 files changed, 241 insertions(+), 152 deletions(-)
> >
> Small fix up. This patch may force the modeset code to no longer do a modeset on boot, causing a black screen
> on Daniel Stone's machine. Fixup below disables all crtc's for the !i915.fastboot case.
Why does this happen? It could be that we're throwing a state difference
under the rug, which wouldn't be great.
-Daniel
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 13b09efeb2a0..b70e3890e2bf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14138,7 +14138,7 @@ void intel_modeset_init(struct drm_device *dev)
> intel_fbc_disable(dev);
>
> drm_modeset_lock_all(dev);
> - intel_modeset_setup_hw_state(dev, false);
> + intel_modeset_setup_hw_state(dev, !i915.fastboot);
> drm_modeset_unlock_all(dev);
>
> for_each_intel_crtc(dev, crtc) {
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct
2015-05-20 16:04 ` [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct maarten.lankhorst
2015-05-27 5:30 ` Maarten Lankhorst
@ 2015-05-29 0:55 ` Matt Roper
2015-06-01 6:35 ` Maarten Lankhorst
1 sibling, 1 reply; 46+ messages in thread
From: Matt Roper @ 2015-05-29 0:55 UTC (permalink / raw)
To: maarten.lankhorst; +Cc: Ander Conselvan de Oliveira, intel-gfx
On Wed, May 20, 2015 at 06:04:27PM +0200, maarten.lankhorst@linux.intel.com wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> To make this work we load the new hardware state into the
> atomic_state, then swap it with the sw state.
>
> This lets us change the force restore path in setup_hw_state()
> to use a single call to intel_mode_set() to restore all the
> previous state.
>
> As a nice bonus this kills off encoder->new_encoder,
> connector->new_enabled and crtc->new_enabled. They were used only
> to restore the state after a modeset.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 377 ++++++++++++++++++++++-------------
> drivers/gpu/drm/i915/intel_drv.h | 14 +-
> 3 files changed, 241 insertions(+), 152 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 83078763ba14..17bf9e80c557 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -395,7 +395,7 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
> return 0;
> }
>
> -static void
> +void
> intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll_config *shared_dpll)
> {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e7aa8610cbdc..a42e7d7bf86b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9639,7 +9639,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
> retry:
> ret = drm_modeset_lock(&config->connection_mutex, ctx);
> if (ret)
> - goto fail_unlock;
> + goto fail;
>
> /*
> * Algorithm gets a little messy:
> @@ -9657,10 +9657,10 @@ retry:
>
> ret = drm_modeset_lock(&crtc->mutex, ctx);
> if (ret)
> - goto fail_unlock;
> + goto fail;
> ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
> if (ret)
> - goto fail_unlock;
> + goto fail;
>
> old->dpms_mode = connector->dpms;
> old->load_detect_temp = false;
> @@ -9679,9 +9679,6 @@ retry:
> continue;
> 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)
> - continue;
>
> crtc = possible_crtc;
> break;
> @@ -9692,20 +9689,17 @@ retry:
> */
> if (!crtc) {
> DRM_DEBUG_KMS("no pipe available for load-detect\n");
> - goto fail_unlock;
> + goto fail;
> }
>
> ret = drm_modeset_lock(&crtc->mutex, ctx);
> if (ret)
> - goto fail_unlock;
> + goto fail;
> ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
> if (ret)
> - goto fail_unlock;
> - intel_encoder->new_crtc = to_intel_crtc(crtc);
> - to_intel_connector(connector)->new_encoder = intel_encoder;
> + goto fail;
>
> intel_crtc = to_intel_crtc(crtc);
> - intel_crtc->new_enabled = true;
> old->dpms_mode = connector->dpms;
> old->load_detect_temp = true;
> old->release_fb = NULL;
> @@ -9773,9 +9767,7 @@ retry:
> intel_wait_for_vblank(dev, intel_crtc->pipe);
> return true;
>
> - fail:
> - intel_crtc->new_enabled = crtc->state->enable;
> -fail_unlock:
> +fail:
> drm_atomic_state_free(state);
> state = NULL;
>
> @@ -9821,10 +9813,6 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> if (IS_ERR(crtc_state))
> goto fail;
>
> - to_intel_connector(connector)->new_encoder = NULL;
> - intel_encoder->new_crtc = NULL;
> - intel_crtc->new_enabled = false;
> -
> connector_state->best_encoder = NULL;
> connector_state->crtc = NULL;
>
> @@ -10969,33 +10957,6 @@ static const struct drm_crtc_helper_funcs intel_helper_funcs = {
> .atomic_flush = intel_finish_crtc_commit,
> };
>
> -/**
> - * 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_crtc *crtc;
> - struct intel_encoder *encoder;
> - struct intel_connector *connector;
> -
> - for_each_intel_connector(dev, connector) {
> - connector->new_encoder =
> - to_intel_encoder(connector->base.encoder);
> - }
> -
> - for_each_intel_encoder(dev, encoder) {
> - encoder->new_crtc =
> - to_intel_crtc(encoder->base.crtc);
> - }
> -
> - for_each_intel_crtc(dev, crtc) {
> - crtc->new_enabled = crtc->base.state->enable;
> - }
> -}
> -
> /* Transitional helper to copy current connector/encoder state to
> * connector->state. This is needed so that code that is partially
> * converted to atomic does the right thing.
> @@ -11526,7 +11487,6 @@ intel_modeset_update_state(struct drm_atomic_state *state)
> }
>
> drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
> - intel_modeset_update_staged_output_state(state->dev);
>
> /* Double check state. */
> for_each_crtc(dev, crtc) {
> @@ -11832,11 +11792,14 @@ check_connector_state(struct drm_device *dev)
> struct intel_connector *connector;
>
> for_each_intel_connector(dev, connector) {
> + struct drm_encoder *encoder = connector->base.encoder;
> + struct drm_connector_state *state = connector->base.state;
> +
> /* This also checks the encoder/connector hw state with the
> * ->get_hw_state callbacks. */
> intel_connector_check_state(connector);
>
> - I915_STATE_WARN(&connector->new_encoder->base != connector->base.encoder,
> + I915_STATE_WARN(state->best_encoder != encoder,
> "connector's staged encoder doesn't match current encoder\n");
> }
> }
> @@ -11856,8 +11819,6 @@ check_encoder_state(struct drm_device *dev)
> encoder->base.base.id,
> encoder->base.name);
>
> - I915_STATE_WARN(&encoder->new_crtc->base != encoder->base.crtc,
> - "encoder's stage crtc doesn't match current crtc\n");
> I915_STATE_WARN(encoder->connectors_active && !encoder->base.crtc,
> "encoder's active_connectors set, but no crtc\n");
>
> @@ -11867,6 +11828,9 @@ check_encoder_state(struct drm_device *dev)
> enabled = true;
> if (connector->base.dpms != DRM_MODE_DPMS_OFF)
> active = true;
> +
> + I915_STATE_WARN(connector->base.state->crtc != encoder->base.crtc,
> + "encoder's stage crtc doesn't match current crtc\n");
> }
> /*
> * for MST connectors if we unplug the connector is gone
> @@ -12296,11 +12260,11 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
> * need to copy the staged config to the atomic state, otherwise the
> * mode set will just reapply the state the HW is already in. */
> for_each_intel_encoder(dev, encoder) {
> - if (&encoder->new_crtc->base != crtc)
> + if (encoder->base.crtc != crtc)
> continue;
>
> for_each_intel_connector(dev, connector) {
> - if (connector->new_encoder != encoder)
> + if (connector->base.state->best_encoder != &encoder->base)
> continue;
>
> connector_state = drm_atomic_get_connector_state(state, &connector->base);
> @@ -12313,14 +12277,10 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
> }
>
> connector_state->crtc = crtc;
> - connector_state->best_encoder = &encoder->base;
> }
> }
>
> for_each_intel_crtc(dev, intel_crtc) {
> - if (intel_crtc->new_enabled == intel_crtc->base.enabled)
> - continue;
> -
> crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
> if (IS_ERR(crtc_state)) {
> DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
> @@ -12329,9 +12289,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
> continue;
> }
>
> - crtc_state->base.active = crtc_state->base.enable =
> - intel_crtc->new_enabled;
> -
> if (&intel_crtc->base == crtc)
> drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
> }
> @@ -14552,99 +14509,224 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>
> - if (!crtc->active)
> + if (!crtc->base.enabled)
> return false;
>
> return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
> }
>
> -static void intel_modeset_readout_hw_state(struct drm_device *dev)
> +static int readout_hw_crtc_state(struct drm_atomic_state *state,
> + struct intel_crtc *crtc)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - enum pipe pipe;
> - struct intel_crtc *crtc;
> - struct intel_encoder *encoder;
> - struct intel_connector *connector;
> - int i;
> + struct drm_i915_private *dev_priv = to_i915(state->dev);
> + struct intel_crtc_state *crtc_state;
> + struct drm_plane *primary = crtc->base.primary;
> + struct drm_plane_state *drm_plane_state;
> + struct intel_plane_state *plane_state;
> + int ret;
>
> - for_each_intel_crtc(dev, crtc) {
> - struct drm_plane *primary = crtc->base.primary;
> - struct intel_plane_state *plane_state;
> + crtc_state = intel_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(crtc_state))
> + return PTR_ERR(crtc_state);
>
> - memset(crtc->config, 0, sizeof(*crtc->config));
> - crtc->config->base.crtc = &crtc->base;
> + ret = drm_atomic_add_affected_planes(state, &crtc->base);
> + if (ret)
> + return ret;
Do we actually try to handle non-primary planes in this function? If
I'm following this properly, at bootup we won't add any sprite or cursor
planes here, even though it's possible that a graphics firmware has
turned some of them on. It seems like our sw and hw states will still
be a bit inconsistent in that case.
Matt
>
> - crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> + memset(crtc_state, 0, sizeof(*crtc_state));
> + crtc_state->base.crtc = &crtc->base;
> + crtc_state->base.state = state;
>
> - crtc->active = dev_priv->display.get_pipe_config(crtc,
> - crtc->config);
> + crtc_state->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
>
> - crtc->base.state->enable = crtc->active;
> - crtc->base.state->active = crtc->active;
> - crtc->base.enabled = crtc->active;
> + crtc_state->base.enable = crtc_state->base.active =
> + crtc->base.enabled = dev_priv->display.get_pipe_config(crtc, crtc_state);
>
> - plane_state = to_intel_plane_state(primary->state);
> - plane_state->visible = primary_get_hw_state(crtc);
> + /* update transitional state */
> + crtc->active = crtc_state->base.active;
> + crtc->config = crtc_state;
>
> - DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> - crtc->base.base.id,
> - crtc->active ? "enabled" : "disabled");
> - }
> + drm_plane_state = drm_atomic_get_plane_state(state, primary);
> + if (IS_ERR(drm_plane_state))
> + return PTR_ERR(drm_plane_state);
> +
> + plane_state = to_intel_plane_state(drm_plane_state);
> + plane_state->visible = primary_get_hw_state(crtc);
> +
> + if (plane_state->visible)
> + crtc_state->base.plane_mask |= 1 << drm_plane_index(primary);
> + else
> + crtc_state->base.plane_mask &= ~(1 << drm_plane_index(primary));
> +
> + DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
> + crtc->base.base.id,
> + crtc_state->base.active ? "enabled" : "disabled");
> +
> + return 0;
> +}
> +
> +static int readout_hw_pll_state(struct drm_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->dev);
> + struct intel_shared_dpll_config *shared_dpll;
> + struct intel_crtc *crtc;
> + struct intel_crtc_state *crtc_state;
> + int i;
>
> + shared_dpll = intel_atomic_get_shared_dpll_state(state);
> for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
>
> pll->on = pll->get_hw_state(dev_priv, pll,
> - &pll->config.hw_state);
> + &shared_dpll[i].hw_state);
> +
> pll->active = 0;
> - pll->config.crtc_mask = 0;
> - for_each_intel_crtc(dev, crtc) {
> - if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) {
> + shared_dpll[i].crtc_mask = 0;
> +
> + for_each_intel_crtc(state->dev, crtc) {
> + crtc_state = intel_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(crtc_state))
> + return PTR_ERR(crtc_state);
> +
> + if (crtc_state->base.active &&
> + crtc_state->shared_dpll == i) {
> pll->active++;
> - pll->config.crtc_mask |= 1 << crtc->pipe;
> + shared_dpll[i].crtc_mask |=
> + 1 << crtc->pipe;
> }
> }
>
> DRM_DEBUG_KMS("%s hw state readout: crtc_mask 0x%08x, on %i\n",
> - pll->name, pll->config.crtc_mask, pll->on);
> + pll->name, shared_dpll[i].crtc_mask,
> + pll->on);
>
> - if (pll->config.crtc_mask)
> + if (shared_dpll[i].crtc_mask)
> intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> }
>
> - for_each_intel_encoder(dev, encoder) {
> - pipe = 0;
> + return 0;
> +}
>
> - if (encoder->get_hw_state(encoder, &pipe)) {
> - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> - encoder->base.crtc = &crtc->base;
> - encoder->get_config(encoder, crtc->config);
> - } else {
> - encoder->base.crtc = NULL;
> - }
> +static struct drm_connector_state *
> +get_connector_state_for_encoder(struct drm_atomic_state *state,
> + struct intel_encoder *encoder)
> +{
> + struct drm_connector *connector;
> + struct drm_connector_state *connector_state;
> + int i;
>
> - encoder->connectors_active = false;
> - DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
> - encoder->base.base.id,
> - encoder->base.name,
> - encoder->base.crtc ? "enabled" : "disabled",
> - pipe_name(pipe));
> - }
> + for_each_connector_in_state(state, connector, connector_state, i)
> + if (connector_state->best_encoder == &encoder->base)
> + return connector_state;
> +
> + return NULL;
> +}
> +
> +static int readout_hw_connector_encoder_state(struct drm_atomic_state *state)
> +{
> + struct drm_device *dev = state->dev;
> + struct drm_i915_private *dev_priv = to_i915(state->dev);
> + struct intel_crtc *crtc;
> + struct drm_crtc_state *drm_crtc_state;
> + struct intel_crtc_state *crtc_state;
> + struct intel_encoder *encoder;
> + struct intel_connector *connector;
> + struct drm_connector_state *connector_state;
> + enum pipe pipe;
>
> for_each_intel_connector(dev, connector) {
> + connector_state =
> + drm_atomic_get_connector_state(state, &connector->base);
> + if (IS_ERR(connector_state))
> + return PTR_ERR(connector_state);
> +
> if (connector->get_hw_state(connector)) {
> connector->base.dpms = DRM_MODE_DPMS_ON;
> - connector->encoder->connectors_active = true;
> connector->base.encoder = &connector->encoder->base;
> } else {
> connector->base.dpms = DRM_MODE_DPMS_OFF;
> connector->base.encoder = NULL;
> }
> +
> + /* We'll update the crtc field when reading encoder state */
> + connector_state->crtc = NULL;
> +
> + connector_state->best_encoder = connector->base.encoder;
> +
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] hw state readout: %s\n",
> connector->base.base.id,
> connector->base.name,
> connector->base.encoder ? "enabled" : "disabled");
> }
> +
> + for_each_intel_encoder(dev, encoder) {
> + pipe = 0;
> +
> + connector_state =
> + get_connector_state_for_encoder(state, encoder);
> +
> + encoder->connectors_active = !!connector_state;
> +
> + if (encoder->get_hw_state(encoder, &pipe)) {
> + encoder->base.crtc =
> + dev_priv->pipe_to_crtc_mapping[pipe];
> + crtc = to_intel_crtc(encoder->base.crtc);
> +
> + drm_crtc_state =
> + state->crtc_states[drm_crtc_index(&crtc->base)];
> + crtc_state = to_intel_crtc_state(drm_crtc_state);
> +
> + encoder->get_config(encoder, crtc_state);
> +
> + if (connector_state)
> + connector_state->crtc = &crtc->base;
> + } else {
> + encoder->base.crtc = NULL;
> + }
> +
> + DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
> + encoder->base.base.id,
> + encoder->base.name,
> + encoder->base.crtc ? "enabled" : "disabled",
> + pipe_name(pipe));
> + }
> +
> + return 0;
> +}
> +
> +static struct drm_atomic_state *
> +intel_modeset_readout_hw_state(struct drm_device *dev)
> +{
> + struct intel_crtc *crtc;
> + int ret = 0;
> +
> + struct drm_atomic_state *state;
> +
> + state = drm_atomic_state_alloc(dev);
> + if (!state)
> + return ERR_PTR(-ENOMEM);
> +
> + state->acquire_ctx = dev->mode_config.acquire_ctx;
> +
> + for_each_intel_crtc(dev, crtc) {
> + ret = readout_hw_crtc_state(state, crtc);
> + if (ret)
> + goto err_free;
> + }
> +
> + ret = readout_hw_pll_state(state);
> + if (ret)
> + goto err_free;
> +
> + ret = readout_hw_connector_encoder_state(state);
> + if (ret)
> + goto err_free;
> +
> + return state;
> +
> +err_free:
> + drm_atomic_state_free(state);
> + return ERR_PTR(ret);
> }
>
> /* Scan out the current hw modeset state, sanitizes it and maps it into the drm
> @@ -14653,37 +14735,57 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> bool force_restore)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - enum pipe pipe;
> - struct intel_crtc *crtc;
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> struct intel_encoder *encoder;
> + struct drm_atomic_state *state;
> + struct intel_shared_dpll_config shared_dplls[I915_NUM_PLLS];
> int i;
>
> - intel_modeset_readout_hw_state(dev);
> -
> - /*
> - * Now that we have the config, copy it to each CRTC struct
> - * Note that this could go away if we move to using crtc_config
> - * checking everywhere.
> - */
> - for_each_intel_crtc(dev, crtc) {
> - if (crtc->active && i915.fastboot) {
> - intel_mode_from_pipe_config(&crtc->base.mode,
> - crtc->config);
> - DRM_DEBUG_KMS("[CRTC:%d] found active mode: ",
> - crtc->base.base.id);
> - drm_mode_debug_printmodeline(&crtc->base.mode);
> - }
> + state = intel_modeset_readout_hw_state(dev);
> + if (IS_ERR(state)) {
> + DRM_ERROR("Failed to read out hw state\n");
> + return;
> }
>
> + drm_atomic_helper_swap_state(dev, state);
> +
> + /* swap sw/hw dpll state */
> + intel_atomic_duplicate_dpll_state(dev_priv, shared_dplls);
> + intel_shared_dpll_commit(state);
> + memcpy(to_intel_atomic_state(state)->shared_dpll,
> + shared_dplls, sizeof(*shared_dplls) * dev_priv->num_shared_dpll);
> +
> /* HW state is read out, now we need to sanitize this mess. */
> for_each_intel_encoder(dev, encoder) {
> intel_sanitize_encoder(encoder);
> }
>
> - for_each_pipe(dev_priv, pipe) {
> - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> - intel_sanitize_crtc(crtc);
> - intel_dump_pipe_config(crtc, crtc->config,
> + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> + /* prevent unnneeded restores with force_restore */
> + crtc_state->active_changed =
> + crtc_state->mode_changed =
> + crtc_state->planes_changed = false;
> +
> + if (crtc->enabled) {
> + intel_mode_from_pipe_config(&crtc->state->mode,
> + to_intel_crtc_state(crtc->state));
> +
> + drm_mode_copy(&crtc->mode, &crtc->state->mode);
> + drm_mode_copy(&crtc->hwmode,
> + &crtc->state->adjusted_mode);
> + }
> +
> + intel_sanitize_crtc(intel_crtc);
> +
> + /*
> + * sanitize_crtc may have forced an update of crtc->state,
> + * so reload in intel_dump_pipe_config
> + */
> + intel_dump_pipe_config(intel_crtc,
> + to_intel_crtc_state(crtc->state),
> "[setup_hw_state]");
> }
>
> @@ -14707,20 +14809,17 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> ilk_wm_get_hw_state(dev);
>
> if (force_restore) {
> - i915_redisable_vga(dev);
> + int ret;
>
> - /*
> - * We need to use raw interfaces for restoring state to avoid
> - * checking (bogus) intermediate states.
> - */
> - for_each_pipe(dev_priv, pipe) {
> - struct drm_crtc *crtc =
> - dev_priv->pipe_to_crtc_mapping[pipe];
> + i915_redisable_vga(dev);
>
> - intel_crtc_restore_mode(crtc);
> + ret = intel_set_mode(state);
> + if (ret) {
> + DRM_ERROR("Failed to restore previous mode\n");
> + drm_atomic_state_free(state);
> }
> } else {
> - intel_modeset_update_staged_output_state(dev);
> + drm_atomic_state_free(state);
> }
>
> intel_modeset_check_state(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7012c67de8d5..65f5f3d41b5a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -130,11 +130,6 @@ struct intel_fbdev {
>
> struct intel_encoder {
> struct drm_encoder base;
> - /*
> - * The new crtc this encoder will be driven from. Only differs from
> - * base->crtc while a modeset is in progress.
> - */
> - struct intel_crtc *new_crtc;
>
> enum intel_output_type type;
> unsigned int cloneable;
> @@ -195,12 +190,6 @@ struct intel_connector {
> */
> struct intel_encoder *encoder;
>
> - /*
> - * The new encoder this connector will be driven. Only differs from
> - * encoder while a modeset is in progress.
> - */
> - struct intel_encoder *new_encoder;
> -
> /* Reads out the current hw, returning true if the connector is enabled
> * and active (i.e. dpms ON state). */
> bool (*get_hw_state)(struct intel_connector *);
> @@ -534,7 +523,6 @@ struct intel_crtc {
>
> struct intel_initial_plane_config plane_config;
> struct intel_crtc_state *config;
> - bool new_enabled;
>
> /* reset counter value when the last flip was submitted */
> unsigned int reset_counter;
> @@ -1414,6 +1402,8 @@ struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
> void intel_atomic_state_clear(struct drm_atomic_state *);
> struct intel_shared_dpll_config *
> intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s);
> +void intel_atomic_duplicate_dpll_state(struct drm_i915_private *,
> + struct intel_shared_dpll_config *);
>
> static inline struct intel_crtc_state *
> intel_atomic_get_crtc_state(struct drm_atomic_state *state,
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct
2015-05-29 0:55 ` Matt Roper
@ 2015-06-01 6:35 ` Maarten Lankhorst
0 siblings, 0 replies; 46+ messages in thread
From: Maarten Lankhorst @ 2015-06-01 6:35 UTC (permalink / raw)
To: Matt Roper; +Cc: Ander Conselvan de Oliveira, intel-gfx
Op 29-05-15 om 02:55 schreef Matt Roper:
> On Wed, May 20, 2015 at 06:04:27PM +0200, maarten.lankhorst@linux.intel.com wrote:
>> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>>
>> To make this work we load the new hardware state into the
>> atomic_state, then swap it with the sw state.
>>
>> This lets us change the force restore path in setup_hw_state()
>> to use a single call to intel_mode_set() to restore all the
>> previous state.
>>
>> As a nice bonus this kills off encoder->new_encoder,
>> connector->new_enabled and crtc->new_enabled. They were used only
>> to restore the state after a modeset.
>>
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_atomic.c | 2 +-
>> drivers/gpu/drm/i915/intel_display.c | 377 ++++++++++++++++++++++-------------
>> drivers/gpu/drm/i915/intel_drv.h | 14 +-
>> 3 files changed, 241 insertions(+), 152 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>> index 83078763ba14..17bf9e80c557 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -395,7 +395,7 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
>> return 0;
>> }
>>
>> -static void
>> +void
>> intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
>> struct intel_shared_dpll_config *shared_dpll)
>> {
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index e7aa8610cbdc..a42e7d7bf86b 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9639,7 +9639,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
>> retry:
>> ret = drm_modeset_lock(&config->connection_mutex, ctx);
>> if (ret)
>> - goto fail_unlock;
>> + goto fail;
>>
>> /*
>> * Algorithm gets a little messy:
>> @@ -9657,10 +9657,10 @@ retry:
>>
>> ret = drm_modeset_lock(&crtc->mutex, ctx);
>> if (ret)
>> - goto fail_unlock;
>> + goto fail;
>> ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
>> if (ret)
>> - goto fail_unlock;
>> + goto fail;
>>
>> old->dpms_mode = connector->dpms;
>> old->load_detect_temp = false;
>> @@ -9679,9 +9679,6 @@ retry:
>> continue;
>> 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)
>> - continue;
>>
>> crtc = possible_crtc;
>> break;
>> @@ -9692,20 +9689,17 @@ retry:
>> */
>> if (!crtc) {
>> DRM_DEBUG_KMS("no pipe available for load-detect\n");
>> - goto fail_unlock;
>> + goto fail;
>> }
>>
>> ret = drm_modeset_lock(&crtc->mutex, ctx);
>> if (ret)
>> - goto fail_unlock;
>> + goto fail;
>> ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
>> if (ret)
>> - goto fail_unlock;
>> - intel_encoder->new_crtc = to_intel_crtc(crtc);
>> - to_intel_connector(connector)->new_encoder = intel_encoder;
>> + goto fail;
>>
>> intel_crtc = to_intel_crtc(crtc);
>> - intel_crtc->new_enabled = true;
>> old->dpms_mode = connector->dpms;
>> old->load_detect_temp = true;
>> old->release_fb = NULL;
>> @@ -9773,9 +9767,7 @@ retry:
>> intel_wait_for_vblank(dev, intel_crtc->pipe);
>> return true;
>>
>> - fail:
>> - intel_crtc->new_enabled = crtc->state->enable;
>> -fail_unlock:
>> +fail:
>> drm_atomic_state_free(state);
>> state = NULL;
>>
>> @@ -9821,10 +9813,6 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>> if (IS_ERR(crtc_state))
>> goto fail;
>>
>> - to_intel_connector(connector)->new_encoder = NULL;
>> - intel_encoder->new_crtc = NULL;
>> - intel_crtc->new_enabled = false;
>> -
>> connector_state->best_encoder = NULL;
>> connector_state->crtc = NULL;
>>
>> @@ -10969,33 +10957,6 @@ static const struct drm_crtc_helper_funcs intel_helper_funcs = {
>> .atomic_flush = intel_finish_crtc_commit,
>> };
>>
>> -/**
>> - * 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_crtc *crtc;
>> - struct intel_encoder *encoder;
>> - struct intel_connector *connector;
>> -
>> - for_each_intel_connector(dev, connector) {
>> - connector->new_encoder =
>> - to_intel_encoder(connector->base.encoder);
>> - }
>> -
>> - for_each_intel_encoder(dev, encoder) {
>> - encoder->new_crtc =
>> - to_intel_crtc(encoder->base.crtc);
>> - }
>> -
>> - for_each_intel_crtc(dev, crtc) {
>> - crtc->new_enabled = crtc->base.state->enable;
>> - }
>> -}
>> -
>> /* Transitional helper to copy current connector/encoder state to
>> * connector->state. This is needed so that code that is partially
>> * converted to atomic does the right thing.
>> @@ -11526,7 +11487,6 @@ intel_modeset_update_state(struct drm_atomic_state *state)
>> }
>>
>> drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
>> - intel_modeset_update_staged_output_state(state->dev);
>>
>> /* Double check state. */
>> for_each_crtc(dev, crtc) {
>> @@ -11832,11 +11792,14 @@ check_connector_state(struct drm_device *dev)
>> struct intel_connector *connector;
>>
>> for_each_intel_connector(dev, connector) {
>> + struct drm_encoder *encoder = connector->base.encoder;
>> + struct drm_connector_state *state = connector->base.state;
>> +
>> /* This also checks the encoder/connector hw state with the
>> * ->get_hw_state callbacks. */
>> intel_connector_check_state(connector);
>>
>> - I915_STATE_WARN(&connector->new_encoder->base != connector->base.encoder,
>> + I915_STATE_WARN(state->best_encoder != encoder,
>> "connector's staged encoder doesn't match current encoder\n");
>> }
>> }
>> @@ -11856,8 +11819,6 @@ check_encoder_state(struct drm_device *dev)
>> encoder->base.base.id,
>> encoder->base.name);
>>
>> - I915_STATE_WARN(&encoder->new_crtc->base != encoder->base.crtc,
>> - "encoder's stage crtc doesn't match current crtc\n");
>> I915_STATE_WARN(encoder->connectors_active && !encoder->base.crtc,
>> "encoder's active_connectors set, but no crtc\n");
>>
>> @@ -11867,6 +11828,9 @@ check_encoder_state(struct drm_device *dev)
>> enabled = true;
>> if (connector->base.dpms != DRM_MODE_DPMS_OFF)
>> active = true;
>> +
>> + I915_STATE_WARN(connector->base.state->crtc != encoder->base.crtc,
>> + "encoder's stage crtc doesn't match current crtc\n");
>> }
>> /*
>> * for MST connectors if we unplug the connector is gone
>> @@ -12296,11 +12260,11 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
>> * need to copy the staged config to the atomic state, otherwise the
>> * mode set will just reapply the state the HW is already in. */
>> for_each_intel_encoder(dev, encoder) {
>> - if (&encoder->new_crtc->base != crtc)
>> + if (encoder->base.crtc != crtc)
>> continue;
>>
>> for_each_intel_connector(dev, connector) {
>> - if (connector->new_encoder != encoder)
>> + if (connector->base.state->best_encoder != &encoder->base)
>> continue;
>>
>> connector_state = drm_atomic_get_connector_state(state, &connector->base);
>> @@ -12313,14 +12277,10 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
>> }
>>
>> connector_state->crtc = crtc;
>> - connector_state->best_encoder = &encoder->base;
>> }
>> }
>>
>> for_each_intel_crtc(dev, intel_crtc) {
>> - if (intel_crtc->new_enabled == intel_crtc->base.enabled)
>> - continue;
>> -
>> crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
>> if (IS_ERR(crtc_state)) {
>> DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
>> @@ -12329,9 +12289,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
>> continue;
>> }
>>
>> - crtc_state->base.active = crtc_state->base.enable =
>> - intel_crtc->new_enabled;
>> -
>> if (&intel_crtc->base == crtc)
>> drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
>> }
>> @@ -14552,99 +14509,224 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
>> {
>> struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>>
>> - if (!crtc->active)
>> + if (!crtc->base.enabled)
>> return false;
>>
>> return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
>> }
>>
>> -static void intel_modeset_readout_hw_state(struct drm_device *dev)
>> +static int readout_hw_crtc_state(struct drm_atomic_state *state,
>> + struct intel_crtc *crtc)
>> {
>> - struct drm_i915_private *dev_priv = dev->dev_private;
>> - enum pipe pipe;
>> - struct intel_crtc *crtc;
>> - struct intel_encoder *encoder;
>> - struct intel_connector *connector;
>> - int i;
>> + struct drm_i915_private *dev_priv = to_i915(state->dev);
>> + struct intel_crtc_state *crtc_state;
>> + struct drm_plane *primary = crtc->base.primary;
>> + struct drm_plane_state *drm_plane_state;
>> + struct intel_plane_state *plane_state;
>> + int ret;
>>
>> - for_each_intel_crtc(dev, crtc) {
>> - struct drm_plane *primary = crtc->base.primary;
>> - struct intel_plane_state *plane_state;
>> + crtc_state = intel_atomic_get_crtc_state(state, crtc);
>> + if (IS_ERR(crtc_state))
>> + return PTR_ERR(crtc_state);
>>
>> - memset(crtc->config, 0, sizeof(*crtc->config));
>> - crtc->config->base.crtc = &crtc->base;
>> + ret = drm_atomic_add_affected_planes(state, &crtc->base);
>> + if (ret)
>> + return ret;
> Do we actually try to handle non-primary planes in this function? If
> I'm following this properly, at bootup we won't add any sprite or cursor
> planes here, even though it's possible that a graphics firmware has
> turned some of them on. It seems like our sw and hw states will still
> be a bit inconsistent in that case.
I'm afraid we don't, but probably should. Adding all possible planes might be better here,
even if we don't track the previous hw state it will cause at least everything to be disabled,
another way of making hw and sw state match up. :-)
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 16/22] drm/i915: Implement intel_crtc_control using atomic state, v3
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (4 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 15/22] drm/i915: Read hw state into an atomic state struct maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-21 12:40 ` [PATCH v3.5 16.5/22] drm/i915: Make intel_display_suspend atomic Maarten Lankhorst
2015-05-26 8:35 ` [PATCH v3.6 16/22] drm/i915: Implement intel_crtc_control using atomic state, v4 Maarten Lankhorst
2015-05-20 16:04 ` [PATCH v3 17/22] drm/i915: move swap state to the right place maarten.lankhorst
` (5 subsequent siblings)
11 siblings, 2 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Assume the callers lock everything with drm_modeset_lock_all.
This change had to be done after converting suspend/resume to
use atomic_state so the atomic state is preserved, otherwise
all transitional state is erased.
Now all callers of .crtc_enable and .crtc_disable go through
atomic modeset! :-D
Changes since v1:
- Only check for crtc_state->active in valleyview_modeset_global_pipes.
- Only check for crtc_state->active in modeset_update_crtc_power_domains.
Changes since v2:
- Rework on top of the changed patch order.
Changes since v3:
- Rename intel_crtc_toggle in description to *_control
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a42e7d7bf86b..e819f1738458 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5952,10 +5952,13 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
void intel_crtc_control(struct drm_crtc *crtc, bool enable)
{
struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- enum intel_display_power_domain domain;
- unsigned long domains;
+ struct intel_crtc_state *pipe_config;
+ struct drm_plane_state *plane_state;
+ struct drm_atomic_state *state;
+ int ret;
if (enable == intel_crtc->active)
return;
@@ -5963,24 +5966,40 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable)
if (enable && !crtc->state->enable)
return;
- crtc->state->active = enable;
- if (enable) {
- domains = get_crtc_power_domains(crtc);
- for_each_power_domain(domain, domains)
- intel_display_power_get(dev_priv, domain);
- intel_crtc->enabled_power_domains = domains;
+ /* this function should be called with drm_modeset_lock_all for now */
+ if (WARN_ON(!ctx))
+ return;
+ lockdep_assert_held(&ctx->ww_ctx);
- dev_priv->display.crtc_enable(crtc);
- intel_crtc_enable_planes(crtc);
- } else {
- intel_crtc_disable_planes(crtc);
- dev_priv->display.crtc_disable(crtc);
+ state = drm_atomic_state_alloc(dev);
+ if (WARN_ON(!state))
+ return;
- domains = intel_crtc->enabled_power_domains;
- for_each_power_domain(domain, domains)
- intel_display_power_put(dev_priv, domain);
- intel_crtc->enabled_power_domains = 0;
+ state->acquire_ctx = ctx;
+ state->allow_modeset = true;
+
+ pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
+ if (IS_ERR(pipe_config)) {
+ ret = PTR_ERR(pipe_config);
+ goto err;
+ }
+ pipe_config->base.active = enable;
+
+ plane_state = drm_atomic_get_plane_state(state, crtc->primary);
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ goto err;
}
+
+ ret = intel_set_mode(state);
+ if (!ret)
+ return;
+
+ DRM_ERROR("Failed to toggle crtc!\n");
+
+err:
+ DRM_ERROR("Updating crtc active failed with %i\n", ret);
+ drm_atomic_state_free(state);
}
/**
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3.5 16.5/22] drm/i915: Make intel_display_suspend atomic.
2015-05-20 16:04 ` [PATCH v3 16/22] drm/i915: Implement intel_crtc_control using atomic state, v3 maarten.lankhorst
@ 2015-05-21 12:40 ` Maarten Lankhorst
2015-05-26 8:33 ` [PATCH v3.6 16.5/22] drm/i915: Make intel_display_suspend atomic, v2 Maarten Lankhorst
2015-05-26 8:35 ` [PATCH v3.6 16/22] drm/i915: Implement intel_crtc_control using atomic state, v4 Maarten Lankhorst
1 sibling, 1 reply; 46+ messages in thread
From: Maarten Lankhorst @ 2015-05-21 12:40 UTC (permalink / raw)
To: intel-gfx
Calculate all state using a normal transition, but afterwards fudge
crtc->state->active back to its old value. This should still allow
state restore in setup_hw_state to work properly.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++++---
1 file changed, 43 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8b1d91136d25..2dc59147d890 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5951,16 +5951,55 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
*/
void intel_display_suspend(struct drm_device *dev)
{
- struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
+ struct drm_atomic_state *state;
struct drm_crtc *crtc;
+ unsigned crtc_mask = 0;
+ int ret = 0;
+
+ if (WARN_ON(!ctx))
+ return;
+
+ lockdep_assert_held(&ctx->ww_ctx);
+ state = drm_atomic_state_alloc(dev);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = ctx;
+ state->allow_modeset = true;
for_each_crtc(dev, crtc) {
- if (!to_intel_crtc(crtc)->active)
+ struct drm_crtc_state *crtc_state =
+ drm_atomic_get_crtc_state(state, crtc);
+
+ ret = PTR_ERR_OR_ZERO(crtc_state);
+ if (ret)
+ goto free;
+
+ if (!crtc_state->active)
continue;
- intel_crtc_disable_planes(crtc);
- dev_priv->display.crtc_disable(crtc);
+ crtc_state->active = false;
+ crtc_mask |= 1 << drm_crtc_index(crtc);
}
+
+ if (crtc_mask) {
+ ret = intel_set_mode(state);
+
+ if (!ret) {
+ for_each_crtc(dev, crtc)
+ if (crtc_mask & (1 << drm_crtc_index(crtc)))
+ crtc->state->active = true;
+
+ return;
+ }
+ }
+
+free:
+ if (ret)
+ DRM_ERROR("Suspending crtc's failed with %i\n", ret);
+ drm_atomic_state_free(state);
}
/* Master function to enable/disable CRTC and corresponding power wells */
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3.6 16.5/22] drm/i915: Make intel_display_suspend atomic, v2.
2015-05-21 12:40 ` [PATCH v3.5 16.5/22] drm/i915: Make intel_display_suspend atomic Maarten Lankhorst
@ 2015-05-26 8:33 ` Maarten Lankhorst
0 siblings, 0 replies; 46+ messages in thread
From: Maarten Lankhorst @ 2015-05-26 8:33 UTC (permalink / raw)
To: intel-gfx
Calculate all state using a normal transition, but afterwards fudge
crtc->state->active back to its old value. This should still allow
state restore in setup_hw_state to work properly.
Calling intel_set_mode will cause intel_display_set_init_power to be
called, make sure init_power gets set again afterwards.
Changes since v1:
- Fix to compile with v2 of the patch that adds intel_display_suspend.
- Add intel_display_set_init_power.
- Set return value to int to allow error checking.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 3 ++
drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++--------
drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d1a090a9f653..69fc3c7057df 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -633,6 +633,9 @@ static int i915_drm_suspend(struct drm_device *dev)
intel_display_suspend(dev);
drm_modeset_unlock_all(dev);
+ /* suspending displays will unsets init power */
+ intel_display_set_init_power(dev_priv, true);
+
intel_dp_mst_suspend(dev);
intel_runtime_pm_disable_interrupts(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 32bab28432f7..392d6d68cb14 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5949,27 +5949,58 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
* turn all crtc's off, but do not adjust state
* This has to be paired with a call to intel_modeset_setup_hw_state.
*/
-void intel_display_suspend(struct drm_device *dev)
+int intel_display_suspend(struct drm_device *dev)
{
- struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
+ struct drm_atomic_state *state;
struct drm_crtc *crtc;
+ unsigned crtc_mask = 0;
+ int ret = 0;
+
+ if (WARN_ON(!ctx))
+ return 0;
+
+ lockdep_assert_held(&ctx->ww_ctx);
+ state = drm_atomic_state_alloc(dev);
+ if (WARN_ON(!state))
+ return -ENOMEM;
+
+ state->acquire_ctx = ctx;
+ state->allow_modeset = true;
for_each_crtc(dev, crtc) {
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- enum intel_display_power_domain domain;
- unsigned long domains;
+ struct drm_crtc_state *crtc_state =
+ drm_atomic_get_crtc_state(state, crtc);
- if (!intel_crtc->active)
+ ret = PTR_ERR_OR_ZERO(crtc_state);
+ if (ret)
+ goto free;
+
+ if (!crtc_state->active)
continue;
- intel_crtc_disable_planes(crtc);
- dev_priv->display.crtc_disable(crtc);
+ crtc_state->active = false;
+ crtc_mask |= 1 << drm_crtc_index(crtc);
+ }
- domains = intel_crtc->enabled_power_domains;
- for_each_power_domain(domain, domains)
- intel_display_power_put(dev_priv, domain);
- intel_crtc->enabled_power_domains = 0;
+ if (crtc_mask) {
+ ret = intel_set_mode(state);
+
+ if (!ret) {
+ for_each_crtc(dev, crtc)
+ if (crtc_mask & (1 << drm_crtc_index(crtc)))
+ crtc->state->active = true;
+
+ return ret;
+ }
}
+
+free:
+ if (ret)
+ DRM_ERROR("Suspending crtc's failed with %i\n", ret);
+ drm_atomic_state_free(state);
+ return ret;
}
/* Master function to enable/disable CRTC and corresponding power wells */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c113f187090f..665e249ae8bf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -988,7 +988,7 @@ int intel_pch_rawclk(struct drm_device *dev);
void intel_mark_busy(struct drm_device *dev);
void intel_mark_idle(struct drm_device *dev);
void intel_crtc_restore_mode(struct drm_crtc *crtc);
-void intel_display_suspend(struct drm_device *dev);
+int intel_display_suspend(struct drm_device *dev);
int intel_crtc_control(struct drm_crtc *crtc, bool enable);
void intel_crtc_update_dpms(struct drm_crtc *crtc);
void intel_encoder_destroy(struct drm_encoder *encoder);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH v3.6 16/22] drm/i915: Implement intel_crtc_control using atomic state, v4.
2015-05-20 16:04 ` [PATCH v3 16/22] drm/i915: Implement intel_crtc_control using atomic state, v3 maarten.lankhorst
2015-05-21 12:40 ` [PATCH v3.5 16.5/22] drm/i915: Make intel_display_suspend atomic Maarten Lankhorst
@ 2015-05-26 8:35 ` Maarten Lankhorst
2015-05-29 0:57 ` Matt Roper
1 sibling, 1 reply; 46+ messages in thread
From: Maarten Lankhorst @ 2015-05-26 8:35 UTC (permalink / raw)
To: Intel Graphics Development
Assume the callers lock everything with drm_modeset_lock_all.
This change had to be done after converting suspend/resume to
use atomic_state so the atomic state is preserved, otherwise
all transitional state is erased.
Now all callers of .crtc_enable and .crtc_disable go through
atomic modeset! :-D
Changes since v1:
- Only check for crtc_state->active in valleyview_modeset_global_pipes.
- Only check for crtc_state->active in modeset_update_crtc_power_domains.
Changes since v2:
- Rework on top of the changed patch order.
Changes since v3:
- Rename intel_crtc_toggle in description to *_control
- Change return value to int.
- Do not add plane state, should be done implicitly already.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++--------------
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e9680eb85bd0..32bab28432f7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5973,38 +5973,49 @@ void intel_display_suspend(struct drm_device *dev)
}
/* Master function to enable/disable CRTC and corresponding power wells */
-void intel_crtc_control(struct drm_crtc *crtc, bool enable)
+int intel_crtc_control(struct drm_crtc *crtc, bool enable)
{
struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- enum intel_display_power_domain domain;
- unsigned long domains;
+ struct intel_crtc_state *pipe_config;
+ struct drm_atomic_state *state;
+ int ret;
if (enable == intel_crtc->active)
- return;
+ return 0;
if (enable && !crtc->state->enable)
- return;
+ return 0;
- crtc->state->active = enable;
- if (enable) {
- domains = get_crtc_power_domains(crtc);
- for_each_power_domain(domain, domains)
- intel_display_power_get(dev_priv, domain);
- intel_crtc->enabled_power_domains = domains;
+ /* this function should be called with drm_modeset_lock_all for now */
+ if (WARN_ON(!ctx))
+ return -EIO;
+ lockdep_assert_held(&ctx->ww_ctx);
- dev_priv->display.crtc_enable(crtc);
- intel_crtc_enable_planes(crtc);
- } else {
- intel_crtc_disable_planes(crtc);
- dev_priv->display.crtc_disable(crtc);
+ state = drm_atomic_state_alloc(dev);
+ if (WARN_ON(!state))
+ return -ENOMEM;
- domains = intel_crtc->enabled_power_domains;
- for_each_power_domain(domain, domains)
- intel_display_power_put(dev_priv, domain);
- intel_crtc->enabled_power_domains = 0;
+ state->acquire_ctx = ctx;
+ state->allow_modeset = true;
+
+ pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
+ if (IS_ERR(pipe_config)) {
+ ret = PTR_ERR(pipe_config);
+ goto err;
}
+ pipe_config->base.active = enable;
+
+ ret = intel_set_mode(state);
+ if (!ret)
+ return ret;
+
+err:
+ DRM_ERROR("Updating crtc active failed with %i\n", ret);
+ drm_atomic_state_free(state);
+ return ret;
}
/**
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 60a29ff65f1f..c113f187090f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -989,7 +989,7 @@ void intel_mark_busy(struct drm_device *dev);
void intel_mark_idle(struct drm_device *dev);
void intel_crtc_restore_mode(struct drm_crtc *crtc);
void intel_display_suspend(struct drm_device *dev);
-void intel_crtc_control(struct drm_crtc *crtc, bool enable);
+int intel_crtc_control(struct drm_crtc *crtc, bool enable);
void intel_crtc_update_dpms(struct drm_crtc *crtc);
void intel_encoder_destroy(struct drm_encoder *encoder);
int intel_connector_init(struct intel_connector *);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3.6 16/22] drm/i915: Implement intel_crtc_control using atomic state, v4.
2015-05-26 8:35 ` [PATCH v3.6 16/22] drm/i915: Implement intel_crtc_control using atomic state, v4 Maarten Lankhorst
@ 2015-05-29 0:57 ` Matt Roper
2015-06-01 6:39 ` Maarten Lankhorst
0 siblings, 1 reply; 46+ messages in thread
From: Matt Roper @ 2015-05-29 0:57 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Intel Graphics Development
On Tue, May 26, 2015 at 10:35:22AM +0200, Maarten Lankhorst wrote:
> Assume the callers lock everything with drm_modeset_lock_all.
>
> This change had to be done after converting suspend/resume to
> use atomic_state so the atomic state is preserved, otherwise
> all transitional state is erased.
>
> Now all callers of .crtc_enable and .crtc_disable go through
> atomic modeset! :-D
>
> Changes since v1:
> - Only check for crtc_state->active in valleyview_modeset_global_pipes.
> - Only check for crtc_state->active in modeset_update_crtc_power_domains.
> Changes since v2:
> - Rework on top of the changed patch order.
> Changes since v3:
> - Rename intel_crtc_toggle in description to *_control
> - Change return value to int.
> - Do not add plane state, should be done implicitly already.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++--------------
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> 2 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e9680eb85bd0..32bab28432f7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5973,38 +5973,49 @@ void intel_display_suspend(struct drm_device *dev)
> }
>
> /* Master function to enable/disable CRTC and corresponding power wells */
> -void intel_crtc_control(struct drm_crtc *crtc, bool enable)
> +int intel_crtc_control(struct drm_crtc *crtc, bool enable)
We change the return value to 'int' here, but we never check the error
code anywhere. Maybe we should hold this off for another patch and then
also propagate the return value back up the call chain? It seems like
at least some of the connector-specific DPMS functions should recognize
the failure of intel_crtc_update_dpms() before trying to continue with
other operations.
> {
> struct drm_device *dev = crtc->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - enum intel_display_power_domain domain;
> - unsigned long domains;
> + struct intel_crtc_state *pipe_config;
> + struct drm_atomic_state *state;
> + int ret;
>
> if (enable == intel_crtc->active)
> - return;
> + return 0;
>
> if (enable && !crtc->state->enable)
> - return;
> + return 0;
>
> - crtc->state->active = enable;
> - if (enable) {
> - domains = get_crtc_power_domains(crtc);
> - for_each_power_domain(domain, domains)
> - intel_display_power_get(dev_priv, domain);
> - intel_crtc->enabled_power_domains = domains;
> + /* this function should be called with drm_modeset_lock_all for now */
> + if (WARN_ON(!ctx))
> + return -EIO;
EIO seems like an unusual choice here (but I'm not really sure what the
best option would be).
Matt
> + lockdep_assert_held(&ctx->ww_ctx);
>
> - dev_priv->display.crtc_enable(crtc);
> - intel_crtc_enable_planes(crtc);
> - } else {
> - intel_crtc_disable_planes(crtc);
> - dev_priv->display.crtc_disable(crtc);
> + state = drm_atomic_state_alloc(dev);
> + if (WARN_ON(!state))
> + return -ENOMEM;
>
> - domains = intel_crtc->enabled_power_domains;
> - for_each_power_domain(domain, domains)
> - intel_display_power_put(dev_priv, domain);
> - intel_crtc->enabled_power_domains = 0;
> + state->acquire_ctx = ctx;
> + state->allow_modeset = true;
> +
> + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
> + if (IS_ERR(pipe_config)) {
> + ret = PTR_ERR(pipe_config);
> + goto err;
> }
> + pipe_config->base.active = enable;
> +
> + ret = intel_set_mode(state);
> + if (!ret)
> + return ret;
> +
> +err:
> + DRM_ERROR("Updating crtc active failed with %i\n", ret);
> + drm_atomic_state_free(state);
> + return ret;
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 60a29ff65f1f..c113f187090f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -989,7 +989,7 @@ void intel_mark_busy(struct drm_device *dev);
> void intel_mark_idle(struct drm_device *dev);
> void intel_crtc_restore_mode(struct drm_crtc *crtc);
> void intel_display_suspend(struct drm_device *dev);
> -void intel_crtc_control(struct drm_crtc *crtc, bool enable);
> +int intel_crtc_control(struct drm_crtc *crtc, bool enable);
> void intel_crtc_update_dpms(struct drm_crtc *crtc);
> void intel_encoder_destroy(struct drm_encoder *encoder);
> int intel_connector_init(struct intel_connector *);
> --
> 2.1.0
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v3.6 16/22] drm/i915: Implement intel_crtc_control using atomic state, v4.
2015-05-29 0:57 ` Matt Roper
@ 2015-06-01 6:39 ` Maarten Lankhorst
0 siblings, 0 replies; 46+ messages in thread
From: Maarten Lankhorst @ 2015-06-01 6:39 UTC (permalink / raw)
To: Matt Roper; +Cc: Intel Graphics Development
Op 29-05-15 om 02:57 schreef Matt Roper:
> On Tue, May 26, 2015 at 10:35:22AM +0200, Maarten Lankhorst wrote:
>> Assume the callers lock everything with drm_modeset_lock_all.
>>
>> This change had to be done after converting suspend/resume to
>> use atomic_state so the atomic state is preserved, otherwise
>> all transitional state is erased.
>>
>> Now all callers of .crtc_enable and .crtc_disable go through
>> atomic modeset! :-D
>>
>> Changes since v1:
>> - Only check for crtc_state->active in valleyview_modeset_global_pipes.
>> - Only check for crtc_state->active in modeset_update_crtc_power_domains.
>> Changes since v2:
>> - Rework on top of the changed patch order.
>> Changes since v3:
>> - Rename intel_crtc_toggle in description to *_control
>> - Change return value to int.
>> - Do not add plane state, should be done implicitly already.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 53 ++++++++++++++++++++++--------------
>> drivers/gpu/drm/i915/intel_drv.h | 2 +-
>> 2 files changed, 33 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index e9680eb85bd0..32bab28432f7 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5973,38 +5973,49 @@ void intel_display_suspend(struct drm_device *dev)
>> }
>>
>> /* Master function to enable/disable CRTC and corresponding power wells */
>> -void intel_crtc_control(struct drm_crtc *crtc, bool enable)
>> +int intel_crtc_control(struct drm_crtc *crtc, bool enable)
> We change the return value to 'int' here, but we never check the error
> code anywhere. Maybe we should hold this off for another patch and then
> also propagate the return value back up the call chain? It seems like
> at least some of the connector-specific DPMS functions should recognize
> the failure of intel_crtc_update_dpms() before trying to continue with
> other operations.
Yeah, but I wasn't sure which ones, and handling is a lot harder. So I wanted to
mark this as 'int' to warn future users that this function may fail so they can check.
>> {
>> struct drm_device *dev = crtc->dev;
>> - struct drm_i915_private *dev_priv = dev->dev_private;
>> + struct drm_mode_config *config = &dev->mode_config;
>> + struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> - enum intel_display_power_domain domain;
>> - unsigned long domains;
>> + struct intel_crtc_state *pipe_config;
>> + struct drm_atomic_state *state;
>> + int ret;
>>
>> if (enable == intel_crtc->active)
>> - return;
>> + return 0;
>>
>> if (enable && !crtc->state->enable)
>> - return;
>> + return 0;
>>
>> - crtc->state->active = enable;
>> - if (enable) {
>> - domains = get_crtc_power_domains(crtc);
>> - for_each_power_domain(domain, domains)
>> - intel_display_power_get(dev_priv, domain);
>> - intel_crtc->enabled_power_domains = domains;
>> + /* this function should be called with drm_modeset_lock_all for now */
>> + if (WARN_ON(!ctx))
>> + return -EIO;
> EIO seems like an unusual choice here (but I'm not really sure what the
> best option would be).
No idea, I just wanted to make sure that it was recognized as programming error.
-EINVAL Is used everywhere in the atomic path, -ENODEV didn't seem appropriate.
>> + lockdep_assert_held(&ctx->ww_ctx);
>>
>> - dev_priv->display.crtc_enable(crtc);
>> - intel_crtc_enable_planes(crtc);
>> - } else {
>> - intel_crtc_disable_planes(crtc);
>> - dev_priv->display.crtc_disable(crtc);
>> + state = drm_atomic_state_alloc(dev);
>> + if (WARN_ON(!state))
>> + return -ENOMEM;
>>
>> - domains = intel_crtc->enabled_power_domains;
>> - for_each_power_domain(domain, domains)
>> - intel_display_power_put(dev_priv, domain);
>> - intel_crtc->enabled_power_domains = 0;
>> + state->acquire_ctx = ctx;
>> + state->allow_modeset = true;
>> +
>> + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
>> + if (IS_ERR(pipe_config)) {
>> + ret = PTR_ERR(pipe_config);
>> + goto err;
>> }
>> + pipe_config->base.active = enable;
>> +
>> + ret = intel_set_mode(state);
>> + if (!ret)
>> + return ret;
>> +
>> +err:
>> + DRM_ERROR("Updating crtc active failed with %i\n", ret);
>> + drm_atomic_state_free(state);
>> + return ret;
>> }
>>
>> /**
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 60a29ff65f1f..c113f187090f 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -989,7 +989,7 @@ void intel_mark_busy(struct drm_device *dev);
>> void intel_mark_idle(struct drm_device *dev);
>> void intel_crtc_restore_mode(struct drm_crtc *crtc);
>> void intel_display_suspend(struct drm_device *dev);
>> -void intel_crtc_control(struct drm_crtc *crtc, bool enable);
>> +int intel_crtc_control(struct drm_crtc *crtc, bool enable);
>> void intel_crtc_update_dpms(struct drm_crtc *crtc);
>> void intel_encoder_destroy(struct drm_encoder *encoder);
>> int intel_connector_init(struct intel_connector *);
>> --
>> 2.1.0
>>
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 17/22] drm/i915: move swap state to the right place
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (5 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 16/22] drm/i915: Implement intel_crtc_control using atomic state, v3 maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 18/22] drm/i915: Use crtc->hwmode for vblanks, v2 maarten.lankhorst
` (4 subsequent siblings)
11 siblings, 0 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
This is a preparation for passing crtc state to the helpers.
When converting all users of crtc->config to use the old or
new state it's easier to find regressions when swap_state is
done first.
If crtc->config is swapped at the same place as swap_state
bugs will never be found.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e819f1738458..12286b4ccd03 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11491,7 +11491,6 @@ intel_modeset_update_state(struct drm_atomic_state *state)
struct drm_connector *connector;
intel_shared_dpll_commit(state);
- drm_atomic_helper_swap_state(state->dev, state);
for_each_intel_encoder(dev, intel_encoder) {
if (!intel_encoder->base.crtc)
@@ -12191,8 +12190,10 @@ static int __intel_set_mode(struct drm_atomic_state *state)
if (ret)
return ret;
+ drm_atomic_helper_swap_state(dev, state);
+
for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!needs_modeset(crtc_state) || !crtc->state->active)
+ if (!needs_modeset(crtc->state) || !crtc_state->active)
continue;
intel_crtc_disable_planes(crtc);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3 18/22] drm/i915: Use crtc->hwmode for vblanks, v2.
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (6 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 17/22] drm/i915: move swap state to the right place maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 19/22] drm/i915: Remove use of crtc->config from i915_debugfs.c maarten.lankhorst
` (3 subsequent siblings)
11 siblings, 0 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
intel_crtc->config will be removed eventually, so use crtc->hwmode.
drm_atomic_helper_update_legacy_modeset_state updates hwmode,
but crtc->active will eventually be gone too. Set dotclock to zero
to indicate the crtc is inactive.
Changes since v1:
- With the hwmode update in drm*update_legacy_modeset_state removed,
intel_modeset_update_state has to assign it instead.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 13 ++++++-------
drivers/gpu/drm/i915/intel_display.c | 6 ++++++
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ca457317a8ac..9359ea2399f1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -564,8 +564,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
struct intel_crtc *intel_crtc =
to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
- const struct drm_display_mode *mode =
- &intel_crtc->config->base.adjusted_mode;
+ const struct drm_display_mode *mode = &intel_crtc->base.hwmode;
htotal = mode->crtc_htotal;
hsync_start = mode->crtc_hsync_start;
@@ -620,7 +619,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- const struct drm_display_mode *mode = &crtc->config->base.adjusted_mode;
+ const struct drm_display_mode *mode = &crtc->base.hwmode;
enum pipe pipe = crtc->pipe;
int position, vtotal;
@@ -647,14 +646,14 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- const struct drm_display_mode *mode = &intel_crtc->config->base.adjusted_mode;
+ const struct drm_display_mode *mode = &intel_crtc->base.hwmode;
int position;
int vbl_start, vbl_end, hsync_start, htotal, vtotal;
bool in_vbl = true;
int ret = 0;
unsigned long irqflags;
- if (!intel_crtc->active) {
+ if (WARN_ON(!mode->crtc_clock)) {
DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled "
"pipe %c\n", pipe_name(pipe));
return 0;
@@ -796,7 +795,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
return -EINVAL;
}
- if (!crtc->state->active) {
+ if (!crtc->hwmode.crtc_clock) {
DRM_DEBUG_KMS("crtc %d is disabled\n", pipe);
return -EBUSY;
}
@@ -805,7 +804,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
vblank_time, flags,
crtc,
- &to_intel_crtc(crtc)->config->base.adjusted_mode);
+ &crtc->hwmode);
}
static bool intel_hpd_irq_event(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 12286b4ccd03..8acc27845fb5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11511,6 +11511,12 @@ intel_modeset_update_state(struct drm_atomic_state *state)
WARN_ON(crtc->state->enable != intel_crtc_in_use(crtc));
to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state);
+
+ /* Update hwmode for vblank functions */
+ if (crtc->state->active)
+ crtc->hwmode = crtc->state->adjusted_mode;
+ else
+ crtc->hwmode.crtc_clock = 0;
}
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3 19/22] drm/i915: Remove use of crtc->config from i915_debugfs.c
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (7 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 18/22] drm/i915: Use crtc->hwmode for vblanks, v2 maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 20/22] drm/i915: Calculate haswell plane workaround, v3 maarten.lankhorst
` (2 subsequent siblings)
11 siblings, 0 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
crtc->config is updated to always contain to the active crtc_state
and only differs from crtc_state during crtc_disable. It will
eventually be removed, so start with some low hanging fruit.
For crtc->active the situation is the same; it will be removed
eventually. Instead use crtc->state->active.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 42 ++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8bf21e8abd93..79a86d3e8f13 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2755,13 +2755,16 @@ static int i915_display_info(struct seq_file *m, void *unused)
seq_printf(m, "---------\n");
for_each_intel_crtc(dev, crtc) {
bool active;
+ struct intel_crtc_state *pipe_config;
int x, y;
+ pipe_config = to_intel_crtc_state(crtc->base.state);
+
seq_printf(m, "CRTC %d: pipe: %c, active=%s (size=%dx%d)\n",
crtc->base.base.id, pipe_name(crtc->pipe),
- yesno(crtc->active), crtc->config->pipe_src_w,
- crtc->config->pipe_src_h);
- if (crtc->active) {
+ yesno(pipe_config->base.active),
+ pipe_config->pipe_src_w, pipe_config->pipe_src_h);
+ if (pipe_config->base.active) {
intel_crtc_info(m, crtc);
active = cursor_position(dev, crtc->pipe, &x, &y);
@@ -3002,7 +3005,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
seq_puts(m, "\n\n");
- if (intel_crtc->config->has_drrs) {
+ if (to_intel_crtc_state(intel_crtc->base.state)->has_drrs) {
struct intel_panel *panel;
mutex_lock(&drrs->mutex);
@@ -3054,7 +3057,7 @@ static int i915_drrs_status(struct seq_file *m, void *unused)
for_each_intel_crtc(dev, intel_crtc) {
drm_modeset_lock(&intel_crtc->base.mutex, NULL);
- if (intel_crtc->active) {
+ if (intel_crtc->base.state->active) {
active_crtc_cnt++;
seq_printf(m, "\nCRTC %d: ", active_crtc_cnt);
@@ -3596,22 +3599,27 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *crtc =
to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_A]);
+ struct intel_crtc_state *pipe_config;
drm_modeset_lock_all(dev);
+ pipe_config = to_intel_crtc_state(crtc->base.state);
+
/*
* If we use the eDP transcoder we need to make sure that we don't
* bypass the pfit, since otherwise the pipe CRC source won't work. Only
* relevant on hsw with pipe A when using the always-on power well
* routing.
*/
- if (crtc->config->cpu_transcoder == TRANSCODER_EDP &&
- !crtc->config->pch_pfit.enabled) {
- bool active = crtc->active;
+ if (pipe_config->cpu_transcoder == TRANSCODER_EDP &&
+ !pipe_config->pch_pfit.enabled) {
+ bool active = pipe_config->base.active;
- if (active)
+ if (active) {
intel_crtc_control(&crtc->base, false);
+ pipe_config = to_intel_crtc_state(crtc->base.state);
+ }
- crtc->config->pch_pfit.force_thru = true;
+ pipe_config->pch_pfit.force_thru = true;
intel_display_power_get(dev_priv,
POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
@@ -3627,6 +3635,7 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *crtc =
to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_A]);
+ struct intel_crtc_state *pipe_config;
drm_modeset_lock_all(dev);
/*
@@ -3635,13 +3644,16 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
* relevant on hsw with pipe A when using the always-on power well
* routing.
*/
- if (crtc->config->pch_pfit.force_thru) {
- bool active = crtc->active;
+ pipe_config = to_intel_crtc_state(crtc->base.state);
+ if (pipe_config->pch_pfit.force_thru) {
+ bool active = pipe_config->base.active;
- if (active)
+ if (active) {
intel_crtc_control(&crtc->base, false);
+ pipe_config = to_intel_crtc_state(crtc->base.state);
+ }
- crtc->config->pch_pfit.force_thru = false;
+ pipe_config->pch_pfit.force_thru = false;
intel_display_power_put(dev_priv,
POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
@@ -3763,7 +3775,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
pipe_name(pipe));
drm_modeset_lock(&crtc->base.mutex, NULL);
- if (crtc->active)
+ if (crtc->base.state->active)
intel_wait_for_vblank(dev, pipe);
drm_modeset_unlock(&crtc->base.mutex);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3 20/22] drm/i915: Calculate haswell plane workaround, v3.
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (8 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 19/22] drm/i915: Remove use of crtc->config from i915_debugfs.c maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-26 8:36 ` [PATCH v3.6 20/22] drm/i915: Calculate haswell plane workaround, v4 Maarten Lankhorst
2015-05-20 16:04 ` [PATCH v3 21/22] drm/i915: Use atomic state for calculating DVO_2X_MODE on i830 maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 22/22] drm/i915: use calculated state for vblank evasion maarten.lankhorst
11 siblings, 1 reply; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
This needs to be a global check because at the time of crtc checking
not all modesets have to be calculated yet. Use intel_crtc->atomic
because there's no reason to keep it in state.
Changes since v1:
- Use intel_crtc->atomic as a place to put hsw_workaround_pipe.
- Make sure quirk only applies to haswell.
- Use first loop to iterate over newly enabled crtc's only.
This increases readability.
Changes since v2:
- Move hsw_workaround_pipe back to crtc_state.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 107 +++++++++++++++++++++++++----------
drivers/gpu/drm/i915/intel_drv.h | 3 +
2 files changed, 79 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8acc27845fb5..2c7581c6803d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4870,42 +4870,15 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
}
-/*
- * This implements the workaround described in the "notes" section of the mode
- * set sequence documentation. When going from no pipes or single pipe to
- * multiple pipes, and planes are enabled after the pipe, we need to wait at
- * least 2 vblanks on the first pipe before enabling planes on the second pipe.
- */
-static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc)
-{
- struct drm_device *dev = crtc->base.dev;
- struct intel_crtc *crtc_it, *other_active_crtc = NULL;
-
- /* We want to get the other_active_crtc only if there's only 1 other
- * active crtc. */
- for_each_intel_crtc(dev, crtc_it) {
- if (!crtc_it->active || crtc_it == crtc)
- continue;
-
- if (other_active_crtc)
- return;
-
- other_active_crtc = crtc_it;
- }
- if (!other_active_crtc)
- return;
-
- intel_wait_for_vblank(dev, other_active_crtc->pipe);
- intel_wait_for_vblank(dev, other_active_crtc->pipe);
-}
-
static void haswell_crtc_enable(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *encoder;
- int pipe = intel_crtc->pipe;
+ int pipe = intel_crtc->pipe, hsw_workaround_pipe;
+ struct intel_crtc_state *pipe_config =
+ to_intel_crtc_state(crtc->state);
if (WARN_ON(intel_crtc->active))
return;
@@ -4982,7 +4955,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
/* If we change the relative order between pipe/planes enabling, we need
* to change the workaround. */
- haswell_mode_set_planes_workaround(intel_crtc);
+ hsw_workaround_pipe = pipe_config->hsw_workaround_pipe;
+ if (IS_HASWELL(dev) && hsw_workaround_pipe != INVALID_PIPE) {
+ intel_wait_for_vblank(dev, hsw_workaround_pipe);
+ intel_wait_for_vblank(dev, hsw_workaround_pipe);
+ }
}
static void ironlake_pfit_disable(struct intel_crtc *crtc)
@@ -12122,6 +12099,71 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
return ret;
}
+/*
+ * This implements the workaround described in the "notes" section of the mode
+ * set sequence documentation. When going from no pipes or single pipe to
+ * multiple pipes, and planes are enabled after the pipe, we need to wait at
+ * least 2 vblanks on the first pipe before enabling planes on the second pipe.
+ */
+static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state)
+{
+ struct drm_crtc_state *crtc_state;
+ struct intel_crtc *intel_crtc;
+ struct drm_crtc *crtc;
+ struct intel_crtc_state *first_crtc_state = NULL;
+ struct intel_crtc_state *other_crtc_state = NULL;
+ enum pipe first_pipe = INVALID_PIPE, enabled_pipe = INVALID_PIPE;
+ int i;
+
+ /* look at all crtc's that are going to be enabled in during modeset */
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ intel_crtc = to_intel_crtc(crtc);
+
+ if (!crtc_state->active || !needs_modeset(crtc_state))
+ continue;
+
+ if (first_crtc_state) {
+ other_crtc_state = to_intel_crtc_state(crtc_state);
+ break;
+ } else {
+ first_crtc_state = to_intel_crtc_state(crtc_state);
+ first_pipe = intel_crtc->pipe;
+ }
+ }
+
+ /* No workaround needed? */
+ if (!first_crtc_state)
+ return 0;
+
+ /* w/a possibly needed, check how many crtc's are already enabled. */
+ for_each_intel_crtc(state->dev, intel_crtc) {
+ struct intel_crtc_state *pipe_config;
+
+ pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
+ if (IS_ERR(pipe_config))
+ return PTR_ERR(pipe_config);
+
+ pipe_config->hsw_workaround_pipe = INVALID_PIPE;
+
+ if (!pipe_config->base.active ||
+ needs_modeset(&pipe_config->base))
+ continue;
+
+ /* 2 or more enabled crtcs means no need for w/a */
+ if (enabled_pipe != INVALID_PIPE)
+ return 0;
+
+ enabled_pipe = intel_crtc->pipe;
+ }
+
+ if (enabled_pipe != INVALID_PIPE)
+ first_crtc_state->hsw_workaround_pipe = enabled_pipe;
+ else if (other_crtc_state)
+ other_crtc_state->hsw_workaround_pipe = first_pipe;
+
+ return 0;
+}
+
/* Code that should eventually be part of atomic_check() */
static int __intel_set_mode_checks(struct drm_atomic_state *state)
{
@@ -12145,6 +12187,9 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
if (ret)
return ret;
+ if (IS_HASWELL(dev))
+ haswell_mode_set_planes_workaround(state);
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 65f5f3d41b5a..bce52c5f4fe4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -443,6 +443,9 @@ struct intel_crtc_state {
int pbn;
struct intel_crtc_scaler_state scaler_state;
+
+ /* w/a for waiting 2 vblanks during crtc enable */
+ enum pipe hsw_workaround_pipe;
};
struct intel_pipe_wm {
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3.6 20/22] drm/i915: Calculate haswell plane workaround, v4.
2015-05-20 16:04 ` [PATCH v3 20/22] drm/i915: Calculate haswell plane workaround, v3 maarten.lankhorst
@ 2015-05-26 8:36 ` Maarten Lankhorst
2015-05-29 0:58 ` Matt Roper
0 siblings, 1 reply; 46+ messages in thread
From: Maarten Lankhorst @ 2015-05-26 8:36 UTC (permalink / raw)
To: Intel Graphics Development
This needs to be a global check because at the time of crtc checking
not all modesets have to be calculated yet. Use intel_crtc->atomic
because there's no reason to keep it in state.
Changes since v1:
- Use intel_crtc->atomic as a place to put hsw_workaround_pipe.
- Make sure quirk only applies to haswell.
- Use first loop to iterate over newly enabled crtc's only.
This increases readability.
Changes since v2:
- Move hsw_workaround_pipe back to crtc_state.
Changes since v3:
- Return errors from haswell_mode_set_planes_workaround.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++++++++----------
drivers/gpu/drm/i915/intel_drv.h | 3 +
2 files changed, 80 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de13c1c14c93..f6733a777590 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4867,42 +4867,15 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
}
-/*
- * This implements the workaround described in the "notes" section of the mode
- * set sequence documentation. When going from no pipes or single pipe to
- * multiple pipes, and planes are enabled after the pipe, we need to wait at
- * least 2 vblanks on the first pipe before enabling planes on the second pipe.
- */
-static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc)
-{
- struct drm_device *dev = crtc->base.dev;
- struct intel_crtc *crtc_it, *other_active_crtc = NULL;
-
- /* We want to get the other_active_crtc only if there's only 1 other
- * active crtc. */
- for_each_intel_crtc(dev, crtc_it) {
- if (!crtc_it->active || crtc_it == crtc)
- continue;
-
- if (other_active_crtc)
- return;
-
- other_active_crtc = crtc_it;
- }
- if (!other_active_crtc)
- return;
-
- intel_wait_for_vblank(dev, other_active_crtc->pipe);
- intel_wait_for_vblank(dev, other_active_crtc->pipe);
-}
-
static void haswell_crtc_enable(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *encoder;
- int pipe = intel_crtc->pipe;
+ int pipe = intel_crtc->pipe, hsw_workaround_pipe;
+ struct intel_crtc_state *pipe_config =
+ to_intel_crtc_state(crtc->state);
if (WARN_ON(intel_crtc->active))
return;
@@ -4979,7 +4952,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
/* If we change the relative order between pipe/planes enabling, we need
* to change the workaround. */
- haswell_mode_set_planes_workaround(intel_crtc);
+ hsw_workaround_pipe = pipe_config->hsw_workaround_pipe;
+ if (IS_HASWELL(dev) && hsw_workaround_pipe != INVALID_PIPE) {
+ intel_wait_for_vblank(dev, hsw_workaround_pipe);
+ intel_wait_for_vblank(dev, hsw_workaround_pipe);
+ }
}
static void ironlake_pfit_disable(struct intel_crtc *crtc)
@@ -12169,6 +12146,71 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
return ret;
}
+/*
+ * This implements the workaround described in the "notes" section of the mode
+ * set sequence documentation. When going from no pipes or single pipe to
+ * multiple pipes, and planes are enabled after the pipe, we need to wait at
+ * least 2 vblanks on the first pipe before enabling planes on the second pipe.
+ */
+static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state)
+{
+ struct drm_crtc_state *crtc_state;
+ struct intel_crtc *intel_crtc;
+ struct drm_crtc *crtc;
+ struct intel_crtc_state *first_crtc_state = NULL;
+ struct intel_crtc_state *other_crtc_state = NULL;
+ enum pipe first_pipe = INVALID_PIPE, enabled_pipe = INVALID_PIPE;
+ int i;
+
+ /* look at all crtc's that are going to be enabled in during modeset */
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ intel_crtc = to_intel_crtc(crtc);
+
+ if (!crtc_state->active || !needs_modeset(crtc_state))
+ continue;
+
+ if (first_crtc_state) {
+ other_crtc_state = to_intel_crtc_state(crtc_state);
+ break;
+ } else {
+ first_crtc_state = to_intel_crtc_state(crtc_state);
+ first_pipe = intel_crtc->pipe;
+ }
+ }
+
+ /* No workaround needed? */
+ if (!first_crtc_state)
+ return 0;
+
+ /* w/a possibly needed, check how many crtc's are already enabled. */
+ for_each_intel_crtc(state->dev, intel_crtc) {
+ struct intel_crtc_state *pipe_config;
+
+ pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
+ if (IS_ERR(pipe_config))
+ return PTR_ERR(pipe_config);
+
+ pipe_config->hsw_workaround_pipe = INVALID_PIPE;
+
+ if (!pipe_config->base.active ||
+ needs_modeset(&pipe_config->base))
+ continue;
+
+ /* 2 or more enabled crtcs means no need for w/a */
+ if (enabled_pipe != INVALID_PIPE)
+ return 0;
+
+ enabled_pipe = intel_crtc->pipe;
+ }
+
+ if (enabled_pipe != INVALID_PIPE)
+ first_crtc_state->hsw_workaround_pipe = enabled_pipe;
+ else if (other_crtc_state)
+ other_crtc_state->hsw_workaround_pipe = first_pipe;
+
+ return 0;
+}
+
/* Code that should eventually be part of atomic_check() */
static int __intel_set_mode_checks(struct drm_atomic_state *state)
{
@@ -12192,7 +12234,10 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
if (ret)
return ret;
- return 0;
+ if (IS_HASWELL(dev))
+ ret = haswell_mode_set_planes_workaround(state);
+
+ return ret;
}
static int
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 665e249ae8bf..d5f38e3d732b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -443,6 +443,9 @@ struct intel_crtc_state {
int pbn;
struct intel_crtc_scaler_state scaler_state;
+
+ /* w/a for waiting 2 vblanks during crtc enable */
+ enum pipe hsw_workaround_pipe;
};
struct intel_pipe_wm {
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v3.6 20/22] drm/i915: Calculate haswell plane workaround, v4.
2015-05-26 8:36 ` [PATCH v3.6 20/22] drm/i915: Calculate haswell plane workaround, v4 Maarten Lankhorst
@ 2015-05-29 0:58 ` Matt Roper
0 siblings, 0 replies; 46+ messages in thread
From: Matt Roper @ 2015-05-29 0:58 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Intel Graphics Development
On Tue, May 26, 2015 at 10:36:14AM +0200, Maarten Lankhorst wrote:
> This needs to be a global check because at the time of crtc checking
> not all modesets have to be calculated yet. Use intel_crtc->atomic
> because there's no reason to keep it in state.
The note about intel_crtc->atomic is out of date as of v3. Otherwise, I
think this looks okay.
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Matt
>
> Changes since v1:
> - Use intel_crtc->atomic as a place to put hsw_workaround_pipe.
> - Make sure quirk only applies to haswell.
> - Use first loop to iterate over newly enabled crtc's only.
> This increases readability.
> Changes since v2:
> - Move hsw_workaround_pipe back to crtc_state.
> Changes since v3:
> - Return errors from haswell_mode_set_planes_workaround.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++++++++----------
> drivers/gpu/drm/i915/intel_drv.h | 3 +
> 2 files changed, 80 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index de13c1c14c93..f6733a777590 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4867,42 +4867,15 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
> return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
> }
>
> -/*
> - * This implements the workaround described in the "notes" section of the mode
> - * set sequence documentation. When going from no pipes or single pipe to
> - * multiple pipes, and planes are enabled after the pipe, we need to wait at
> - * least 2 vblanks on the first pipe before enabling planes on the second pipe.
> - */
> -static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc)
> -{
> - struct drm_device *dev = crtc->base.dev;
> - struct intel_crtc *crtc_it, *other_active_crtc = NULL;
> -
> - /* We want to get the other_active_crtc only if there's only 1 other
> - * active crtc. */
> - for_each_intel_crtc(dev, crtc_it) {
> - if (!crtc_it->active || crtc_it == crtc)
> - continue;
> -
> - if (other_active_crtc)
> - return;
> -
> - other_active_crtc = crtc_it;
> - }
> - if (!other_active_crtc)
> - return;
> -
> - intel_wait_for_vblank(dev, other_active_crtc->pipe);
> - intel_wait_for_vblank(dev, other_active_crtc->pipe);
> -}
> -
> static void haswell_crtc_enable(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_encoder *encoder;
> - int pipe = intel_crtc->pipe;
> + int pipe = intel_crtc->pipe, hsw_workaround_pipe;
> + struct intel_crtc_state *pipe_config =
> + to_intel_crtc_state(crtc->state);
>
> if (WARN_ON(intel_crtc->active))
> return;
> @@ -4979,7 +4952,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> /* If we change the relative order between pipe/planes enabling, we need
> * to change the workaround. */
> - haswell_mode_set_planes_workaround(intel_crtc);
> + hsw_workaround_pipe = pipe_config->hsw_workaround_pipe;
> + if (IS_HASWELL(dev) && hsw_workaround_pipe != INVALID_PIPE) {
> + intel_wait_for_vblank(dev, hsw_workaround_pipe);
> + intel_wait_for_vblank(dev, hsw_workaround_pipe);
> + }
> }
>
> static void ironlake_pfit_disable(struct intel_crtc *crtc)
> @@ -12169,6 +12146,71 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
> return ret;
> }
>
> +/*
> + * This implements the workaround described in the "notes" section of the mode
> + * set sequence documentation. When going from no pipes or single pipe to
> + * multiple pipes, and planes are enabled after the pipe, we need to wait at
> + * least 2 vblanks on the first pipe before enabling planes on the second pipe.
> + */
> +static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state)
> +{
> + struct drm_crtc_state *crtc_state;
> + struct intel_crtc *intel_crtc;
> + struct drm_crtc *crtc;
> + struct intel_crtc_state *first_crtc_state = NULL;
> + struct intel_crtc_state *other_crtc_state = NULL;
> + enum pipe first_pipe = INVALID_PIPE, enabled_pipe = INVALID_PIPE;
> + int i;
> +
> + /* look at all crtc's that are going to be enabled in during modeset */
> + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + intel_crtc = to_intel_crtc(crtc);
> +
> + if (!crtc_state->active || !needs_modeset(crtc_state))
> + continue;
> +
> + if (first_crtc_state) {
> + other_crtc_state = to_intel_crtc_state(crtc_state);
> + break;
> + } else {
> + first_crtc_state = to_intel_crtc_state(crtc_state);
> + first_pipe = intel_crtc->pipe;
> + }
> + }
> +
> + /* No workaround needed? */
> + if (!first_crtc_state)
> + return 0;
> +
> + /* w/a possibly needed, check how many crtc's are already enabled. */
> + for_each_intel_crtc(state->dev, intel_crtc) {
> + struct intel_crtc_state *pipe_config;
> +
> + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
> + if (IS_ERR(pipe_config))
> + return PTR_ERR(pipe_config);
> +
> + pipe_config->hsw_workaround_pipe = INVALID_PIPE;
> +
> + if (!pipe_config->base.active ||
> + needs_modeset(&pipe_config->base))
> + continue;
> +
> + /* 2 or more enabled crtcs means no need for w/a */
> + if (enabled_pipe != INVALID_PIPE)
> + return 0;
> +
> + enabled_pipe = intel_crtc->pipe;
> + }
> +
> + if (enabled_pipe != INVALID_PIPE)
> + first_crtc_state->hsw_workaround_pipe = enabled_pipe;
> + else if (other_crtc_state)
> + other_crtc_state->hsw_workaround_pipe = first_pipe;
> +
> + return 0;
> +}
> +
> /* Code that should eventually be part of atomic_check() */
> static int __intel_set_mode_checks(struct drm_atomic_state *state)
> {
> @@ -12192,7 +12234,10 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
> if (ret)
> return ret;
>
> - return 0;
> + if (IS_HASWELL(dev))
> + ret = haswell_mode_set_planes_workaround(state);
> +
> + return ret;
> }
>
> static int
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 665e249ae8bf..d5f38e3d732b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -443,6 +443,9 @@ struct intel_crtc_state {
> int pbn;
>
> struct intel_crtc_scaler_state scaler_state;
> +
> + /* w/a for waiting 2 vblanks during crtc enable */
> + enum pipe hsw_workaround_pipe;
> };
>
> struct intel_pipe_wm {
> --
> 2.1.0
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v3 21/22] drm/i915: Use atomic state for calculating DVO_2X_MODE on i830.
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (9 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 20/22] drm/i915: Calculate haswell plane workaround, v3 maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
2015-05-20 16:04 ` [PATCH v3 22/22] drm/i915: use calculated state for vblank evasion maarten.lankhorst
11 siblings, 0 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
This is a small behavioral change because it leaves DVO_2X_MODE
set between crtc_disable and crtc_enable. This is probably harmless
though and if not should be fixed by calculating 2x mode before
enable/disable pll.
This is needed because intel_crtc->active will be removed eventually.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2c7581c6803d..008b011308de 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1698,7 +1698,7 @@ static int intel_num_dvo_pipes(struct drm_device *dev)
int count = 0;
for_each_intel_crtc(dev, crtc)
- count += crtc->active &&
+ count += crtc->base.state->active &&
intel_pipe_has_type(crtc, INTEL_OUTPUT_DVO);
return count;
@@ -1779,7 +1779,7 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
/* Disable DVO 2x clock on both PLLs if necessary */
if (IS_I830(dev) &&
intel_pipe_has_type(crtc, INTEL_OUTPUT_DVO) &&
- intel_num_dvo_pipes(dev) == 1) {
+ !intel_num_dvo_pipes(dev)) {
I915_WRITE(DPLL(PIPE_B),
I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
I915_WRITE(DPLL(PIPE_A),
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v3 22/22] drm/i915: use calculated state for vblank evasion
2015-05-20 16:04 ` [PATCH v3 10/22] drm/i915: Support modeset across multiple pipes maarten.lankhorst
` (10 preceding siblings ...)
2015-05-20 16:04 ` [PATCH v3 21/22] drm/i915: Use atomic state for calculating DVO_2X_MODE on i830 maarten.lankhorst
@ 2015-05-20 16:04 ` maarten.lankhorst
11 siblings, 0 replies; 46+ messages in thread
From: maarten.lankhorst @ 2015-05-20 16:04 UTC (permalink / raw)
To: intel-gfx
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
crtc->active will be gone eventually, and this check should be just as good.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 008b011308de..d050a59ce9f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12953,6 +12953,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_crtc_state *crtc_state = intel_crtc->base.state;
struct intel_plane *intel_plane;
struct drm_plane *p;
unsigned fb_bits = 0;
@@ -12996,7 +12997,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
intel_runtime_pm_get(dev_priv);
/* Perform vblank evasion around commit operation */
- if (intel_crtc->active)
+ if (crtc_state->active && !needs_modeset(crtc_state))
intel_crtc->atomic.evade =
intel_pipe_update_start(intel_crtc,
&intel_crtc->atomic.start_vbl_count);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 46+ messages in thread