On Tue, 24 Aug 2010 00:16:37 +0100, Peter Clifton <pc...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
