| Message ID | 20171018204825.2500-8-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 18, 2017 at 08:48:24PM +0000, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Track the system agent voltage we request from pcode in the cdclk state > on CNL. Annoyingly we can't actually read out the current value since > there's no pcode command to do that, so we'll have to just assume that > it worked. +static u32 cnl_cur_voltage(struct drm_i915_private *dev_priv) +{ + u32 voltage; + sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &voltage); + return voltage; +} + static void cnl_get_cdclk(struct drm_i915_private *dev_priv, struct intel_cdclk_state *cdclk_state) { @@ -1486,8 +1493,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); + cdclk_state->voltage = cnl_cur_voltage(dev_priv); } static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) @@ -1594,8 +1600,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, intel_update_cdclk(dev_priv); + dev_priv->cdclk.hw.voltage = cnl_cur_voltage(dev_priv); > > 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> > --- > drivers/gpu/drm/i915/intel_cdclk.c | 44 ++++++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index 1b4dcd9689da..795a18f64c4c 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1505,6 +1505,19 @@ static int cnl_calc_cdclk(int min_cdclk) > return 168000; > } > > +static u8 cnl_calc_voltage(int cdclk) > +{ > + switch (cdclk) { > + default: > + case 168000: > + return 0; > + case 336000: > + return 1; > + case 528000: > + return 2; > + } > +} where is the port_clock taking into account? > + > static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv, > struct intel_cdclk_state *cdclk_state) > { > @@ -1538,7 +1551,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > cdclk_state->cdclk = cdclk_state->ref; > > if (cdclk_state->vco == 0) > - return; > + goto out; > > divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK; > > @@ -1555,6 +1568,13 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > } > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); > + > + out: > + /* > + * Can't read this out :( Let's assume it's > + * at least what the CDCLK frequency requires. > + */ > + cdclk_state->voltage = cnl_calc_voltage(cdclk_state->cdclk); > } > > static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) > @@ -1595,7 +1615,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > { > int cdclk = cdclk_state->cdclk; > int vco = cdclk_state->vco; > - u32 val, divider, pcu_ack; > + u32 val, divider; > int ret; > > mutex_lock(&dev_priv->pcu_lock); > @@ -1624,19 +1644,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > break; > } > > - switch (cdclk) { > - case 528000: > - pcu_ack = 2; > - break; > - case 336000: > - pcu_ack = 1; > - break; > - case 168000: > - default: > - pcu_ack = 0; > - break; > - } > - > if (dev_priv->cdclk.hw.vco != 0 && > dev_priv->cdclk.hw.vco != vco) > cnl_cdclk_pll_disable(dev_priv); > @@ -1654,7 +1661,8 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > /* inform PCU of the change */ > mutex_lock(&dev_priv->pcu_lock); > - sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack); > + sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, > + cdclk_state->voltage); > mutex_unlock(&dev_priv->pcu_lock); > > intel_update_cdclk(dev_priv); > @@ -1747,6 +1755,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv) > > cdclk_state.cdclk = cnl_calc_cdclk(0); > cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk); > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > cnl_set_cdclk(dev_priv, &cdclk_state); > } > @@ -1764,6 +1773,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv) > > cdclk_state.cdclk = cdclk_state.ref; > cdclk_state.vco = 0; > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > cnl_set_cdclk(dev_priv, &cdclk_state); > } > @@ -2081,6 +2091,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > intel_state->cdclk.logical.vco = vco; > intel_state->cdclk.logical.cdclk = cdclk; > + intel_state->cdclk.logical.voltage = cnl_calc_voltage(cdclk); > > if (!intel_state->active_crtcs) { > cdclk = cnl_calc_cdclk(0); > @@ -2088,6 +2099,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > intel_state->cdclk.actual.vco = vco; > intel_state->cdclk.actual.cdclk = cdclk; > + intel_state->cdclk.actual.voltage = cnl_calc_voltage(cdclk); > } else { > intel_state->cdclk.actual = > intel_state->cdclk.logical; > -- > 2.13.6 >
On Wed, Oct 18, 2017 at 09:50:59PM +0000, Rodrigo Vivi wrote: > On Wed, Oct 18, 2017 at 08:48:24PM +0000, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Track the system agent voltage we request from pcode in the cdclk state > > on CNL. Annoyingly we can't actually read out the current value since > > there's no pcode command to do that, so we'll have to just assume that > > it worked. > > +static u32 cnl_cur_voltage(struct drm_i915_private *dev_priv) > +{ > + u32 voltage; > + sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &voltage); > + return voltage; > +} > + > static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > struct intel_cdclk_state *cdclk_state) > { > @@ -1486,8 +1493,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); > > + cdclk_state->voltage = cnl_cur_voltage(dev_priv); > } > > static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) > @@ -1594,8 +1600,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > intel_update_cdclk(dev_priv); > > + dev_priv->cdclk.hw.voltage = cnl_cur_voltage(dev_priv); Hmm... read works, but it triggers a [ 337.907234] [drm:pipe_config_err [i915]] *ERROR* mismatch in min_voltage (expected 0, found 1) Because the hw voltage is global while the minimal local is only relative to that pipe. I'm playing here with changes on top of the branch you sent yesterday... git://people.freedesktop.org/~vivijim/drm-intel dvfs-atomic trying to add the port_clock to the picture and the hw_state readout.. main doubt is where to get port_clock from during cnl_modeset_calc_cdclk? > > > > > > 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> > > --- > > drivers/gpu/drm/i915/intel_cdclk.c | 44 ++++++++++++++++++++++++-------------- > > 1 file changed, 28 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > > index 1b4dcd9689da..795a18f64c4c 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -1505,6 +1505,19 @@ static int cnl_calc_cdclk(int min_cdclk) > > return 168000; > > } > > > > +static u8 cnl_calc_voltage(int cdclk) > > +{ > > + switch (cdclk) { > > + default: > > + case 168000: > > + return 0; > > + case 336000: > > + return 1; > > + case 528000: > > + return 2; > > + } > > +} > > where is the port_clock taking into account? > > > + > > static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv, > > struct intel_cdclk_state *cdclk_state) > > { > > @@ -1538,7 +1551,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > cdclk_state->cdclk = cdclk_state->ref; > > > > if (cdclk_state->vco == 0) > > - return; > > + goto out; > > > > divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK; > > > > @@ -1555,6 +1568,13 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > } > > > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); > > + > > + out: > > + /* > > + * Can't read this out :( Let's assume it's > > + * at least what the CDCLK frequency requires. > > + */ > > + cdclk_state->voltage = cnl_calc_voltage(cdclk_state->cdclk); > > } > > > > static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) > > @@ -1595,7 +1615,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > { > > int cdclk = cdclk_state->cdclk; > > int vco = cdclk_state->vco; > > - u32 val, divider, pcu_ack; > > + u32 val, divider; > > int ret; > > > > mutex_lock(&dev_priv->pcu_lock); > > @@ -1624,19 +1644,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > break; > > } > > > > - switch (cdclk) { > > - case 528000: > > - pcu_ack = 2; > > - break; > > - case 336000: > > - pcu_ack = 1; > > - break; > > - case 168000: > > - default: > > - pcu_ack = 0; > > - break; > > - } > > - > > if (dev_priv->cdclk.hw.vco != 0 && > > dev_priv->cdclk.hw.vco != vco) > > cnl_cdclk_pll_disable(dev_priv); > > @@ -1654,7 +1661,8 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > > > /* inform PCU of the change */ > > mutex_lock(&dev_priv->pcu_lock); > > - sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack); > > + sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, > > + cdclk_state->voltage); > > mutex_unlock(&dev_priv->pcu_lock); > > > > intel_update_cdclk(dev_priv); > > @@ -1747,6 +1755,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv) > > > > cdclk_state.cdclk = cnl_calc_cdclk(0); > > cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk); > > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > > > cnl_set_cdclk(dev_priv, &cdclk_state); > > } > > @@ -1764,6 +1773,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv) > > > > cdclk_state.cdclk = cdclk_state.ref; > > cdclk_state.vco = 0; > > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > > > cnl_set_cdclk(dev_priv, &cdclk_state); > > } > > @@ -2081,6 +2091,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > > > intel_state->cdclk.logical.vco = vco; > > intel_state->cdclk.logical.cdclk = cdclk; > > + intel_state->cdclk.logical.voltage = cnl_calc_voltage(cdclk); > > > > if (!intel_state->active_crtcs) { > > cdclk = cnl_calc_cdclk(0); > > @@ -2088,6 +2099,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > > > intel_state->cdclk.actual.vco = vco; > > intel_state->cdclk.actual.cdclk = cdclk; > > + intel_state->cdclk.actual.voltage = cnl_calc_voltage(cdclk); > > } else { > > intel_state->cdclk.actual = > > intel_state->cdclk.logical; > > -- > > 2.13.6 > >
On Wed, Oct 18, 2017 at 02:50:59PM -0700, Rodrigo Vivi wrote: > On Wed, Oct 18, 2017 at 08:48:24PM +0000, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Track the system agent voltage we request from pcode in the cdclk state > > on CNL. Annoyingly we can't actually read out the current value since > > there's no pcode command to do that, so we'll have to just assume that > > it worked. > > +static u32 cnl_cur_voltage(struct drm_i915_private *dev_priv) > +{ > + u32 voltage; > + sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &voltage); I don't think that'll work. _pcode_read() and _pcode_write() are actually the same thing, the only difference is whether we return the resulting value or not. Thus if we actually tried to do this we'd end up setting the voltage instead of reading it. The only way to actually read anything is for pcode to specify a specific read command. It's a rather unfortunate design of the mailbox interface :( We should perhaps put some WARNS into the pcode read/write functions so that people don't accidentally use them in the wrong way. > + return voltage; > +} > + > static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > struct intel_cdclk_state *cdclk_state) > { > @@ -1486,8 +1493,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); > > + cdclk_state->voltage = cnl_cur_voltage(dev_priv); > } > > static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) > @@ -1594,8 +1600,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > intel_update_cdclk(dev_priv); > > + dev_priv->cdclk.hw.voltage = cnl_cur_voltage(dev_priv); > > > > > > 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> > > --- > > drivers/gpu/drm/i915/intel_cdclk.c | 44 ++++++++++++++++++++++++-------------- > > 1 file changed, 28 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > > index 1b4dcd9689da..795a18f64c4c 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -1505,6 +1505,19 @@ static int cnl_calc_cdclk(int min_cdclk) > > return 168000; > > } > > > > +static u8 cnl_calc_voltage(int cdclk) > > +{ > > + switch (cdclk) { > > + default: > > + case 168000: > > + return 0; > > + case 336000: > > + return 1; > > + case 528000: > > + return 2; > > + } > > +} > > where is the port_clock taking into account? Last patch of the series. > > > + > > static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv, > > struct intel_cdclk_state *cdclk_state) > > { > > @@ -1538,7 +1551,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > cdclk_state->cdclk = cdclk_state->ref; > > > > if (cdclk_state->vco == 0) > > - return; > > + goto out; > > > > divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK; > > > > @@ -1555,6 +1568,13 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > } > > > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); > > + > > + out: > > + /* > > + * Can't read this out :( Let's assume it's > > + * at least what the CDCLK frequency requires. > > + */ > > + cdclk_state->voltage = cnl_calc_voltage(cdclk_state->cdclk); > > } > > > > static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) > > @@ -1595,7 +1615,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > { > > int cdclk = cdclk_state->cdclk; > > int vco = cdclk_state->vco; > > - u32 val, divider, pcu_ack; > > + u32 val, divider; > > int ret; > > > > mutex_lock(&dev_priv->pcu_lock); > > @@ -1624,19 +1644,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > break; > > } > > > > - switch (cdclk) { > > - case 528000: > > - pcu_ack = 2; > > - break; > > - case 336000: > > - pcu_ack = 1; > > - break; > > - case 168000: > > - default: > > - pcu_ack = 0; > > - break; > > - } > > - > > if (dev_priv->cdclk.hw.vco != 0 && > > dev_priv->cdclk.hw.vco != vco) > > cnl_cdclk_pll_disable(dev_priv); > > @@ -1654,7 +1661,8 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > > > /* inform PCU of the change */ > > mutex_lock(&dev_priv->pcu_lock); > > - sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack); > > + sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, > > + cdclk_state->voltage); > > mutex_unlock(&dev_priv->pcu_lock); > > > > intel_update_cdclk(dev_priv); > > @@ -1747,6 +1755,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv) > > > > cdclk_state.cdclk = cnl_calc_cdclk(0); > > cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk); > > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > > > cnl_set_cdclk(dev_priv, &cdclk_state); > > } > > @@ -1764,6 +1773,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv) > > > > cdclk_state.cdclk = cdclk_state.ref; > > cdclk_state.vco = 0; > > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > > > cnl_set_cdclk(dev_priv, &cdclk_state); > > } > > @@ -2081,6 +2091,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > > > intel_state->cdclk.logical.vco = vco; > > intel_state->cdclk.logical.cdclk = cdclk; > > + intel_state->cdclk.logical.voltage = cnl_calc_voltage(cdclk); > > > > if (!intel_state->active_crtcs) { > > cdclk = cnl_calc_cdclk(0); > > @@ -2088,6 +2099,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > > > intel_state->cdclk.actual.vco = vco; > > intel_state->cdclk.actual.cdclk = cdclk; > > + intel_state->cdclk.actual.voltage = cnl_calc_voltage(cdclk); > > } else { > > intel_state->cdclk.actual = > > intel_state->cdclk.logical; > > -- > > 2.13.6 > >
On Thu, 2017-10-19 at 13:48 +0300, Ville Syrjälä wrote: > On Wed, Oct 18, 2017 at 02:50:59PM -0700, Rodrigo Vivi wrote: > > > > On Wed, Oct 18, 2017 at 08:48:24PM +0000, Ville Syrjala wrote: > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Track the system agent voltage we request from pcode in the cdclk > > > state > > > on CNL. Annoyingly we can't actually read out the current value > > > since > > > there's no pcode command to do that, so we'll have to just assume > > > that > > > it worked. > > +static u32 cnl_cur_voltage(struct drm_i915_private *dev_priv) > > +{ > > + u32 voltage; > > + sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, > > &voltage); > I don't think that'll work. _pcode_read() and _pcode_write() are > actually > the same thing, the only difference is whether we return the > resulting value > or not. Thus if we actually tried to do this we'd end up setting the > voltage instead of reading it. > > The only way to actually read anything is for pcode to specify a > specific read command. It's a rather unfortunate design of the > mailbox interface :( > > We should perhaps put some WARNS into the pcode read/write functions > so > that people don't accidentally use them in the wrong way. +1 for this. I was under the assumption that *read* means just reading out the value. > > > > > + return voltage; > > +} > > + > > static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > struct intel_cdclk_state *cdclk_state) > > { > > @@ -1486,8 +1493,7 @@ static void cnl_get_cdclk(struct > > drm_i915_private *dev_priv, > > > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, > > div); > > > > + cdclk_state->voltage = cnl_cur_voltage(dev_priv); > > } > > > > static void cnl_cdclk_pll_disable(struct drm_i915_private > > *dev_priv) > > @@ -1594,8 +1600,7 @@ static void cnl_set_cdclk(struct > > drm_i915_private *dev_priv, > > > > intel_update_cdclk(dev_priv); > > > > + dev_priv->cdclk.hw.voltage = cnl_cur_voltage(dev_priv); > > > > > > > > > > > > > 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> > > > --- > > > drivers/gpu/drm/i915/intel_cdclk.c | 44 > > > ++++++++++++++++++++++++-------------- > > > 1 file changed, 28 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c > > > b/drivers/gpu/drm/i915/intel_cdclk.c > > > index 1b4dcd9689da..795a18f64c4c 100644 > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > > @@ -1505,6 +1505,19 @@ static int cnl_calc_cdclk(int min_cdclk) > > > return 168000; > > > } > > > > > > +static u8 cnl_calc_voltage(int cdclk) > > > +{ > > > + switch (cdclk) { > > > + default: > > > + case 168000: > > > + return 0; > > > + case 336000: > > > + return 1; > > > + case 528000: > > > + return 2; > > > + } > > > +} > > where is the port_clock taking into account? > Last patch of the series. > > > > > > > > > > > + > > > static void cnl_cdclk_pll_update(struct drm_i915_private > > > *dev_priv, > > > struct intel_cdclk_state > > > *cdclk_state) > > > { > > > @@ -1538,7 +1551,7 @@ static void cnl_get_cdclk(struct > > > drm_i915_private *dev_priv, > > > cdclk_state->cdclk = cdclk_state->ref; > > > > > > if (cdclk_state->vco == 0) > > > - return; > > > + goto out; > > > > > > divider = I915_READ(CDCLK_CTL) & > > > BXT_CDCLK_CD2X_DIV_SEL_MASK; > > > > > > @@ -1555,6 +1568,13 @@ static void cnl_get_cdclk(struct > > > drm_i915_private *dev_priv, > > > } > > > > > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, > > > div); > > > + > > > + out: > > > + /* > > > + * Can't read this out :( Let's assume it's > > > + * at least what the CDCLK frequency requires. > > > + */ > > > + cdclk_state->voltage = cnl_calc_voltage(cdclk_state- > > > >cdclk); > > > } > > > > > > static void cnl_cdclk_pll_disable(struct drm_i915_private > > > *dev_priv) > > > @@ -1595,7 +1615,7 @@ static void cnl_set_cdclk(struct > > > drm_i915_private *dev_priv, > > > { > > > int cdclk = cdclk_state->cdclk; > > > int vco = cdclk_state->vco; > > > - u32 val, divider, pcu_ack; > > > + u32 val, divider; > > > int ret; > > > > > > mutex_lock(&dev_priv->pcu_lock); > > > @@ -1624,19 +1644,6 @@ static void cnl_set_cdclk(struct > > > drm_i915_private *dev_priv, > > > break; > > > } > > > > > > - switch (cdclk) { > > > - case 528000: > > > - pcu_ack = 2; > > > - break; > > > - case 336000: > > > - pcu_ack = 1; > > > - break; > > > - case 168000: > > > - default: > > > - pcu_ack = 0; > > > - break; > > > - } > > > - > > > if (dev_priv->cdclk.hw.vco != 0 && > > > dev_priv->cdclk.hw.vco != vco) > > > cnl_cdclk_pll_disable(dev_priv); > > > @@ -1654,7 +1661,8 @@ static void cnl_set_cdclk(struct > > > drm_i915_private *dev_priv, > > > > > > /* inform PCU of the change */ > > > mutex_lock(&dev_priv->pcu_lock); > > > - sandybridge_pcode_write(dev_priv, > > > SKL_PCODE_CDCLK_CONTROL, pcu_ack); > > > + sandybridge_pcode_write(dev_priv, > > > SKL_PCODE_CDCLK_CONTROL, > > > + cdclk_state->voltage); > > > mutex_unlock(&dev_priv->pcu_lock); > > > > > > intel_update_cdclk(dev_priv); > > > @@ -1747,6 +1755,7 @@ void cnl_init_cdclk(struct drm_i915_private > > > *dev_priv) > > > > > > cdclk_state.cdclk = cnl_calc_cdclk(0); > > > cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, > > > cdclk_state.cdclk); > > > + cdclk_state.voltage = > > > cnl_calc_voltage(cdclk_state.cdclk); > > > > > > cnl_set_cdclk(dev_priv, &cdclk_state); > > > } > > > @@ -1764,6 +1773,7 @@ void cnl_uninit_cdclk(struct > > > drm_i915_private *dev_priv) > > > > > > cdclk_state.cdclk = cdclk_state.ref; > > > cdclk_state.vco = 0; > > > + cdclk_state.voltage = > > > cnl_calc_voltage(cdclk_state.cdclk); > > > > > > cnl_set_cdclk(dev_priv, &cdclk_state); > > > } > > > @@ -2081,6 +2091,7 @@ static int cnl_modeset_calc_cdclk(struct > > > drm_atomic_state *state) > > > > > > intel_state->cdclk.logical.vco = vco; > > > intel_state->cdclk.logical.cdclk = cdclk; > > > + intel_state->cdclk.logical.voltage = > > > cnl_calc_voltage(cdclk); > > > > > > if (!intel_state->active_crtcs) { > > > cdclk = cnl_calc_cdclk(0); > > > @@ -2088,6 +2099,7 @@ static int cnl_modeset_calc_cdclk(struct > > > drm_atomic_state *state) > > > > > > intel_state->cdclk.actual.vco = vco; > > > intel_state->cdclk.actual.cdclk = cdclk; > > > + intel_state->cdclk.actual.voltage = > > > cnl_calc_voltage(cdclk); > > > } else { > > > intel_state->cdclk.actual = > > > intel_state->cdclk.logical; > > > -- > > > 2.13.6 > > >
On Thu, Oct 19, 2017 at 01:56:38PM +0300, Mika Kahola wrote: > On Thu, 2017-10-19 at 13:48 +0300, Ville Syrjälä wrote: > > On Wed, Oct 18, 2017 at 02:50:59PM -0700, Rodrigo Vivi wrote: > > > > > > On Wed, Oct 18, 2017 at 08:48:24PM +0000, Ville Syrjala wrote: > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > Track the system agent voltage we request from pcode in the cdclk > > > > state > > > > on CNL. Annoyingly we can't actually read out the current value > > > > since > > > > there's no pcode command to do that, so we'll have to just assume > > > > that > > > > it worked. > > > +static u32 cnl_cur_voltage(struct drm_i915_private *dev_priv) > > > +{ > > > + u32 voltage; > > > + sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, > > > &voltage); > > I don't think that'll work. _pcode_read() and _pcode_write() are > > actually > > the same thing, the only difference is whether we return the > > resulting value > > or not. Thus if we actually tried to do this we'd end up setting the > > voltage instead of reading it. > > > > The only way to actually read anything is for pcode to specify a > > specific read command. It's a rather unfortunate design of the > > mailbox interface :( > > > > We should perhaps put some WARNS into the pcode read/write functions > > so > > that people don't accidentally use them in the wrong way. > +1 for this. I was under the assumption that *read* means just reading > out the value. Do you want to volunteer for that? Looking at the current _read() uses, the only one that looks a bit suspicious is the DUTY_CYCLE_CONTROL. Not sure that's actually working as expected... Hmm, looks like it's fine actually. Some HSW PM doc I have says that data0=0x0 means "read dynamic RPe", and data0=0x1 means "send PCU interrupt when we can go back to RPe". We might want to add some defines for those magic values to make the code look less suspicious. Another thing we could do is deduplicate the code in the pcode_read/write functions. Eg. write() could just call read() maybe, or pull the code into a common _rw() helper (like we have for the VLV/CHV sideband stuff). Oh, and looks like intel_sbi_read/write could use the same treatment as well.
On Thu, Oct 19, 2017 at 10:48:04AM +0000, Ville Syrjälä wrote: > On Wed, Oct 18, 2017 at 02:50:59PM -0700, Rodrigo Vivi wrote: > > On Wed, Oct 18, 2017 at 08:48:24PM +0000, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Track the system agent voltage we request from pcode in the cdclk state > > > on CNL. Annoyingly we can't actually read out the current value since > > > there's no pcode command to do that, so we'll have to just assume that > > > it worked. > > > > +static u32 cnl_cur_voltage(struct drm_i915_private *dev_priv) > > +{ > > + u32 voltage; > > + sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &voltage); > > I don't think that'll work. _pcode_read() and _pcode_write() are actually > the same thing, the only difference is whether we return the resulting value > or not. Thus if we actually tried to do this we'd end up setting the > voltage instead of reading it. :( > > The only way to actually read anything is for pcode to specify a > specific read command. It's a rather unfortunate design of the > mailbox interface :( > > We should perhaps put some WARNS into the pcode read/write functions so > that people don't accidentally use them in the wrong way. > > > + return voltage; > > +} > > + > > static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > struct intel_cdclk_state *cdclk_state) > > { > > @@ -1486,8 +1493,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); > > > > + cdclk_state->voltage = cnl_cur_voltage(dev_priv); > > } > > > > static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) > > @@ -1594,8 +1600,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > > > intel_update_cdclk(dev_priv); > > > > + dev_priv->cdclk.hw.voltage = cnl_cur_voltage(dev_priv); > > > > > > > > > > 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> > > > --- > > > drivers/gpu/drm/i915/intel_cdclk.c | 44 ++++++++++++++++++++++++-------------- > > > 1 file changed, 28 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > > > index 1b4dcd9689da..795a18f64c4c 100644 > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > > @@ -1505,6 +1505,19 @@ static int cnl_calc_cdclk(int min_cdclk) > > > return 168000; > > > } > > > > > > +static u8 cnl_calc_voltage(int cdclk) > > > +{ > > > + switch (cdclk) { > > > + default: > > > + case 168000: > > > + return 0; > > > + case 336000: > > > + return 1; > > > + case 528000: > > > + return 2; > > > + } > > > +} > > > > where is the port_clock taking into account? > > Last patch of the series. > > > > > > + > > > static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv, > > > struct intel_cdclk_state *cdclk_state) > > > { > > > @@ -1538,7 +1551,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > > cdclk_state->cdclk = cdclk_state->ref; > > > > > > if (cdclk_state->vco == 0) > > > - return; > > > + goto out; > > > > > > divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK; > > > > > > @@ -1555,6 +1568,13 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > > > } > > > > > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); > > > + > > > + out: > > > + /* > > > + * Can't read this out :( Let's assume it's > > > + * at least what the CDCLK frequency requires. > > > + */ > > > + cdclk_state->voltage = cnl_calc_voltage(cdclk_state->cdclk); > > > } > > > > > > static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) > > > @@ -1595,7 +1615,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > > { > > > int cdclk = cdclk_state->cdclk; > > > int vco = cdclk_state->vco; > > > - u32 val, divider, pcu_ack; > > > + u32 val, divider; > > > int ret; > > > > > > mutex_lock(&dev_priv->pcu_lock); > > > @@ -1624,19 +1644,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > > break; > > > } > > > > > > - switch (cdclk) { > > > - case 528000: > > > - pcu_ack = 2; > > > - break; > > > - case 336000: > > > - pcu_ack = 1; > > > - break; > > > - case 168000: > > > - default: > > > - pcu_ack = 0; > > > - break; > > > - } > > > - > > > if (dev_priv->cdclk.hw.vco != 0 && > > > dev_priv->cdclk.hw.vco != vco) > > > cnl_cdclk_pll_disable(dev_priv); > > > @@ -1654,7 +1661,8 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > > > > > /* inform PCU of the change */ > > > mutex_lock(&dev_priv->pcu_lock); > > > - sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack); > > > + sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, > > > + cdclk_state->voltage); > > > mutex_unlock(&dev_priv->pcu_lock); > > > > > > intel_update_cdclk(dev_priv); > > > @@ -1747,6 +1755,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv) > > > > > > cdclk_state.cdclk = cnl_calc_cdclk(0); > > > cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk); > > > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > > > > > cnl_set_cdclk(dev_priv, &cdclk_state); > > > } > > > @@ -1764,6 +1773,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv) > > > > > > cdclk_state.cdclk = cdclk_state.ref; > > > cdclk_state.vco = 0; > > > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > > > > > cnl_set_cdclk(dev_priv, &cdclk_state); > > > } > > > @@ -2081,6 +2091,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > > > > > intel_state->cdclk.logical.vco = vco; > > > intel_state->cdclk.logical.cdclk = cdclk; > > > + intel_state->cdclk.logical.voltage = cnl_calc_voltage(cdclk); > > > > > > if (!intel_state->active_crtcs) { > > > cdclk = cnl_calc_cdclk(0); > > > @@ -2088,6 +2099,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > > > > > intel_state->cdclk.actual.vco = vco; > > > intel_state->cdclk.actual.cdclk = cdclk; > > > + intel_state->cdclk.actual.voltage = cnl_calc_voltage(cdclk); > > > } else { > > > intel_state->cdclk.actual = > > > intel_state->cdclk.logical; > > > -- > > > 2.13.6 > > > > > -- > Ville Syrjälä > Intel OTC
On Wed, Oct 18, 2017 at 08:48:24PM +0000, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Track the system agent voltage we request from pcode in the cdclk state > on CNL. Annoyingly we can't actually read out the current value since > there's no pcode command to do that, so we'll have to just assume that > it worked. > > 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> (even with new v with voltage_level name) Thanks for all explanations and discussions, this seems the right approach. > --- > drivers/gpu/drm/i915/intel_cdclk.c | 44 ++++++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index 1b4dcd9689da..795a18f64c4c 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1505,6 +1505,19 @@ static int cnl_calc_cdclk(int min_cdclk) > return 168000; > } > > +static u8 cnl_calc_voltage(int cdclk) > +{ > + switch (cdclk) { > + default: > + case 168000: > + return 0; > + case 336000: > + return 1; > + case 528000: > + return 2; > + } > +} > + > static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv, > struct intel_cdclk_state *cdclk_state) > { > @@ -1538,7 +1551,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > cdclk_state->cdclk = cdclk_state->ref; > > if (cdclk_state->vco == 0) > - return; > + goto out; > > divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK; > > @@ -1555,6 +1568,13 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, > } > > cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); > + > + out: > + /* > + * Can't read this out :( Let's assume it's > + * at least what the CDCLK frequency requires. > + */ > + cdclk_state->voltage = cnl_calc_voltage(cdclk_state->cdclk); > } > > static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) > @@ -1595,7 +1615,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > { > int cdclk = cdclk_state->cdclk; > int vco = cdclk_state->vco; > - u32 val, divider, pcu_ack; > + u32 val, divider; > int ret; > > mutex_lock(&dev_priv->pcu_lock); > @@ -1624,19 +1644,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > break; > } > > - switch (cdclk) { > - case 528000: > - pcu_ack = 2; > - break; > - case 336000: > - pcu_ack = 1; > - break; > - case 168000: > - default: > - pcu_ack = 0; > - break; > - } > - > if (dev_priv->cdclk.hw.vco != 0 && > dev_priv->cdclk.hw.vco != vco) > cnl_cdclk_pll_disable(dev_priv); > @@ -1654,7 +1661,8 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, > > /* inform PCU of the change */ > mutex_lock(&dev_priv->pcu_lock); > - sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack); > + sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, > + cdclk_state->voltage); > mutex_unlock(&dev_priv->pcu_lock); > > intel_update_cdclk(dev_priv); > @@ -1747,6 +1755,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv) > > cdclk_state.cdclk = cnl_calc_cdclk(0); > cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk); > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > cnl_set_cdclk(dev_priv, &cdclk_state); > } > @@ -1764,6 +1773,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv) > > cdclk_state.cdclk = cdclk_state.ref; > cdclk_state.vco = 0; > + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); > > cnl_set_cdclk(dev_priv, &cdclk_state); > } > @@ -2081,6 +2091,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > intel_state->cdclk.logical.vco = vco; > intel_state->cdclk.logical.cdclk = cdclk; > + intel_state->cdclk.logical.voltage = cnl_calc_voltage(cdclk); > > if (!intel_state->active_crtcs) { > cdclk = cnl_calc_cdclk(0); > @@ -2088,6 +2099,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) > > intel_state->cdclk.actual.vco = vco; > intel_state->cdclk.actual.cdclk = cdclk; > + intel_state->cdclk.actual.voltage = cnl_calc_voltage(cdclk); > } else { > intel_state->cdclk.actual = > intel_state->cdclk.logical; > -- > 2.13.6 >
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index 1b4dcd9689da..795a18f64c4c 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1505,6 +1505,19 @@ static int cnl_calc_cdclk(int min_cdclk) return 168000; } +static u8 cnl_calc_voltage(int cdclk) +{ + switch (cdclk) { + default: + case 168000: + return 0; + case 336000: + return 1; + case 528000: + return 2; + } +} + static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv, struct intel_cdclk_state *cdclk_state) { @@ -1538,7 +1551,7 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, cdclk_state->cdclk = cdclk_state->ref; if (cdclk_state->vco == 0) - return; + goto out; divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK; @@ -1555,6 +1568,13 @@ static void cnl_get_cdclk(struct drm_i915_private *dev_priv, } cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div); + + out: + /* + * Can't read this out :( Let's assume it's + * at least what the CDCLK frequency requires. + */ + cdclk_state->voltage = cnl_calc_voltage(cdclk_state->cdclk); } static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv) @@ -1595,7 +1615,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, { int cdclk = cdclk_state->cdclk; int vco = cdclk_state->vco; - u32 val, divider, pcu_ack; + u32 val, divider; int ret; mutex_lock(&dev_priv->pcu_lock); @@ -1624,19 +1644,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, break; } - switch (cdclk) { - case 528000: - pcu_ack = 2; - break; - case 336000: - pcu_ack = 1; - break; - case 168000: - default: - pcu_ack = 0; - break; - } - if (dev_priv->cdclk.hw.vco != 0 && dev_priv->cdclk.hw.vco != vco) cnl_cdclk_pll_disable(dev_priv); @@ -1654,7 +1661,8 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv, /* inform PCU of the change */ mutex_lock(&dev_priv->pcu_lock); - sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack); + sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, + cdclk_state->voltage); mutex_unlock(&dev_priv->pcu_lock); intel_update_cdclk(dev_priv); @@ -1747,6 +1755,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv) cdclk_state.cdclk = cnl_calc_cdclk(0); cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk); + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); cnl_set_cdclk(dev_priv, &cdclk_state); } @@ -1764,6 +1773,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv) cdclk_state.cdclk = cdclk_state.ref; cdclk_state.vco = 0; + cdclk_state.voltage = cnl_calc_voltage(cdclk_state.cdclk); cnl_set_cdclk(dev_priv, &cdclk_state); } @@ -2081,6 +2091,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) intel_state->cdclk.logical.vco = vco; intel_state->cdclk.logical.cdclk = cdclk; + intel_state->cdclk.logical.voltage = cnl_calc_voltage(cdclk); if (!intel_state->active_crtcs) { cdclk = cnl_calc_cdclk(0); @@ -2088,6 +2099,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state) intel_state->cdclk.actual.vco = vco; intel_state->cdclk.actual.cdclk = cdclk; + intel_state->cdclk.actual.voltage = cnl_calc_voltage(cdclk); } else { intel_state->cdclk.actual = intel_state->cdclk.logical;