On Thu, Mar 26, 2015 at 06:32:15PM -0300, Paulo Zanoni wrote: > 2015-03-19 11:14 GMT-03:00 <deepak.s@xxxxxxxxxxxxxxx>: > > From: Deepak S <deepak.s@xxxxxxxxxxxxxxx> > > > > After feedback from the hardware team, now we set the GPU min/idel freq to RPe. > > Punit is expecting us to operate GPU between Rpe & Rp0. If we drop the > > frequency to RPn, punit is failing to change the input voltage to > > minimum :( > > Since this is far away from the obvious, I am imagining some > programmer from the future looking at this code and imagining > min_freq_softlimit was "accidentally" set to RPe instead of RPn. Can't > we add a comment in the code - not just the commit message -, to make > it clear that we're doing this since the punit is weird? > > Another thing which I noticed is that your patch title mentions CHV, > but your patch touches the VLV function instead of the CHV one. This > also leads me to think that maybe the power measurement experiments > you did were done using the non-patched CHV code... Can you please > clarify your intentions here? And also maybe redo the power > measurements if needed. > > Also, I think we need at least an ACK from Chris here, especially > since he was already discussing the previous version of this patch. If you include a comment like (and note we want to set dev_priv->rps.min_freq not dev_priv->rps.min_freq_softlimit): /* PUnit validated range is only [RPe, RP0] */ dev_priv->rps.min_freq = dev_priv->rps.efficient_freq; and make sure that is set before we derive dev_priv->rps.idle_freq. You can have my Acked-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
