* [PATCH v2 1/2] drm/i915: do not update cursor in crtc mode set
2013-09-17 15:33 [PATCH 0/2] drm/i915: HSW modeset hang fix v2 ville.syrjala
@ 2013-09-17 15:33 ` ville.syrjala
2013-09-17 15:33 ` [PATCH v2 2/2] drm/i915: Don't enable the cursor on a disable pipe ville.syrjala
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: ville.syrjala @ 2013-09-17 15:33 UTC (permalink / raw)
To: intel-gfx; +Cc: stable
From: Jani Nikula <jani.nikula@intel.com>
The cursor is disabled before crtc mode set in crtc disable (and we
assert this is the case), and enabled afterwards in crtc enable. Do not
update it in crtc mode set.
On HSW enabling a plane on a disabled pipe may hang the entire system.
And there's no good reason for doing it ever, so just don't.
v2: Add note about HSW hangs - vsyrjala
Cc: stable@vger.kernel.org
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bb3bcc7..b4f8f71 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4932,9 +4932,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
}
}
- /* Ensure that the cursor is valid for the new mode before changing... */
- intel_crtc_update_cursor(crtc, true);
-
if (!is_dsi && is_lvds && dev_priv->lvds_downclock_avail) {
/*
* Ensure we match the reduced clock's P to the target clock.
@@ -5845,9 +5842,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
intel_crtc->config.dpll.p2 = clock.p2;
}
- /* Ensure that the cursor is valid for the new mode before changing... */
- intel_crtc_update_cursor(crtc, true);
-
/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
if (intel_crtc->config.has_pch_encoder) {
fp = i9xx_dpll_compute_fp(&intel_crtc->config.dpll);
@@ -6398,9 +6392,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
if (!intel_ddi_pll_mode_set(crtc))
return -EINVAL;
- /* Ensure that the cursor is valid for the new mode before changing... */
- intel_crtc_update_cursor(crtc, true);
-
if (intel_crtc->config.has_dp_encoder)
intel_dp_set_m_n(intel_crtc);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/2] drm/i915: Don't enable the cursor on a disable pipe
2013-09-17 15:33 [PATCH 0/2] drm/i915: HSW modeset hang fix v2 ville.syrjala
2013-09-17 15:33 ` [PATCH v2 1/2] drm/i915: do not update cursor in crtc mode set ville.syrjala
@ 2013-09-17 15:33 ` ville.syrjala
2013-09-17 16:24 ` [Intel-gfx] " Chris Wilson
2013-09-17 16:07 ` [PATCH 0/2] drm/i915: HSW modeset hang fix v2 Jani Nikula
2013-09-17 19:17 ` [Intel-gfx] " Paulo Zanoni
3 siblings, 1 reply; 9+ messages in thread
From: ville.syrjala @ 2013-09-17 15:33 UTC (permalink / raw)
To: intel-gfx; +Cc: stable
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
On HSW enabling a plane on a disabled pipe may hang the entire system.
And there's no good reason for doing it ever, so just don't.
v2: Move the crtc active checks to intel_crtc_cursor_{set,move} to
avoid confusing people during modeset
Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b4f8f71..8f485e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7066,7 +7066,8 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
intel_crtc->cursor_width = width;
intel_crtc->cursor_height = height;
- intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
+ if (intel_crtc->active)
+ intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
return 0;
fail_unpin:
@@ -7085,7 +7086,8 @@ static int intel_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
intel_crtc->cursor_x = x;
intel_crtc->cursor_y = y;
- intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
+ if (intel_crtc->active)
+ intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
return 0;
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Don't enable the cursor on a disable pipe
2013-09-17 15:33 ` [PATCH v2 2/2] drm/i915: Don't enable the cursor on a disable pipe ville.syrjala
@ 2013-09-17 16:24 ` Chris Wilson
2013-09-17 16:53 ` Ville Syrjälä
0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2013-09-17 16:24 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, stable
On Tue, Sep 17, 2013 at 06:33:44PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On HSW enabling a plane on a disabled pipe may hang the entire system.
> And there's no good reason for doing it ever, so just don't.
>
> v2: Move the crtc active checks to intel_crtc_cursor_{set,move} to
> avoid confusing people during modeset
But outside of modeset the existing checks are accurate.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Don't enable the cursor on a disable pipe
2013-09-17 16:24 ` [Intel-gfx] " Chris Wilson
@ 2013-09-17 16:53 ` Ville Syrjälä
2013-09-17 20:15 ` Paulo Zanoni
0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2013-09-17 16:53 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, stable
On Tue, Sep 17, 2013 at 05:24:01PM +0100, Chris Wilson wrote:
> On Tue, Sep 17, 2013 at 06:33:44PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > On HSW enabling a plane on a disabled pipe may hang the entire system.
> > And there's no good reason for doing it ever, so just don't.
> >
> > v2: Move the crtc active checks to intel_crtc_cursor_{set,move} to
> > avoid confusing people during modeset
>
> But outside of modeset the existing checks are accurate.
There are no existing checks anymore. The crtc->enabled check ended up
as collateral damage in the cursor visibility patches.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Don't enable the cursor on a disable pipe
2013-09-17 16:53 ` Ville Syrjälä
@ 2013-09-17 20:15 ` Paulo Zanoni
0 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-09-17 20:15 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Chris Wilson, Intel Graphics Development, stable
2013/9/17 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Tue, Sep 17, 2013 at 05:24:01PM +0100, Chris Wilson wrote:
>> On Tue, Sep 17, 2013 at 06:33:44PM +0300, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > On HSW enabling a plane on a disabled pipe may hang the entire system.
>> > And there's no good reason for doing it ever, so just don't.
>> >
>> > v2: Move the crtc active checks to intel_crtc_cursor_{set,move} to
>> > avoid confusing people during modeset
>>
>> But outside of modeset the existing checks are accurate.
>
> There are no existing checks anymore. The crtc->enabled check ended up
> as collateral damage in the cursor visibility patches.
I'm not really an expert in the cursor code, but as far as I could
understand, this patch looks correct.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
(I propose a "Weak-reviewed-by:" tag for cases like this!)
Also, isn't this one of those cases where we can try to write an IGT
test? (unset all modes, call the cursor IOCTLs)
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] drm/i915: HSW modeset hang fix v2
2013-09-17 15:33 [PATCH 0/2] drm/i915: HSW modeset hang fix v2 ville.syrjala
2013-09-17 15:33 ` [PATCH v2 1/2] drm/i915: do not update cursor in crtc mode set ville.syrjala
2013-09-17 15:33 ` [PATCH v2 2/2] drm/i915: Don't enable the cursor on a disable pipe ville.syrjala
@ 2013-09-17 16:07 ` Jani Nikula
2013-09-18 8:01 ` Daniel Vetter
2013-09-17 19:17 ` [Intel-gfx] " Paulo Zanoni
3 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2013-09-17 16:07 UTC (permalink / raw)
To: ville.syrjala, intel-gfx
On Tue, 17 Sep 2013, ville.syrjala@linux.intel.com wrote:
> v2 as requested.
>
> The first patch just got rebased, the second one I adjusted a bit.
[dropping cc: stable]
So these two are for -fixes, right?
Unrelated to any hangs, there's still [1] that's destined to dinq, but
depends on patch 1/2 from this series. This is because Ville lifted
patch 1/2 from my original series of three, when I didn't know it would
fix a hang... Daniel, have fun. ;)
Cheers,
Jani.
[1] http://mid.gmane.org/1379059389-2890-2-git-send-email-jani.nikula@intel.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] drm/i915: HSW modeset hang fix v2
2013-09-17 16:07 ` [PATCH 0/2] drm/i915: HSW modeset hang fix v2 Jani Nikula
@ 2013-09-18 8:01 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2013-09-18 8:01 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Tue, Sep 17, 2013 at 07:07:45PM +0300, Jani Nikula wrote:
> On Tue, 17 Sep 2013, ville.syrjala@linux.intel.com wrote:
> > v2 as requested.
> >
> > The first patch just got rebased, the second one I adjusted a bit.
>
> [dropping cc: stable]
>
> So these two are for -fixes, right?
>
> Unrelated to any hangs, there's still [1] that's destined to dinq, but
> depends on patch 1/2 from this series. This is because Ville lifted
> patch 1/2 from my original series of three, when I didn't know it would
> fix a hang... Daniel, have fun. ;)
I'll do a backmerge when -rc2 is out or so. Both patches merged to -fixes.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH 0/2] drm/i915: HSW modeset hang fix v2
2013-09-17 15:33 [PATCH 0/2] drm/i915: HSW modeset hang fix v2 ville.syrjala
` (2 preceding siblings ...)
2013-09-17 16:07 ` [PATCH 0/2] drm/i915: HSW modeset hang fix v2 Jani Nikula
@ 2013-09-17 19:17 ` Paulo Zanoni
3 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-09-17 19:17 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development, stable
2013/9/17 <ville.syrjala@linux.intel.com>:
> v2 as requested.
>
> The first patch just got rebased, the second one I adjusted a bit.
I've been testing this for the last 5 minutes and it seems the problem
is fixed. Now instead of a hard hang I get a FIFO underrun :)
Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
I'll keep using these patches (because this machine is almost unusable
without them) and I'll report if I see the hang again. I'll also start
investigating the underrun.
Thanks,
Paulo
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 9+ messages in thread