* [PATCH] drm/i915: fixup the plane->pipe fixup code
@ 2012-09-06 20:09 Daniel Vetter
2012-09-07 9:46 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2012-09-06 20:09 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
We need to check whether the _other plane is on our pipe, not whether
our plane is on the other pipe. Otherwise if not both pipes/planes are
active, we won't properly clean up the mess and set up our desired
plane->pipe mapping.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
Tested-by: Vladyslav Shtabovenko <DFEW.Entwickler@googlemail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd9c275..ce9b247 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7994,11 +7994,27 @@ static void intel_enable_pipe_a(struct drm_device *dev)
}
+static bool
+intel_check_plane_mapping(struct intel_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+ u32 reg, val;
+
+ reg = DSPCNTR(!crtc->plane);
+ val = I915_READ(reg);
+
+ if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
+ (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
+ return false;
+
+ return true;
+}
+
static void intel_sanitize_crtc(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- u32 reg, val;
+ u32 reg;
/* Clear any frame start delays used for debugging left by the BIOS */
reg = PIPECONF(crtc->pipe);
@@ -8006,17 +8022,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
/* We need to sanitize the plane -> pipe mapping first because this will
* disable the crtc (and hence change the state) if it is wrong. */
- if (!HAS_PCH_SPLIT(dev)) {
+ if (!HAS_PCH_SPLIT(dev) && !intel_check_plane_mapping(crtc)) {
struct intel_connector *connector;
bool plane;
- reg = DSPCNTR(crtc->plane);
- val = I915_READ(reg);
-
- if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
- (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
- goto ok;
-
DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
crtc->base.base.id);
@@ -8040,7 +8049,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
WARN_ON(crtc->active);
crtc->base.enabled = false;
}
-ok:
if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
crtc->pipe == PIPE_A && !crtc->active) {
--
1.7.11.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: fixup the plane->pipe fixup code
2012-09-06 20:09 [PATCH] drm/i915: fixup the plane->pipe fixup code Daniel Vetter
@ 2012-09-07 9:46 ` Chris Wilson
2012-09-08 16:14 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2012-09-07 9:46 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Thu, 6 Sep 2012 22:09:35 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We need to check whether the _other plane is on our pipe, not whether
> our plane is on the other pipe. Otherwise if not both pipes/planes are
> active, we won't properly clean up the mess and set up our desired
> plane->pipe mapping.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
> Tested-by: Vladyslav Shtabovenko <DFEW.Entwickler@googlemail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++----------
> 1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fd9c275..ce9b247 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7994,11 +7994,27 @@ static void intel_enable_pipe_a(struct drm_device *dev)
>
> }
>
> +static bool
> +intel_check_plane_mapping(struct intel_crtc *crtc)
> +{
> + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> + u32 reg, val;
> +
> + reg = DSPCNTR(!crtc->plane);
> + val = I915_READ(reg);
> +
> + if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
> + (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
> + return false;
If the other plane is off but has our pipe selected, turn off our plane.
Try again.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] drm/i915: fixup the plane->pipe fixup code
2012-09-07 9:46 ` Chris Wilson
@ 2012-09-08 16:14 ` Daniel Vetter
2012-09-08 19:00 ` Chris Wilson
2012-09-11 6:17 ` Daniel Vetter
0 siblings, 2 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-09-08 16:14 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
We need to check whether the _other plane is on our pipe, not whether
our plane is on the other pipe. Otherwise if not both pipes/planes are
active, we won't properly clean up the mess and set up our desired
plane->pipe mapping.
v2: Fixup the logic, I've totally fumbled it. Noticed by Chris Wilson.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49838
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd9c275..46f4598 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7994,11 +7994,27 @@ static void intel_enable_pipe_a(struct drm_device *dev)
}
+static bool
+intel_check_plane_mapping(struct intel_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+ u32 reg, val;
+
+ reg = DSPCNTR(!crtc->plane);
+ val = I915_READ(reg);
+
+ if ((val & DISPLAY_PLANE_ENABLE) &&
+ (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
+ return false;
+
+ return true;
+}
+
static void intel_sanitize_crtc(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- u32 reg, val;
+ u32 reg;
/* Clear any frame start delays used for debugging left by the BIOS */
reg = PIPECONF(crtc->pipe);
@@ -8006,17 +8022,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
/* We need to sanitize the plane -> pipe mapping first because this will
* disable the crtc (and hence change the state) if it is wrong. */
- if (!HAS_PCH_SPLIT(dev)) {
+ if (!HAS_PCH_SPLIT(dev) && !intel_check_plane_mapping(crtc)) {
struct intel_connector *connector;
bool plane;
- reg = DSPCNTR(crtc->plane);
- val = I915_READ(reg);
-
- if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
- (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
- goto ok;
-
DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
crtc->base.base.id);
@@ -8040,7 +8049,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
WARN_ON(crtc->active);
crtc->base.enabled = false;
}
-ok:
if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
crtc->pipe == PIPE_A && !crtc->active) {
--
1.7.11.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: fixup the plane->pipe fixup code
2012-09-08 16:14 ` Daniel Vetter
@ 2012-09-08 19:00 ` Chris Wilson
2012-09-11 6:17 ` Daniel Vetter
1 sibling, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2012-09-08 19:00 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Sat, 8 Sep 2012 18:14:52 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We need to check whether the _other plane is on our pipe, not whether
> our plane is on the other pipe. Otherwise if not both pipes/planes are
> active, we won't properly clean up the mess and set up our desired
> plane->pipe mapping.
>
> v2: Fixup the logic, I've totally fumbled it. Noticed by Chris Wilson.
Frankly I'd be happier if we did both as that way we should be
idiotproof, and more importantly obviously safe.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/i915: fixup the plane->pipe fixup code
2012-09-08 16:14 ` Daniel Vetter
2012-09-08 19:00 ` Chris Wilson
@ 2012-09-11 6:17 ` Daniel Vetter
2012-09-12 10:34 ` Daniel Vetter
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2012-09-11 6:17 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
We need to check whether the _other plane is on our pipe, not whether
our plane is on the other pipe. Otherwise if not both pipes/planes are
active, we won't properly clean up the mess and set up our desired
plane->pipe mapping.
v2: Fixup the logic, I've totally fumbled it. Noticed by Chris Wilson.
v3: I've checked Bspec, and the flexible plane->pipe mapping is a
gen2/3 feature, so test for that instead of PCH_SPLIT
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49838
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd9c275..7454df6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7994,11 +7994,27 @@ static void intel_enable_pipe_a(struct drm_device *dev)
}
+static bool
+intel_check_plane_mapping(struct intel_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+ u32 reg, val;
+
+ reg = DSPCNTR(!crtc->plane);
+ val = I915_READ(reg);
+
+ if ((val & DISPLAY_PLANE_ENABLE) &&
+ (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
+ return false;
+
+ return true;
+}
+
static void intel_sanitize_crtc(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- u32 reg, val;
+ u32 reg;
/* Clear any frame start delays used for debugging left by the BIOS */
reg = PIPECONF(crtc->pipe);
@@ -8006,17 +8022,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
/* We need to sanitize the plane -> pipe mapping first because this will
* disable the crtc (and hence change the state) if it is wrong. */
- if (!HAS_PCH_SPLIT(dev)) {
+ if (INTEL_INFO(dev)->gen < 4 && !intel_check_plane_mapping(crtc)) {
struct intel_connector *connector;
bool plane;
- reg = DSPCNTR(crtc->plane);
- val = I915_READ(reg);
-
- if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
- (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
- goto ok;
-
DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
crtc->base.base.id);
@@ -8040,7 +8049,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
WARN_ON(crtc->active);
crtc->base.enabled = false;
}
-ok:
if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
crtc->pipe == PIPE_A && !crtc->active) {
--
1.7.11.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: fixup the plane->pipe fixup code
2012-09-11 6:17 ` Daniel Vetter
@ 2012-09-12 10:34 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-09-12 10:34 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Tue, Sep 11, 2012 at 08:17:33AM +0200, Daniel Vetter wrote:
> We need to check whether the _other plane is on our pipe, not whether
> our plane is on the other pipe. Otherwise if not both pipes/planes are
> active, we won't properly clean up the mess and set up our desired
> plane->pipe mapping.
>
> v2: Fixup the logic, I've totally fumbled it. Noticed by Chris Wilson.
>
> v3: I've checked Bspec, and the flexible plane->pipe mapping is a
> gen2/3 feature, so test for that instead of PCH_SPLIT
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49838
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
For a reason that seems to totally ellude me this breaks resume on my
i855gm. Reliably, and that despite that the plane fixup code isn't run
(according to the debug printk) both with and without this patch ...
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++----------
> 1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fd9c275..7454df6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7994,11 +7994,27 @@ static void intel_enable_pipe_a(struct drm_device *dev)
>
> }
>
> +static bool
> +intel_check_plane_mapping(struct intel_crtc *crtc)
> +{
> + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> + u32 reg, val;
> +
> + reg = DSPCNTR(!crtc->plane);
> + val = I915_READ(reg);
> +
> + if ((val & DISPLAY_PLANE_ENABLE) &&
> + (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
> + return false;
> +
> + return true;
> +}
> +
> static void intel_sanitize_crtc(struct intel_crtc *crtc)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 reg, val;
> + u32 reg;
>
> /* Clear any frame start delays used for debugging left by the BIOS */
> reg = PIPECONF(crtc->pipe);
> @@ -8006,17 +8022,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>
> /* We need to sanitize the plane -> pipe mapping first because this will
> * disable the crtc (and hence change the state) if it is wrong. */
> - if (!HAS_PCH_SPLIT(dev)) {
> + if (INTEL_INFO(dev)->gen < 4 && !intel_check_plane_mapping(crtc)) {
> struct intel_connector *connector;
> bool plane;
>
> - reg = DSPCNTR(crtc->plane);
> - val = I915_READ(reg);
> -
> - if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
> - (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
> - goto ok;
> -
> DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
> crtc->base.base.id);
>
> @@ -8040,7 +8049,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
> WARN_ON(crtc->active);
> crtc->base.enabled = false;
> }
> -ok:
>
> if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
> crtc->pipe == PIPE_A && !crtc->active) {
> --
> 1.7.11.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-12 10:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-06 20:09 [PATCH] drm/i915: fixup the plane->pipe fixup code Daniel Vetter
2012-09-07 9:46 ` Chris Wilson
2012-09-08 16:14 ` Daniel Vetter
2012-09-08 19:00 ` Chris Wilson
2012-09-11 6:17 ` Daniel Vetter
2012-09-12 10:34 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox