[Intel-gfx] [PATCH] drm/i915: Share the common work of disabling active FBC before updating

[Keith Packard]

On Thu,  7 Jul 2011 21:30:19 +0100, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> Upon review, all path share the same dependencies for updating the
> registers and so we can benefit from sharing the code and checking
> early.

yeah, looks good.

> +	if (intel_fbc_enabled(dev)) {
> +		/* We update FBC along two paths, after changing fb/crtc
> +		 * configuration (modeswitching) and after page-flipping
> +		 * finishes. For the latter, we know that not only did
> +		 * we disable the FBC at the start of the page-flip
> +		 * sequence, but also more than one vblank has passed.
> +		 *
> +		 * For the former case of modeswitching, it is possible
> +		 * to switch between two FBC valid configurations
> +		 * instantaneously so we do need to disable the FBC
> +		 * before we can modify its control registers. We also
> +		 * have to wait for the next vblank for that to take
> +		 * effect.
> +		 *
> +		 * In the scenario that we go from a valid to invalid
> +		 * and then back to valid FBC configuration we have
> +		 * no strict enforcement that a vblank occurred since
> +		 * disabling the FBC. However, along all current pipe
> +		 * disabling paths we do need to wait for a vblank at
> +		 * some point...
> +		 */
> +		DRM_DEBUG_KMS("disabling active FBC for update\n");
> +		intel_disable_fbc(dev);
> +		intel_wait_for_vblank(dev, intel_crtc->pipe);
> +	}
> +
>  	intel_enable_fbc(crtc, 500);

Should this path queue a worker function to re-enable FBC after 'a
while'? Is there any particular reason it needs to be done synchronously
here? That would avoid a 'wait_for_vblank' call with the mutex held.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20110707/c9dff5ae/attachment.sig>