[PATCH] drm/i915: Fix deadlock in i830_disable_pipe()

[PATCH] drm/i915: Fix deadlock in i830_disable_pipe()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

i830_disable_pipe() gets called from the power well code, and thus
we're already holding the power domain mutex. That means we can't
call plane->get_hw_state() as it will also try to grab the
same mutex and will thus deadlock.

Replace the assert_plane() calls (which calls ->get_hw_state()) with
just raw register reads in i830_disable_pipe(). As a bonus we can
now get a warning if plane C is enabled even though we don't even
expose it as a drm plane.

Fixes: 51f5a0963984 ("drm/i915: Add .get_hw_state() method for planes")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d67c7c498b34..48d9332b196f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14731,8 +14731,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 	DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n",
 		      pipe_name(pipe));
 
-	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_A));
-	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_B));
+	WARN_ON(I915_READ(DSPCNTR(PLANE_A)) & DISPLAY_PLANE_ENABLE ||
+		I915_READ(DSPCNTR(PLANE_B)) & DISPLAY_PLANE_ENABLE ||
+		I915_READ(DSPCNTR(PLANE_C)) & DISPLAY_PLANE_ENABLE ||
+		I915_READ(CURCNTR(PIPE_A)) & CURSOR_MODE ||
+		I915_READ(CURCNTR(PIPE_B)) & CURSOR_MODE);
 
 	I915_WRITE(PIPECONF(pipe), 0);
 	POSTING_READ(PIPECONF(pipe));
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Fix deadlock in i830_disable_pipe()

[Patchwork]

== Series Details ==

Series: drm/i915: Fix deadlock in i830_disable_pipe()
URL   : https://patchwork.freedesktop.org/series/34553/
State : success

== Summary ==

Series 34553v1 drm/i915: Fix deadlock in i830_disable_pipe()
https://patchwork.freedesktop.org/api/1.0/series/34553/revisions/1/mbox/

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:440s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:441s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:387s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:514s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:278s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:498s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:481s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:464s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:429s
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:265s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:530s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:423s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:431s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:451s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:485s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:525s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:472s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:525s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:584s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:537s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:561s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:514s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:504s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:449s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:550s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:414s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:602s
fi-cnl-y         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:545s
fi-glk-dsi       total:288  pass:164  dwarn:0   dfail:10  fail:2   skip:112 time:410s

73f135dccc60c8fa6e2a200286b00877209a9c29 drm-tip: 2017y-11m-28d-14h-52m-10s UTC integration manifest
0fc6657b6208 drm/i915: Fix deadlock in i830_disable_pipe()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7327/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix deadlock in i830_disable_pipe()

[Chris Wilson]

Quoting Ville Syrjala (2017-11-28 15:48:53)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> i830_disable_pipe() gets called from the power well code, and thus
> we're already holding the power domain mutex. That means we can't
> call plane->get_hw_state() as it will also try to grab the
> same mutex and will thus deadlock.
> 
> Replace the assert_plane() calls (which calls ->get_hw_state()) with
> just raw register reads in i830_disable_pipe(). As a bonus we can
> now get a warning if plane C is enabled even though we don't even
> expose it as a drm plane.
> 
> Fixes: 51f5a0963984 ("drm/i915: Add .get_hw_state() method for planes")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d67c7c498b34..48d9332b196f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14731,8 +14731,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
>         DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n",
>                       pipe_name(pipe));
>  
> -       assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_A));
> -       assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_B));
> +       WARN_ON(I915_READ(DSPCNTR(PLANE_A)) & DISPLAY_PLANE_ENABLE ||
> +               I915_READ(DSPCNTR(PLANE_B)) & DISPLAY_PLANE_ENABLE ||
> +               I915_READ(DSPCNTR(PLANE_C)) & DISPLAY_PLANE_ENABLE ||
> +               I915_READ(CURCNTR(PIPE_A)) & CURSOR_MODE ||
> +               I915_READ(CURCNTR(PIPE_B)) & CURSOR_MODE);

Ok. Avoiding mutex recursion sounds sensible, but a recursion sounds
like a layering violation. i830_disable_pipe is only used by the
powerwell, and I guess you could make the i830_enable_pipe in
i9xx_crtc_disable into a call to the powerdomain instead.

That sounds like more work than desired to fix the immediate problem!

However, I think you will miss the loss in precision by putting all the
checks into one WARN_ON. If it either fires, we've no idea what went
wrong?

Would it be worth just making each of those a separate WARN_ON()? I
think so.

