On Wed, Oct 04, 2017 at 12:36:42PM -0700, Rodrigo Vivi wrote:
> On Wed, Oct 04, 2017 at 09:46:41AM +0000, Mika Kahola wrote:
> > On Tue, 2017-10-03 at 00:06 -0700, Rodrigo Vivi wrote:
> > > According to spec "If voltage is set too low,
> > > it will break functionality. If voltage is set too high,
> > > it will waste power."
> > >
> > > So, let's prefer the waste of power instead of breaking
> > > functionality.
> > >
> > > But also the logic of deciding the level on spec
> > > tells "Else, use level 2."
> > > So, default is actually "2", not "0".
> > The spec also says
> >
> > "If CD clock = 168 MHz AND DDI clock ≤ 594 MHz, use level 0.
> > Else If CD clock = 336 MHz AND DDI clock ≤ 594 MHz, use level 1.
> > Else, use level 2."
> >
> > Should we add check for DDI clock rate as well here?
>
> By this time dpll are disabled and not associated to any
> port yet. So portclock is 0. So if we invert the default
> we do respect the same algorightm you pasted.
> Also if you notice I'm just inverting this in a separeted
> patch to make it clear and if needed to bisect we end up
> on this single point of change. But on a later patch on
> this series I change to use your function with this algoright
> there, but giving portclock = 0.
>
> So by the end of this series the flow is:
>
> - enable cdclk
> - request dvfs level enough for cdclk in question.
> - enable pll
> - adjust dvfs level only if needed taking the port clock into account.
> - disable pll
> - put back level to cdclock level if needed.
>
> >
> > >
> > > v2: Rebase moving it up to avoid some temporary code
> > > duplication.
> > >
> > > Cc: Mika Kahola <mika.kahola@xxxxxxxxx>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index af8411c2a6b9..7e9c4444c844 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -1562,15 +1562,15 @@ static void cnl_set_cdclk(struct
> > > drm_i915_private *dev_priv,
> > > }
> > >
> > > switch (cdclk) {
> > > - case 528000:
> > > - pcu_ack = 2;
> > > + case 168000:
> > > + pcu_ack = 0;
> > > break;
> > > case 336000:
> > > pcu_ack = 1;
> > > break;
> > > - case 168000:
> > > + case 528000:
> > > default:
> > > - pcu_ack = 0;
> > > + pcu_ack = 2;
> > > break;
The spec says "Else If CD clock ≤ 556.8 AND DDI clock > 594 MHz, use level 1"
So in case of cdclock = 528000, it is still < 556.8Mhz and the level should be 1
Manasi
> > > }
> > >
> > --
> > Mika Kahola - Intel OTC
> >
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
