On Fri, Oct 20, 2017 at 05:48:54PM +0000, Runyan, Arthur J wrote: > Sorry about top reply from corporate email... > If you know in advance that you will just be temporarily disabling the PLL, then your sequence works. Actually we would end up using this sequence even if disable the PLL for good. Eg. if we're just disabling the entire pipe+DPLL, we'd do this (assuming cdclk doesn't need changing, but voltage can be lowered due to the port no longer requiring it): 1. disable DPLL 2. disable DPLL power ... 3. DVFS pre sequence 4. DVFS post sequence And when starting up a pipe from the cold with a new DPLL we'd conversly do this (again assuming no cdclk change, but voltage would have to be ramped up for the port): 1. DVFS pre sequence 2. DVFS post sequence ... 3. enable DPLL power 4. enable DPLL It does look a bit strange doing the DVFS sequences on their own like that, but I don't see why that should be problem. From where I'm sitting it doesn't really look any different than the following seqeunces: Disable with cdclk changing but port clock not having any effect on the voltage: 1. disable DPLL 2. disable DPLL power ... 3. DVFS pre sequence 4. change cdclk 5. DVFS post sequence Enable with cdclk changing but port clock not having any effect on the voltage: 1. DVFS pre sequence 2. change cdclk 3. DVFS post sequence ... 4. enable DPLL power 5. enable DPLL So unless something is snooping the actual state of the DPLLs, and based on that second guessing our choice of voltage, I don't really see how these two cases differ at all. > > -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > Sent: Friday, 20 October, 2017 4:12 AM > To: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Kahola, Mika <mika.kahola@xxxxxxxxx>; Navare, Manasi D <manasi.d.navare@xxxxxxxxx>; Runyan, Arthur J <arthur.j.runyan@xxxxxxxxx> > Subject: Re: [PATCH 8/8] drm/i915: Adjust system agent voltage on CNL if required by DDI ports > > 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@xxxxxxxxxxxxxxx> > > > > > > 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@xxxxxxxxx> > > > Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx> > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > > Tested-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > (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 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