Either way, I've checked that your explanation matches the code...
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Fix deadlock in i830_disable_pipe()

[Patchwork]

== Series Details ==

Series: drm/i915: Fix deadlock in i830_disable_pipe()
URL   : https://patchwork.freedesktop.org/series/34553/
State : success

== Summary ==

Blacklisted hosts:
shard-hsw        total:2654 pass:1520 dwarn:8   dfail:0   fail:10  skip:1115 time:8970s
shard-snb        total:2554 pass:1238 dwarn:17  dfail:5   fail:7   skip:1282 time:6951s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7327/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix deadlock in i830_disable_pipe()

[Ville Syrjälä]

On Tue, Nov 28, 2017 at 09:05:00PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-28 15:48:53)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > i830_disable_pipe() gets called from the power well code, and thus
> > we're already holding the power domain mutex. That means we can't
> > call plane->get_hw_state() as it will also try to grab the
> > same mutex and will thus deadlock.
> > 
> > Replace the assert_plane() calls (which calls ->get_hw_state()) with
> > just raw register reads in i830_disable_pipe(). As a bonus we can
> > now get a warning if plane C is enabled even though we don't even
> > expose it as a drm plane.
> > 
> > Fixes: 51f5a0963984 ("drm/i915: Add .get_hw_state() method for planes")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d67c7c498b34..48d9332b196f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14731,8 +14731,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> >         DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n",
> >                       pipe_name(pipe));
> >  
> > -       assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_A));
> > -       assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_B));
> > +       WARN_ON(I915_READ(DSPCNTR(PLANE_A)) & DISPLAY_PLANE_ENABLE ||
> > +               I915_READ(DSPCNTR(PLANE_B)) & DISPLAY_PLANE_ENABLE ||
> > +               I915_READ(DSPCNTR(PLANE_C)) & DISPLAY_PLANE_ENABLE ||
> > +               I915_READ(CURCNTR(PIPE_A)) & CURSOR_MODE ||
> > +               I915_READ(CURCNTR(PIPE_B)) & CURSOR_MODE);
> 
> Ok. Avoiding mutex recursion sounds sensible, but a recursion sounds
> like a layering violation. i830_disable_pipe is only used by the
> powerwell, and I guess you could make the i830_enable_pipe in
> i9xx_crtc_disable into a call to the powerdomain instead.

That wouldn't actully work. We already hold the power domain
reference so the power well enable hook wouldn't even be called,
and even if it was it would just be a nop as the pipe is already
up and running.

I guess it would be possible to redesign that stuff somehow to do what
we want, but that probably means we'd have to redesign the power domain
initialization as well to never call the enable hook if the power well
is already active on boot.

So this would involve a bunch of work, and I can't actually see any
benefit from taking a detour through the power well code.

> 
> That sounds like more work than desired to fix the immediate problem!
> 
> However, I think you will miss the loss in precision by putting all the
> checks into one WARN_ON. If it either fires, we've no idea what went
> wrong?
> 
> Would it be worth just making each of those a separate WARN_ON()? I
> think so.

Yeah, I guess having that extra bit of information might be somewhat
helpful. I'll split it up.

> 
> Either way, I've checked that your explanation matches the code...
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks.

> -Chris

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Fix deadlock in i830_disable_pipe()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

i830_disable_pipe() gets called from the power well code, and thus
we're already holding the power domain mutex. That means we can't
call plane->get_hw_state() as it will also try to grab the
same mutex and will thus deadlock.

Replace the assert_plane() calls (which calls ->get_hw_state()) with
just raw register reads in i830_disable_pipe(). As a bonus we can
now get a warning if plane C is enabled even though we don't even
expose it as a drm plane.

v2: Do a separate WARN_ON() for each plane (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Fixes: 51f5a0963984 ("drm/i915: Add .get_hw_state() method for planes")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d67c7c498b34..674b86bbe7d7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14731,8 +14731,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 	DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n",
 		      pipe_name(pipe));
 
-	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_A));
-	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_B));
+	WARN_ON(I915_READ(DSPCNTR(PLANE_A)) & DISPLAY_PLANE_ENABLE);
+	WARN_ON(I915_READ(DSPCNTR(PLANE_B)) & DISPLAY_PLANE_ENABLE);
+	WARN_ON(I915_READ(DSPCNTR(PLANE_C)) & DISPLAY_PLANE_ENABLE);
+	WARN_ON(I915_READ(CURCNTR(PIPE_A)) & CURSOR_MODE);
+	WARN_ON(I915_READ(CURCNTR(PIPE_B)) & CURSOR_MODE);
 
 	I915_WRITE(PIPECONF(pipe), 0);
 	POSTING_READ(PIPECONF(pipe));
