diff mbox

[1/5] drm/i915: Increase maximum polling time to 50ms for forcewake request/clear ack

Message ID 1440332571-20761-2-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com Aug. 23, 2015, 12:22 p.m. UTC
On BXT, We Observe timeout for forcewake request completion with 2ms polling period as given here:
[drm:fw_domains_get] ERROR render: timed out waiting for forcewake ack request.
Polling for 50ms is recommended to avoid these timeouts.

Change-Id: Ie715b0069a3049606e9602bc5e97a6511890864d
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Vetter Aug. 26, 2015, 9:26 a.m. UTC | #1
On Sun, Aug 23, 2015 at 05:52:47PM +0530, Sagar Arun Kamble wrote:
> On BXT, We Observe timeout for forcewake request completion with 2ms polling period as given here:
> [drm:fw_domains_get] ERROR render: timed out waiting for forcewake ack request.
> Polling for 50ms is recommended to avoid these timeouts.
> 
> Change-Id: Ie715b0069a3049606e9602bc5e97a6511890864d
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Probably needs cc: stable plus a pile of bugzilla links all over ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 9d3c2e4..2df03b1 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -27,7 +27,7 @@
>  
>  #include <linux/pm_runtime.h>
>  
> -#define FORCEWAKE_ACK_TIMEOUT_MS 2
> +#define FORCEWAKE_ACK_TIMEOUT_MS 50
>  
>  #define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
>  #define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, (dev_priv__)->regs + (reg__))
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
yu.dai@intel.com Sept. 21, 2015, 4:43 p.m. UTC | #2
Looks fine to me.
Reviewed by: Alex Dai <yu.dai@intel.com>.

On 08/23/2015 05:22 AM, Sagar Arun Kamble wrote:
> On BXT, We Observe timeout for forcewake request completion with 2ms polling period as given here:
> [drm:fw_domains_get] ERROR render: timed out waiting for forcewake ack request.
> Polling for 50ms is recommended to avoid these timeouts.
>
> Change-Id: Ie715b0069a3049606e9602bc5e97a6511890864d
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_uncore.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 9d3c2e4..2df03b1 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -27,7 +27,7 @@
>   
>   #include <linux/pm_runtime.h>
>   
> -#define FORCEWAKE_ACK_TIMEOUT_MS 2
> +#define FORCEWAKE_ACK_TIMEOUT_MS 50
>   
>   #define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
>   #define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, (dev_priv__)->regs + (reg__))
Tvrtko Ursulin Sept. 22, 2015, 9:06 a.m. UTC | #3
On 08/26/2015 10:26 AM, Daniel Vetter wrote:
> On Sun, Aug 23, 2015 at 05:52:47PM +0530, Sagar Arun Kamble wrote:
>> On BXT, We Observe timeout for forcewake request completion with 2ms polling period as given here:
>> [drm:fw_domains_get] ERROR render: timed out waiting for forcewake ack request.
>> Polling for 50ms is recommended to avoid these timeouts.
>>
>> Change-Id: Ie715b0069a3049606e9602bc5e97a6511890864d
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>
> Probably needs cc: stable plus a pile of bugzilla links all over ...

If the required wait is typically that long, shouldn't we be worried 
about busy polling?

Regards,

Tvrtko
Chris Wilson Sept. 22, 2015, 9:15 a.m. UTC | #4
On Tue, Sep 22, 2015 at 10:06:47AM +0100, Tvrtko Ursulin wrote:
> 
> On 08/26/2015 10:26 AM, Daniel Vetter wrote:
> >On Sun, Aug 23, 2015 at 05:52:47PM +0530, Sagar Arun Kamble wrote:
> >>On BXT, We Observe timeout for forcewake request completion with 2ms polling period as given here:
> >>[drm:fw_domains_get] ERROR render: timed out waiting for forcewake ack request.
> >>Polling for 50ms is recommended to avoid these timeouts.
> >>
> >>Change-Id: Ie715b0069a3049606e9602bc5e97a6511890864d
> >>Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >
> >Probably needs cc: stable plus a pile of bugzilla links all over ...
> 
> If the required wait is typically that long, shouldn't we be worried
> about busy polling?

