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

[Paulo Zanoni]

2013/10/10 Jani Nikula <jani.nikula at intel.com>:
> On Wed, 09 Oct 2013, Paulo Zanoni <przanoni at gmail.com> wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at 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?
>

We definitely should do something in case we get a timeout. I don't
really know the best solution here, I need to study the problem. I
just tried to solve the immediate problem of "make my machine work" :)
I also think we should be careful to not mess with someone else's operation.

Also, I think this problem is kinda "orthogonal" to increasing the
timeout (although increasing the timeout hides it, hopefully forever).
I added this to my TODO list, but I don't guarantee I will fix it in
the next weeks.


> In any case, this patch is
>
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>


Thanks for the review :)

>
>
>> 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 at intel.com>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at 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



-- 
Paulo Zanoni