-- 
2.13.6

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Fix deadlock in i830_disable_pipe() (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Fix deadlock in i830_disable_pipe() (rev2)
URL   : https://patchwork.freedesktop.org/series/34553/
State : success

== Summary ==

Series 34553v2 drm/i915: Fix deadlock in i830_disable_pipe()
https://patchwork.freedesktop.org/api/1.0/series/34553/revisions/2/mbox/

Test gem_exec_reloc:
        Subgroup basic-cpu-active:
                fail       -> PASS       (fi-gdg-551) fdo#102582 +1
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575

fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:442s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:383s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:518s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:283s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:505s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:507s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:484s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:477s
fi-elk-e7500     total:224  pass:162  dwarn:16  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:1   dfail:0   fail:0   skip:108 time:268s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:545s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:377s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:261s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:393s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:476s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:444s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:480s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:529s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:480s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:535s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:585s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:539s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:566s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:521s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:442s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:548s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:421s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:613s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:495s

6d6c48b9b35806aba461d2c8285db2689de9095f drm-tip: 2017y-11m-30d-12h-22m-59s UTC integration manifest
97a8b12e8774 drm/i915: Fix deadlock in i830_disable_pipe()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7369/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915: Fix deadlock in i830_disable_pipe() (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Fix deadlock in i830_disable_pipe() (rev2)
URL   : https://patchwork.freedesktop.org/series/34553/
State : warning

== Summary ==

Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test kms_flip:
        Subgroup flip-vs-expired-vblank:
                pass       -> SKIP       (shard-hsw) fdo#102887
Test kms_chv_cursor_fail:
        Subgroup pipe-a-64x64-top-edge:
                pass       -> SKIP       (shard-hsw)
Test gem_tiled_swapping:
        Subgroup non-threaded:
                incomplete -> PASS       (shard-snb)
Test kms_vblank:
        Subgroup query-busy:
                pass       -> SKIP       (shard-hsw)
Test kms_atomic_transition:
        Subgroup plane-all-transition-nonblocking:
                pass       -> SKIP       (shard-hsw)
Test drv_selftest:
        Subgroup mock_sanitycheck:
                pass       -> DMESG-WARN (shard-snb) fdo#102707 +1

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707

shard-hsw        total:2650 pass:1521 dwarn:2   dfail:0   fail:9   skip:1117 time:9144s
shard-snb        total:2663 pass:1305 dwarn:2   dfail:0   fail:14  skip:1342 time:8125s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7369/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fix deadlock in i830_disable_pipe()

[Ville Syrjälä]

On Wed, Nov 29, 2017 at 02:54:11PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> i830_disable_pipe() gets called from the power well code, and thus
> we're already holding the power domain mutex. That means we can't
> call plane->get_hw_state() as it will also try to grab the
> same mutex and will thus deadlock.
> 
> Replace the assert_plane() calls (which calls ->get_hw_state()) with
> just raw register reads in i830_disable_pipe(). As a bonus we can
> now get a warning if plane C is enabled even though we don't even
> expose it as a drm plane.
> 
> v2: Do a separate WARN_ON() for each plane (Chris)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Fixes: 51f5a0963984 ("drm/i915: Add .get_hw_state() method for planes")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pushed to dinq. Thanks for the review.

> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d67c7c498b34..674b86bbe7d7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14731,8 +14731,11 @@ void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	DRM_DEBUG_KMS("disabling pipe %c due to force quirk\n",
>  		      pipe_name(pipe));
>  
> -	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_A));
> -	assert_planes_disabled(intel_get_crtc_for_pipe(dev_priv, PIPE_B));
> +	WARN_ON(I915_READ(DSPCNTR(PLANE_A)) & DISPLAY_PLANE_ENABLE);
> +	WARN_ON(I915_READ(DSPCNTR(PLANE_B)) & DISPLAY_PLANE_ENABLE);
> +	WARN_ON(I915_READ(DSPCNTR(PLANE_C)) & DISPLAY_PLANE_ENABLE);
> +	WARN_ON(I915_READ(CURCNTR(PIPE_A)) & CURSOR_MODE);
> +	WARN_ON(I915_READ(CURCNTR(PIPE_B)) & CURSOR_MODE);
>  
>  	I915_WRITE(PIPECONF(pipe), 0);
>  	POSTING_READ(PIPECONF(pipe));
> -- 
> 2.13.6

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx