[Intel-gfx] [PATCH] drm/i915: Drop the msleep parameter to wait_for()

[Chris Wilson]

On Tue, 24 Aug 2010 00:16:37 +0100, Peter Clifton <pcjc2 at cam.ac.uk> wrote:
> I noticed that the patch changes the semantics of some of the wait_for
> calls. Previously, many were called with a zero msleep parameter -
> meaning the call would not msleep. With this patch, the cases below will
> now msleep(1), rather than not msleep'ing at all.

Intentionally. The choices I made when adding the wait_for() were fairly
arbitrary. I'd err on the side of sleeping the extra milliseconds rather
than spend 500 microseconds busy-spinning. (I've have a different opinion
if these ever become the rate-limiting step in modesetting... ;-)

> >  	/* Wait for compressing bit to clear */
> > -	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10, 0)) {
> > +	if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10)) {
>                 ^___ wait_for_atomic?

This is perhaps the most debatable as I have no feel for what the
compression delay is and the spin may only be on the order of a few
hundred microseconds. However, I think the wait_for() here is entirely
superfluous and have removed it in my drm-testing.

A couple of the later patches increase those short timeouts you
highlighted to fix reported issues, so those are poor candidates for
busy-spinning. So outside of kdb, I don't see a reason where we need to be
continuously polling the register.

-- 
Chris Wilson, Intel Open Source Technology Centre