Re: [PATCH 07/11] drm/i915/skl: Updated the gen9_enable_rps function

Jesse Barnes
2015-03-11T20:07:49Z

On 03/11/2015 12:46 PM, Chris Wilson wrote:
> On Wed, Mar 11, 2015 at 12:27:59PM -0700, Jesse Barnes wrote:
>> On 03/05/2015 09:37 PM, akash.goel@intel.com wrote:
>>> + /* Leaning on the below call to gen6_set_rps to program/setup the
>>> + * Up/Down EI & threshold registers, as well as the RP_CONTROL,
>>> + * RP_INTERRUPT_LIMITS & RPNSWREQ registers */
>>> + dev_priv->rps.power = HIGH_POWER; /* force a reset */
>>
>> Are you also sure that dev_priv->rps.cur_freq != min_freq_softlimit at
>> this point? That's the condition for calling into the threshold update
>> function (maybe gen6_set_rps should check both variables though).
>
> It's a good point, but Akash has inherited that bug from me. What I
> think we want is removing the actual intel_set_rps() calls here (and the
> rest of the *_enable_rps()) and do an intel_set_rps_idle() call from the
> common point in the caller, where we can put all the dancing required to
> force the RPS initialisation.

Yeah, that would make things a little clearer. I'd be fine with that as
a patch on top, fixing up the other functions as well.

Jesse