public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: flush cursors harder
@ 2013-11-04  7:13 Daniel Vetter
  2013-11-04 16:02 ` [Intel-gfx] " Ville Syrjälä
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2013-11-04  7:13 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, stable, Thomas Richter

Apparently they need the same treatment as primary planes. This fixes
modesetting failures because of stuck cursors (!) on Thomas' i830M
machine.

I've figured while at it I'll also roll it out for the ivb 3 pipe
version of this function. I didn't do this for i845/i865 since Bspec
says the update mechanism works differently, and there's some
additional rules about what can be updated in which order.

Tested-by: Thomas Richter <thor@math.tu-berlin.de>
Cc: stable@vger.kernel.org
Cc:  Thomas Richter <thor@math.tu-berlin.de>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f34252d134b6..04d2699f51b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7123,7 +7123,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
 		intel_crtc->cursor_visible = visible;
 	}
 	/* and commit changes on next vblank */
+	POSTING_READ(CURCNTR(pipe));
 	I915_WRITE(CURBASE(pipe), base);
+	POSTING_READ(CURBASE(pipe));
 }
 
 static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
@@ -7152,7 +7154,9 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
 		intel_crtc->cursor_visible = visible;
 	}
 	/* and commit changes on next vblank */
+	POSTING_READ(CURCNTR_IVB(pipe));
 	I915_WRITE(CURBASE_IVB(pipe), base);
+	POSTING_READ(CURBASE_IVB(pipe));
 }
 
 /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
-- 
1.8.4.rc3

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

* Re: [Intel-gfx] [PATCH] drm/i915: flush cursors harder
  2013-11-04  7:13 [PATCH] drm/i915: flush cursors harder Daniel Vetter
@ 2013-11-04 16:02 ` Ville Syrjälä
  2013-11-04 16:34   ` Daniel Vetter
  0 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2013-11-04 16:02 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development, stable

On Mon, Nov 04, 2013 at 08:13:45AM +0100, Daniel Vetter wrote:
> Apparently they need the same treatment as primary planes. This fixes
> modesetting failures because of stuck cursors (!) on Thomas' i830M
> machine.

What treatment? Primary planes don't need any extra posting reads AFAIK.

> 
> I've figured while at it I'll also roll it out for the ivb 3 pipe
> version of this function. I didn't do this for i845/i865 since Bspec
> says the update mechanism works differently, and there's some
> additional rules about what can be updated in which order.
> 
> Tested-by: Thomas Richter <thor@math.tu-berlin.de>

I didn't see an explicit note from Thomas saying that he tested it.

> Cc: stable@vger.kernel.org
> Cc:  Thomas Richter <thor@math.tu-berlin.de>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f34252d134b6..04d2699f51b4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7123,7 +7123,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
>  		intel_crtc->cursor_visible = visible;
>  	}
>  	/* and commit changes on next vblank */
> +	POSTING_READ(CURCNTR(pipe));
>  	I915_WRITE(CURBASE(pipe), base);
> +	POSTING_READ(CURBASE(pipe));
>  }
>  
>  static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> @@ -7152,7 +7154,9 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
>  		intel_crtc->cursor_visible = visible;
>  	}
>  	/* and commit changes on next vblank */
> +	POSTING_READ(CURCNTR_IVB(pipe));
>  	I915_WRITE(CURBASE_IVB(pipe), base);
> +	POSTING_READ(CURBASE_IVB(pipe));
>  }
>  
>  /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
> -- 
> 1.8.4.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

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

* Re: [PATCH] drm/i915: flush cursors harder
  2013-11-04 16:02 ` [Intel-gfx] " Ville Syrjälä
@ 2013-11-04 16:34   ` Daniel Vetter
  2013-11-04 16:58     ` [Intel-gfx] " Ville Syrjälä
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2013-11-04 16:34 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Daniel Vetter, Intel Graphics Development, stable

On Mon, Nov 04, 2013 at 06:02:24PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 04, 2013 at 08:13:45AM +0100, Daniel Vetter wrote:
> > Apparently they need the same treatment as primary planes. This fixes
> > modesetting failures because of stuck cursors (!) on Thomas' i830M
> > machine.
> 
> What treatment? Primary planes don't need any extra posting reads AFAIK.

