Re: [PATCH 8/8] drm/i915: Adjust system agent voltage on CNL if required by DDI ports

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 19, 2017 at 04:54:56PM -0700, Rodrigo Vivi wrote:
> 
> On Wed, Oct 18, 2017 at 08:48:25PM +0000, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On CNL we may need to bump up the system agent voltage not only due
> > to CDCLK but also when driving DDI port with a sufficiently high clock.
> > To that end start tracking the minimum acceptable voltage for each crtc.
> > We do the tracking via crtcs because we don't have any kind of encoder
> > state. Also there's no downside to doing it this way, and it matches how
> > we track cdclk requirements on account of pixel rate.
> > 
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> (Although I didn't find cases where I could force a higher voltage level,
> everything works well on CNL with this.)

I went and actually read the spec now :) so I can now see that this
doesn't really match the sequence listed in the spec.

For example, let's assume we have a modeset where we have to disable
a DPLL, change CDCLK, and finally enable a DPLL again.

What the spec says we should do is something like this:
1.  DVFS pre sequence
2.  disable DPLL
3.  DVFS post sequence
4.  disable DPLL power
...
5.  DVFS pre sequence
6.  configure CDCLK_CTL
7.  DVFS post sequence
...
8.  enable DPLL power
9.  DVFS pre sequence
10. enable DPLL
11. DVFS post sequence

With my code what we'd end up doing is this instead:
1. disable DPLL
2. disable DPLL power
...
3. DVFS pre sequence
4. configure CDCLK_CTL
5. DVFS post sequence
...
6. enable DPLL power
7. enable DPLL

So my way always results in running the DVFS sequences at most once.
With the way the spec has things listed we might have to run the
sequences multiple times. 

Art, is there a good reason why we'd actually have to run the
DVFS sequences around each DPLL_ENABLE write, instead of just
doing it once at any point between disabling and enabling
DPLLs?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx