[Intel-gfx] [PATCH] DRI2 CopyRegion scanline wait fixes

[Łukasz Kuryło]

Dnia sobota 05 grudnia 2009 o 07:52:34 Jesse Barnes napisał(a):
> On Thu, 03 Dec 2009 11:36:49 -0800
> 
> Eric Anholt <eric at anholt.net> wrote:
> > On Wed, 2 Dec 2009 12:29:28 -0800, Jesse Barnes
> >
> > <jbarnes at virtuousgeek.org> wrote:
> > > In the case of full height blits, we were getting tearing on the
> > > first line. This could be due to batch buffer startup time or late
> > > vblank events.  On 965 and above we have a "start of vblank" event
> > > (as opposed to the one that comes in after various in-chip
> > > housekeeping is done on pre-965), so we can use that for full
> > > screen copies.  For pre-965, we just have to set a scanline wait
> > > for somewhere near the bottom of the screen to give us time to set
> > > up the copy.
> > >
> > > Fixes bug #22475.
> > >
> > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > >
> > > diff --git a/src/i830_dri.c b/src/i830_dri.c
> > > index 98c1a15..313cc96 100644
> > > --- a/src/i830_dri.c
> > > +++ b/src/i830_dri.c
> > > @@ -302,6 +302,7 @@ I830DRI2CopyRegion(DrawablePtr drawable,
> > > RegionPtr pRegion, int y1, y2;
> > >  		int pipe = -1, event, load_scan_lines_pipe;
> > >  		xf86CrtcPtr crtc;
> > > +		Bool full_height = FALSE;
> > >
> > >  		box = REGION_EXTENTS(unused, gc->pCompositeClip);
> > >  		crtc = i830_covering_crtc(scrn, box, NULL,
> > > &crtcbox); @@ -310,21 +311,34 @@ I830DRI2CopyRegion(DrawablePtr
> > > drawable, RegionPtr pRegion, if (crtc != NULL
> > > && !crtc->rotatedData) { pipe = i830_crtc_to_pipe(crtc);
> > >
> > > +			/* Make sure we don't wait for a scanline
> > > that will never occur */
> > > +			y1 = (crtcbox.y1 <= box->y1) ? box->y1 -
> > > crtcbox.y1 : 0;
> > > +			y2 = (box->y2 <= crtcbox.y2) ?
> > > +			    box->y2 - crtcbox.y1 : crtcbox.y2 -
> > > crtcbox.y1; +
> > > +			if (y1 == 0 && y2 == crtcbox.y2)
> > > +			    full_height = TRUE;

This condition does not work with multiple monitors (see) clipping above.
I believe it should be if (y1 == 0 && y2 == crtcbox.y2 - crtcbox.y1).

> > > +
> > > +			/* Pre-965 doesn't have SVBLANK, so we
> > > need a bit
> > > +			 * of extra time for the blitter to start
> > > up and
> > > +			 * do its job for a full height blit */
> > > +			if (full_height && !IS_I965G(intel))
> > > +			    y2 -= 2;
> >
> > Don't you mean y1 here?  And won't this blow up for y < 2?
> 
> It'll only happen for full height blits, so I expect y2 to be greater
> than 2.  But theoretically it could be I suppose.
> 
> And no, I mean y2, since we want to submit the batch just before the
> beam reaches the bottom of the screen, not slightly after it leaves the
> top.
> 
> > > +
> > >  			if (pipe == 0) {
> > >  				event =
> > > MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW; load_scan_lines_pipe =
> > >  				    MI_LOAD_SCAN_LINES_DISPLAY_PIPEA;
> > > +				if (full_height && IS_I965G(intel))
> > > +				    event =
> > > MI_WAIT_FOR_PIPEA_SVBLANK; } else {
> > >  				event =
> > > MI_WAIT_FOR_PIPEB_SCAN_LINE_WINDOW; load_scan_lines_pipe =
> > >  				    MI_LOAD_SCAN_LINES_DISPLAY_PIPEB;
> > > +				if (full_height && IS_I965G(intel))
> > > +				    event =
> > > MI_WAIT_FOR_PIPEB_SVBLANK;
> >
> > If we're doing an SVBLANK wait, shouldn't we bail on the scan lines
> > loading?
> 
> Yeah, probably.  Seems to be harmless though.
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20091208/aa13049f/attachment.sig>