If you look at flush_primary_plane it's definitely there. So I've copied
it over (it's just a real I915_READ, not a posting one).

> 
> > 
> > I've figured while at it I'll also roll it out for the ivb 3 pipe
> > version of this function. I didn't do this for i845/i865 since Bspec
> > says the update mechanism works differently, and there's some
> > additional rules about what can be updated in which order.
> > 
> > Tested-by: Thomas Richter <thor@math.tu-berlin.de>
> 
> I didn't see an explicit note from Thomas saying that he tested it.

It's burried somewhere in the thread, but he said that with the 2 earlier
dvo patches + this one here the lvds-only use-case now works well. Before
that he had issues with the display just showing a cursor and the kernel
complaining about the cursor being stuck in the enabled position when
trying to re-enable it.
-Daniel

> 
> > Cc: stable@vger.kernel.org
> > Cc:  Thomas Richter <thor@math.tu-berlin.de>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f34252d134b6..04d2699f51b4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7123,7 +7123,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> >  		intel_crtc->cursor_visible = visible;
> >  	}
> >  	/* and commit changes on next vblank */
> > +	POSTING_READ(CURCNTR(pipe));
> >  	I915_WRITE(CURBASE(pipe), base);
> > +	POSTING_READ(CURBASE(pipe));
> >  }
> >  
> >  static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> > @@ -7152,7 +7154,9 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> >  		intel_crtc->cursor_visible = visible;
> >  	}
> >  	/* and commit changes on next vblank */
> > +	POSTING_READ(CURCNTR_IVB(pipe));
> >  	I915_WRITE(CURBASE_IVB(pipe), base);
> > +	POSTING_READ(CURBASE_IVB(pipe));
> >  }
> >  
> >  /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
> > -- 
> > 1.8.4.rc3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [Intel-gfx] [PATCH] drm/i915: flush cursors harder
  2013-11-04 16:34   ` Daniel Vetter
@ 2013-11-04 16:58     ` Ville Syrjälä
  2013-11-15 19:22       ` Daniel Vetter
  0 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2013-11-04 16:58 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, stable

On Mon, Nov 04, 2013 at 05:34:41PM +0100, Daniel Vetter wrote:
> On Mon, Nov 04, 2013 at 06:02:24PM +0200, Ville Syrjälä wrote:
> > On Mon, Nov 04, 2013 at 08:13:45AM +0100, Daniel Vetter wrote:
> > > Apparently they need the same treatment as primary planes. This fixes
> > > modesetting failures because of stuck cursors (!) on Thomas' i830M
> > > machine.
> > 
> > What treatment? Primary planes don't need any extra posting reads AFAIK.
> 
> If you look at flush_primary_plane it's definitely there. So I've copied
> it over (it's just a real I915_READ, not a posting one).

The regular read is there just so that we don't modify the surface
base address. We should be able to replace the read w/ recomputing
the address.

> 
> > 
> > > 
> > > I've figured while at it I'll also roll it out for the ivb 3 pipe
> > > version of this function. I didn't do this for i845/i865 since Bspec
> > > says the update mechanism works differently, and there's some
> > > additional rules about what can be updated in which order.
> > > 
> > > Tested-by: Thomas Richter <thor@math.tu-berlin.de>
> > 
> > I didn't see an explicit note from Thomas saying that he tested it.
> 
> It's burried somewhere in the thread, but he said that with the 2 earlier
> dvo patches + this one here the lvds-only use-case now works well. Before
> that he had issues with the display just showing a cursor and the kernel
> complaining about the cursor being stuck in the enabled position when
> trying to re-enable it.

From the mails, I couldn't figure out if he tried them all or just the
two patches you posted earlier.

> -Daniel
> 
> > 
> > > Cc: stable@vger.kernel.org
> > > Cc:  Thomas Richter <thor@math.tu-berlin.de>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index f34252d134b6..04d2699f51b4 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -7123,7 +7123,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> > >  		intel_crtc->cursor_visible = visible;
> > >  	}
> > >  	/* and commit changes on next vblank */
> > > +	POSTING_READ(CURCNTR(pipe));
> > >  	I915_WRITE(CURBASE(pipe), base);
> > > +	POSTING_READ(CURBASE(pipe));
> > >  }
> > >  
> > >  static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> > > @@ -7152,7 +7154,9 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> > >  		intel_crtc->cursor_visible = visible;
> > >  	}
> > >  	/* and commit changes on next vblank */
> > > +	POSTING_READ(CURCNTR_IVB(pipe));
> > >  	I915_WRITE(CURBASE_IVB(pipe), base);
> > > +	POSTING_READ(CURBASE_IVB(pipe));
> > >  }
> > >  
> > >  /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
> > > -- 
> > > 1.8.4.rc3
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

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

* Re: [PATCH] drm/i915: flush cursors harder
  2013-11-04 16:58     ` [Intel-gfx] " Ville Syrjälä