With an exponential backoff (i.e. busyspin for the first ~jiffie/1ms)
then usleep_range 1, 2, 4, 8,..?

And also keeping the fw wakelock for longer than a jiffie to
optimistically avoid these multi-jiffie stalls. Whilst grumbling.
-Chris
Tvrtko Ursulin Sept. 22, 2015, 9:48 a.m. UTC | #5
On 09/22/2015 10:15 AM, Chris Wilson wrote:
> On Tue, Sep 22, 2015 at 10:06:47AM +0100, Tvrtko Ursulin wrote:
>>
>> On 08/26/2015 10:26 AM, Daniel Vetter wrote:
>>> On Sun, Aug 23, 2015 at 05:52:47PM +0530, Sagar Arun Kamble wrote:
>>>> On BXT, We Observe timeout for forcewake request completion with 2ms polling period as given here:
>>>> [drm:fw_domains_get] ERROR render: timed out waiting for forcewake ack request.
>>>> Polling for 50ms is recommended to avoid these timeouts.
>>>>
>>>> Change-Id: Ie715b0069a3049606e9602bc5e97a6511890864d
>>>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>>
>>> Probably needs cc: stable plus a pile of bugzilla links all over ...
>>
>> If the required wait is typically that long, shouldn't we be worried
>> about busy polling?
>
> With an exponential backoff (i.e. busyspin for the first ~jiffie/1ms)
> then usleep_range 1, 2, 4, 8,..?
>
> And also keeping the fw wakelock for longer than a jiffie to
> optimistically avoid these multi-jiffie stalls. Whilst grumbling.

For the atomic case holding it longer is the only option.

But key is how typical or atypical these long waits are. Measuring that 
together with power use sounds critical.

Spread of required waits would also give the answer to what the best 
sleeping strategy for the non-atomic case is.

Regards,

Tvrtko
Daniel Vetter Sept. 23, 2015, 8:04 a.m. UTC | #6
On Mon, Sep 21, 2015 at 09:43:06AM -0700, Yu Dai wrote:
> Looks fine to me.
> Reviewed by: Alex Dai <yu.dai@intel.com>.
> 
> On 08/23/2015 05:22 AM, Sagar Arun Kamble wrote:
> >On BXT, We Observe timeout for forcewake request completion with 2ms polling period as given here:
> >[drm:fw_domains_get] ERROR render: timed out waiting for forcewake ack request.
> >Polling for 50ms is recommended to avoid these timeouts.
> >
> >Change-Id: Ie715b0069a3049606e9602bc5e97a6511890864d
> >Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> >---
> >  drivers/gpu/drm/i915/intel_uncore.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> >index 9d3c2e4..2df03b1 100644
> >--- a/drivers/gpu/drm/i915/intel_uncore.c
> >+++ b/drivers/gpu/drm/i915/intel_uncore.c
> >@@ -27,7 +27,7 @@
> >  #include <linux/pm_runtime.h>
> >-#define FORCEWAKE_ACK_TIMEOUT_MS 2
> >+#define FORCEWAKE_ACK_TIMEOUT_MS 50
> >  #define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
> >  #define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, (dev_priv__)->regs + (reg__))
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 9d3c2e4..2df03b1 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -27,7 +27,7 @@ 
 
 #include <linux/pm_runtime.h>
 
-#define FORCEWAKE_ACK_TIMEOUT_MS 2
+#define FORCEWAKE_ACK_TIMEOUT_MS 50
 
 #define __raw_i915_read8(dev_priv__, reg__) readb((dev_priv__)->regs + (reg__))
 #define __raw_i915_write8(dev_priv__, reg__, val__) writeb(val__, (dev_priv__)->regs + (reg__))