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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 15 Dec 2016 10:57:55 +0200
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     Nicholas Mc Guire <hofrat@...dl.org>,
        Daniel Vetter <daniel.vetter@...el.com>
Cc:     ymohanma <yogesh.mohan.marimuthu@...el.com>,
        David Airlie <airlied@...ux.ie>,
        intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, Nicholas Mc Guire <hofrat@...dl.org>
Subject: Re: [PATCH] drm/i915: use udelay for very small delays

On Thu, 15 Dec 2016, Jani Nikula <jani.nikula@...ux.intel.com> wrote:
> On Thu, 15 Dec 2016, Nicholas Mc Guire <hofrat@...dl.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@...dl.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