Re: [PATCH] drm/i915: use udelay for very small delays

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 15 Dec 2016, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 15 Dec 2016, Nicholas Mc Guire <hofrat@osadl.org> wrote:
>> usleep_range() is intended for delays in the 10us to 10ms range that need
>> good precision. a useleep_range(1, will effectively be no more than an
>> imprecise udelay with some added cache disruption as it will fire more or
>> less immediately - use udelay() here.
>>
>> Fixes: commit be4fc046bed3 ("drm/i915: add VLV DSI PLL Calculations")
>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
>> ---
>>
>> Problem located by coccinelle
>>
>> The requirement of waiting at least 0.5 us is assured with the udelay(1)
>> here which should be more effective than a usleep_range() - would 
>> ndelay(500) make sense here ?
>
> This is in the modeset path, i.e. pretty slow anyway. In this case, the
> point is not to try hard to minimize the wait, the point is to guarantee
> "at least 0.5 us" has passed. If the CPU can do something else,
> including dozing off, in the mean time, great. I think we should stick
> with usleep_range().
>
> I think the question is, how do we express this in code? IMO udelay() is
> not the answer.
>
> And why doesn't usleep_range() kernel-doc mention anything about the
> ranges?
>
>
> BR,
> Jani.
>
>
>>
>> Patch was compile tested with: x86_64_defconfig (implies CONFIG_DRM_I915)
>>
>> Patch is against 4.9.0 (localvrsion-next is next-20161214)
>>
>>  drivers/gpu/drm/i915/intel_dsi_pll.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> index 56eff60..0ec040e 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> @@ -157,7 +157,7 @@ static void vlv_enable_dsi_pll(struct intel_encoder *encoder,
>>  		      config->dsi_pll.ctrl & ~DSI_PLL_VCO_EN);
>>  
>>  	/* wait at least 0.5 us after ungating before enabling VCO */
>> -	usleep_range(1, 10);
>> +	udelay(1);
>>  
>>  	vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, config->dsi_pll.ctrl);

PS. This vlv_cck_write() call will do sideband communication with
millisecond range timeouts.


-- 
Jani Nikula, Intel Open Source Technology Center