@ 2013-11-15 19:22       ` Daniel Vetter
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2013-11-15 19:22 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Daniel Vetter, Intel Graphics Development, stable

On Mon, Nov 04, 2013 at 06:58:13PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 04, 2013 at 05:34:41PM +0100, Daniel Vetter wrote:
> > On Mon, Nov 04, 2013 at 06:02:24PM +0200, Ville Syrjälä wrote:
> > > On Mon, Nov 04, 2013 at 08:13:45AM +0100, Daniel Vetter wrote:
> > > > Apparently they need the same treatment as primary planes. This fixes
> > > > modesetting failures because of stuck cursors (!) on Thomas' i830M
> > > > machine.
> > > 
> > > What treatment? Primary planes don't need any extra posting reads AFAIK.
> > 
> > If you look at flush_primary_plane it's definitely there. So I've copied
> > it over (it's just a real I915_READ, not a posting one).
> 
> The regular read is there just so that we don't modify the surface
> base address. We should be able to replace the read w/ recomputing
> the address.
> 
> > 
> > > 
> > > > 
> > > > I've figured while at it I'll also roll it out for the ivb 3 pipe
> > > > version of this function. I didn't do this for i845/i865 since Bspec
> > > > says the update mechanism works differently, and there's some
> > > > additional rules about what can be updated in which order.
> > > > 
> > > > Tested-by: Thomas Richter <thor@math.tu-berlin.de>
> > > 
> > > I didn't see an explicit note from Thomas saying that he tested it.
> > 
> > It's burried somewhere in the thread, but he said that with the 2 earlier
> > dvo patches + this one here the lvds-only use-case now works well. Before
> > that he had issues with the display just showing a cursor and the kernel
> > complaining about the cursor being stuck in the enabled position when
> > trying to re-enable it.
> 
> From the mails, I couldn't figure out if he tried them all or just the
> two patches you posted earlier.

Well I've written that patch to combat a WARN backtrace where the cursor
somehow didn't turn of. Thomas said he hasn't seen it since, so might have
worked. In any case it shouldn't be harmful, so I've merged it to -fixes.
-Daniel

> 
> > -Daniel
> > 
> > > 
> > > > Cc: stable@vger.kernel.org
> > > > Cc:  Thomas Richter <thor@math.tu-berlin.de>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index f34252d134b6..04d2699f51b4 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -7123,7 +7123,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
> > > >  		intel_crtc->cursor_visible = visible;
> > > >  	}
> > > >  	/* and commit changes on next vblank */
> > > > +	POSTING_READ(CURCNTR(pipe));
> > > >  	I915_WRITE(CURBASE(pipe), base);
> > > > +	POSTING_READ(CURBASE(pipe));
> > > >  }
> > > >  
> > > >  static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> > > > @@ -7152,7 +7154,9 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32 base)
> > > >  		intel_crtc->cursor_visible = visible;
> > > >  	}
> > > >  	/* and commit changes on next vblank */
> > > > +	POSTING_READ(CURCNTR_IVB(pipe));
> > > >  	I915_WRITE(CURBASE_IVB(pipe), base);
> > > > +	POSTING_READ(CURBASE_IVB(pipe));
> > > >  }
> > > >  
> > > >  /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
> > > > -- 
> > > > 1.8.4.rc3
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel OTC
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

end of thread, other threads:[~2013-11-15 19:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-04  7:13 [PATCH] drm/i915: flush cursors harder Daniel Vetter
2013-11-04 16:02 ` [Intel-gfx] " Ville Syrjälä
2013-11-04 16:34   ` Daniel Vetter
2013-11-04 16:58     ` [Intel-gfx] " Ville Syrjälä
2013-11-15 19:22       ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox