[Intel-gfx] [PATCH] treat disabled CRTCs as "not covering" for scanline wait purposes

Eric Anholt eric at anholt.net
Wed Jun 24 23:29:09 CEST 2009


On Wed, 2009-06-24 at 12:21 -0700, Jesse Barnes wrote:
> Now that swapbuffers does a scanline wait to avoid tearing, it's
> important to take into account the CRTC status to avoid hangs.  If we
> do a scanline wait when the CRTC is off (due to DPMS for example) we'll
> hang the GPU.  So add some code to check the CRTC DPMS status to the
> i830_covering_crtc function, returning NULL if none of the covering
> CRTCs are actually active.  KMS vs UMS logic is hidden in new i830*
> functions, cleaning up both DRI2 & video paths a bit.
> 
> Fixes fdo bug #22383.

Looks good but one note:

> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> 
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index 02a71ae..8f45e84 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -71,7 +71,7 @@ typedef struct {
>      int num_props;
>      drmmode_prop_ptr props;
>      void *private_data;
> -	/* this is used to store private data */
> +    int dpms_mode;
>  } drmmode_output_private_rec, *drmmode_output_private_ptr;
>  
>  static void
> @@ -727,6 +727,7 @@ drmmode_output_dpms(xf86OutputPtr output, int mode)
>                                  drmmode_output->output_id,
>                                  props->prop_id,
>                                  mode);
> +			drmmode_output->dpms_mode = mode;
>                          drmModeFreeProperty(props);
>                          return;
>  		}
> @@ -734,6 +735,14 @@ drmmode_output_dpms(xf86OutputPtr output, int mode)
>  	}
>  }
>  
> +int
> +drmmode_output_dpms_status(xf86OutputPtr output)
> +{
> +	drmmode_output_private_ptr drmmode_output = output->driver_private;
> +
> +	return drmmode_output->dpms_mode;
> +}
> +
>  static Bool
>  drmmode_property_ignore(drmModePropertyPtr prop)
>  {
> diff --git a/src/i830.h b/src/i830.h
> index 6825e77..25ed424 100644
> --- a/src/i830.h
> +++ b/src/i830.h
> @@ -691,7 +691,10 @@ void I830DRI2CloseScreen(ScreenPtr pScreen);
>  
>  extern Bool drmmode_pre_init(ScrnInfoPtr pScrn, int fd, int cpp);
>  extern int drmmode_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, xf86CrtcPtr crtc);
> +extern int drmmode_output_dpms_status(xf86OutputPtr output);
>  
> +extern Bool i830_crtc_on(xf86CrtcPtr crtc);
> +extern int i830_crtc_to_pipe(xf86CrtcPtr crtc);
>  extern Bool I830AccelInit(ScreenPtr pScreen);
>  extern void I830SetupForScreenToScreenCopy(ScrnInfoPtr pScrn, int xdir,
>  					   int ydir, int rop,
> diff --git a/src/i830_dri.c b/src/i830_dri.c
> index d00b42c..be09126 100644
> --- a/src/i830_dri.c
> +++ b/src/i830_dri.c
> @@ -311,12 +311,7 @@ I830DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion,
>  
>  	/* Make sure the CRTC is valid and this is the real front buffer */
>  	if (crtc != NULL && !crtc->rotatedData) {
> -	    if (pI830->use_drm_mode)
> -		pipe = drmmode_get_pipe_from_crtc_id(pI830->bufmgr, crtc);
> -	    else {
> -		I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
> -		pipe = intel_crtc->pipe;
> -	    }
> +	    pipe = i830_crtc_to_pipe(crtc);
>  
>  	    if (pipe == 0) {
>  		event = MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW;
> diff --git a/src/i830_driver.c b/src/i830_driver.c
> index 70b8788..b9b9d0b 100644
> --- a/src/i830_driver.c
> +++ b/src/i830_driver.c
> @@ -2456,6 +2456,49 @@ i830_init_bufmgr(ScrnInfoPtr pScrn)
>     }
>  }
>  
> +Bool i830_crtc_on(xf86CrtcPtr crtc)
> +{
> +    ScrnInfoPtr pScrn = crtc->scrn;
> +    xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
> +    I830Ptr pI830 = I830PTR(pScrn);
> +
> +    if (pI830->use_drm_mode) {
> +	int i, active_outputs = 0;
> +
> +	/* Kernel manages CRTC status based out output config */
> +	for (i = 0; i < xf86_config->num_output; i++) {
> +	    xf86OutputPtr output = xf86_config->output[i];
> +	    if (drmmode_output_dpms_status(output) == DPMSModeOn)
> +		active_outputs++;
> +	}
> +
> +	if (active_outputs)
> +	    return TRUE;
> +	return FALSE;

shouldn't this be checking that the output is attached to the crtc we're
checking?

> +    } else {
> +	I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
> +
> +	if (intel_crtc->dpms_mode == DPMSModeOn)
> +	    return TRUE;
> +	return FALSE;
> +    }
> +}
> +
> +int i830_crtc_to_pipe(xf86CrtcPtr crtc)
> +{
> +    ScrnInfoPtr pScrn = crtc->scrn;
> +    I830Ptr pI830 = I830PTR(pScrn);
> +    int pipe;
> +
> +    if (pI830->use_drm_mode) {
> +	pipe = drmmode_get_pipe_from_crtc_id(pI830->bufmgr, crtc);
> +    } else {
> +	I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
> +	pipe = intel_crtc->pipe;
> +    }
> +
> +    return pipe;
> +}
>  
>  static void
>  I830AdjustMemory(ScreenPtr pScreen)
> diff --git a/src/i830_video.c b/src/i830_video.c
> index 1ff80a0..d543cc4 100644
> --- a/src/i830_video.c
> +++ b/src/i830_video.c
> @@ -1734,6 +1734,11 @@ i830_covering_crtc (ScrnInfoPtr pScrn,
>      for (c = 0; c < xf86_config->num_crtc; c++)
>      {
>  	crtc = xf86_config->crtc[c];
> +
> +	/* If the CRTC is off, treat it as not covering */
> +	if (!i830_crtc_on(crtc))
> +	    continue;
> +
>  	i830_crtc_box (crtc, &crtc_box);
>  	i830_box_intersect (&cover_box, &crtc_box, box);
>  	coverage = i830_box_area (&cover_box);
> @@ -2491,14 +2496,8 @@ I830PutImage(ScrnInfoPtr pScrn,
>  	    int y1, y2;
>  	    int pipe = -1, event, load_scan_lines_pipe;
>  
> -	    if (pixmap_is_scanout(pPixmap)) {
> -		if (pI830->use_drm_mode)
> -		    pipe = drmmode_get_pipe_from_crtc_id(pI830->bufmgr, crtc);
> -		else {
> -		    I830CrtcPrivatePtr intel_crtc = crtc->driver_private;
> -		    pipe = intel_crtc->pipe;
> -		}
> -	    }
> +	    if (pixmap_is_scanout(pPixmap))
> +		pipe = i830_crtc_to_pipe(crtc);
>  
>  	    if (pipe >= 0) {
>  		if (pipe == 0) {
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
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/20090624/1bb3083e/attachment.sig>


More information about the Intel-gfx mailing list