Re: [PATCH] drm/i915: Drop the msleep parameter to wait_for()

[Peter Clifton <pcjc2@cam.ac.uk>]

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)