From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Clifton Subject: Re: [PATCH] drm/i915: Drop the msleep parameter to wait_for() Date: Tue, 24 Aug 2010 00:16:37 +0100 Message-ID: <1282605397.29375.7.camel@pcjc2lap> References: <1282475148-15951-10-git-send-email-chris@chris-wilson.co.uk> <1282582569-24138-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from ppsw-33.csi.cam.ac.uk (ppsw-33.csi.cam.ac.uk [131.111.8.133]) by gabe.freedesktop.org (Postfix) with ESMTP id 990BA9E7D5 for ; Mon, 23 Aug 2010 16:16:42 -0700 (PDT) In-Reply-To: <1282582569-24138-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson , "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Mon, 2010-08-23 at 17:56 +0100, Chris Wilson wrote: > Jesse's feedback from using the wait_for() macro was that the msleep > argument was that it was superfluous and made the macro more difficult > to use and to read. As the actually amount of time to sleep is not > critical, the crucial part is to sleep and let the processor schedule > something else whilst we wait for the event, replace the argument with a > hardcoded value. 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. > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 6ccb797..ecf2d41 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -993,7 +993,7 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe) > /* Wait for vblank interrupt bit to set */ > if (wait_for((I915_READ(pipestat_reg) & ^___ wait_for_atomic?? > PIPE_VBLANK_INTERRUPT_STATUS) == 0, > - 50, 0)) > + 50)) > DRM_DEBUG_KMS("vblank wait timed out\n"); > } > > @@ -1092,7 +1092,7 @@ void i8xx_disable_fbc(struct drm_device *dev) > I915_WRITE(FBC_CONTROL, fbc_ctl); > > /* 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? > DRM_DEBUG_KMS("FBC idle timed out\n"); > return; > } > @@ -2115,7 +2115,7 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode) > I915_WRITE(transconf_reg, temp | TRANS_ENABLE); > I915_READ(transconf_reg); > > - if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 10, 0)) > + if (wait_for(I915_READ(transconf_reg) & TRANS_STATE_ENABLE, 10)) ^___ wait_for_atomic? > @@ -5553,7 +5553,7 @@ void ironlake_enable_drps(struct drm_device *dev) > rgvmodectl |= MEMMODE_SWMODE_EN; > I915_WRITE(MEMMODECTL, rgvmodectl); > > - if (wait_for((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 1, 0)) > + if (wait_for((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10)) ^___ wait_for_atomic? > DRM_ERROR("stuck trying to change perf mode\n"); > msleep(1); > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index b819c10..c9b62fe 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -114,7 +114,7 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on) > > I915_WRITE(ctl_reg, I915_READ(ctl_reg) | > POWER_TARGET_ON); > - if (wait_for(I915_READ(status_reg) & PP_ON, 1000, 0)) > + if (wait_for(I915_READ(status_reg) & PP_ON, 1000)) ^___ wait_for_atomic? > DRM_ERROR("timed out waiting to enable LVDS pipe"); > > intel_lvds_set_backlight(dev, dev_priv->backlight_duty_cycle); > @@ -123,7 +123,7 @@ static void intel_lvds_set_power(struct drm_device *dev, bool on) > > I915_WRITE(ctl_reg, I915_READ(ctl_reg) & > ~POWER_TARGET_ON); > - if (wait_for((I915_READ(status_reg) & PP_ON) == 0, 1000, 0)) > + if (wait_for((I915_READ(status_reg) & PP_ON) == 0, 1000)) ^___ wait_for_atomic? -- Peter Clifton Electrical Engineering Division, Engineering Department, University of Cambridge, 9, JJ Thomson Avenue, Cambridge CB3 0FA Tel: +44 (0)7729 980173 - (No signal in the lab!) Tel: +44 (0)1223 748328 - (Shared lab phone, ask for me)