Re: [PATCH] drm/i915: s/mdelay/msleep/

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: s/mdelay/msleep/
Date: Tue, 7 Jul 2015 11:03:10 +0300	[thread overview]
Message-ID: <20150707080310.GR5176@intel.com> (raw)
In-Reply-To: <1436253040-20151-1-git-send-email-daniel.vetter@ffwll.ch>

On Tue, Jul 07, 2015 at 09:10:40AM +0200, Daniel Vetter wrote:
> Burning cpu cycles isn't awesome, so use sleeps instead.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 56316c1e945c..87be3f307e40 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1026,7 +1026,7 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
>  		line_mask = DSL_LINEMASK_GEN3;
>  
>  	line1 = I915_READ(reg) & line_mask;
> -	mdelay(5);
> +	msleep(5);

msleep() is quite inaccurate, but the accuracy of the delay shouldn't matter
too much here. The only functional worry would be if it manages to sleep for
exactly one frame and then we'd think DSL had stopped. But since we've already
told the pipe to shut down, I don't think it should ever end up scanning out
more than one frame here, so I suppose that problem should never happen.
So yeah, this seems safe.

It might make the modeset a bit slower if it ends up sleeping for a long
time, but I suppose it shouldn't be too bad. If we want to optimize for
that we could use usleep_range() and change the sleep duration to be
just a few scanlines.

>  	line2 = I915_READ(reg) & line_mask;
>  
>  	return line1 == line2;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6eb5d76e6912..1efac89cb738 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4255,7 +4255,7 @@ static void ironlake_enable_drps(struct drm_device *dev)
>  
>  	if (wait_for_atomic((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10))
>  		DRM_ERROR("stuck trying to change perf mode\n");
> -	mdelay(1);
> +	msleep(1);
>  
>  	ironlake_set_drps(dev, fstart);
>  
> @@ -4286,10 +4286,10 @@ static void ironlake_disable_drps(struct drm_device *dev)
>  
>  	/* Go back to the starting frequency */
>  	ironlake_set_drps(dev, dev_priv->ips.fstart);
> -	mdelay(1);
> +	msleep(1);
>  	rgvswctl |= MEMCTL_CMD_STS;
>  	I915_WRITE(MEMSWCTL, rgvswctl);
> -	mdelay(1);
> +	msleep(1);

Init/resume stuff. Doesn't seem too performance critical so msleep()
ought to be fine.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	spin_unlock_irq(&mchdev_lock);
>  }
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx