[Intel-gfx] [PATCH 1/2] Use WAIT_FOR_SCAN_LINE instead of WAIT_FOR_VBLANK

[Eric Anholt]

On Mon, 2009-04-06 at 11:34 -0700, Carl Worth wrote:
> Either way, the goal is tear-free video playing. But waiting for
> a scan-line window not only has the advantage of being cheaper
> for small windows, but also avoids hanging the GPU in the case
> of the pipe getting turned off, (by screensaver, for example),
> while a batch is waiting for a VBLANK that will never occur.
> 
> This fixes that GPU hang.
> ---
>  src/i810_reg.h   |   19 +++++++++++++------
>  src/i830_video.c |   36 ++++++++++++++++++++----------------
>  2 files changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/src/i810_reg.h b/src/i810_reg.h
> index bc462fa..102097c 100644
> --- a/src/i810_reg.h
> +++ b/src/i810_reg.h
> @@ -2436,12 +2436,19 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>  #define MI_OVERLAY_FLIP_OFF		(2<<21)
>  
>  /* Wait for Events */
> -#define MI_WAIT_FOR_EVENT		(0x03<<23)
> -#define MI_WAIT_FOR_PIPEB_SVBLANK	(1<<18)
> -#define MI_WAIT_FOR_PIPEA_SVBLANK	(1<<17)
> -#define MI_WAIT_FOR_OVERLAY_FLIP	(1<<16)
> -#define MI_WAIT_FOR_PIPEB_VBLANK	(1<<7)
> -#define MI_WAIT_FOR_PIPEA_VBLANK	(1<<3)
> +#define MI_WAIT_FOR_EVENT			(0x03<<23)
> +#define MI_WAIT_FOR_PIPEB_SVBLANK		(1<<18)
> +#define MI_WAIT_FOR_PIPEA_SVBLANK		(1<<17)
> +#define MI_WAIT_FOR_OVERLAY_FLIP		(1<<16)
> +#define MI_WAIT_FOR_PIPEB_VBLANK		(1<<7)
> +#define MI_WAIT_FOR_PIPEB_SCAN_LINE_WINDOW	(1<<5)
> +#define MI_WAIT_FOR_PIPEA_VBLANK		(1<<3)
> +#define MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW	(1<<2)
> +
> +/* Set the scan line for MI_WAIT_FOR_PIPE?_SCAN_LINE_WINDOW */
> +#define MI_LOAD_SCAN_LINES_INCL			(0x12<<23)
> +#define MI_LOAD_SCAN_LINES_DISPLAY_PIPEA	(0)
> +#define MI_LOAD_SCAN_LINES_DISPLAY_PIPEB	(0x1<<20)
>  
>  /* Flush */
>  #define MI_FLUSH			(0x04<<23)
> diff --git a/src/i830_video.c b/src/i830_video.c
> index 3f3aaac..3dde5b4 100644
> --- a/src/i830_video.c
> +++ b/src/i830_video.c
> @@ -2533,24 +2533,28 @@ I830PutImage(ScrnInfoPtr pScrn,
>          }
>  
>          if (sync) {
> -            I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
> -            int event;
> -
> -            if (IS_I965G(pI830)) {
> -                if (intel_crtc->pipe == 0)
> -                    event = MI_WAIT_FOR_PIPEA_SVBLANK;
> -                else
> -                    event = MI_WAIT_FOR_PIPEB_SVBLANK;
> -            } else {
> -                if (intel_crtc->pipe == 0)
> -                    event = MI_WAIT_FOR_PIPEA_VBLANK;
> -                else
> -                    event = MI_WAIT_FOR_PIPEB_VBLANK;
> -            }
> +	    BoxPtr box;
> +            int event, pipe;
> +	    I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
> +
> +	    if (intel_crtc->pipe == 0) {
> +		event = MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW;
> +		pipe = MI_LOAD_SCAN_LINES_DISPLAY_PIPEA;
> +	    } else {
> +		event = MI_WAIT_FOR_PIPEB_SCAN_LINE_WINDOW;
> +		pipe = MI_LOAD_SCAN_LINES_DISPLAY_PIPEB;
> +	    }
> +
> +	    box = REGION_EXTENTS(unused, clipBoxes);
>  
> -            BEGIN_BATCH(2);
> +            BEGIN_BATCH(5);
> +	    /* The documentation says that the LOAD_SCAN_LINES command
> +	     * always comes in pairs. Don't ask me why. */
> +	    OUT_BATCH(MI_LOAD_SCAN_LINES_INCL | pipe);
> +	    OUT_BATCH((box->y1 << 16) | box->y2);
> +	    OUT_BATCH(MI_LOAD_SCAN_LINES_INCL | pipe);
> +	    OUT_BATCH((box->y1 << 16) | box->y2);
>              OUT_BATCH(MI_WAIT_FOR_EVENT | event);
> -            OUT_BATCH(MI_NOOP);
>              ADVANCE_BATCH();
>          }

This looks to me like it'll result in tearing for a monitor positioned
below 0,0, since we're not offsetting by the crtc origin (so the
exclusion window ends up way off the end of the screen for an
above/below configuration).

Still, not hanging the machine is the most important thing here, and I'd
accept tearing in exchange.

-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- 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/20090406/91c9711e/attachment.sig>