All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Sanitize shared dpll state
@ 2013-07-17  4:55 Daniel Vetter
  2013-07-17  8:19 ` Chris Wilson
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2013-07-17  4:55 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

There seems to be no limit to the amount of gunk the firmware can
leave behind. Some platforms leave pch dplls on which are not in
active use at all. The example in the bug report is a Apple Macbook
Pro.

Note that this escape scrunity of the hw state checker until we've
tried to use this enabled, but unused pll since we did only check for
the inverse case of a in-used, but disabled pll.

v2: Add a WARN in the pll state checker which would have caught this
case.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66952
Reported-and-tested-by: shui yangwei <yangweix.shui@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3cda57d..38ea99b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8437,6 +8437,8 @@ check_shared_dpll_state(struct drm_device *dev)
 		     pll->active, pll->refcount);
 		WARN(pll->active && !pll->on,
 		     "pll in active use but not on in sw tracking\n");
+		WARN(pll->on && !pll->active,
+		     "pll in on but not on in use in sw tracking\n");
 		WARN(pll->on != active,
 		     "pll on state mismatch (expected %i, found %i)\n",
 		     pll->on, active);
@@ -9958,8 +9960,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		}
 		pll->refcount = pll->active;
 
-		DRM_DEBUG_KMS("%s hw state readout: refcount %i\n",
-			      pll->name, pll->refcount);
+		DRM_DEBUG_KMS("%s hw state readout: refcount %i, on %i\n",
+			      pll->name, pll->refcount, pll->on);
 	}
 
 	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
@@ -10019,6 +10021,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 	struct drm_plane *plane;
 	struct intel_crtc *crtc;
 	struct intel_encoder *encoder;
+	int i;
 
 	intel_modeset_readout_hw_state(dev);
 
@@ -10050,6 +10053,18 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 		intel_dump_pipe_config(crtc, &crtc->config, "[setup_hw_state]");
 	}
 
+	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)
+			continue;
+
+		DRM_DEBUG_KMS("%s enabled but not in use, disabling\n", pll->name);
+
+		pll->disable(dev_priv, pll);
+		pll->on = false;
+	}
+
 	if (force_restore) {
 		/*
 		 * We need to use raw interfaces for restoring state to avoid
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915: Sanitize shared dpll state
  2013-07-17  4:55 [PATCH] drm/i915: Sanitize shared dpll state Daniel Vetter
@ 2013-07-17  8:19 ` Chris Wilson
  2013-07-17  9:51   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2013-07-17  8:19 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development

On Wed, Jul 17, 2013 at 06:55:04AM +0200, Daniel Vetter wrote:
> There seems to be no limit to the amount of gunk the firmware can
> leave behind. Some platforms leave pch dplls on which are not in
> active use at all. The example in the bug report is a Apple Macbook
> Pro.
> 
> Note that this escape scrunity of the hw state checker until we've
> tried to use this enabled, but unused pll since we did only check for
> the inverse case of a in-used, but disabled pll.
> 
> v2: Add a WARN in the pll state checker which would have caught this
> case.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66952
> Reported-and-tested-by: shui yangwei <yangweix.shui@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Plonk it in intel_sanitize_plls(), and preferably move all the sanitze
encoder/crtc/pll into intel_sanitize_display() (in a later patch), so that
intel_modeset_setup_hw_state() is not quite so broken up.

Other than that minor request,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915: Sanitize shared dpll state
  2013-07-17  8:19 ` Chris Wilson
@ 2013-07-17  9:51   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2013-07-17  9:51 UTC (permalink / raw)
  To: Chris Wilson, Daniel Vetter, Intel Graphics Development

On Wed, Jul 17, 2013 at 09:19:03AM +0100, Chris Wilson wrote:
> On Wed, Jul 17, 2013 at 06:55:04AM +0200, Daniel Vetter wrote:
> > There seems to be no limit to the amount of gunk the firmware can
> > leave behind. Some platforms leave pch dplls on which are not in
> > active use at all. The example in the bug report is a Apple Macbook
> > Pro.
> > 
> > Note that this escape scrunity of the hw state checker until we've
> > tried to use this enabled, but unused pll since we did only check for
> > the inverse case of a in-used, but disabled pll.
> > 
> > v2: Add a WARN in the pll state checker which would have caught this
> > case.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66952
> > Reported-and-tested-by: shui yangwei <yangweix.shui@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Plonk it in intel_sanitize_plls(), and preferably move all the sanitze
> encoder/crtc/pll into intel_sanitize_display() (in a later patch), so that
> intel_modeset_setup_hw_state() is not quite so broken up.

Agreed, but soemthing for -next.

> Other than that minor request,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Picked up for -fixes, thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-07-17  9:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-17  4:55 [PATCH] drm/i915: Sanitize shared dpll state Daniel Vetter
2013-07-17  8:19 ` Chris Wilson
2013-07-17  9: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.