[PATCH 2/3] drm/i915: WARN if RPS adjustment is more than range

[Kamble, Sagar A]

On 1/13/2017 11:30 PM, Chris Wilson wrote:
> On Fri, Jan 13, 2017 at 05:43:47PM +0000, Kamble, Sagar A wrote:
>> Hi Chris,
>>
>> Details of the issue with GuC and trace that shows the adjustment overflow to negative values is given below.
>> Even when Host RPS has requested RP0, Up Threshold Interrupts are coming due to kernel bug.
> Who's kernel bug? When we hit min/max we apply the interrupt mask to
> pmintrmsk.
it was bug with i915 handling of pm_intr_keep with GuC enabled. This was 
not correctly updated hence interrupts were staying unmasked at min/max.
earlier patch in series https://patchwork.freedesktop.org/patch/133084/ 
will solve this problem.
I prepared this patch to WARN to know in future if we are doing 
something abnormal with RPS adjustments.
>
> Are we happy with say:
Yes. I am happy with this but ideally I would want pm_intr_keep 
sanitizing patch as well.
Could you please review that also.
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9d98d646342d..7425e3a94854 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1170,6 +1170,8 @@ static void gen6_pm_rps_work(struct work_struct *work)
>                          adj *= 2;
>                  else /* CHV needs even encode values */
>                          adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1;
> +               if (new_delay >= dev_priv->rps.max_freq_softlimit)
> +                       adj = 0;
>                  /*
>                   * For better performance, jump directly
>                   * to RPe if we're below it.
> @@ -1191,6 +1193,8 @@ static void gen6_pm_rps_work(struct work_struct *work)
>                          adj *= 2;
>                  else /* CHV needs even encode values */
>                          adj = IS_CHERRYVIEW(dev_priv) ? -2 : -1;
> +               if (new_delay <= dev_priv->rps.min_freq_softlimit)
> +                       adj = 0;
>          } else { /* unknown event */
>                  adj = 0;
>          }
>
> so that we completely prevent RPS from jumping the shark?
> -Chris
>