Re: [PATCH] drm/i915: increase the SWSCI DSLP default timeout to 50ms

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

On Wed, 09 Oct 2013, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> The spec says the default timeout should be 2ms, but on my machine
> this doesn't seem to be enough. Sometimes it works, sometimes I get
> these messages when booting:
>   - SWSCI request timed out

BTW if this happens, and BIOS missed our call, will anyone clear
SWSCI_SCIC_INDICATOR? If not, we're bound to keep hitting:

>   - SWSCI request already in progress

So maybe we should just clear SWSCI_SCIC_INDICATOR ourselves on timeout?
Would that help?

In any case, this patch is

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> And my guess is that the "already in progress" message is because the
> first one is still happening.
>
> I did some experiments on my machine (that has CONFIG_HZ=1000) and the
> wait_for function usually takes 4-6 jiffies to finish, but I've seen
> up to 9. So increase the timeout to 50ms. We only expect to wait for
> the actual amount of time the operation takes, so even a huge timeout
> shouldn't delay us more than what the hardware actually requires.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 2acf5ca..9044640 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -258,7 +258,9 @@ static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
>  	/* Driver sleep timeout in ms. */
>  	dslp = ioread32(&swsci->dslp);
>  	if (!dslp) {
> -		dslp = 2;
> +		/* The spec says 2ms should be the default, but it's too small
> +		 * for some machines. */
> +		dslp = 50;
>  	} else if (dslp > 500) {
>  		/* Hey bios, trust must be earned. */
>  		WARN_ONCE(1, "excessive driver sleep timeout (DSPL) %u\n", dslp);
> -- 
> 1.8.3.1
>

-- 
Jani Nikula, Intel Open Source Technology Center