* [PATCH 1/2] drm/i915: Use a crtc mask instead of a active refcount for dpll functions.
@ 2015-11-24 9:44 Maarten Lankhorst
2015-11-24 9:44 ` [PATCH 2/2] drm/i915: Perform dpll commit first Maarten Lankhorst
2015-11-26 14:36 ` [PATCH 1/2] drm/i915: Use a crtc mask instead of a active refcount for dpll functions Ander Conselvan De Oliveira
0 siblings, 2 replies; 5+ messages in thread
From: Maarten Lankhorst @ 2015-11-24 9:44 UTC (permalink / raw)
To: intel-gfx
This makes it easier to verify correct dpll setup with only a single crtc.
It it also useful to detect double dpll enable/disable.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 68 ++++++++++++++++++++----------------
3 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b86feeeafc45..37d4a7f3d112 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3236,7 +3236,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->name, pll->id);
seq_printf(m, " crtc_mask: 0x%08x, active: %d, on: %s\n",
- pll->config.crtc_mask, pll->active, yesno(pll->on));
+ pll->config.crtc_mask, hweight32(pll->active_mask), yesno(pll->on));
seq_printf(m, " tracked hardware state:\n");
seq_printf(m, " dpll: 0x%08x\n", pll->config.hw_state.dpll);
seq_printf(m, " dpll_md: 0x%08x\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f3ad72abeea7..e0763c3005e8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -393,7 +393,7 @@ struct intel_shared_dpll_config {
struct intel_shared_dpll {
struct intel_shared_dpll_config config;
- int active; /* count of number of active CRTCs (i.e. DPMS on) */
+ unsigned active_mask; /* mask of active CRTCs (i.e. DPMS on) */
bool on; /* is the PLL actually active? Disabled during modeset */
const char *name;
/* should match the index in the dev_priv->shared_dplls array */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3317db3806a8..79bb22ef67dc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1870,7 +1870,7 @@ static void intel_prepare_shared_dpll(struct intel_crtc *crtc)
return;
WARN_ON(!pll->config.crtc_mask);
- if (pll->active == 0) {
+ if (pll->active_mask == 0) {
DRM_DEBUG_DRIVER("setting up %s\n", pll->name);
WARN_ON(pll->on);
assert_shared_dpll_disabled(dev_priv, pll);
@@ -1892,6 +1892,7 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
+ unsigned crtc_mask = 1 << drm_crtc_index(&crtc->base);
if (WARN_ON(pll == NULL))
return;
@@ -1899,11 +1900,16 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
if (WARN_ON(pll->config.crtc_mask == 0))
return;
+ if (WARN_ON(pll->active_mask & crtc_mask))
+ return;
+
DRM_DEBUG_KMS("enable %s (active %d, on? %d) for crtc %d\n",
- pll->name, pll->active, pll->on,
+ pll->name, hweight32(pll->active_mask), pll->on,
crtc->base.base.id);
- if (pll->active++) {
+ pll->active_mask |= crtc_mask;
+
+ if (pll->active_mask != crtc_mask) {
WARN_ON(!pll->on);
assert_shared_dpll_enabled(dev_priv, pll);
return;
@@ -1922,30 +1928,33 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc)
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
+ unsigned crtc_mask = 1 << drm_crtc_index(&crtc->base);
/* PCH only available on ILK+ */
- if (INTEL_INFO(dev)->gen < 5)
+ if (INTEL_INFO(dev_priv)->gen < 5)
return;
if (pll == NULL)
return;
- if (WARN_ON(!(pll->config.crtc_mask & (1 << drm_crtc_index(&crtc->base)))))
+ if (WARN_ON(!(pll->config.crtc_mask & crtc_mask)))
return;
- DRM_DEBUG_KMS("disable %s (active %d, on? %d) for crtc %d\n",
- pll->name, pll->active, pll->on,
+ if (WARN_ON(!(pll->active_mask & crtc_mask)))
+ return;
+
+ DRM_DEBUG_KMS("disable %s (active %x, on? %d) for crtc %d\n",
+ pll->name, pll->active_mask, pll->on,
crtc->base.base.id);
- if (WARN_ON(pll->active == 0)) {
+ pll->active_mask &= ~crtc_mask;
+ if (pll->active_mask) {
assert_shared_dpll_disabled(dev_priv, pll);
return;
}
assert_shared_dpll_enabled(dev_priv, pll);
WARN_ON(!pll->on);
- if (--pll->active)
- return;
DRM_DEBUG_KMS("disabling %s\n", pll->name);
pll->disable(dev_priv, pll);
@@ -4239,10 +4248,10 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
if (memcmp(&crtc_state->dpll_hw_state,
&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",
+ DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, active 0x%08x)\n",
crtc->base.base.id, pll->name,
shared_dpll[i].crtc_mask,
- pll->active);
+ pll->active_mask);
goto found;
}
}
@@ -12841,12 +12850,12 @@ check_shared_dpll_state(struct drm_device *dev)
active = pll->get_hw_state(dev_priv, pll, &dpll_hw_state);
- I915_STATE_WARN(pll->active > hweight32(pll->config.crtc_mask),
- "more active pll users than references: %i vs %i\n",
- pll->active, hweight32(pll->config.crtc_mask));
- I915_STATE_WARN(pll->active && !pll->on,
+ I915_STATE_WARN(pll->active_mask & ~pll->config.crtc_mask,
+ "more active pll users than references: %x vs %x\n",
+ pll->active_mask, pll->config.crtc_mask);
+ I915_STATE_WARN(pll->active_mask && !pll->on,
"pll in active use but not on in sw tracking\n");
- I915_STATE_WARN(pll->on && !pll->active,
+ I915_STATE_WARN(pll->on && !pll->active_mask,
"pll in on but not on in use in sw tracking\n");
I915_STATE_WARN(pll->on != active,
"pll on state mismatch (expected %i, found %i)\n",
@@ -12854,16 +12863,16 @@ check_shared_dpll_state(struct drm_device *dev)
for_each_intel_crtc(dev, crtc) {
if (crtc->base.state->enable && intel_crtc_to_shared_dpll(crtc) == pll)
- enabled_crtcs++;
+ enabled_crtcs |= 1 << drm_crtc_index(&crtc->base);
if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll)
- active_crtcs++;
+ active_crtcs |= 1 << drm_crtc_index(&crtc->base);
}
- I915_STATE_WARN(pll->active != active_crtcs,
- "pll active crtcs mismatch (expected %i, found %i)\n",
- pll->active, active_crtcs);
- I915_STATE_WARN(hweight32(pll->config.crtc_mask) != enabled_crtcs,
- "pll enabled crtcs mismatch (expected %i, found %i)\n",
- hweight32(pll->config.crtc_mask), enabled_crtcs);
+ I915_STATE_WARN(pll->active_mask != active_crtcs,
+ "pll active crtcs mismatch (expected %x, found %x)\n",
+ pll->active_mask, active_crtcs);
+ I915_STATE_WARN(pll->config.crtc_mask != enabled_crtcs,
+ "pll enabled crtcs mismatch (expected %x, found %x)\n",
+ pll->config.crtc_mask, enabled_crtcs);
I915_STATE_WARN(pll->on && memcmp(&pll->config.hw_state, &dpll_hw_state,
sizeof(dpll_hw_state)),
@@ -15502,14 +15511,13 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
pll->on = pll->get_hw_state(dev_priv, pll,
&pll->config.hw_state);
- pll->active = 0;
+ pll->active_mask = 0;
pll->config.crtc_mask = 0;
for_each_intel_crtc(dev, crtc) {
- if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) {
- pll->active++;
+ if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll)
pll->config.crtc_mask |= 1 << crtc->pipe;
- }
}
+ pll->active_mask = pll->config.crtc_mask;
DRM_DEBUG_KMS("%s hw state readout: crtc_mask 0x%08x, on %i\n",
pll->name, pll->config.crtc_mask, pll->on);
@@ -15615,7 +15623,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
for (i = 0; i < dev_priv->num_shared_dpll; i++) {
struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
- if (!pll->on || pll->active)
+ if (!pll->on || pll->active_mask)
continue;
DRM_DEBUG_KMS("%s enabled but not in use, disabling\n", pll->name);
--
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] 5+ messages in thread
* [PATCH 2/2] drm/i915: Perform dpll commit first.
2015-11-24 9:44 [PATCH 1/2] drm/i915: Use a crtc mask instead of a active refcount for dpll functions Maarten Lankhorst
@ 2015-11-24 9:44 ` Maarten Lankhorst
2015-11-26 14:41 ` Ander Conselvan De Oliveira
2015-11-26 14:36 ` [PATCH 1/2] drm/i915: Use a crtc mask instead of a active refcount for dpll functions Ander Conselvan De Oliveira
1 sibling, 1 reply; 5+ messages in thread
From: Maarten Lankhorst @ 2015-11-24 9:44 UTC (permalink / raw)
To: intel-gfx
Warn for the wrong mask in enable only. Disable will have the wrong mask
now because the new state is committed before disabling the old state.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 79bb22ef67dc..3eb51195da72 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1897,7 +1897,7 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
if (WARN_ON(pll == NULL))
return;
- if (WARN_ON(pll->config.crtc_mask == 0))
+ if (WARN_ON(!(pll->config.crtc_mask & (1 << drm_crtc_index(&crtc->base)))))
return;
if (WARN_ON(pll->active_mask & crtc_mask))
@@ -1937,9 +1937,6 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc)
if (pll == NULL)
return;
- if (WARN_ON(!(pll->config.crtc_mask & crtc_mask)))
- return;
-
if (WARN_ON(!(pll->active_mask & crtc_mask)))
return;
@@ -13382,7 +13379,8 @@ static int intel_atomic_commit(struct drm_device *dev,
}
drm_atomic_helper_swap_state(dev, state);
- dev_priv->wm.config = to_intel_atomic_state(state)->wm_config;
+ dev_priv->wm.config = intel_state->wm_config;
+ intel_shared_dpll_commit(state);
if (intel_state->modeset) {
memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
@@ -13415,8 +13413,6 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_modeset_update_crtc_state(state);
if (intel_state->modeset) {
- intel_shared_dpll_commit(state);
-
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
modeset_update_crtc_power_domains(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] 5+ messages in thread
* Re: [PATCH 1/2] drm/i915: Use a crtc mask instead of a active refcount for dpll functions.
2015-11-24 9:44 [PATCH 1/2] drm/i915: Use a crtc mask instead of a active refcount for dpll functions Maarten Lankhorst
2015-11-24 9:44 ` [PATCH 2/2] drm/i915: Perform dpll commit first Maarten Lankhorst
@ 2015-11-26 14:36 ` Ander Conselvan De Oliveira
1 sibling, 0 replies; 5+ messages in thread
From: Ander Conselvan De Oliveira @ 2015-11-26 14:36 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Tue, 2015-11-24 at 10:44 +0100, Maarten Lankhorst wrote:
> This makes it easier to verify correct dpll setup with only a single crtc.
> It it also useful to detect double dpll enable/disable.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 68 ++++++++++++++++++++---------------
> -
> 3 files changed, 40 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index b86feeeafc45..37d4a7f3d112 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3236,7 +3236,7 @@ static int i915_shared_dplls_info(struct seq_file *m,
> void *unused)
>
> seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->name, pll->id);
> seq_printf(m, " crtc_mask: 0x%08x, active: %d, on: %s\n",
> - pll->config.crtc_mask, pll->active, yesno(pll
> ->on));
> + pll->config.crtc_mask, hweight32(pll
> ->active_mask), yesno(pll->on));
> seq_printf(m, " tracked hardware state:\n");
> seq_printf(m, " dpll: 0x%08x\n", pll
> ->config.hw_state.dpll);
> seq_printf(m, " dpll_md: 0x%08x\n",
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f3ad72abeea7..e0763c3005e8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -393,7 +393,7 @@ struct intel_shared_dpll_config {
> struct intel_shared_dpll {
> struct intel_shared_dpll_config config;
>
> - int active; /* count of number of active CRTCs (i.e. DPMS on) */
> + unsigned active_mask; /* mask of active CRTCs (i.e. DPMS on) */
> bool on; /* is the PLL actually active? Disabled during modeset */
> const char *name;
> /* should match the index in the dev_priv->shared_dplls array */
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 3317db3806a8..79bb22ef67dc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1870,7 +1870,7 @@ static void intel_prepare_shared_dpll(struct intel_crtc
> *crtc)
> return;
>
> WARN_ON(!pll->config.crtc_mask);
> - if (pll->active == 0) {
> + if (pll->active_mask == 0) {
> DRM_DEBUG_DRIVER("setting up %s\n", pll->name);
> WARN_ON(pll->on);
> assert_shared_dpll_disabled(dev_priv, pll);
> @@ -1892,6 +1892,7 @@ static void intel_enable_shared_dpll(struct intel_crtc
> *crtc)
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
> + unsigned crtc_mask = 1 << drm_crtc_index(&crtc->base);
>
> if (WARN_ON(pll == NULL))
> return;
> @@ -1899,11 +1900,16 @@ static void intel_enable_shared_dpll(struct intel_crtc
> *crtc)
> if (WARN_ON(pll->config.crtc_mask == 0))
> return;
>
> + if (WARN_ON(pll->active_mask & crtc_mask))
> + return;
> +
> DRM_DEBUG_KMS("enable %s (active %d, on? %d) for crtc %d\n",
> - pll->name, pll->active, pll->on,
> + pll->name, hweight32(pll->active_mask), pll->on,
Maybe just print active_mask, it is more informative anyway.
> crtc->base.base.id);
>
> - if (pll->active++) {
> + pll->active_mask |= crtc_mask;
> +
> + if (pll->active_mask != crtc_mask) {
> WARN_ON(!pll->on);
> assert_shared_dpll_enabled(dev_priv, pll);
> return;
> @@ -1922,30 +1928,33 @@ static void intel_disable_shared_dpll(struct
> intel_crtc *crtc)
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
> + unsigned crtc_mask = 1 << drm_crtc_index(&crtc->base);
>
> /* PCH only available on ILK+ */
> - if (INTEL_INFO(dev)->gen < 5)
> + if (INTEL_INFO(dev_priv)->gen < 5)
> return;
>
> if (pll == NULL)
> return;
>
> - if (WARN_ON(!(pll->config.crtc_mask & (1 << drm_crtc_index(&crtc
> ->base)))))
> + if (WARN_ON(!(pll->config.crtc_mask & crtc_mask)))
> return;
>
> - DRM_DEBUG_KMS("disable %s (active %d, on? %d) for crtc %d\n",
> - pll->name, pll->active, pll->on,
> + if (WARN_ON(!(pll->active_mask & crtc_mask)))
> + return;
> +
> + DRM_DEBUG_KMS("disable %s (active %x, on? %d) for crtc %d\n",
> + pll->name, pll->active_mask, pll->on,
> crtc->base.base.id);
>
> - if (WARN_ON(pll->active == 0)) {
> + pll->active_mask &= ~crtc_mask;
> + if (pll->active_mask) {
> assert_shared_dpll_disabled(dev_priv, pll);
> return;
> }
That call to assert_shared_dpll_disabled() looks wrong now. Just remove it, move
the assert enable after the DRM_DEBUG_KMS and add
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Ander
>
> assert_shared_dpll_enabled(dev_priv, pll);
> WARN_ON(!pll->on);
> - if (--pll->active)
> - return;
>
> DRM_DEBUG_KMS("disabling %s\n", pll->name);
> pll->disable(dev_priv, pll);
> @@ -4239,10 +4248,10 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct
> intel_crtc *crtc,
> if (memcmp(&crtc_state->dpll_hw_state,
> &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",
> + DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask
> 0x%08x, active 0x%08x)\n",
> crtc->base.base.id, pll->name,
> shared_dpll[i].crtc_mask,
> - pll->active);
> + pll->active_mask);
> goto found;
> }
> }
> @@ -12841,12 +12850,12 @@ check_shared_dpll_state(struct drm_device *dev)
>
> active = pll->get_hw_state(dev_priv, pll, &dpll_hw_state);
>
> - I915_STATE_WARN(pll->active > hweight32(pll
> ->config.crtc_mask),
> - "more active pll users than references: %i vs %i\n",
> - pll->active, hweight32(pll->config.crtc_mask));
> - I915_STATE_WARN(pll->active && !pll->on,
> + I915_STATE_WARN(pll->active_mask & ~pll->config.crtc_mask,
> + "more active pll users than references: %x vs %x\n",
> + pll->active_mask, pll->config.crtc_mask);
> + I915_STATE_WARN(pll->active_mask && !pll->on,
> "pll in active use but not on in sw tracking\n");
> - I915_STATE_WARN(pll->on && !pll->active,
> + I915_STATE_WARN(pll->on && !pll->active_mask,
> "pll in on but not on in use in sw tracking\n");
> I915_STATE_WARN(pll->on != active,
> "pll on state mismatch (expected %i, found %i)\n",
> @@ -12854,16 +12863,16 @@ check_shared_dpll_state(struct drm_device *dev)
>
> for_each_intel_crtc(dev, crtc) {
> if (crtc->base.state->enable &&
> intel_crtc_to_shared_dpll(crtc) == pll)
> - enabled_crtcs++;
> + enabled_crtcs |= 1 << drm_crtc_index(&crtc
> ->base);
> if (crtc->active && intel_crtc_to_shared_dpll(crtc)
> == pll)
> - active_crtcs++;
> + active_crtcs |= 1 << drm_crtc_index(&crtc
> ->base);
> }
> - I915_STATE_WARN(pll->active != active_crtcs,
> - "pll active crtcs mismatch (expected %i, found %i)\n",
> - pll->active, active_crtcs);
> - I915_STATE_WARN(hweight32(pll->config.crtc_mask) !=
> enabled_crtcs,
> - "pll enabled crtcs mismatch (expected %i, found %i)\n",
> - hweight32(pll->config.crtc_mask), enabled_crtcs);
> + I915_STATE_WARN(pll->active_mask != active_crtcs,
> + "pll active crtcs mismatch (expected %x, found %x)\n",
> + pll->active_mask, active_crtcs);
> + I915_STATE_WARN(pll->config.crtc_mask != enabled_crtcs,
> + "pll enabled crtcs mismatch (expected %x, found %x)\n",
> + pll->config.crtc_mask, enabled_crtcs);
>
> I915_STATE_WARN(pll->on && memcmp(&pll->config.hw_state,
> &dpll_hw_state,
> sizeof(dpll_hw_state)),
> @@ -15502,14 +15511,13 @@ static void intel_modeset_readout_hw_state(struct
> drm_device *dev)
>
> pll->on = pll->get_hw_state(dev_priv, pll,
> &pll->config.hw_state);
> - pll->active = 0;
> + pll->active_mask = 0;
> pll->config.crtc_mask = 0;
> for_each_intel_crtc(dev, crtc) {
> - if (crtc->active && intel_crtc_to_shared_dpll(crtc)
> == pll) {
> - pll->active++;
> + if (crtc->active && intel_crtc_to_shared_dpll(crtc)
> == pll)
> pll->config.crtc_mask |= 1 << crtc->pipe;
> - }
> }
> + pll->active_mask = pll->config.crtc_mask;
>
> DRM_DEBUG_KMS("%s hw state readout: crtc_mask 0x%08x, on
> %i\n",
> pll->name, pll->config.crtc_mask, pll->on);
> @@ -15615,7 +15623,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
> for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
>
> - if (!pll->on || pll->active)
> + if (!pll->on || pll->active_mask)
> continue;
>
> DRM_DEBUG_KMS("%s enabled but not in use, disabling\n", pll
> ->name);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: Perform dpll commit first.
2015-11-24 9:44 ` [PATCH 2/2] drm/i915: Perform dpll commit first Maarten Lankhorst
@ 2015-11-26 14:41 ` Ander Conselvan De Oliveira
2015-11-30 9:08 ` Maarten Lankhorst
0 siblings, 1 reply; 5+ messages in thread
From: Ander Conselvan De Oliveira @ 2015-11-26 14:41 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Tue, 2015-11-24 at 10:44 +0100, Maarten Lankhorst wrote:
> Warn for the wrong mask in enable only. Disable will have the wrong mask
> now because the new state is committed before disabling the old state.
Why?
Ander
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 79bb22ef67dc..3eb51195da72 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1897,7 +1897,7 @@ static void intel_enable_shared_dpll(struct intel_crtc
> *crtc)
> if (WARN_ON(pll == NULL))
> return;
>
> - if (WARN_ON(pll->config.crtc_mask == 0))
> + if (WARN_ON(!(pll->config.crtc_mask & (1 << drm_crtc_index(&crtc
> ->base)))))
> return;
>
> if (WARN_ON(pll->active_mask & crtc_mask))
> @@ -1937,9 +1937,6 @@ static void intel_disable_shared_dpll(struct intel_crtc
> *crtc)
> if (pll == NULL)
> return;
>
> - if (WARN_ON(!(pll->config.crtc_mask & crtc_mask)))
> - return;
> -
> if (WARN_ON(!(pll->active_mask & crtc_mask)))
> return;
>
> @@ -13382,7 +13379,8 @@ static int intel_atomic_commit(struct drm_device *dev,
> }
>
> drm_atomic_helper_swap_state(dev, state);
> - dev_priv->wm.config = to_intel_atomic_state(state)->wm_config;
> + dev_priv->wm.config = intel_state->wm_config;
> + intel_shared_dpll_commit(state);
>
> if (intel_state->modeset) {
> memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
> @@ -13415,8 +13413,6 @@ static int intel_atomic_commit(struct drm_device *dev,
> intel_modeset_update_crtc_state(state);
>
> if (intel_state->modeset) {
> - intel_shared_dpll_commit(state);
> -
> drm_atomic_helper_update_legacy_modeset_state(state->dev,
> state);
> modeset_update_crtc_power_domains(state);
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: Perform dpll commit first.
2015-11-26 14:41 ` Ander Conselvan De Oliveira
@ 2015-11-30 9:08 ` Maarten Lankhorst
0 siblings, 0 replies; 5+ messages in thread
From: Maarten Lankhorst @ 2015-11-30 9:08 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, intel-gfx
Op 26-11-15 om 15:41 schreef Ander Conselvan De Oliveira:
> On Tue, 2015-11-24 at 10:44 +0100, Maarten Lankhorst wrote:
>> Warn for the wrong mask in enable only. Disable will have the wrong mask
>> now because the new state is committed before disabling the old state.
> Why?
>
> Ander
>
This is required for async commit. In that case the state has to be committed while
still holding the connection_mutex, but the modeset itself is run asynchronously.
This also means that any modeset has to wait for previous modesets to complete,
otherwise the pll state may be updated incorrectly.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-11-30 9:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-24 9:44 [PATCH 1/2] drm/i915: Use a crtc mask instead of a active refcount for dpll functions Maarten Lankhorst
2015-11-24 9:44 ` [PATCH 2/2] drm/i915: Perform dpll commit first Maarten Lankhorst
2015-11-26 14:41 ` Ander Conselvan De Oliveira
2015-11-30 9:08 ` Maarten Lankhorst
2015-11-26 14:36 ` [PATCH 1/2] drm/i915: Use a crtc mask instead of a active refcount for dpll functions Ander Conselvan De Oliveira
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.