* [PATCH v2 00/17] drm/i915: Convert to atomic, part 2.
@ 2015-05-13 20:23 Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 01/17] drm/atomic: update crtc->hwmode in legacy state Maarten Lankhorst
` (16 more replies)
0 siblings, 17 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
I made a small mistake in the original ordering of this series.
This resulted in patches trying to do too much, and things became
too confusing. I removed the atomic plane conversion patches,
and changed ordering.
The new ordering should be more logical, instead of converting planes
to atomic first I do it after this series, to reduce the review burden
on this series and to unblock others.
The goal of this patch series is to implement hardware readout using
atomic state, and restore sw state with a call to intel_set_mode.
After that's done intel_crtc_toggle can be safely converted to
atomic modeset, because nothing relies on transitional state any
more.
This series contains everything except conversion to atomic planes
and almost all of the crtc->config and crtc->active removal patches
are removed too, I need to split those up some more.
Ander Conselvan de Oliveira (6):
drm/i915: Set mode_changed for audio in intel_modeset_pipe_config()
drm/i915: Make __intel_set_mode() take only atomic state as argument
drm/i915: Support modeset across multiple pipes
drm/i915: Use global atomic state for staged pll config
drm/i915: Read hw state into an atomic state struct
drm/i915: Move cdclk and pll setup to intel_modeset_compute_config()
Maarten Lankhorst (11):
drm/atomic: update crtc->hwmode in legacy state
drm/atomic: Allow drivers to subclass drm_atomic_state, v2
drm/i915: get rid of put_shared_dpll
drm/i915: get rid of intel_crtc_disable and related code, v2
drm/i915: use intel_crtc_control everywhere
drm/i915: Use drm_atomic_helper_update_legacy_modeset_state
drm/i915: Use crtc_state->active instead of crtc_state->enable
drm/i915: Implement intel_crtc_toggle using atomic state, v3
drm/i915: Calculate haswell plane workaround, v2.
drm/i915: Use crtc->hwmode for vblanks.
drm/i915: Remove use of crtc->config from i915_debugfs.c
drivers/gpu/drm/drm_atomic.c | 116 +++-
drivers/gpu/drm/drm_atomic_helper.c | 1 +
drivers/gpu/drm/i915/i915_debugfs.c | 50 +-
drivers/gpu/drm/i915/i915_drv.h | 2 -
drivers/gpu/drm/i915/i915_irq.c | 13 +-
drivers/gpu/drm/i915/intel_atomic.c | 49 ++
drivers/gpu/drm/i915/intel_display.c | 1105 ++++++++++++++++------------------
drivers/gpu/drm/i915/intel_drv.h | 32 +-
include/drm/drm_atomic.h | 5 +
include/drm/drm_crtc.h | 6 +
10 files changed, 729 insertions(+), 650 deletions(-)
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 01/17] drm/atomic: update crtc->hwmode in legacy state
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-18 8:01 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v2 Maarten Lankhorst
` (15 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
This is useful when calculating vblank in drivers that support it.
During a modeset the atomic state may not match the hardware state,
so if the driver wants to wait on a vblank they'll want to use
crtc->hwmode rather than crtc->state->adjusted_mode.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d192b48d919a..63613ce673c7 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -673,6 +673,7 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
/* set legacy state in the crtc structure */
for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) {
crtc->mode = crtc->state->mode;
+ crtc->hwmode = crtc->state->adjusted_mode;
crtc->enabled = crtc->state->enable;
crtc->x = crtc->primary->state->src_x >> 16;
crtc->y = crtc->primary->state->src_y >> 16;
--
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] 37+ messages in thread
* [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v2
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 01/17] drm/atomic: update crtc->hwmode in legacy state Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-18 8:06 ` [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v3 Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 03/17] drm/i915: get rid of put_shared_dpll Maarten Lankhorst
` (14 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
Drivers may need to store the state of shared resources, such as PLLs
or FIFO space, into the atomic state. Allow this by making it possible
to subclass drm_atomic_state.
Changes since v1:
- Change member names for functions to atomic_state_(alloc,clear)
- Change __drm_atomic_state_new to drm_atomic_state_init
- Allow free function to be overridden too, in case extra memory is
allocated in alloc.
Cc: dri-devel@lists.freedesktop.org
Acked-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/drm_atomic.c | 116 ++++++++++++++++++++++++++++++++-----------
include/drm/drm_atomic.h | 5 ++
include/drm/drm_crtc.h | 6 +++
3 files changed, 99 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index c6277a4a1f2f..47364f244dc0 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -30,7 +30,15 @@
#include <drm/drm_atomic.h>
#include <drm/drm_plane_helper.h>
-static void kfree_state(struct drm_atomic_state *state)
+/**
+ * drm_atomic_state_default_free -
+ * free memory initialized by drm_atomic_state_init
+ * @state: atomic state
+ *
+ * Free all the memory allocated by drm_atomic_state_init.
+ * This is useful for drivers that subclass the atomic state.
+ */
+void drm_atomic_state_default_free(struct drm_atomic_state *state)
{
kfree(state->connectors);
kfree(state->connector_states);
@@ -38,24 +46,20 @@ static void kfree_state(struct drm_atomic_state *state)
kfree(state->crtc_states);
kfree(state->planes);
kfree(state->plane_states);
- kfree(state);
}
+EXPORT_SYMBOL(drm_atomic_state_default_free);
/**
- * drm_atomic_state_alloc - allocate atomic state
+ * drm_atomic_state_init - init new atomic state
* @dev: DRM device
+ * @state: atomic state
*
- * This allocates an empty atomic state to track updates.
+ * Default implementation for filling in a new atomic state.
+ * This is useful for drivers that subclass the atomic state.
*/
-struct drm_atomic_state *
-drm_atomic_state_alloc(struct drm_device *dev)
+int
+drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
{
- struct drm_atomic_state *state;
-
- state = kzalloc(sizeof(*state), GFP_KERNEL);
- if (!state)
- return NULL;
-
/* TODO legacy paths should maybe do a better job about
* setting this appropriately?
*/
@@ -92,31 +96,50 @@ drm_atomic_state_alloc(struct drm_device *dev)
state->dev = dev;
- DRM_DEBUG_ATOMIC("Allocate atomic state %p\n", state);
+ DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state);
- return state;
+ return 0;
fail:
- kfree_state(state);
+ drm_atomic_state_default_free(state);
+ return -ENOMEM;
+}
+EXPORT_SYMBOL(drm_atomic_state_init);
+
+/**
+ * drm_atomic_state_alloc - allocate atomic state
+ * @dev: DRM device
+ *
+ * This allocates an empty atomic state to track updates.
+ */
+struct drm_atomic_state *
+drm_atomic_state_alloc(struct drm_device *dev)
+{
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_atomic_state *state;
+
+ if (!config->funcs->atomic_state_alloc) {
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return NULL;
+ if (drm_atomic_state_init(dev, state) < 0) {
+ kfree(state);
+ return NULL;
+ }
+ return state;
+ }
- return NULL;
+ return config->funcs->atomic_state_alloc(dev);
}
EXPORT_SYMBOL(drm_atomic_state_alloc);
/**
- * drm_atomic_state_clear - clear state object
+ * drm_atomic_state_default_clear - clear base atomic state
* @state: atomic state
*
- * When the w/w mutex algorithm detects a deadlock we need to back off and drop
- * all locks. So someone else could sneak in and change the current modeset
- * configuration. Which means that all the state assembled in @state is no
- * longer an atomic update to the current state, but to some arbitrary earlier
- * state. Which could break assumptions the driver's ->atomic_check likely
- * relies on.
- *
- * Hence we must clear all cached state and completely start over, using this
- * function.
+ * Default implementation for clearing atomic state.
+ * This is useful for drivers that subclass the atomic state.
*/
-void drm_atomic_state_clear(struct drm_atomic_state *state)
+void drm_atomic_state_default_clear(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
struct drm_mode_config *config = &dev->mode_config;
@@ -162,6 +185,32 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)
state->plane_states[i] = NULL;
}
}
+EXPORT_SYMBOL(drm_atomic_state_default_clear);
+
+/**
+ * drm_atomic_state_clear - clear state object
+ * @state: atomic state
+ *
+ * When the w/w mutex algorithm detects a deadlock we need to back off and drop
+ * all locks. So someone else could sneak in and change the current modeset
+ * configuration. Which means that all the state assembled in @state is no
+ * longer an atomic update to the current state, but to some arbitrary earlier
+ * state. Which could break assumptions the driver's ->atomic_check likely
+ * relies on.
+ *
+ * Hence we must clear all cached state and completely start over, using this
+ * function.
+ */
+void drm_atomic_state_clear(struct drm_atomic_state *state)
+{
+ struct drm_device *dev = state->dev;
+ struct drm_mode_config *config = &dev->mode_config;
+
+ if (config->funcs->atomic_state_clear)
+ config->funcs->atomic_state_clear(state);
+ else
+ drm_atomic_state_default_clear(state);
+}
EXPORT_SYMBOL(drm_atomic_state_clear);
/**
@@ -173,14 +222,25 @@ EXPORT_SYMBOL(drm_atomic_state_clear);
*/
void drm_atomic_state_free(struct drm_atomic_state *state)
{
+ struct drm_device *dev;
+ struct drm_mode_config *config;
+
if (!state)
return;
+ dev = state->dev;
+ config = &dev->mode_config;
+
drm_atomic_state_clear(state);
DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
- kfree_state(state);
+ if (config->funcs->atomic_state_free) {
+ config->funcs->atomic_state_free(state);
+ } else {
+ drm_atomic_state_default_free(state);
+ kfree(state);
+ }
}
EXPORT_SYMBOL(drm_atomic_state_free);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index d78543067700..6445970535ec 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -35,6 +35,11 @@ drm_atomic_state_alloc(struct drm_device *dev);
void drm_atomic_state_clear(struct drm_atomic_state *state);
void drm_atomic_state_free(struct drm_atomic_state *state);
+int __must_check
+drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state);
+void drm_atomic_state_default_clear(struct drm_atomic_state *state);
+void drm_atomic_state_default_free(struct drm_atomic_state *state);
+
struct drm_crtc_state * __must_check
drm_atomic_get_crtc_state(struct drm_atomic_state *state,
struct drm_crtc *crtc);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 840e9e62878c..bff25b0cada9 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -985,6 +985,9 @@ struct drm_mode_set {
* @atomic_check: check whether a given atomic state update is possible
* @atomic_commit: commit an atomic state update previously verified with
* atomic_check()
+ * @atomic_state_alloc: allocate a new atomic state
+ * @atomic_state_clear: clear the atomic state
+ * @atomic_state_free: free the atomic state
*
* Some global (i.e. not per-CRTC, connector, etc) mode setting functions that
* involve drivers.
@@ -1000,6 +1003,9 @@ struct drm_mode_config_funcs {
int (*atomic_commit)(struct drm_device *dev,
struct drm_atomic_state *a,
bool async);
+ struct drm_atomic_state *(*atomic_state_alloc)(struct drm_device *dev);
+ void (*atomic_state_clear)(struct drm_atomic_state *state);
+ void (*atomic_state_free)(struct drm_atomic_state *state);
};
/**
--
2.1.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v2 03/17] drm/i915: get rid of put_shared_dpll
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 01/17] drm/atomic: update crtc->hwmode in legacy state Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v2 Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 04/17] drm/i915: get rid of intel_crtc_disable and related code, v2 Maarten Lankhorst
` (13 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
Now that the pll updates are staged the put_shared_dpll function
consists only of checks that are done in check_shared_dpll_state
after a modeset too.
The changes to pll->config are overwritten by
intel_shared_dpll_commit, so this entire function is a noop.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 34 +++-------------------------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
2 files changed, 3 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c8510493468f..5d233c83f82c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4239,27 +4239,6 @@ static void lpt_pch_enable(struct drm_crtc *crtc)
lpt_enable_pch_transcoder(dev_priv, cpu_transcoder);
}
-void intel_put_shared_dpll(struct intel_crtc *crtc)
-{
- struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
-
- if (pll == NULL)
- return;
-
- if (!(pll->config.crtc_mask & (1 << crtc->pipe))) {
- WARN(1, "bad %s crtc mask\n", pll->name);
- return;
- }
-
- pll->config.crtc_mask &= ~(1 << crtc->pipe);
- if (pll->config.crtc_mask == 0) {
- WARN_ON(pll->on);
- WARN_ON(pll->active);
- }
-
- crtc->config->shared_dpll = DPLL_ID_PRIVATE;
-}
-
struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
struct intel_crtc_state *crtc_state)
{
@@ -5235,13 +5214,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
intel_disable_shared_dpll(intel_crtc);
}
-static void ironlake_crtc_off(struct drm_crtc *crtc)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- intel_put_shared_dpll(intel_crtc);
-}
-
-
static void i9xx_pfit_enable(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
@@ -14084,7 +14056,7 @@ static void intel_init_display(struct drm_device *dev)
haswell_crtc_compute_clock;
dev_priv->display.crtc_enable = haswell_crtc_enable;
dev_priv->display.crtc_disable = haswell_crtc_disable;
- dev_priv->display.off = ironlake_crtc_off;
+ dev_priv->display.off = i9xx_crtc_off;
dev_priv->display.update_primary_plane =
skylake_update_primary_plane;
} else if (HAS_DDI(dev)) {
@@ -14095,7 +14067,7 @@ static void intel_init_display(struct drm_device *dev)
haswell_crtc_compute_clock;
dev_priv->display.crtc_enable = haswell_crtc_enable;
dev_priv->display.crtc_disable = haswell_crtc_disable;
- dev_priv->display.off = ironlake_crtc_off;
+ dev_priv->display.off = i9xx_crtc_off;
dev_priv->display.update_primary_plane =
ironlake_update_primary_plane;
} else if (HAS_PCH_SPLIT(dev)) {
@@ -14106,7 +14078,7 @@ static void intel_init_display(struct drm_device *dev)
ironlake_crtc_compute_clock;
dev_priv->display.crtc_enable = ironlake_crtc_enable;
dev_priv->display.crtc_disable = ironlake_crtc_disable;
- dev_priv->display.off = ironlake_crtc_off;
+ dev_priv->display.off = i9xx_crtc_off;
dev_priv->display.update_primary_plane =
ironlake_update_primary_plane;
} else if (IS_VALLEYVIEW(dev)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47bc729043c5..70d0cc8fe70e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1082,7 +1082,6 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
#define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
struct intel_crtc_state *state);
-void intel_put_shared_dpll(struct intel_crtc *crtc);
void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
const struct dpll *dpll);
--
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] 37+ messages in thread
* [PATCH v2 04/17] drm/i915: get rid of intel_crtc_disable and related code, v2
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (2 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 03/17] drm/i915: get rid of put_shared_dpll Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 05/17] drm/i915: use intel_crtc_control everywhere Maarten Lankhorst
` (12 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
Now that the dpll updates are (mostly) atomic, the .off() code is a noop,
and intel_crtc_disable does mostly the same as intel_modeset_update_state.
Move all logic for connectors_active and setting dpms to that function.
Changes since v1:
- Move drm_atomic_helper_swap_state up.
Changes since v2:
- Split out intel_put_shared_dpll removal.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_display.c | 75 ++++++++----------------------------
2 files changed, 15 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6a66d6b7d33b..a0e4868653f2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -574,7 +574,6 @@ struct drm_i915_display_funcs {
struct intel_crtc_state *crtc_state);
void (*crtc_enable)(struct drm_crtc *crtc);
void (*crtc_disable)(struct drm_crtc *crtc);
- void (*off)(struct drm_crtc *crtc);
void (*audio_codec_enable)(struct drm_connector *connector,
struct intel_encoder *encoder,
struct drm_display_mode *mode);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5d233c83f82c..655c12477796 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6045,10 +6045,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
mutex_unlock(&dev->struct_mutex);
}
-static void i9xx_crtc_off(struct drm_crtc *crtc)
-{
-}
-
/* Master function to enable/disable CRTC and corresponding power wells */
void intel_crtc_control(struct drm_crtc *crtc, bool enable)
{
@@ -6098,34 +6094,6 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
crtc->state->active = enable;
}
-static void intel_crtc_disable(struct drm_crtc *crtc)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_connector *connector;
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- /* crtc should still be enabled when we disable it. */
- WARN_ON(!crtc->state->enable);
-
- intel_crtc_disable_planes(crtc);
- dev_priv->display.crtc_disable(crtc);
- dev_priv->display.off(crtc);
-
- drm_plane_helper_disable(crtc->primary);
-
- /* Update computed state. */
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
- if (!connector->encoder || !connector->encoder->crtc)
- continue;
-
- if (connector->encoder->crtc != crtc)
- continue;
-
- connector->dpms = DRM_MODE_DPMS_OFF;
- to_intel_encoder(connector->encoder)->connectors_active = false;
- }
-}
-
void intel_encoder_destroy(struct drm_encoder *encoder)
{
struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
@@ -11606,26 +11574,22 @@ intel_modeset_update_state(struct drm_atomic_state *state)
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
- int i;
intel_shared_dpll_commit(dev_priv);
+ drm_atomic_helper_swap_state(state->dev, state);
for_each_intel_encoder(dev, intel_encoder) {
if (!intel_encoder->base.crtc)
continue;
- for_each_crtc_in_state(state, crtc, crtc_state, i)
- if (crtc == intel_encoder->base.crtc)
- break;
-
- if (crtc != intel_encoder->base.crtc)
+ crtc = intel_encoder->base.crtc;
+ crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+ if (!crtc_state || !needs_modeset(crtc->state))
continue;
- if (crtc_state->enable && needs_modeset(crtc_state))
- intel_encoder->connectors_active = false;
+ intel_encoder->connectors_active = false;
}
- drm_atomic_helper_swap_state(state->dev, state);
intel_modeset_fixup_state(state);
/* Double check state. */
@@ -11637,14 +11601,12 @@ intel_modeset_update_state(struct drm_atomic_state *state)
if (!connector->encoder || !connector->encoder->crtc)
continue;
- for_each_crtc_in_state(state, crtc, crtc_state, i)
- if (crtc == connector->encoder->crtc)
- break;
-
- if (crtc != connector->encoder->crtc)
+ crtc = connector->encoder->crtc;
+ crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+ if (!crtc_state || !needs_modeset(crtc->state))
continue;
- if (crtc->state->enable && needs_modeset(crtc->state)) {
+ if (crtc->state->enable) {
struct drm_property *dpms_property =
dev->mode_config.dpms_property;
@@ -11655,9 +11617,9 @@ intel_modeset_update_state(struct drm_atomic_state *state)
intel_encoder = to_intel_encoder(connector->encoder);
intel_encoder->connectors_active = true;
- }
+ } else
+ connector->dpms = DRM_MODE_DPMS_OFF;
}
-
}
static bool intel_fuzzy_clock_check(int clock1, int clock2)
@@ -12333,12 +12295,10 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
if (!needs_modeset(crtc_state))
continue;
- if (!crtc_state->enable) {
- intel_crtc_disable(crtc);
- } else if (crtc->state->enable) {
- intel_crtc_disable_planes(crtc);
- dev_priv->display.crtc_disable(crtc);
- }
+ intel_crtc_disable_planes(crtc);
+ dev_priv->display.crtc_disable(crtc);
+ if (!crtc_state->enable)
+ drm_plane_helper_disable(crtc->primary);
}
/* crtc->mode is already used by the ->mode_set callbacks, hence we need
@@ -14056,7 +14016,6 @@ static void intel_init_display(struct drm_device *dev)
haswell_crtc_compute_clock;
dev_priv->display.crtc_enable = haswell_crtc_enable;
dev_priv->display.crtc_disable = haswell_crtc_disable;
- dev_priv->display.off = i9xx_crtc_off;
dev_priv->display.update_primary_plane =
skylake_update_primary_plane;
} else if (HAS_DDI(dev)) {
@@ -14067,7 +14026,6 @@ static void intel_init_display(struct drm_device *dev)
haswell_crtc_compute_clock;
dev_priv->display.crtc_enable = haswell_crtc_enable;
dev_priv->display.crtc_disable = haswell_crtc_disable;
- dev_priv->display.off = i9xx_crtc_off;
dev_priv->display.update_primary_plane =
ironlake_update_primary_plane;
} else if (HAS_PCH_SPLIT(dev)) {
@@ -14078,7 +14036,6 @@ static void intel_init_display(struct drm_device *dev)
ironlake_crtc_compute_clock;
dev_priv->display.crtc_enable = ironlake_crtc_enable;
dev_priv->display.crtc_disable = ironlake_crtc_disable;
- dev_priv->display.off = i9xx_crtc_off;
dev_priv->display.update_primary_plane =
ironlake_update_primary_plane;
} else if (IS_VALLEYVIEW(dev)) {
@@ -14088,7 +14045,6 @@ static void intel_init_display(struct drm_device *dev)
dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
dev_priv->display.crtc_enable = valleyview_crtc_enable;
dev_priv->display.crtc_disable = i9xx_crtc_disable;
- dev_priv->display.off = i9xx_crtc_off;
dev_priv->display.update_primary_plane =
i9xx_update_primary_plane;
} else {
@@ -14098,7 +14054,6 @@ static void intel_init_display(struct drm_device *dev)
dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
dev_priv->display.crtc_enable = i9xx_crtc_enable;
dev_priv->display.crtc_disable = i9xx_crtc_disable;
- dev_priv->display.off = i9xx_crtc_off;
dev_priv->display.update_primary_plane =
i9xx_update_primary_plane;
}
--
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] 37+ messages in thread
* [PATCH v2 05/17] drm/i915: use intel_crtc_control everywhere
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (3 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 04/17] drm/i915: get rid of intel_crtc_disable and related code, v2 Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 06/17] drm/i915: Use drm_atomic_helper_update_legacy_modeset_state Maarten Lankhorst
` (11 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
Having a single path for everything makes it a lot easier to keep
crtc_state->active in sync with intel_crtc->active.
A crtc cannot be changed to active when not enabled, because it means
no mode is set and no connectors are connected.
This should also make intel_crtc->active match crtc_state->active.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 18 ++++++++--
drivers/gpu/drm/i915/intel_display.c | 64 +++++++++++++-----------------------
drivers/gpu/drm/i915/intel_drv.h | 1 -
3 files changed, 37 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index adbbddab42c6..acd4d2c7613a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3606,12 +3606,18 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
*/
if (crtc->config->cpu_transcoder == TRANSCODER_EDP &&
!crtc->config->pch_pfit.enabled) {
+ bool active = crtc->active;
+
+ if (active)
+ intel_crtc_control(&crtc->base, false);
+
crtc->config->pch_pfit.force_thru = true;
intel_display_power_get(dev_priv,
POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
- intel_crtc_reset(crtc);
+ if (active)
+ intel_crtc_control(&crtc->base, true);
}
drm_modeset_unlock_all(dev);
}
@@ -3630,12 +3636,18 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
* routing.
*/
if (crtc->config->pch_pfit.force_thru) {
- crtc->config->pch_pfit.force_thru = false;
+ bool active = crtc->active;
- intel_crtc_reset(crtc);
+ if (active)
+ intel_crtc_control(&crtc->base, false);
+
+ crtc->config->pch_pfit.force_thru = false;
intel_display_power_put(dev_priv,
POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
+
+ if (active)
+ intel_crtc_control(&crtc->base, true);
}
drm_modeset_unlock_all(dev);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 655c12477796..232a9c06fbad 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3214,22 +3214,8 @@ static void intel_update_primary_planes(struct drm_device *dev)
}
}
-void intel_crtc_reset(struct intel_crtc *crtc)
-{
- struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-
- if (!crtc->active)
- return;
-
- intel_crtc_disable_planes(&crtc->base);
- dev_priv->display.crtc_disable(&crtc->base);
- dev_priv->display.crtc_enable(&crtc->base);
- intel_crtc_enable_planes(&crtc->base);
-}
-
void intel_prepare_reset(struct drm_device *dev)
{
- struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *crtc;
/* no reset support for gen2 */
@@ -3241,18 +3227,12 @@ void intel_prepare_reset(struct drm_device *dev)
return;
drm_modeset_lock_all(dev);
-
/*
* Disabling the crtcs gracefully seems nicer. Also the
* g33 docs say we should at least disable all the planes.
*/
- for_each_intel_crtc(dev, crtc) {
- if (!crtc->active)
- continue;
-
- intel_crtc_disable_planes(&crtc->base);
- dev_priv->display.crtc_disable(&crtc->base);
- }
+ for_each_intel_crtc(dev, crtc)
+ intel_crtc_control(&crtc->base, false);
}
void intel_finish_reset(struct drm_device *dev)
@@ -6054,26 +6034,29 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable)
enum intel_display_power_domain domain;
unsigned long domains;
+ if (enable == intel_crtc->active)
+ return;
+
+ if (enable && !crtc->state->enable)
+ return;
+
+ crtc->state->active = enable;
if (enable) {
- if (!intel_crtc->active) {
- 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;
+ 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;
- dev_priv->display.crtc_enable(crtc);
- intel_crtc_enable_planes(crtc);
- }
+ dev_priv->display.crtc_enable(crtc);
+ intel_crtc_enable_planes(crtc);
} else {
- if (intel_crtc->active) {
- intel_crtc_disable_planes(crtc);
- dev_priv->display.crtc_disable(crtc);
+ intel_crtc_disable_planes(crtc);
+ dev_priv->display.crtc_disable(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;
- }
+ 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;
}
}
@@ -6090,8 +6073,6 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
enable |= intel_encoder->connectors_active;
intel_crtc_control(crtc, enable);
-
- crtc->state->active = enable;
}
void intel_encoder_destroy(struct drm_encoder *encoder)
@@ -14528,8 +14509,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
plane = crtc->plane;
to_intel_plane_state(crtc->base.primary->state)->visible = true;
crtc->plane = !plane;
- intel_crtc_disable_planes(&crtc->base);
- dev_priv->display.crtc_disable(&crtc->base);
+ intel_crtc_control(&crtc->base, false);
crtc->plane = plane;
/* ... and break all links. */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 70d0cc8fe70e..d0d99669aa00 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -993,7 +993,6 @@ 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_crtc_control(struct drm_crtc *crtc, bool enable);
-void intel_crtc_reset(struct intel_crtc *crtc);
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] 37+ messages in thread
* [PATCH v2 06/17] drm/i915: Use drm_atomic_helper_update_legacy_modeset_state
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (4 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 05/17] drm/i915: use intel_crtc_control everywhere Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable Maarten Lankhorst
` (10 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
Now that the helper is exported there's no need to duplicate
this code any more.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 61 +++---------------------------------
1 file changed, 4 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 232a9c06fbad..db1b47febb27 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11088,43 +11088,6 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
}
}
-/* Fixup legacy state after an atomic state swap.
- */
-static void intel_modeset_fixup_state(struct drm_atomic_state *state)
-{
- struct intel_crtc *crtc;
- struct intel_encoder *encoder;
- struct intel_connector *connector;
-
- for_each_intel_connector(state->dev, connector) {
- connector->base.encoder = connector->base.state->best_encoder;
- if (connector->base.encoder)
- connector->base.encoder->crtc =
- connector->base.state->crtc;
- }
-
- /* Update crtc of disabled encoders */
- for_each_intel_encoder(state->dev, encoder) {
- int num_connectors = 0;
-
- for_each_intel_connector(state->dev, connector)
- if (connector->base.encoder == &encoder->base)
- num_connectors++;
-
- if (num_connectors == 0)
- encoder->base.crtc = NULL;
- }
-
- for_each_intel_crtc(state->dev, crtc) {
- crtc->base.enabled = crtc->base.state->enable;
- crtc->config = to_intel_crtc_state(crtc->base.state);
- }
-
- /* Copy the new configuration to the staged state, to keep the few
- * pieces of code that haven't been converted yet happy */
- intel_modeset_update_staged_output_state(state->dev);
-}
-
static void
connected_sink_compute_bpp(struct intel_connector *connector,
struct intel_crtc_state *pipe_config)
@@ -11571,11 +11534,14 @@ intel_modeset_update_state(struct drm_atomic_state *state)
intel_encoder->connectors_active = false;
}
- intel_modeset_fixup_state(state);
+ intel_modeset_update_staged_output_state(state->dev);
+ drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
/* Double check state. */
for_each_crtc(dev, crtc) {
WARN_ON(crtc->state->enable != intel_crtc_in_use(crtc));
+
+ to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state);
}
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -12282,25 +12248,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
drm_plane_helper_disable(crtc->primary);
}
- /* crtc->mode is already used by the ->mode_set callbacks, hence we need
- * to set it here already despite that we pass it down the callchain.
- *
- * Note we'll need to fix this up when we start tracking multiple
- * pipes; here we assume a single modeset_pipe and only track the
- * single crtc and mode.
- */
- if (pipe_config->base.enable && needs_modeset(&pipe_config->base)) {
- modeset_crtc->mode = pipe_config->base.mode;
-
- /*
- * Calculate and store various constants which
- * are later needed by vblank and swap-completion
- * timestamping. They are derived from true hwmode.
- */
- drm_calc_timestamping_constants(modeset_crtc,
- &pipe_config->base.adjusted_mode);
- }
-
/* Only after disabling all output pipelines that will be changed can we
* update the the output configuration. */
intel_modeset_update_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] 37+ messages in thread
* [PATCH v2 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (5 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 06/17] drm/i915: Use drm_atomic_helper_update_legacy_modeset_state Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-18 15:30 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 08/17] drm/i915: Set mode_changed for audio in intel_modeset_pipe_config() Maarten Lankhorst
` (9 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
crtc_state->enable means a crtc is configured, but it may be turned
off for dpms. Until the previous commit crtc_state->active was not
updated on crtc off, but now that we do we should use that for tracking
whether a crtc is scanning out or not.
At this point crtc->active should mirror crtc_state->active,
so some paranoia from the crtc_disable functions can be removed.
Note that intel_set_mode_setup_plls still checks for ->enable,
because all resources that are needed have to be calculated, so
dpms changes will still succeed.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++------------------
2 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 557af8877a2e..ca457317a8ac 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
return -EINVAL;
}
- if (!crtc->state->enable) {
+ if (!crtc->state->active) {
DRM_DEBUG_KMS("crtc %d is disabled\n", pipe);
return -EBUSY;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index db1b47febb27..583c9105cf49 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4667,7 +4667,7 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc)
bool reenable_ips = false;
/* The clocks have to be on to load the palette. */
- if (!crtc->state->enable || !intel_crtc->active)
+ if (!crtc->state->active)
return;
if (HAS_GMCH_DISPLAY(dev_priv->dev)) {
@@ -4872,9 +4872,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc->pipe;
- WARN_ON(!crtc->state->enable);
-
- if (intel_crtc->active)
+ if (WARN_ON(intel_crtc->active))
return;
if (intel_crtc->config->has_pch_encoder)
@@ -4978,9 +4976,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc->pipe;
- WARN_ON(!crtc->state->enable);
-
- if (intel_crtc->active)
+ if (WARN_ON(intel_crtc->active))
return;
if (intel_crtc_to_shared_dpll(intel_crtc))
@@ -5082,7 +5078,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
int pipe = intel_crtc->pipe;
u32 reg, temp;
- if (!intel_crtc->active)
+ if (WARN_ON(!intel_crtc->active))
return;
for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -5144,7 +5140,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
- if (!intel_crtc->active)
+ if (WARN_ON(!intel_crtc->active))
return;
for_each_encoder_on_crtc(dev, crtc, encoder) {
@@ -5742,7 +5738,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
/* add all active pipes to the state */
for_each_crtc(state->dev, crtc) {
- if (!crtc->state->enable)
+ if (!crtc->state->active)
continue;
crtc_state = drm_atomic_get_crtc_state(state, crtc);
@@ -5752,7 +5748,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
/* disable/enable all currently active pipes while we change cdclk */
for_each_crtc_in_state(state, crtc, crtc_state, i)
- if (crtc_state->enable)
+ if (crtc_state->active)
crtc_state->mode_changed = true;
return 0;
@@ -5840,9 +5836,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
int pipe = intel_crtc->pipe;
bool is_dsi;
- WARN_ON(!crtc->state->enable);
-
- if (intel_crtc->active)
+ if (WARN_ON(intel_crtc->active))
return;
is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
@@ -5918,9 +5912,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc->pipe;
- WARN_ON(!crtc->state->enable);
-
- if (intel_crtc->active)
+ if (WARN_ON(intel_crtc->active))
return;
i9xx_set_pll_dividers(intel_crtc);
@@ -5980,7 +5972,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc->pipe;
- if (!intel_crtc->active)
+ if (WARN_ON(!intel_crtc->active))
return;
/*
@@ -11553,7 +11545,7 @@ intel_modeset_update_state(struct drm_atomic_state *state)
if (!crtc_state || !needs_modeset(crtc->state))
continue;
- if (crtc->state->enable) {
+ if (crtc->state->active) {
struct drm_property *dpms_property =
dev->mode_config.dpms_property;
@@ -11970,6 +11962,10 @@ check_crtc_state(struct drm_device *dev)
"crtc active state doesn't match with hw state "
"(expected %i, found %i)\n", crtc->active, active);
+ I915_STATE_WARN(crtc->active != crtc->base.state->active,
+ "transitional active state does not match atomic hw state "
+ "(expected %i, found %i)\n", crtc->base.state->active, crtc->active);
+
if (active &&
!intel_pipe_config_compare(dev, crtc->config, &pipe_config)) {
I915_STATE_WARN(1, "pipe state doesn't match!\n");
@@ -12115,6 +12111,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
if (IS_ERR(pipe_config))
return pipe_config;
+ if (!pipe_config->base.enable &&
+ WARN_ON(pipe_config->base.active))
+ pipe_config->base.active = false;
+
if (!pipe_config->base.enable)
return pipe_config;
@@ -12239,7 +12239,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
return ret;
for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!needs_modeset(crtc_state))
+ if (!needs_modeset(crtc_state) || !crtc->state->active)
continue;
intel_crtc_disable_planes(crtc);
@@ -12261,7 +12261,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!needs_modeset(crtc->state) || !crtc->state->enable)
+ if (!needs_modeset(crtc->state) || !crtc->state->active)
continue;
update_scanline_offset(to_intel_crtc(crtc));
@@ -14494,7 +14494,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
* have active connectors/encoders. */
intel_crtc_update_dpms(&crtc->base);
- if (crtc->active != crtc->base.state->enable) {
+ if (crtc->active != crtc->base.state->active) {
struct intel_encoder *encoder;
/* This can happen either due to bugs in the get_hw_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] 37+ messages in thread
* [PATCH v2 08/17] drm/i915: Set mode_changed for audio in intel_modeset_pipe_config()
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (6 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-18 15:36 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 09/17] drm/i915: Make __intel_set_mode() take only atomic state as argument Maarten Lankhorst
` (8 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
A follow up patch will make intel_modeset_compute_config() deal with
multiple crtcs, so move crtc specific stuff into the lower level crtc
specific function.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 583c9105cf49..ec548cbb06ee 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -427,6 +427,12 @@ static void vlv_clock(int refclk, intel_clock_t *clock)
clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
}
+static bool
+needs_modeset(struct drm_crtc_state *state)
+{
+ return state->mode_changed || state->active_changed;
+}
+
/**
* Returns whether any output on the specified pipe is of the specified type
*/
@@ -11387,6 +11393,15 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
return -EINVAL;
}
+ /*
+ * XXX: Add all connectors to make the crtc state match the encoders.
+ */
+ if (!needs_modeset(&pipe_config->base)) {
+ ret = drm_atomic_add_affected_connectors(state, crtc);
+ if (ret)
+ return ret;
+ }
+
clear_intel_crtc_state(pipe_config);
pipe_config->cpu_transcoder =
@@ -11478,6 +11493,18 @@ encoder_retry:
DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
+ /* Check if we need to force a modeset */
+ if (pipe_config->has_audio !=
+ to_intel_crtc_state(crtc->state)->has_audio)
+ pipe_config->base.mode_changed = true;
+
+ /*
+ * Note we have an issue here with infoframes: current code
+ * only updates them on the full mode set path per hw
+ * requirements. So here we should be checking for any
+ * required changes and forcing a mode set.
+ */
+
return 0;
fail:
return ret;
@@ -11495,12 +11522,6 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)
return false;
}
-static bool
-needs_modeset(struct drm_crtc_state *state)
-{
- return state->mode_changed || state->active_changed;
-}
-
static void
intel_modeset_update_state(struct drm_atomic_state *state)
{
@@ -12093,10 +12114,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
struct intel_crtc_state *pipe_config;
int ret = 0;
- ret = drm_atomic_add_affected_connectors(state, crtc);
- if (ret)
- return ERR_PTR(ret);
-
ret = drm_atomic_helper_check_modeset(state->dev, state);
if (ret)
return ERR_PTR(ret);
@@ -12122,19 +12139,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
if (ret)
return ERR_PTR(ret);
- /* Check things that can only be changed through modeset */
- if (pipe_config->has_audio !=
- to_intel_crtc(crtc)->config->has_audio)
- pipe_config->base.mode_changed = true;
-
- /*
- * Note we have an issue here with infoframes: current code
- * only updates them on the full mode set path per hw
- * requirements. So here we should be checking for any
- * required changes and forcing a mode set.
- */
-
- intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]");
+ intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, "[modeset]");
ret = drm_atomic_helper_check_planes(state->dev, state);
if (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] 37+ messages in thread
* [PATCH v2 09/17] drm/i915: Make __intel_set_mode() take only atomic state as argument
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (7 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 08/17] drm/i915: Set mode_changed for audio in intel_modeset_pipe_config() Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-15 7:42 ` Ander Conselvan De Oliveira
2015-05-13 20:23 ` [PATCH v2 10/17] drm/i915: Support modeset across multiple pipes Maarten Lankhorst
` (7 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
The last remaining portion that required the modeset_crtc argument is
converted to deal with all crtcs in the state that need_modeset(). By
doing that, __intel_set_mode() is generic enough, behaving like a commit
function for the atomic state.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ec548cbb06ee..1c706623f99c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12224,12 +12224,10 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
return 0;
}
-static int __intel_set_mode(struct drm_crtc *modeset_crtc,
- struct intel_crtc_state *pipe_config)
+static int __intel_set_mode(struct drm_atomic_state *state)
{
- struct drm_device *dev = modeset_crtc->dev;
+ struct drm_device *dev = state->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_atomic_state *state = pipe_config->base.state;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
int ret = 0;
@@ -12289,7 +12287,7 @@ static int intel_set_mode_with_config(struct drm_crtc *crtc,
{
int ret;
- ret = __intel_set_mode(crtc, pipe_config);
+ ret = __intel_set_mode(pipe_config->base.state);
if (ret == 0)
intel_modeset_check_state(crtc->dev);
--
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] 37+ messages in thread
* [PATCH v2 10/17] drm/i915: Support modeset across multiple pipes
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (8 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 09/17] drm/i915: Make __intel_set_mode() take only atomic state as argument Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 11/17] drm/i915: Use global atomic state for staged pll config Maarten Lankhorst
` (6 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Compute new pipe_configs for all crtcs in the atomic state. The commit
part of the mode set (__intel_set_mode()) is already enabled to support
multiple pipes, the only thing missing was calculating a new pipe_config
for every crtc.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 138 +++++++++++++----------------------
1 file changed, 52 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1c706623f99c..0a2a7b6e198c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -82,8 +82,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
static void ironlake_pch_clock_get(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
-static int intel_set_mode(struct drm_crtc *crtc,
- struct drm_atomic_state *state);
+static int intel_set_mode(struct drm_atomic_state *state);
static int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *ifb,
struct drm_mode_fb_cmd2 *mode_cmd,
@@ -9830,7 +9829,7 @@ retry:
drm_mode_copy(&crtc_state->base.mode, mode);
- if (intel_set_mode(crtc, state)) {
+ if (intel_set_mode(state)) {
DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
if (old->release_fb)
old->release_fb->funcs->destroy(old->release_fb);
@@ -9904,7 +9903,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
if (ret)
goto fail;
- ret = intel_set_mode(crtc, state);
+ ret = intel_set_mode(state);
if (ret)
goto fail;
@@ -11373,9 +11372,10 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
static int
intel_modeset_pipe_config(struct drm_crtc *crtc,
- struct drm_atomic_state *state,
- struct intel_crtc_state *pipe_config)
+ struct drm_atomic_state *state)
{
+ struct drm_crtc_state *crtc_state;
+ struct intel_crtc_state *pipe_config;
struct intel_encoder *encoder;
struct drm_connector *connector;
struct drm_connector_state *connector_state;
@@ -11393,6 +11393,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
return -EINVAL;
}
+ crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+ if (WARN_ON(!crtc_state))
+ return -EINVAL;
+
+ pipe_config = to_intel_crtc_state(crtc_state);
+
/*
* XXX: Add all connectors to make the crtc state match the encoders.
*/
@@ -12107,45 +12113,35 @@ static void update_scanline_offset(struct intel_crtc *crtc)
crtc->scanline_offset = 1;
}
-static struct intel_crtc_state *
-intel_modeset_compute_config(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
+static int
+intel_modeset_compute_config(struct drm_atomic_state *state)
{
- struct intel_crtc_state *pipe_config;
- int ret = 0;
+ 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 ERR_PTR(ret);
-
- /*
- * Note this needs changes when we start tracking multiple modes
- * and crtcs. At that point we'll need to compute the whole config
- * (i.e. one pipe_config for each crtc) rather than just the one
- * for this crtc.
- */
- pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
- if (IS_ERR(pipe_config))
- return pipe_config;
-
- if (!pipe_config->base.enable &&
- WARN_ON(pipe_config->base.active))
- pipe_config->base.active = false;
+ return ret;
- if (!pipe_config->base.enable)
- return pipe_config;
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ if (!crtc_state->enable &&
+ WARN_ON(crtc_state->active))
+ crtc_state->active = false;
- ret = intel_modeset_pipe_config(crtc, state, pipe_config);
- if (ret)
- return ERR_PTR(ret);
+ if (!crtc_state->enable)
+ continue;
- intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, "[modeset]");
+ ret = intel_modeset_pipe_config(crtc, state);
+ if (ret)
+ return ret;
- ret = drm_atomic_helper_check_planes(state->dev, state);
- if (ret)
- return ERR_PTR(ret);
+ intel_dump_pipe_config(to_intel_crtc(crtc),
+ to_intel_crtc_state(crtc_state),
+ "[modeset]");
+ }
- return pipe_config;
+ return drm_atomic_helper_check_planes(state->dev, state);
}
static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
@@ -12282,37 +12278,27 @@ static int __intel_set_mode(struct drm_atomic_state *state)
return 0;
}
-static int intel_set_mode_with_config(struct drm_crtc *crtc,
- struct intel_crtc_state *pipe_config)
+static int intel_set_mode_checked(struct drm_atomic_state *state)
{
+ struct drm_device *dev = state->dev;
int ret;
- ret = __intel_set_mode(pipe_config->base.state);
-
+ ret = __intel_set_mode(state);
if (ret == 0)
- intel_modeset_check_state(crtc->dev);
+ intel_modeset_check_state(dev);
return ret;
}
-static int intel_set_mode(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
+static int intel_set_mode(struct drm_atomic_state *state)
{
- struct intel_crtc_state *pipe_config;
- int ret = 0;
-
- pipe_config = intel_modeset_compute_config(crtc, state);
- if (IS_ERR(pipe_config)) {
- ret = PTR_ERR(pipe_config);
- goto out;
- }
+ int ret;
- ret = intel_set_mode_with_config(crtc, pipe_config);
+ ret = intel_modeset_compute_config(state);
if (ret)
- goto out;
+ return ret;
-out:
- return ret;
+ return intel_set_mode_checked(state);
}
void intel_crtc_restore_mode(struct drm_crtc *crtc)
@@ -12384,7 +12370,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
crtc->primary->fb, crtc->x, crtc->y);
- ret = intel_set_mode(crtc, state);
+ ret = intel_set_mode(state);
if (ret)
drm_atomic_state_free(state);
}
@@ -12542,7 +12528,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
{
struct drm_device *dev;
struct drm_atomic_state *state = NULL;
- struct intel_crtc_state *pipe_config;
bool primary_plane_was_visible;
int ret;
@@ -12574,44 +12559,25 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
if (ret)
goto out;
- pipe_config = intel_modeset_compute_config(set->crtc, state);
- if (IS_ERR(pipe_config)) {
- ret = PTR_ERR(pipe_config);
+ primary_plane_was_visible = primary_plane_visible(set->crtc);
+
+ ret = intel_modeset_compute_config(state);
+ if (ret)
goto out;
- }
intel_update_pipe_size(to_intel_crtc(set->crtc));
- primary_plane_was_visible = primary_plane_visible(set->crtc);
-
- ret = intel_set_mode_with_config(set->crtc, pipe_config);
-
+ ret = intel_set_mode_checked(state);
if (ret == 0 &&
- pipe_config->base.enable &&
- pipe_config->base.planes_changed &&
- !needs_modeset(&pipe_config->base)) {
- struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
-
+ set->crtc->state->active &&
+ !needs_modeset(set->crtc->state) &&
+ !primary_plane_was_visible &&
+ primary_plane_visible(set->crtc)) {
/*
* We need to make sure the primary plane is re-enabled if it
* has previously been turned off.
*/
- if (ret == 0 && !primary_plane_was_visible &&
- primary_plane_visible(set->crtc)) {
- WARN_ON(!intel_crtc->active);
- intel_post_enable_primary(set->crtc);
- }
-
- /*
- * In the fastboot case this may be our only check of the
- * state after boot. It would be better to only do it on
- * the first update, but we don't have a nice way of doing that
- * (and really, set_config isn't used much for high freq page
- * flipping, so increasing its cost here shouldn't be a big
- * deal).
- */
- if (i915.fastboot && ret == 0)
- intel_modeset_check_state(set->crtc->dev);
+ intel_post_enable_primary(set->crtc);
}
if (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] 37+ messages in thread
* [PATCH v2 11/17] drm/i915: Use global atomic state for staged pll config
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (9 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 10/17] drm/i915: Support modeset across multiple pipes Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-18 15:45 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 12/17] drm/i915: Read hw state into an atomic state struct Maarten Lankhorst
` (5 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 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.
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 | 49 ++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 111 +++++++++++------------------------
drivers/gpu/drm/i915/intel_drv.h | 13 ++++
4 files changed, 95 insertions(+), 79 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0e4868653f2..0e200018c9aa 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..aff87054406c 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -421,3 +421,52 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
return 0;
}
+
+static void intel_atomic_duplicate_dpll_state(struct drm_device *dev,
+ struct intel_atomic_state *state)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_shared_dpll *pll;
+ enum intel_dpll_id i;
+
+ state->dpll_set = true;
+
+ /* Copy shared dpll state */
+ for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+ pll = &dev_priv->shared_dplls[i];
+
+ memcpy(&state->shared_dpll[i],
+ &pll->config, sizeof(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);
+
+ if (!state->dpll_set)
+ intel_atomic_duplicate_dpll_state(state->base.dev, state);
+
+ 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 0a2a7b6e198c..ed69eddf457e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4229,8 +4229,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;
@@ -4239,7 +4242,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;
}
@@ -4259,7 +4262,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;
}
@@ -4268,15 +4271,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;
}
@@ -4285,7 +4288,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;
@@ -4295,83 +4298,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];
}
}
@@ -11532,13 +11485,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) {
@@ -12171,9 +12123,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)
@@ -12184,13 +12140,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;
}
@@ -13887,6 +13840,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] 37+ messages in thread
* [PATCH v2 12/17] drm/i915: Read hw state into an atomic state struct
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (10 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 11/17] drm/i915: Use global atomic state for staged pll config Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 13/17] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config() Maarten Lankhorst
` (4 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 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 | 22 +--
drivers/gpu/drm/i915/intel_display.c | 371 ++++++++++++++++++++++-------------
drivers/gpu/drm/i915/intel_drv.h | 14 +-
3 files changed, 246 insertions(+), 161 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index aff87054406c..08a43030231c 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -422,21 +422,17 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
return 0;
}
-static void intel_atomic_duplicate_dpll_state(struct drm_device *dev,
- struct intel_atomic_state *state)
+void
+intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
+ struct intel_shared_dpll_config *shared_dpll)
{
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_shared_dpll *pll;
enum intel_dpll_id i;
- state->dpll_set = true;
-
/* Copy shared dpll state */
for (i = 0; i < dev_priv->num_shared_dpll; i++) {
- pll = &dev_priv->shared_dplls[i];
+ struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
- memcpy(&state->shared_dpll[i],
- &pll->config, sizeof(pll->config));
+ shared_dpll[i] = pll->config;
}
}
@@ -445,8 +441,12 @@ intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s)
{
struct intel_atomic_state *state = to_intel_atomic_state(s);
- if (!state->dpll_set)
- intel_atomic_duplicate_dpll_state(state->base.dev, state);
+ if (!state->dpll_set) {
+ state->dpll_set = true;
+
+ intel_atomic_duplicate_dpll_state(to_i915(state->base.dev),
+ state->shared_dpll);
+ }
return state->shared_dpll;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ed69eddf457e..e316a0d0a253 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9660,7 +9660,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:
@@ -9678,10 +9678,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;
@@ -9700,9 +9700,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;
@@ -9713,20 +9710,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;
@@ -9794,9 +9788,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;
@@ -9842,10 +9834,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;
@@ -10990,33 +10978,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.
@@ -11505,7 +11466,6 @@ intel_modeset_update_state(struct drm_atomic_state *state)
intel_encoder->connectors_active = false;
}
- intel_modeset_update_staged_output_state(state->dev);
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
/* Double check state. */
@@ -11812,11 +11772,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");
}
}
@@ -11836,8 +11799,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");
@@ -11847,6 +11808,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
@@ -12280,11 +12244,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);
@@ -12297,14 +12261,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",
@@ -12313,9 +12273,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);
}
@@ -14542,98 +14499,219 @@ 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;
- 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));
+ memset(crtc_state, 0, sizeof(*crtc_state));
+ crtc_state->base.crtc = &crtc->base;
+ crtc_state->base.state = state;
- crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
+ crtc_state->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
- crtc->active = dev_priv->display.get_pipe_config(crtc,
- crtc->config);
+ crtc_state->base.enable = crtc_state->base.active =
+ crtc->base.enabled = dev_priv->display.get_pipe_config(crtc, crtc_state);
- crtc->base.state->enable = crtc->active;
- crtc->base.state->active = crtc->active;
- crtc->base.enabled = crtc->active;
+ /* update transitional state */
+ crtc->active = crtc_state->base.active;
+ crtc->config = crtc_state;
- plane_state = to_intel_plane_state(primary->state);
- plane_state->visible = primary_get_hw_state(crtc);
+ drm_plane_state = drm_atomic_get_plane_state(state, primary);
+ if (IS_ERR(drm_plane_state))
+ return PTR_ERR(drm_plane_state);
- DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
- crtc->base.base.id,
- crtc->active ? "enabled" : "disabled");
- }
+ 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
@@ -14642,37 +14720,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]");
}
@@ -14696,20 +14794,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] 37+ messages in thread
* [PATCH v2 13/17] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config()
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (11 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 12/17] drm/i915: Read hw state into an atomic state struct Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 14/17] drm/i915: Implement intel_crtc_toggle using atomic state, v3 Maarten Lankhorst
` (3 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 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 e316a0d0a253..7247f757c5ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12029,37 +12029,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;
@@ -12137,6 +12106,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;
@@ -12146,10 +12150,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] 37+ messages in thread
* [PATCH v2 14/17] drm/i915: Implement intel_crtc_toggle using atomic state, v3
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (12 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 13/17] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config() Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 15/17] drm/i915: Calculate haswell plane workaround, v2 Maarten Lankhorst
` (2 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
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.
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 7247f757c5ce..1d3b0233f070 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5979,10 +5979,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;
@@ -5990,24 +5993,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] 37+ messages in thread
* [PATCH v2 15/17] drm/i915: Calculate haswell plane workaround, v2.
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (13 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 14/17] drm/i915: Implement intel_crtc_toggle using atomic state, v3 Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-18 15:47 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 17/17] drm/i915: Remove use of crtc->config from i915_debugfs.c Maarten Lankhorst
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
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.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 100 ++++++++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_drv.h | 3 ++
2 files changed, 72 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1d3b0233f070..9316b0be4f5b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4897,42 +4897,13 @@ 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;
if (WARN_ON(intel_crtc->active))
return;
@@ -5009,7 +4980,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 = intel_crtc->atomic.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)
@@ -12099,6 +12074,66 @@ 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 *crtc;
+ struct drm_crtc_state *crtc_state;
+ struct intel_crtc *intel_crtc, *enabled_crtc;
+ struct intel_crtc *first_crtc = NULL, *other_crtc = NULL;
+ int enabled_crtcs = 0, 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) {
+ other_crtc = intel_crtc;
+ break;
+ } else {
+ first_crtc = intel_crtc;
+ }
+ }
+
+ /* No workaround needed? */
+ if (!first_crtc)
+ return 0;
+
+ /* w/a possibly needed, check how many crtc's are already enabled. */
+ for_each_intel_crtc(state->dev, intel_crtc) {
+ intel_crtc->atomic.hsw_workaround_pipe = INVALID_PIPE;
+
+ crtc_state = drm_atomic_get_crtc_state(state,
+ &intel_crtc->base);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ if (!crtc_state->active || needs_modeset(crtc_state))
+ continue;
+
+ /* 2 or more enabled crtcs means no need for w/a */
+ if (++enabled_crtcs >= 2)
+ return 0;
+
+ enabled_crtc = intel_crtc;
+ }
+
+ if (enabled_crtcs == 1)
+ first_crtc->atomic.hsw_workaround_pipe = enabled_crtc->pipe;
+ else if (other_crtc)
+ other_crtc->atomic.hsw_workaround_pipe = first_crtc->pipe;
+
+ return 0;
+}
+
/* Code that should eventually be part of atomic_check() */
static int __intel_set_mode_checks(struct drm_atomic_state *state)
{
@@ -12122,6 +12157,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..1aa10a9445de 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -490,6 +490,9 @@ struct intel_crtc_atomic_commit {
bool update_fbc;
bool post_enable_primary;
unsigned update_sprite_watermarks;
+
+ /* Operations to perform during modeset */
+ enum pipe hsw_workaround_pipe;
};
struct intel_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] 37+ messages in thread
* [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks.
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (14 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 15/17] drm/i915: Calculate haswell plane workaround, v2 Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-18 15:49 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 17/17] drm/i915: Remove use of crtc->config from i915_debugfs.c Maarten Lankhorst
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
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.
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 | 3 +++
2 files changed, 9 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 9316b0be4f5b..5063ac3d028d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11467,6 +11467,9 @@ 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);
+
+ if (!crtc->state->active)
+ 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] 37+ messages in thread
* [PATCH v2 17/17] drm/i915: Remove use of crtc->config from i915_debugfs.c
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
` (15 preceding siblings ...)
2015-05-13 20:23 ` [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks Maarten Lankhorst
@ 2015-05-13 20:23 ` Maarten Lankhorst
2015-05-18 15:51 ` Daniel Vetter
16 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-13 20:23 UTC (permalink / raw)
To: intel-gfx
crtc->config is a pointer to the current crtc->state, and will be
removed eventually. Same for crtc->active, 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 acd4d2c7613a..5439ff9f7e6b 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] 37+ messages in thread
* Re: [PATCH v2 09/17] drm/i915: Make __intel_set_mode() take only atomic state as argument
2015-05-13 20:23 ` [PATCH v2 09/17] drm/i915: Make __intel_set_mode() take only atomic state as argument Maarten Lankhorst
@ 2015-05-15 7:42 ` Ander Conselvan De Oliveira
0 siblings, 0 replies; 37+ messages in thread
From: Ander Conselvan De Oliveira @ 2015-05-15 7:42 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, 2015-05-13 at 22:23 +0200, Maarten Lankhorst wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> The last remaining portion that required the modeset_crtc argument is
> converted to deal with all crtcs in the state that need_modeset(). By
> doing that, __intel_set_mode() is generic enough, behaving like a commit
> function for the atomic state.
Now that I read this I realized I didn't do a good job writing that
commit message. It refers to the move drm_calc_timestamping_constants()
into intel_modeset_update_state(), which is not part of this patch
anymore, so the text makes even less sense now.
Ander
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ec548cbb06ee..1c706623f99c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12224,12 +12224,10 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
> return 0;
> }
>
> -static int __intel_set_mode(struct drm_crtc *modeset_crtc,
> - struct intel_crtc_state *pipe_config)
> +static int __intel_set_mode(struct drm_atomic_state *state)
> {
> - struct drm_device *dev = modeset_crtc->dev;
> + struct drm_device *dev = state->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct drm_atomic_state *state = pipe_config->base.state;
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
> int ret = 0;
> @@ -12289,7 +12287,7 @@ static int intel_set_mode_with_config(struct drm_crtc *crtc,
> {
> int ret;
>
> - ret = __intel_set_mode(crtc, pipe_config);
> + ret = __intel_set_mode(pipe_config->base.state);
>
> if (ret == 0)
> intel_modeset_check_state(crtc->dev);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 01/17] drm/atomic: update crtc->hwmode in legacy state
2015-05-13 20:23 ` [PATCH v2 01/17] drm/atomic: update crtc->hwmode in legacy state Maarten Lankhorst
@ 2015-05-18 8:01 ` Maarten Lankhorst
0 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-18 8:01 UTC (permalink / raw)
To: intel-gfx
Op 13-05-15 om 22:23 schreef Maarten Lankhorst:
> This is useful when calculating vblank in drivers that support it.
> During a modeset the atomic state may not match the hardware state,
> so if the driver wants to wait on a vblank they'll want to use
> crtc->hwmode rather than crtc->state->adjusted_mode.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
Drop this patch please, after discussion with danvet it seems better to do this inside the intel code of
"[PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks." instead.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v3
2015-05-13 20:23 ` [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v2 Maarten Lankhorst
@ 2015-05-18 8:06 ` Maarten Lankhorst
2015-05-18 14:40 ` Daniel Vetter
0 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-18 8:06 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
Drivers may need to store the state of shared resources, such as PLLs
or FIFO space, into the atomic state. Allow this by making it possible
to subclass drm_atomic_state.
Changes since v1:
- Change member names for functions to atomic_state_(alloc,clear)
- Change __drm_atomic_state_new to drm_atomic_state_init
- Allow free function to be overridden too, in case extra memory is
allocated in alloc.
Changes since v2:
- Rename *_default_free to default_release, to make clear it doesn't
free the state object itself.
Cc: dri-devel@lists.freedesktop.org
Acked-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index c6277a4a1f2f..cd1b16b25716 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -30,7 +30,15 @@
#include <drm/drm_atomic.h>
#include <drm/drm_plane_helper.h>
-static void kfree_state(struct drm_atomic_state *state)
+/**
+ * drm_atomic_state_default_release -
+ * release memory initialized by drm_atomic_state_init
+ * @state: atomic state
+ *
+ * Free all the memory allocated by drm_atomic_state_init.
+ * This is useful for drivers that subclass the atomic state.
+ */
+void drm_atomic_state_default_release(struct drm_atomic_state *state)
{
kfree(state->connectors);
kfree(state->connector_states);
@@ -38,24 +46,20 @@ static void kfree_state(struct drm_atomic_state *state)
kfree(state->crtc_states);
kfree(state->planes);
kfree(state->plane_states);
- kfree(state);
}
+EXPORT_SYMBOL(drm_atomic_state_default_release);
/**
- * drm_atomic_state_alloc - allocate atomic state
+ * drm_atomic_state_init - init new atomic state
* @dev: DRM device
+ * @state: atomic state
*
- * This allocates an empty atomic state to track updates.
+ * Default implementation for filling in a new atomic state.
+ * This is useful for drivers that subclass the atomic state.
*/
-struct drm_atomic_state *
-drm_atomic_state_alloc(struct drm_device *dev)
+int
+drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
{
- struct drm_atomic_state *state;
-
- state = kzalloc(sizeof(*state), GFP_KERNEL);
- if (!state)
- return NULL;
-
/* TODO legacy paths should maybe do a better job about
* setting this appropriately?
*/
@@ -92,31 +96,50 @@ drm_atomic_state_alloc(struct drm_device *dev)
state->dev = dev;
- DRM_DEBUG_ATOMIC("Allocate atomic state %p\n", state);
+ DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state);
- return state;
+ return 0;
fail:
- kfree_state(state);
+ drm_atomic_state_default_release(state);
+ return -ENOMEM;
+}
+EXPORT_SYMBOL(drm_atomic_state_init);
+
+/**
+ * drm_atomic_state_alloc - allocate atomic state
+ * @dev: DRM device
+ *
+ * This allocates an empty atomic state to track updates.
+ */
+struct drm_atomic_state *
+drm_atomic_state_alloc(struct drm_device *dev)
+{
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_atomic_state *state;
+
+ if (!config->funcs->atomic_state_alloc) {
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return NULL;
+ if (drm_atomic_state_init(dev, state) < 0) {
+ kfree(state);
+ return NULL;
+ }
+ return state;
+ }
- return NULL;
+ return config->funcs->atomic_state_alloc(dev);
}
EXPORT_SYMBOL(drm_atomic_state_alloc);
/**
- * drm_atomic_state_clear - clear state object
+ * drm_atomic_state_default_clear - clear base atomic state
* @state: atomic state
*
- * When the w/w mutex algorithm detects a deadlock we need to back off and drop
- * all locks. So someone else could sneak in and change the current modeset
- * configuration. Which means that all the state assembled in @state is no
- * longer an atomic update to the current state, but to some arbitrary earlier
- * state. Which could break assumptions the driver's ->atomic_check likely
- * relies on.
- *
- * Hence we must clear all cached state and completely start over, using this
- * function.
+ * Default implementation for clearing atomic state.
+ * This is useful for drivers that subclass the atomic state.
*/
-void drm_atomic_state_clear(struct drm_atomic_state *state)
+void drm_atomic_state_default_clear(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
struct drm_mode_config *config = &dev->mode_config;
@@ -162,6 +185,32 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)
state->plane_states[i] = NULL;
}
}
+EXPORT_SYMBOL(drm_atomic_state_default_clear);
+
+/**
+ * drm_atomic_state_clear - clear state object
+ * @state: atomic state
+ *
+ * When the w/w mutex algorithm detects a deadlock we need to back off and drop
+ * all locks. So someone else could sneak in and change the current modeset
+ * configuration. Which means that all the state assembled in @state is no
+ * longer an atomic update to the current state, but to some arbitrary earlier
+ * state. Which could break assumptions the driver's ->atomic_check likely
+ * relies on.
+ *
+ * Hence we must clear all cached state and completely start over, using this
+ * function.
+ */
+void drm_atomic_state_clear(struct drm_atomic_state *state)
+{
+ struct drm_device *dev = state->dev;
+ struct drm_mode_config *config = &dev->mode_config;
+
+ if (config->funcs->atomic_state_clear)
+ config->funcs->atomic_state_clear(state);
+ else
+ drm_atomic_state_default_clear(state);
+}
EXPORT_SYMBOL(drm_atomic_state_clear);
/**
@@ -173,14 +222,25 @@ EXPORT_SYMBOL(drm_atomic_state_clear);
*/
void drm_atomic_state_free(struct drm_atomic_state *state)
{
+ struct drm_device *dev;
+ struct drm_mode_config *config;
+
if (!state)
return;
+ dev = state->dev;
+ config = &dev->mode_config;
+
drm_atomic_state_clear(state);
DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
- kfree_state(state);
+ if (config->funcs->atomic_state_free) {
+ config->funcs->atomic_state_free(state);
+ } else {
+ drm_atomic_state_default_release(state);
+ kfree(state);
+ }
}
EXPORT_SYMBOL(drm_atomic_state_free);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index d78543067700..f0d3a7387d99 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -35,6 +35,11 @@ drm_atomic_state_alloc(struct drm_device *dev);
void drm_atomic_state_clear(struct drm_atomic_state *state);
void drm_atomic_state_free(struct drm_atomic_state *state);
+int __must_check
+drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state);
+void drm_atomic_state_default_clear(struct drm_atomic_state *state);
+void drm_atomic_state_default_release(struct drm_atomic_state *state);
+
struct drm_crtc_state * __must_check
drm_atomic_get_crtc_state(struct drm_atomic_state *state,
struct drm_crtc *crtc);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 840e9e62878c..bff25b0cada9 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -985,6 +985,9 @@ struct drm_mode_set {
* @atomic_check: check whether a given atomic state update is possible
* @atomic_commit: commit an atomic state update previously verified with
* atomic_check()
+ * @atomic_state_alloc: allocate a new atomic state
+ * @atomic_state_clear: clear the atomic state
+ * @atomic_state_free: free the atomic state
*
* Some global (i.e. not per-CRTC, connector, etc) mode setting functions that
* involve drivers.
@@ -1000,6 +1003,9 @@ struct drm_mode_config_funcs {
int (*atomic_commit)(struct drm_device *dev,
struct drm_atomic_state *a,
bool async);
+ struct drm_atomic_state *(*atomic_state_alloc)(struct drm_device *dev);
+ void (*atomic_state_clear)(struct drm_atomic_state *state);
+ void (*atomic_state_free)(struct drm_atomic_state *state);
};
/**
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v3
2015-05-18 8:06 ` [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v3 Maarten Lankhorst
@ 2015-05-18 14:40 ` Daniel Vetter
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-05-18 14:40 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel
On Mon, May 18, 2015 at 10:06:40AM +0200, Maarten Lankhorst wrote:
> Drivers may need to store the state of shared resources, such as PLLs
> or FIFO space, into the atomic state. Allow this by making it possible
> to subclass drm_atomic_state.
>
> Changes since v1:
> - Change member names for functions to atomic_state_(alloc,clear)
> - Change __drm_atomic_state_new to drm_atomic_state_init
> - Allow free function to be overridden too, in case extra memory is
> allocated in alloc.
> Changes since v2:
> - Rename *_default_free to default_release, to make clear it doesn't
> free the state object itself.
I'd have gone with just _release since we don't have a ->release hook nor
a drm_atomic_helper_state_release wrapper. But that's really a bikeshed
now ;-)
> Cc: dri-devel@lists.freedesktop.org
> Acked-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Applied to topic/drm-misc, thanks.
-Daniel
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c6277a4a1f2f..cd1b16b25716 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -30,7 +30,15 @@
> #include <drm/drm_atomic.h>
> #include <drm/drm_plane_helper.h>
>
> -static void kfree_state(struct drm_atomic_state *state)
> +/**
> + * drm_atomic_state_default_release -
> + * release memory initialized by drm_atomic_state_init
> + * @state: atomic state
> + *
> + * Free all the memory allocated by drm_atomic_state_init.
> + * This is useful for drivers that subclass the atomic state.
> + */
> +void drm_atomic_state_default_release(struct drm_atomic_state *state)
> {
> kfree(state->connectors);
> kfree(state->connector_states);
> @@ -38,24 +46,20 @@ static void kfree_state(struct drm_atomic_state *state)
> kfree(state->crtc_states);
> kfree(state->planes);
> kfree(state->plane_states);
> - kfree(state);
> }
> +EXPORT_SYMBOL(drm_atomic_state_default_release);
>
> /**
> - * drm_atomic_state_alloc - allocate atomic state
> + * drm_atomic_state_init - init new atomic state
> * @dev: DRM device
> + * @state: atomic state
> *
> - * This allocates an empty atomic state to track updates.
> + * Default implementation for filling in a new atomic state.
> + * This is useful for drivers that subclass the atomic state.
> */
> -struct drm_atomic_state *
> -drm_atomic_state_alloc(struct drm_device *dev)
> +int
> +drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
> {
> - struct drm_atomic_state *state;
> -
> - state = kzalloc(sizeof(*state), GFP_KERNEL);
> - if (!state)
> - return NULL;
> -
> /* TODO legacy paths should maybe do a better job about
> * setting this appropriately?
> */
> @@ -92,31 +96,50 @@ drm_atomic_state_alloc(struct drm_device *dev)
>
> state->dev = dev;
>
> - DRM_DEBUG_ATOMIC("Allocate atomic state %p\n", state);
> + DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state);
>
> - return state;
> + return 0;
> fail:
> - kfree_state(state);
> + drm_atomic_state_default_release(state);
> + return -ENOMEM;
> +}
> +EXPORT_SYMBOL(drm_atomic_state_init);
> +
> +/**
> + * drm_atomic_state_alloc - allocate atomic state
> + * @dev: DRM device
> + *
> + * This allocates an empty atomic state to track updates.
> + */
> +struct drm_atomic_state *
> +drm_atomic_state_alloc(struct drm_device *dev)
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_atomic_state *state;
> +
> + if (!config->funcs->atomic_state_alloc) {
> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> + if (!state)
> + return NULL;
> + if (drm_atomic_state_init(dev, state) < 0) {
> + kfree(state);
> + return NULL;
> + }
> + return state;
> + }
>
> - return NULL;
> + return config->funcs->atomic_state_alloc(dev);
> }
> EXPORT_SYMBOL(drm_atomic_state_alloc);
>
> /**
> - * drm_atomic_state_clear - clear state object
> + * drm_atomic_state_default_clear - clear base atomic state
> * @state: atomic state
> *
> - * When the w/w mutex algorithm detects a deadlock we need to back off and drop
> - * all locks. So someone else could sneak in and change the current modeset
> - * configuration. Which means that all the state assembled in @state is no
> - * longer an atomic update to the current state, but to some arbitrary earlier
> - * state. Which could break assumptions the driver's ->atomic_check likely
> - * relies on.
> - *
> - * Hence we must clear all cached state and completely start over, using this
> - * function.
> + * Default implementation for clearing atomic state.
> + * This is useful for drivers that subclass the atomic state.
> */
> -void drm_atomic_state_clear(struct drm_atomic_state *state)
> +void drm_atomic_state_default_clear(struct drm_atomic_state *state)
> {
> struct drm_device *dev = state->dev;
> struct drm_mode_config *config = &dev->mode_config;
> @@ -162,6 +185,32 @@ void drm_atomic_state_clear(struct drm_atomic_state *state)
> state->plane_states[i] = NULL;
> }
> }
> +EXPORT_SYMBOL(drm_atomic_state_default_clear);
> +
> +/**
> + * drm_atomic_state_clear - clear state object
> + * @state: atomic state
> + *
> + * When the w/w mutex algorithm detects a deadlock we need to back off and drop
> + * all locks. So someone else could sneak in and change the current modeset
> + * configuration. Which means that all the state assembled in @state is no
> + * longer an atomic update to the current state, but to some arbitrary earlier
> + * state. Which could break assumptions the driver's ->atomic_check likely
> + * relies on.
> + *
> + * Hence we must clear all cached state and completely start over, using this
> + * function.
> + */
> +void drm_atomic_state_clear(struct drm_atomic_state *state)
> +{
> + struct drm_device *dev = state->dev;
> + struct drm_mode_config *config = &dev->mode_config;
> +
> + if (config->funcs->atomic_state_clear)
> + config->funcs->atomic_state_clear(state);
> + else
> + drm_atomic_state_default_clear(state);
> +}
> EXPORT_SYMBOL(drm_atomic_state_clear);
>
> /**
> @@ -173,14 +222,25 @@ EXPORT_SYMBOL(drm_atomic_state_clear);
> */
> void drm_atomic_state_free(struct drm_atomic_state *state)
> {
> + struct drm_device *dev;
> + struct drm_mode_config *config;
> +
> if (!state)
> return;
>
> + dev = state->dev;
> + config = &dev->mode_config;
> +
> drm_atomic_state_clear(state);
>
> DRM_DEBUG_ATOMIC("Freeing atomic state %p\n", state);
>
> - kfree_state(state);
> + if (config->funcs->atomic_state_free) {
> + config->funcs->atomic_state_free(state);
> + } else {
> + drm_atomic_state_default_release(state);
> + kfree(state);
> + }
> }
> EXPORT_SYMBOL(drm_atomic_state_free);
>
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index d78543067700..f0d3a7387d99 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -35,6 +35,11 @@ drm_atomic_state_alloc(struct drm_device *dev);
> void drm_atomic_state_clear(struct drm_atomic_state *state);
> void drm_atomic_state_free(struct drm_atomic_state *state);
>
> +int __must_check
> +drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state);
> +void drm_atomic_state_default_clear(struct drm_atomic_state *state);
> +void drm_atomic_state_default_release(struct drm_atomic_state *state);
> +
> struct drm_crtc_state * __must_check
> drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> struct drm_crtc *crtc);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 840e9e62878c..bff25b0cada9 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -985,6 +985,9 @@ struct drm_mode_set {
> * @atomic_check: check whether a given atomic state update is possible
> * @atomic_commit: commit an atomic state update previously verified with
> * atomic_check()
> + * @atomic_state_alloc: allocate a new atomic state
> + * @atomic_state_clear: clear the atomic state
> + * @atomic_state_free: free the atomic state
> *
> * Some global (i.e. not per-CRTC, connector, etc) mode setting functions that
> * involve drivers.
> @@ -1000,6 +1003,9 @@ struct drm_mode_config_funcs {
> int (*atomic_commit)(struct drm_device *dev,
> struct drm_atomic_state *a,
> bool async);
> + struct drm_atomic_state *(*atomic_state_alloc)(struct drm_device *dev);
> + void (*atomic_state_clear)(struct drm_atomic_state *state);
> + void (*atomic_state_free)(struct drm_atomic_state *state);
> };
>
> /**
>
> _______________________________________________
> 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] 37+ messages in thread
* Re: [PATCH v2 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable
2015-05-13 20:23 ` [PATCH v2 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable Maarten Lankhorst
@ 2015-05-18 15:30 ` Daniel Vetter
2015-05-18 16:35 ` Maarten Lankhorst
0 siblings, 1 reply; 37+ messages in thread
From: Daniel Vetter @ 2015-05-18 15:30 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, May 13, 2015 at 10:23:37PM +0200, Maarten Lankhorst wrote:
> crtc_state->enable means a crtc is configured, but it may be turned
> off for dpms. Until the previous commit crtc_state->active was not
> updated on crtc off, but now that we do we should use that for tracking
> whether a crtc is scanning out or not.
>
> At this point crtc->active should mirror crtc_state->active,
> so some paranoia from the crtc_disable functions can be removed.
>
> Note that intel_set_mode_setup_plls still checks for ->enable,
> because all resources that are needed have to be calculated, so
> dpms changes will still succeed.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
A few detail comments below.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_irq.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++------------------
> 2 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 557af8877a2e..ca457317a8ac 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
> return -EINVAL;
> }
>
> - if (!crtc->state->enable) {
> + if (!crtc->state->active) {
This change looks unjustified I think.
> DRM_DEBUG_KMS("crtc %d is disabled\n", pipe);
> return -EBUSY;
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index db1b47febb27..583c9105cf49 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4667,7 +4667,7 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc)
> bool reenable_ips = false;
>
> /* The clocks have to be on to load the palette. */
> - if (!crtc->state->enable || !intel_crtc->active)
> + if (!crtc->state->active)
> return;
>
> if (HAS_GMCH_DISPLAY(dev_priv->dev)) {
> @@ -4872,9 +4872,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
>
> - WARN_ON(!crtc->state->enable);
> -
> - if (intel_crtc->active)
> + if (WARN_ON(intel_crtc->active))
> return;
>
> if (intel_crtc->config->has_pch_encoder)
> @@ -4978,9 +4976,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
>
> - WARN_ON(!crtc->state->enable);
> -
> - if (intel_crtc->active)
> + if (WARN_ON(intel_crtc->active))
> return;
>
> if (intel_crtc_to_shared_dpll(intel_crtc))
> @@ -5082,7 +5078,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> int pipe = intel_crtc->pipe;
> u32 reg, temp;
>
> - if (!intel_crtc->active)
> + if (WARN_ON(!intel_crtc->active))
> return;
>
> for_each_encoder_on_crtc(dev, crtc, encoder)
> @@ -5144,7 +5140,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> struct intel_encoder *encoder;
> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>
> - if (!intel_crtc->active)
> + if (WARN_ON(!intel_crtc->active))
> return;
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> @@ -5742,7 +5738,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
>
> /* add all active pipes to the state */
> for_each_crtc(state->dev, crtc) {
> - if (!crtc->state->enable)
> + if (!crtc->state->active)
This is a functional change to the cdclk code and might break it and/or
conflict with the ongoing cdclk work from Ville/Mika. Definitely needs to
be split out.
> continue;
>
> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> @@ -5752,7 +5748,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
>
> /* disable/enable all currently active pipes while we change cdclk */
> for_each_crtc_in_state(state, crtc, crtc_state, i)
> - if (crtc_state->enable)
> + if (crtc_state->active)
> crtc_state->mode_changed = true;
Same here.
Hm, aside of all that maybe we should drop vlv_modeset_global_pipes and
instead just look at crtc_state->mode_changed? That way we don't need to
duplicate the same checks twice, once to set ->mode_changed and once to
for the prepare_pipes mask. Or is that duplication already getting
removed?
>
> return 0;
> @@ -5840,9 +5836,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
> int pipe = intel_crtc->pipe;
> bool is_dsi;
>
> - WARN_ON(!crtc->state->enable);
> -
> - if (intel_crtc->active)
> + if (WARN_ON(intel_crtc->active))
> return;
>
> is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
> @@ -5918,9 +5912,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
>
> - WARN_ON(!crtc->state->enable);
> -
> - if (intel_crtc->active)
> + if (WARN_ON(intel_crtc->active))
> return;
>
> i9xx_set_pll_dividers(intel_crtc);
> @@ -5980,7 +5972,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> struct intel_encoder *encoder;
> int pipe = intel_crtc->pipe;
>
> - if (!intel_crtc->active)
> + if (WARN_ON(!intel_crtc->active))
> return;
>
> /*
> @@ -11553,7 +11545,7 @@ intel_modeset_update_state(struct drm_atomic_state *state)
> if (!crtc_state || !needs_modeset(crtc->state))
> continue;
>
> - if (crtc->state->enable) {
> + if (crtc->state->active) {
> struct drm_property *dpms_property =
> dev->mode_config.dpms_property;
>
> @@ -11970,6 +11962,10 @@ check_crtc_state(struct drm_device *dev)
> "crtc active state doesn't match with hw state "
> "(expected %i, found %i)\n", crtc->active, active);
>
> + I915_STATE_WARN(crtc->active != crtc->base.state->active,
> + "transitional active state does not match atomic hw state "
> + "(expected %i, found %i)\n", crtc->base.state->active, crtc->active);
> +
> if (active &&
> !intel_pipe_config_compare(dev, crtc->config, &pipe_config)) {
> I915_STATE_WARN(1, "pipe state doesn't match!\n");
> @@ -12115,6 +12111,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
> if (IS_ERR(pipe_config))
> return pipe_config;
>
> + if (!pipe_config->base.enable &&
> + WARN_ON(pipe_config->base.active))
> + pipe_config->base.active = false;
> +
> if (!pipe_config->base.enable)
> return pipe_config;
>
> @@ -12239,7 +12239,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
> return ret;
>
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> - if (!needs_modeset(crtc_state))
> + if (!needs_modeset(crtc_state) || !crtc->state->active)
> continue;
>
> intel_crtc_disable_planes(crtc);
> @@ -12261,7 +12261,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
>
> /* Now enable the clocks, plane, pipe, and connectors that we set up. */
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> - if (!needs_modeset(crtc->state) || !crtc->state->enable)
> + if (!needs_modeset(crtc->state) || !crtc->state->active)
> continue;
>
> update_scanline_offset(to_intel_crtc(crtc));
> @@ -14494,7 +14494,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
> * have active connectors/encoders. */
> intel_crtc_update_dpms(&crtc->base);
>
> - if (crtc->active != crtc->base.state->enable) {
> + if (crtc->active != crtc->base.state->active) {
> struct intel_encoder *encoder;
>
> /* This can happen either due to bugs in the get_hw_state
> --
> 2.1.0
>
> _______________________________________________
> 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] 37+ messages in thread
* Re: [PATCH v2 08/17] drm/i915: Set mode_changed for audio in intel_modeset_pipe_config()
2015-05-13 20:23 ` [PATCH v2 08/17] drm/i915: Set mode_changed for audio in intel_modeset_pipe_config() Maarten Lankhorst
@ 2015-05-18 15:36 ` Daniel Vetter
2015-05-18 16:37 ` Maarten Lankhorst
0 siblings, 1 reply; 37+ messages in thread
From: Daniel Vetter @ 2015-05-18 15:36 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Ander Conselvan de Oliveira, intel-gfx
On Wed, May 13, 2015 at 10:23:38PM +0200, Maarten Lankhorst wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> A follow up patch will make intel_modeset_compute_config() deal with
> multiple crtcs, so move crtc specific stuff into the lower level crtc
> specific function.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++----------------
> 1 file changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 583c9105cf49..ec548cbb06ee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -427,6 +427,12 @@ static void vlv_clock(int refclk, intel_clock_t *clock)
> clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
> }
>
> +static bool
> +needs_modeset(struct drm_crtc_state *state)
> +{
> + return state->mode_changed || state->active_changed;
> +}
> +
> /**
> * Returns whether any output on the specified pipe is of the specified type
> */
> @@ -11387,6 +11393,15 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> return -EINVAL;
> }
>
> + /*
> + * XXX: Add all connectors to make the crtc state match the encoders.
> + */
> + if (!needs_modeset(&pipe_config->base)) {
> + ret = drm_atomic_add_affected_connectors(state, crtc);
> + if (ret)
> + return ret;
> + }
Comment aside: Eventually we need to invert this check and use
intel_pipe_config_compare with a special mode to a) not scream into dmesg
b) ignore any changes we can fixup without a full modeset (i.e. pfit). And
then use that to decide whether we need a modeset or not for the crtc.
We still rely upon our internal set_mode functions to at least in some
cases do an unconditional modeset on the passed-in crtc for property
updates. But once converted to atomic we need to be more intelligent,
especially since userspace expects no-op changes to get filtered out. So
unconditionally doing a modeset wont cut it either (with the current code
all the set_prop hooks have such checks hand-rolled).
-Daniel
> +
> clear_intel_crtc_state(pipe_config);
>
> pipe_config->cpu_transcoder =
> @@ -11478,6 +11493,18 @@ encoder_retry:
> DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
> base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
>
> + /* Check if we need to force a modeset */
> + if (pipe_config->has_audio !=
> + to_intel_crtc_state(crtc->state)->has_audio)
> + pipe_config->base.mode_changed = true;
> +
> + /*
> + * Note we have an issue here with infoframes: current code
> + * only updates them on the full mode set path per hw
> + * requirements. So here we should be checking for any
> + * required changes and forcing a mode set.
> + */
> +
> return 0;
> fail:
> return ret;
> @@ -11495,12 +11522,6 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)
> return false;
> }
>
> -static bool
> -needs_modeset(struct drm_crtc_state *state)
> -{
> - return state->mode_changed || state->active_changed;
> -}
> -
> static void
> intel_modeset_update_state(struct drm_atomic_state *state)
> {
> @@ -12093,10 +12114,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
> struct intel_crtc_state *pipe_config;
> int ret = 0;
>
> - ret = drm_atomic_add_affected_connectors(state, crtc);
> - if (ret)
> - return ERR_PTR(ret);
> -
> ret = drm_atomic_helper_check_modeset(state->dev, state);
> if (ret)
> return ERR_PTR(ret);
> @@ -12122,19 +12139,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
> if (ret)
> return ERR_PTR(ret);
>
> - /* Check things that can only be changed through modeset */
> - if (pipe_config->has_audio !=
> - to_intel_crtc(crtc)->config->has_audio)
> - pipe_config->base.mode_changed = true;
> -
> - /*
> - * Note we have an issue here with infoframes: current code
> - * only updates them on the full mode set path per hw
> - * requirements. So here we should be checking for any
> - * required changes and forcing a mode set.
> - */
> -
> - intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]");
> + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, "[modeset]");
>
> ret = drm_atomic_helper_check_planes(state->dev, state);
> if (ret)
> --
> 2.1.0
>
> _______________________________________________
> 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] 37+ messages in thread
* Re: [PATCH v2 11/17] drm/i915: Use global atomic state for staged pll config
2015-05-13 20:23 ` [PATCH v2 11/17] drm/i915: Use global atomic state for staged pll config Maarten Lankhorst
@ 2015-05-18 15:45 ` Daniel Vetter
2015-05-18 16:27 ` Maarten Lankhorst
0 siblings, 1 reply; 37+ messages in thread
From: Daniel Vetter @ 2015-05-18 15:45 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Ander Conselvan de Oliveira, intel-gfx
On Wed, May 13, 2015 at 10:23:41PM +0200, Maarten Lankhorst wrote:
> 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.
>
> 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 | 49 ++++++++++++++++
> drivers/gpu/drm/i915/intel_display.c | 111 +++++++++++------------------------
> drivers/gpu/drm/i915/intel_drv.h | 13 ++++
> 4 files changed, 95 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a0e4868653f2..0e200018c9aa 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..aff87054406c 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -421,3 +421,52 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
>
> return 0;
> }
> +
> +static void intel_atomic_duplicate_dpll_state(struct drm_device *dev,
> + struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_shared_dpll *pll;
> + enum intel_dpll_id i;
> +
> + state->dpll_set = true;
The ww mutex locking dance here is missing. For simplicity I think we
could just repurpose the dev->mode_config.connection_mutex. We need that
anyway full modesets, and dpll changes should only be required in those.
Which means this wont introduce any unecessary parallelism.
It means though that we need to wire up a proper error code to all callers
of duplicate/get_dpll_state, like with all the other atomic states. Might
be best to follow the design for connector/crtc/planes an pass around
pointers with error codes explicitly (instead of returning
state->shared_dpll below since that can only cope with NULL and not with
-EDEADLK).
Sorry that I didn't spot this earlier.
-Daniel
> +
> + /* Copy shared dpll state */
> + for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> + pll = &dev_priv->shared_dplls[i];
> +
> + memcpy(&state->shared_dpll[i],
> + &pll->config, sizeof(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);
> +
> + if (!state->dpll_set)
> + intel_atomic_duplicate_dpll_state(state->base.dev, state);
> +
> + 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 0a2a7b6e198c..ed69eddf457e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4229,8 +4229,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;
> @@ -4239,7 +4242,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;
> }
> @@ -4259,7 +4262,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;
> }
> @@ -4268,15 +4271,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;
> }
> @@ -4285,7 +4288,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;
> @@ -4295,83 +4298,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];
> }
> }
>
> @@ -11532,13 +11485,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) {
> @@ -12171,9 +12123,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)
> @@ -12184,13 +12140,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;
> }
>
> @@ -13887,6 +13840,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
--
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] 37+ messages in thread
* Re: [PATCH v2 15/17] drm/i915: Calculate haswell plane workaround, v2.
2015-05-13 20:23 ` [PATCH v2 15/17] drm/i915: Calculate haswell plane workaround, v2 Maarten Lankhorst
@ 2015-05-18 15:47 ` Daniel Vetter
2015-05-18 15:51 ` Daniel Stone
0 siblings, 1 reply; 37+ messages in thread
From: Daniel Vetter @ 2015-05-18 15:47 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, May 13, 2015 at 10:23:45PM +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.
Hm, intel_crtc->atomic needs to be moved into intel_crtc_state eventually,
so I don't really understand your reasoning here. Yes it's not really a
static state, but we carry all the other state transition hints like
->need_modeset or ->active_changed around in state objects too. Imo it's
the right place for this stuff, the only tricky part is that we must clear
them all in the relevant duplicate_state hooks.
Anyway nothing that blocks this one, just something to keep in mind for
the road ahead.
-Daniel
>
> 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.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 100 ++++++++++++++++++++++++-----------
> drivers/gpu/drm/i915/intel_drv.h | 3 ++
> 2 files changed, 72 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1d3b0233f070..9316b0be4f5b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4897,42 +4897,13 @@ 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;
>
> if (WARN_ON(intel_crtc->active))
> return;
> @@ -5009,7 +4980,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 = intel_crtc->atomic.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)
> @@ -12099,6 +12074,66 @@ 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 *crtc;
> + struct drm_crtc_state *crtc_state;
> + struct intel_crtc *intel_crtc, *enabled_crtc;
> + struct intel_crtc *first_crtc = NULL, *other_crtc = NULL;
> + int enabled_crtcs = 0, 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) {
> + other_crtc = intel_crtc;
> + break;
> + } else {
> + first_crtc = intel_crtc;
> + }
> + }
> +
> + /* No workaround needed? */
> + if (!first_crtc)
> + return 0;
> +
> + /* w/a possibly needed, check how many crtc's are already enabled. */
> + for_each_intel_crtc(state->dev, intel_crtc) {
> + intel_crtc->atomic.hsw_workaround_pipe = INVALID_PIPE;
> +
> + crtc_state = drm_atomic_get_crtc_state(state,
> + &intel_crtc->base);
> + if (IS_ERR(crtc_state))
> + return PTR_ERR(crtc_state);
> +
> + if (!crtc_state->active || needs_modeset(crtc_state))
> + continue;
> +
> + /* 2 or more enabled crtcs means no need for w/a */
> + if (++enabled_crtcs >= 2)
> + return 0;
> +
> + enabled_crtc = intel_crtc;
> + }
> +
> + if (enabled_crtcs == 1)
> + first_crtc->atomic.hsw_workaround_pipe = enabled_crtc->pipe;
> + else if (other_crtc)
> + other_crtc->atomic.hsw_workaround_pipe = first_crtc->pipe;
> +
> + return 0;
> +}
> +
> /* Code that should eventually be part of atomic_check() */
> static int __intel_set_mode_checks(struct drm_atomic_state *state)
> {
> @@ -12122,6 +12157,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..1aa10a9445de 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -490,6 +490,9 @@ struct intel_crtc_atomic_commit {
> bool update_fbc;
> bool post_enable_primary;
> unsigned update_sprite_watermarks;
> +
> + /* Operations to perform during modeset */
> + enum pipe hsw_workaround_pipe;
> };
>
> struct intel_crtc {
> --
> 2.1.0
>
> _______________________________________________
> 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] 37+ messages in thread
* Re: [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks.
2015-05-13 20:23 ` [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks Maarten Lankhorst
@ 2015-05-18 15:49 ` Daniel Vetter
2015-05-18 16:28 ` Ville Syrjälä
0 siblings, 1 reply; 37+ messages in thread
From: Daniel Vetter @ 2015-05-18 15:49 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, May 13, 2015 at 10:23:46PM +0200, Maarten Lankhorst wrote:
> 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.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
I think adding a code comment to our assignment of crtc->hw_mode that we
need this for i915_get_vblank_timestamp (and only for that) would be
really good. Especially since I can't find it with a quick grep, at least
in current upstream ;-)
-Daniel
> ---
> drivers/gpu/drm/i915/i915_irq.c | 13 ++++++-------
> drivers/gpu/drm/i915/intel_display.c | 3 +++
> 2 files changed, 9 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 9316b0be4f5b..5063ac3d028d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11467,6 +11467,9 @@ 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);
> +
> + if (!crtc->state->active)
> + 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
--
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] 37+ messages in thread
* Re: [PATCH v2 17/17] drm/i915: Remove use of crtc->config from i915_debugfs.c
2015-05-13 20:23 ` [PATCH v2 17/17] drm/i915: Remove use of crtc->config from i915_debugfs.c Maarten Lankhorst
@ 2015-05-18 15:51 ` Daniel Vetter
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-05-18 15:51 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, May 13, 2015 at 10:23:47PM +0200, Maarten Lankhorst wrote:
> crtc->config is a pointer to the current crtc->state, and will be
> removed eventually. Same for crtc->active, instead use crtc->state->active.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Maybe assert here in the commit message that with all the previous changes
->config and ->state are now updated in lockstep and hence we can switch
them over. Same for ->active. Just to make sure that you and reviewers
agree on what the crucial bit is here.
-Daniel
> ---
> 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 acd4d2c7613a..5439ff9f7e6b 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
--
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] 37+ messages in thread
* Re: [PATCH v2 15/17] drm/i915: Calculate haswell plane workaround, v2.
2015-05-18 15:47 ` Daniel Vetter
@ 2015-05-18 15:51 ` Daniel Stone
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Stone @ 2015-05-18 15:51 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
Hi,
On 18 May 2015 at 16:47, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, May 13, 2015 at 10:23:45PM +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.
>
> Hm, intel_crtc->atomic needs to be moved into intel_crtc_state eventually,
> so I don't really understand your reasoning here. Yes it's not really a
> static state, but we carry all the other state transition hints like
> ->need_modeset or ->active_changed around in state objects too. Imo it's
> the right place for this stuff, the only tricky part is that we must clear
> them all in the relevant duplicate_state hooks.
>
> Anyway nothing that blocks this one, just something to keep in mind for
> the road ahead.
What makes me slightly nervous is the number of places where we
duplicate the state, meaning that we could easily bleed those if we're
not extremely careful about how we do it. I argued for this one to go
into intel_crtc->atomic as it was much more suited than the static
state, but if we're implementing a transient state section of
intel_crtc_state (ideally under its own sub-struct, so it can easily
be zeroed), then I guess that wfm.
Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 11/17] drm/i915: Use global atomic state for staged pll config
2015-05-18 15:45 ` Daniel Vetter
@ 2015-05-18 16:27 ` Maarten Lankhorst
2015-05-19 8:13 ` Daniel Vetter
0 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-18 16:27 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Ander Conselvan de Oliveira, intel-gfx
Op 18-05-15 om 17:45 schreef Daniel Vetter:
> On Wed, May 13, 2015 at 10:23:41PM +0200, Maarten Lankhorst wrote:
>> 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.
>>
>> 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 | 49 ++++++++++++++++
>> drivers/gpu/drm/i915/intel_display.c | 111 +++++++++++------------------------
>> drivers/gpu/drm/i915/intel_drv.h | 13 ++++
>> 4 files changed, 95 insertions(+), 79 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index a0e4868653f2..0e200018c9aa 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..aff87054406c 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -421,3 +421,52 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
>>
>> return 0;
>> }
>> +
>> +static void intel_atomic_duplicate_dpll_state(struct drm_device *dev,
>> + struct intel_atomic_state *state)
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(dev);
>> + struct intel_shared_dpll *pll;
>> + enum intel_dpll_id i;
>> +
>> + state->dpll_set = true;
> The ww mutex locking dance here is missing. For simplicity I think we
> could just repurpose the dev->mode_config.connection_mutex. We need that
> anyway full modesets, and dpll changes should only be required in those.
> Which means this wont introduce any unecessary parallelism.
>
> It means though that we need to wire up a proper error code to all callers
> of duplicate/get_dpll_state, like with all the other atomic states. Might
> be best to follow the design for connector/crtc/planes an pass around
> pointers with error codes explicitly (instead of returning
> state->shared_dpll below since that can only cope with NULL and not with
> -EDEADLK).
>
> Sorry that I didn't spot this earlier.
>
The dpll state should only ever be done during a modeset in which case the connection_mutex is guaranteed to be held.
Instead of making this return a value can we add a lockdep_assert_held ?
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks.
2015-05-18 15:49 ` Daniel Vetter
@ 2015-05-18 16:28 ` Ville Syrjälä
2015-05-19 6:10 ` Maarten Lankhorst
0 siblings, 1 reply; 37+ messages in thread
From: Ville Syrjälä @ 2015-05-18 16:28 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Mon, May 18, 2015 at 05:49:23PM +0200, Daniel Vetter wrote:
> On Wed, May 13, 2015 at 10:23:46PM +0200, Maarten Lankhorst wrote:
> > 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.
> >
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> I think adding a code comment to our assignment of crtc->hw_mode that we
> need this for i915_get_vblank_timestamp (and only for that) would be
> really good. Especially since I can't find it with a quick grep, at least
> in current upstream ;-)
I don't particularly like resurrecting this zombie. Why we can't just use
crtc->state->adjusted_mode (or wherever the current adjusted mode is kept)?
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 13 ++++++-------
> > drivers/gpu/drm/i915/intel_display.c | 3 +++
> > 2 files changed, 9 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 9316b0be4f5b..5063ac3d028d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11467,6 +11467,9 @@ 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);
> > +
> > + if (!crtc->state->active)
> > + 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
>
> --
> 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable
2015-05-18 15:30 ` Daniel Vetter
@ 2015-05-18 16:35 ` Maarten Lankhorst
2015-05-19 8:09 ` Daniel Vetter
0 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-18 16:35 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
Op 18-05-15 om 17:30 schreef Daniel Vetter:
> On Wed, May 13, 2015 at 10:23:37PM +0200, Maarten Lankhorst wrote:
>> crtc_state->enable means a crtc is configured, but it may be turned
>> off for dpms. Until the previous commit crtc_state->active was not
>> updated on crtc off, but now that we do we should use that for tracking
>> whether a crtc is scanning out or not.
>>
>> At this point crtc->active should mirror crtc_state->active,
>> so some paranoia from the crtc_disable functions can be removed.
>>
>> Note that intel_set_mode_setup_plls still checks for ->enable,
>> because all resources that are needed have to be calculated, so
>> dpms changes will still succeed.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> A few detail comments below.
> -Daniel
>
>> ---
>> drivers/gpu/drm/i915/i915_irq.c | 2 +-
>> drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++------------------
>> 2 files changed, 23 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 557af8877a2e..ca457317a8ac 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
>> return -EINVAL;
>> }
>>
>> - if (!crtc->state->enable) {
>> + if (!crtc->state->active) {
> This change looks unjustified I think.
Why? Can you get a vblank on crtc that is dpms off? I think not..
Either way later on I use hwmode->crtc_clock which renders this moot.
>> <snip>
>> @@ -5742,7 +5738,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
>>
>> /* add all active pipes to the state */
>> for_each_crtc(state->dev, crtc) {
>> - if (!crtc->state->enable)
>> + if (!crtc->state->active)
> This is a functional change to the cdclk code and might break it and/or
> conflict with the ongoing cdclk work from Ville/Mika. Definitely needs to
> be split out.
No this function just sets mode_changed on all active crtc's.
This is done to turn them off while changing the clock. In case they're already turned off you don't have to turn them off.
>> continue;
>>
>> crtc_state = drm_atomic_get_crtc_state(state, crtc);
>> @@ -5752,7 +5748,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
>>
>> /* disable/enable all currently active pipes while we change cdclk */
>> for_each_crtc_in_state(state, crtc, crtc_state, i)
>> - if (crtc_state->enable)
>> + if (crtc_state->active)
>> crtc_state->mode_changed = true;
> Same here.
>
> Hm, aside of all that maybe we should drop vlv_modeset_global_pipes and
> instead just look at crtc_state->mode_changed? That way we don't need to
> duplicate the same checks twice, once to set ->mode_changed and once to
> for the prepare_pipes mask. Or is that duplication already getting
> removed?
I think we could get rid of a lot of those extra loops if we want to later,
but for now readability is more important.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 08/17] drm/i915: Set mode_changed for audio in intel_modeset_pipe_config()
2015-05-18 15:36 ` Daniel Vetter
@ 2015-05-18 16:37 ` Maarten Lankhorst
0 siblings, 0 replies; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-18 16:37 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Ander Conselvan de Oliveira, intel-gfx
Op 18-05-15 om 17:36 schreef Daniel Vetter:
> On Wed, May 13, 2015 at 10:23:38PM +0200, Maarten Lankhorst wrote:
>> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>>
>> A follow up patch will make intel_modeset_compute_config() deal with
>> multiple crtcs, so move crtc specific stuff into the lower level crtc
>> specific function.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++----------------
>> 1 file changed, 28 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 583c9105cf49..ec548cbb06ee 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -427,6 +427,12 @@ static void vlv_clock(int refclk, intel_clock_t *clock)
>> clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p);
>> }
>>
>> +static bool
>> +needs_modeset(struct drm_crtc_state *state)
>> +{
>> + return state->mode_changed || state->active_changed;
>> +}
>> +
>> /**
>> * Returns whether any output on the specified pipe is of the specified type
>> */
>> @@ -11387,6 +11393,15 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>> return -EINVAL;
>> }
>>
>> + /*
>> + * XXX: Add all connectors to make the crtc state match the encoders.
>> + */
>> + if (!needs_modeset(&pipe_config->base)) {
>> + ret = drm_atomic_add_affected_connectors(state, crtc);
>> + if (ret)
>> + return ret;
>> + }
> Comment aside: Eventually we need to invert this check and use
> intel_pipe_config_compare with a special mode to a) not scream into dmesg
> b) ignore any changes we can fixup without a full modeset (i.e. pfit). And
> then use that to decide whether we need a modeset or not for the crtc.
> We still rely upon our internal set_mode functions to at least in some
> cases do an unconditional modeset on the passed-in crtc for property
> updates. But once converted to atomic we need to be more intelligent,
> especially since userspace expects no-op changes to get filtered out. So
> unconditionally doing a modeset wont cut it either (with the current code
> all the set_prop hooks have such checks hand-rolled).
Yeah, but for now I can't get that to work properly until everything looks more atomic first.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks.
2015-05-18 16:28 ` Ville Syrjälä
@ 2015-05-19 6:10 ` Maarten Lankhorst
2015-05-19 8:16 ` Daniel Vetter
0 siblings, 1 reply; 37+ messages in thread
From: Maarten Lankhorst @ 2015-05-19 6:10 UTC (permalink / raw)
To: Ville Syrjälä, Daniel Vetter; +Cc: intel-gfx
Op 18-05-15 om 18:28 schreef Ville Syrjälä:
> On Mon, May 18, 2015 at 05:49:23PM +0200, Daniel Vetter wrote:
>> On Wed, May 13, 2015 at 10:23:46PM +0200, Maarten Lankhorst wrote:
>>> 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.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> I think adding a code comment to our assignment of crtc->hw_mode that we
>> need this for i915_get_vblank_timestamp (and only for that) would be
>> really good. Especially since I can't find it with a quick grep, at least
>> in current upstream ;-)
> I don't particularly like resurrecting this zombie. Why we can't just use
> crtc->state->adjusted_mode (or wherever the current adjusted mode is kept)?
>
Because we want to get rid of intel_crtc->config, and if drm_atomic_swap_state
is moved to be done before any call to then crtc->state->adjusted_mode will not
be in sync with the hw state, and any wai tfor vblank will produce funny results.
Since I don't think you should want to pass a state to vblank you would have to use
some crtc local variable somewhere, in this case I chose to use hwmode for that.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable
2015-05-18 16:35 ` Maarten Lankhorst
@ 2015-05-19 8:09 ` Daniel Vetter
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-05-19 8:09 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, May 18, 2015 at 06:35:59PM +0200, Maarten Lankhorst wrote:
> Op 18-05-15 om 17:30 schreef Daniel Vetter:
> > On Wed, May 13, 2015 at 10:23:37PM +0200, Maarten Lankhorst wrote:
> >> crtc_state->enable means a crtc is configured, but it may be turned
> >> off for dpms. Until the previous commit crtc_state->active was not
> >> updated on crtc off, but now that we do we should use that for tracking
> >> whether a crtc is scanning out or not.
> >>
> >> At this point crtc->active should mirror crtc_state->active,
> >> so some paranoia from the crtc_disable functions can be removed.
> >>
> >> Note that intel_set_mode_setup_plls still checks for ->enable,
> >> because all resources that are needed have to be calculated, so
> >> dpms changes will still succeed.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > A few detail comments below.
> > -Daniel
> >
> >> ---
> >> drivers/gpu/drm/i915/i915_irq.c | 2 +-
> >> drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++------------------
> >> 2 files changed, 23 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index 557af8877a2e..ca457317a8ac 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
> >> return -EINVAL;
> >> }
> >>
> >> - if (!crtc->state->enable) {
> >> + if (!crtc->state->active) {
> > This change looks unjustified I think.
> Why? Can you get a vblank on crtc that is dpms off? I think not..
>
> Either way later on I use hwmode->crtc_clock which renders this moot.
> >> <snip>
> >> @@ -5742,7 +5738,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
> >>
> >> /* add all active pipes to the state */
> >> for_each_crtc(state->dev, crtc) {
> >> - if (!crtc->state->enable)
> >> + if (!crtc->state->active)
> > This is a functional change to the cdclk code and might break it and/or
> > conflict with the ongoing cdclk work from Ville/Mika. Definitely needs to
> > be split out.
> No this function just sets mode_changed on all active crtc's.
>
> This is done to turn them off while changing the clock. In case they're already turned off you don't have to turn them off.
>
> >> continue;
> >>
> >> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> >> @@ -5752,7 +5748,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
> >>
> >> /* disable/enable all currently active pipes while we change cdclk */
> >> for_each_crtc_in_state(state, crtc, crtc_state, i)
> >> - if (crtc_state->enable)
> >> + if (crtc_state->active)
> >> crtc_state->mode_changed = true;
> > Same here.
> >
> > Hm, aside of all that maybe we should drop vlv_modeset_global_pipes and
> > instead just look at crtc_state->mode_changed? That way we don't need to
> > duplicate the same checks twice, once to set ->mode_changed and once to
> > for the prepare_pipes mask. Or is that duplication already getting
> > removed?
> I think we could get rid of a lot of those extra loops if we want to later,
> but for now readability is more important.
Hm makes sense with your replies - there's a few cases indeed where we
need to switch from checking ->enable to ->active if we start using the
mode_set machinery for dpms. I think it'd be good to explain that a bit in
the commit message for all the different cases, so that reviewers can go
hunting for more and make sure there aren't ;-)
-Daniel
--
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] 37+ messages in thread
* Re: [PATCH v2 11/17] drm/i915: Use global atomic state for staged pll config
2015-05-18 16:27 ` Maarten Lankhorst
@ 2015-05-19 8:13 ` Daniel Vetter
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-05-19 8:13 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: Ander Conselvan de Oliveira, intel-gfx
On Mon, May 18, 2015 at 06:27:51PM +0200, Maarten Lankhorst wrote:
> Op 18-05-15 om 17:45 schreef Daniel Vetter:
> > On Wed, May 13, 2015 at 10:23:41PM +0200, Maarten Lankhorst wrote:
> >> 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.
> >>
> >> 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 | 49 ++++++++++++++++
> >> drivers/gpu/drm/i915/intel_display.c | 111 +++++++++++------------------------
> >> drivers/gpu/drm/i915/intel_drv.h | 13 ++++
> >> 4 files changed, 95 insertions(+), 79 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index a0e4868653f2..0e200018c9aa 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..aff87054406c 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >> @@ -421,3 +421,52 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
> >>
> >> return 0;
> >> }
> >> +
> >> +static void intel_atomic_duplicate_dpll_state(struct drm_device *dev,
> >> + struct intel_atomic_state *state)
> >> +{
> >> + struct drm_i915_private *dev_priv = to_i915(dev);
> >> + struct intel_shared_dpll *pll;
> >> + enum intel_dpll_id i;
> >> +
> >> + state->dpll_set = true;
> > The ww mutex locking dance here is missing. For simplicity I think we
> > could just repurpose the dev->mode_config.connection_mutex. We need that
> > anyway full modesets, and dpll changes should only be required in those.
> > Which means this wont introduce any unecessary parallelism.
> >
> > It means though that we need to wire up a proper error code to all callers
> > of duplicate/get_dpll_state, like with all the other atomic states. Might
> > be best to follow the design for connector/crtc/planes an pass around
> > pointers with error codes explicitly (instead of returning
> > state->shared_dpll below since that can only cope with NULL and not with
> > -EDEADLK).
> >
> > Sorry that I didn't spot this earlier.
> >
> The dpll state should only ever be done during a modeset in which case the connection_mutex is guaranteed to be held.
> Instead of making this return a value can we add a lockdep_assert_held ?
The problem is that the atomic core might only grab the crtc state and
crtc mutex if e.g. userspace only changes the mode. But I've forgotten
that we're using the helpers to handle modesets, and those will acquire
all the needed connector states and hence the connection_mutex.
A locking check is therefore indeed enough. But please use
WARN_ON(!mutex_is_locked) since imo a locking check which can be compiled
out is useless. And the additional correctness lockdep provides isn't
needed - we'll catch the bug since no one always hits the race by doing
modesets in parallel ;-)
-Daniel
--
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] 37+ messages in thread
* Re: [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks.
2015-05-19 6:10 ` Maarten Lankhorst
@ 2015-05-19 8:16 ` Daniel Vetter
0 siblings, 0 replies; 37+ messages in thread
From: Daniel Vetter @ 2015-05-19 8:16 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Tue, May 19, 2015 at 08:10:46AM +0200, Maarten Lankhorst wrote:
> Op 18-05-15 om 18:28 schreef Ville Syrjälä:
> > On Mon, May 18, 2015 at 05:49:23PM +0200, Daniel Vetter wrote:
> >> On Wed, May 13, 2015 at 10:23:46PM +0200, Maarten Lankhorst wrote:
> >>> 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.
> >>>
> >>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> I think adding a code comment to our assignment of crtc->hw_mode that we
> >> need this for i915_get_vblank_timestamp (and only for that) would be
> >> really good. Especially since I can't find it with a quick grep, at least
> >> in current upstream ;-)
> > I don't particularly like resurrecting this zombie. Why we can't just use
> > crtc->state->adjusted_mode (or wherever the current adjusted mode is kept)?
> >
> Because we want to get rid of intel_crtc->config, and if drm_atomic_swap_state
> is moved to be done before any call to then crtc->state->adjusted_mode will not
> be in sync with the hw state, and any wai tfor vblank will produce funny results.
>
> Since I don't think you should want to pass a state to vblank you would have to use
> some crtc local variable somewhere, in this case I chose to use hwmode for that.
I guess plan be could be to the required values (and only those we need,
not the entire mode struct) in struct intel_crtc. Not sure whether that's
worth the bother.
-Daniel
--
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] 37+ messages in thread
end of thread, other threads:[~2015-05-19 8:14 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-13 20:23 [PATCH v2 00/17] drm/i915: Convert to atomic, part 2 Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 01/17] drm/atomic: update crtc->hwmode in legacy state Maarten Lankhorst
2015-05-18 8:01 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v2 Maarten Lankhorst
2015-05-18 8:06 ` [PATCH v2 02/17] drm/atomic: Allow drivers to subclass drm_atomic_state, v3 Maarten Lankhorst
2015-05-18 14:40 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 03/17] drm/i915: get rid of put_shared_dpll Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 04/17] drm/i915: get rid of intel_crtc_disable and related code, v2 Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 05/17] drm/i915: use intel_crtc_control everywhere Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 06/17] drm/i915: Use drm_atomic_helper_update_legacy_modeset_state Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 07/17] drm/i915: Use crtc_state->active instead of crtc_state->enable Maarten Lankhorst
2015-05-18 15:30 ` Daniel Vetter
2015-05-18 16:35 ` Maarten Lankhorst
2015-05-19 8:09 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 08/17] drm/i915: Set mode_changed for audio in intel_modeset_pipe_config() Maarten Lankhorst
2015-05-18 15:36 ` Daniel Vetter
2015-05-18 16:37 ` Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 09/17] drm/i915: Make __intel_set_mode() take only atomic state as argument Maarten Lankhorst
2015-05-15 7:42 ` Ander Conselvan De Oliveira
2015-05-13 20:23 ` [PATCH v2 10/17] drm/i915: Support modeset across multiple pipes Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 11/17] drm/i915: Use global atomic state for staged pll config Maarten Lankhorst
2015-05-18 15:45 ` Daniel Vetter
2015-05-18 16:27 ` Maarten Lankhorst
2015-05-19 8:13 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 12/17] drm/i915: Read hw state into an atomic state struct Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 13/17] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config() Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 14/17] drm/i915: Implement intel_crtc_toggle using atomic state, v3 Maarten Lankhorst
2015-05-13 20:23 ` [PATCH v2 15/17] drm/i915: Calculate haswell plane workaround, v2 Maarten Lankhorst
2015-05-18 15:47 ` Daniel Vetter
2015-05-18 15:51 ` Daniel Stone
2015-05-13 20:23 ` [PATCH v2 16/17] drm/i915: Use crtc->hwmode for vblanks Maarten Lankhorst
2015-05-18 15:49 ` Daniel Vetter
2015-05-18 16:28 ` Ville Syrjälä
2015-05-19 6:10 ` Maarten Lankhorst
2015-05-19 8:16 ` Daniel Vetter
2015-05-13 20:23 ` [PATCH v2 17/17] drm/i915: Remove use of crtc->config from i915_debugfs.c Maarten Lankhorst
2015-05-18 15:51 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.