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

[Jesse Barnes]

On Wed, 24 Jun 2009 14:29:09 -0700
Eric Anholt <eric at anholt.net> wrote:

> 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:
>
> > +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?

Yes, of course.  I had that before I moved the code, arg.

-- 
Jesse Barnes, Intel Open Source Technology Center