Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-18 Thread Ville Syrjälä 
On Tue, Oct 17, 2017 at 04:23:07PM -0700, Rodrigo Vivi wrote:
> On Tue, Oct 17, 2017 at 08:36:14PM +, Ville Syrjälä wrote:
> > On Tue, Oct 17, 2017 at 09:02:05PM +0300, Ville Syrjälä wrote:
> > > On Tue, Oct 17, 2017 at 10:45:22AM -0700, Rodrigo Vivi wrote:
> > > > On Tue, Oct 17, 2017 at 05:23:20PM +, Ville Syrjälä wrote:
> > > > > On Tue, Oct 17, 2017 at 09:47:05AM -0700, Rodrigo Vivi wrote:
> > > > > > On Tue, Oct 17, 2017 at 03:44:21PM +, Ville Syrjälä wrote:
> > > > > > > On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> > > > > > > > From: "Kahola, Mika" 
> > > > > > > > 
> > > > > > > > Display Voltage and Frequency Switching (DVFS) is used to 
> > > > > > > > adjust the
> > > > > > > > display voltage to match the display clock frequencies. If 
> > > > > > > > voltage is
> > > > > > > > set too low, it will break functionality. If voltage is set too 
> > > > > > > > high,
> > > > > > > > it will waste power. Voltage level is selected based on CD 
> > > > > > > > clock and
> > > > > > > > DDI clock.
> > > > > > > > 
> > > > > > > > The sequence before frequency change is the following and it 
> > > > > > > > requests
> > > > > > > > the power controller to raise voltage to maximum
> > > > > > > > 
> > > > > > > > - Ensure any previous GT Driver Mailbox transaction is complete.
> > > > > > > > - Write GT Driver Mailbox Data Low = 0x3.
> > > > > > > > - Write GT Driver Mailbox Data High = 0x0.
> > > > > > > > - Write GT Driver Mailbox Interface = 0x8007.
> > > > > > > > - Poll GT Driver Mailbox Interface for Run/Busy indication 
> > > > > > > > cleared (bit 31 = 0).
> > > > > > > > - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, 
> > > > > > > > else restart the sequence.
> > > > > > > >   Timeout after 3ms
> > > > > > > > 
> > > > > > > > The sequence after frequency change is the following and it 
> > > > > > > > requests
> > > > > > > > the port controller to raise voltage to the requested level.
> > > > > > > > 
> > > > > > > > - Write GT Driver Mailbox Data Low
> > > > > > > >  * For level 0, write 0x0
> > > > > > > >  * For level 1, write 0x1
> > > > > > > >  * For level 2, write 0x2
> > > > > > > >  * For level 3, write 0x3
> > > > > > > >- Write GT Driver Mailbox Data High = 0x0.
> > > > > > > >- Write GT Driver Mailbox Interface = 0x8007.
> > > > > > > > 
> > > > > > > > For Cannonlake, the level 3 is not used and it aliases to level 
> > > > > > > > 2.
> > > > > > > > 
> > > > > > > > v2: reuse Paulo's work on cdclk. This patch depends on Paulo's 
> > > > > > > > patch
> > > > > > > > [PATCH 02/12] drm/i915/cnl: extract 
> > > > > > > > cnl_dvfs_{pre,post}_change
> > > > > > > > v3: (By Rodrigo): Remove duplicated commend and fix typo on 
> > > > > > > > Paulo's name.
> > > > > > > > v4: (By Rodrigo): Rebase on top "Unify and export gen9+ 
> > > > > > > > port_clock calculation"
> > > > > > > > The port clock calculation here was only addressing DP, so 
> > > > > > > > let's reuse
> > > > > > > > the current port calculation that is already in place 
> > > > > > > > without any duplication.
> > > > > > > > Alos fix portclk <= 594000 instead of portclk < 594000.
> > > > > > > > 
> > > > > > > > Cc: Paulo Zanoni 
> > > > > > > > Cc: Ville Syrjälä 
> > > > > > > > Signed-off-by: Kahola, Mika 
> > > > > > > > Signed-off-by: Rodrigo Vivi 
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 
> > > > > > > > ++
> > > > > > > >  1 file changed, 22 insertions(+), 8 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> > > > > > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > > > index a2a3d93d67bd..6030fbafa580 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > > > @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr 
> > > > > > > > bxt_pll_mgr = {
> > > > > > > > .dump_hw_state = bxt_dump_hw_state,
> > > > > > > >  };
> > > > > > > >  
> > > > > > > > +static int cnl_get_dvfs_level(int cdclk, int portclk)
> > > > > > > > +{
> > > > > > > > +   if (cdclk == 168000 && portclk <= 594000)
> > > > > > > > +   return 0;
> > > > > > > > +   else if (cdclk == 336000 && portclk <= 594000)
> > > > > > > > +   return 1;
> > > > > > > > +   else
> > > > > > > > +   return 2;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  static void cnl_ddi_pll_enable(struct drm_i915_private 
> > > > > > > > *dev_priv,
> > > > > > > >struct intel_shared_dpll *pll)
> > > > > > > >  {
> > > > > > > > uint32_t val;
> > > > > > > > +   int ret;
> > > > > > > > +   int level;
> > > > > > > > +   int cdclk, portclk;
> > > > > > > >  
> > > > > > > > /* 1. Enable DPLL power in DPLL_ENABLE. */
> > > > > > > > val = I915_READ(CNL_DPLL_E

Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-17 Thread Rodrigo Vivi 
On Tue, Oct 17, 2017 at 08:36:14PM +, Ville Syrjälä wrote:
> On Tue, Oct 17, 2017 at 09:02:05PM +0300, Ville Syrjälä wrote:
> > On Tue, Oct 17, 2017 at 10:45:22AM -0700, Rodrigo Vivi wrote:
> > > On Tue, Oct 17, 2017 at 05:23:20PM +, Ville Syrjälä wrote:
> > > > On Tue, Oct 17, 2017 at 09:47:05AM -0700, Rodrigo Vivi wrote:
> > > > > On Tue, Oct 17, 2017 at 03:44:21PM +, Ville Syrjälä wrote:
> > > > > > On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> > > > > > > From: "Kahola, Mika" 
> > > > > > > 
> > > > > > > Display Voltage and Frequency Switching (DVFS) is used to adjust 
> > > > > > > the
> > > > > > > display voltage to match the display clock frequencies. If 
> > > > > > > voltage is
> > > > > > > set too low, it will break functionality. If voltage is set too 
> > > > > > > high,
> > > > > > > it will waste power. Voltage level is selected based on CD clock 
> > > > > > > and
> > > > > > > DDI clock.
> > > > > > > 
> > > > > > > The sequence before frequency change is the following and it 
> > > > > > > requests
> > > > > > > the power controller to raise voltage to maximum
> > > > > > > 
> > > > > > > - Ensure any previous GT Driver Mailbox transaction is complete.
> > > > > > > - Write GT Driver Mailbox Data Low = 0x3.
> > > > > > > - Write GT Driver Mailbox Data High = 0x0.
> > > > > > > - Write GT Driver Mailbox Interface = 0x8007.
> > > > > > > - Poll GT Driver Mailbox Interface for Run/Busy indication 
> > > > > > > cleared (bit 31 = 0).
> > > > > > > - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, 
> > > > > > > else restart the sequence.
> > > > > > >   Timeout after 3ms
> > > > > > > 
> > > > > > > The sequence after frequency change is the following and it 
> > > > > > > requests
> > > > > > > the port controller to raise voltage to the requested level.
> > > > > > > 
> > > > > > > - Write GT Driver Mailbox Data Low
> > > > > > >  * For level 0, write 0x0
> > > > > > >  * For level 1, write 0x1
> > > > > > >  * For level 2, write 0x2
> > > > > > >  * For level 3, write 0x3
> > > > > > >- Write GT Driver Mailbox Data High = 0x0.
> > > > > > >- Write GT Driver Mailbox Interface = 0x8007.
> > > > > > > 
> > > > > > > For Cannonlake, the level 3 is not used and it aliases to level 2.
> > > > > > > 
> > > > > > > v2: reuse Paulo's work on cdclk. This patch depends on Paulo's 
> > > > > > > patch
> > > > > > > [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
> > > > > > > v3: (By Rodrigo): Remove duplicated commend and fix typo on 
> > > > > > > Paulo's name.
> > > > > > > v4: (By Rodrigo): Rebase on top "Unify and export gen9+ 
> > > > > > > port_clock calculation"
> > > > > > > The port clock calculation here was only addressing DP, so 
> > > > > > > let's reuse
> > > > > > > the current port calculation that is already in place without 
> > > > > > > any duplication.
> > > > > > > Alos fix portclk <= 594000 instead of portclk < 594000.
> > > > > > > 
> > > > > > > Cc: Paulo Zanoni 
> > > > > > > Cc: Ville Syrjälä 
> > > > > > > Signed-off-by: Kahola, Mika 
> > > > > > > Signed-off-by: Rodrigo Vivi 
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 
> > > > > > > ++
> > > > > > >  1 file changed, 22 insertions(+), 8 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> > > > > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > > index a2a3d93d67bd..6030fbafa580 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > > @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr 
> > > > > > > bxt_pll_mgr = {
> > > > > > >   .dump_hw_state = bxt_dump_hw_state,
> > > > > > >  };
> > > > > > >  
> > > > > > > +static int cnl_get_dvfs_level(int cdclk, int portclk)
> > > > > > > +{
> > > > > > > + if (cdclk == 168000 && portclk <= 594000)
> > > > > > > + return 0;
> > > > > > > + else if (cdclk == 336000 && portclk <= 594000)
> > > > > > > + return 1;
> > > > > > > + else
> > > > > > > + return 2;
> > > > > > > +}
> > > > > > > +
> > > > > > >  static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
> > > > > > >  struct intel_shared_dpll *pll)
> > > > > > >  {
> > > > > > >   uint32_t val;
> > > > > > > + int ret;
> > > > > > > + int level;
> > > > > > > + int cdclk, portclk;
> > > > > > >  
> > > > > > >   /* 1. Enable DPLL power in DPLL_ENABLE. */
> > > > > > >   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > > > > > > @@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct 
> > > > > > > drm_i915_private *dev_priv,
> > > > > > >   /*
> > > > > > >* 5. If the frequency will result in a change to the voltage
> > > > > > >* requirement, follow the Display Voltage Frequency Switching
> > > > > > > -  * Sequence Before Frequency Change
> > > > > > > -  *
> > > > > > >

Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-17 Thread Ville Syrjälä 
On Tue, Oct 17, 2017 at 09:02:05PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 17, 2017 at 10:45:22AM -0700, Rodrigo Vivi wrote:
> > On Tue, Oct 17, 2017 at 05:23:20PM +, Ville Syrjälä wrote:
> > > On Tue, Oct 17, 2017 at 09:47:05AM -0700, Rodrigo Vivi wrote:
> > > > On Tue, Oct 17, 2017 at 03:44:21PM +, Ville Syrjälä wrote:
> > > > > On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> > > > > > From: "Kahola, Mika" 
> > > > > > 
> > > > > > Display Voltage and Frequency Switching (DVFS) is used to adjust the
> > > > > > display voltage to match the display clock frequencies. If voltage 
> > > > > > is
> > > > > > set too low, it will break functionality. If voltage is set too 
> > > > > > high,
> > > > > > it will waste power. Voltage level is selected based on CD clock and
> > > > > > DDI clock.
> > > > > > 
> > > > > > The sequence before frequency change is the following and it 
> > > > > > requests
> > > > > > the power controller to raise voltage to maximum
> > > > > > 
> > > > > > - Ensure any previous GT Driver Mailbox transaction is complete.
> > > > > > - Write GT Driver Mailbox Data Low = 0x3.
> > > > > > - Write GT Driver Mailbox Data High = 0x0.
> > > > > > - Write GT Driver Mailbox Interface = 0x8007.
> > > > > > - Poll GT Driver Mailbox Interface for Run/Busy indication cleared 
> > > > > > (bit 31 = 0).
> > > > > > - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else 
> > > > > > restart the sequence.
> > > > > >   Timeout after 3ms
> > > > > > 
> > > > > > The sequence after frequency change is the following and it requests
> > > > > > the port controller to raise voltage to the requested level.
> > > > > > 
> > > > > > - Write GT Driver Mailbox Data Low
> > > > > >  * For level 0, write 0x0
> > > > > >  * For level 1, write 0x1
> > > > > >  * For level 2, write 0x2
> > > > > >  * For level 3, write 0x3
> > > > > >- Write GT Driver Mailbox Data High = 0x0.
> > > > > >- Write GT Driver Mailbox Interface = 0x8007.
> > > > > > 
> > > > > > For Cannonlake, the level 3 is not used and it aliases to level 2.
> > > > > > 
> > > > > > v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
> > > > > > [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
> > > > > > v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's 
> > > > > > name.
> > > > > > v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock 
> > > > > > calculation"
> > > > > > The port clock calculation here was only addressing DP, so 
> > > > > > let's reuse
> > > > > > the current port calculation that is already in place without 
> > > > > > any duplication.
> > > > > > Alos fix portclk <= 594000 instead of portclk < 594000.
> > > > > > 
> > > > > > Cc: Paulo Zanoni 
> > > > > > Cc: Ville Syrjälä 
> > > > > > Signed-off-by: Kahola, Mika 
> > > > > > Signed-off-by: Rodrigo Vivi 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 
> > > > > > ++
> > > > > >  1 file changed, 22 insertions(+), 8 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> > > > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > index a2a3d93d67bd..6030fbafa580 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr 
> > > > > > bxt_pll_mgr = {
> > > > > > .dump_hw_state = bxt_dump_hw_state,
> > > > > >  };
> > > > > >  
> > > > > > +static int cnl_get_dvfs_level(int cdclk, int portclk)
> > > > > > +{
> > > > > > +   if (cdclk == 168000 && portclk <= 594000)
> > > > > > +   return 0;
> > > > > > +   else if (cdclk == 336000 && portclk <= 594000)
> > > > > > +   return 1;
> > > > > > +   else
> > > > > > +   return 2;
> > > > > > +}
> > > > > > +
> > > > > >  static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
> > > > > >struct intel_shared_dpll *pll)
> > > > > >  {
> > > > > > uint32_t val;
> > > > > > +   int ret;
> > > > > > +   int level;
> > > > > > +   int cdclk, portclk;
> > > > > >  
> > > > > > /* 1. Enable DPLL power in DPLL_ENABLE. */
> > > > > > val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > > > > > @@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct 
> > > > > > drm_i915_private *dev_priv,
> > > > > > /*
> > > > > >  * 5. If the frequency will result in a change to the voltage
> > > > > >  * requirement, follow the Display Voltage Frequency Switching
> > > > > > -* Sequence Before Frequency Change
> > > > > > -*
> > > > > > -* FIXME: (DVFS) is used to adjust the display voltage to match 
> > > > > > the
> > > > > > -* display clock frequencies
> > > > > > +* (DVFS) Sequence Before Frequency Change
> > > > > >  */
> > > > > > +   ret = cnl_dvfs_pre_change(dev_priv);
> > > > > >  
> > > > > >

Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-17 Thread Ville Syrjälä 
On Tue, Oct 17, 2017 at 10:45:22AM -0700, Rodrigo Vivi wrote:
> On Tue, Oct 17, 2017 at 05:23:20PM +, Ville Syrjälä wrote:
> > On Tue, Oct 17, 2017 at 09:47:05AM -0700, Rodrigo Vivi wrote:
> > > On Tue, Oct 17, 2017 at 03:44:21PM +, Ville Syrjälä wrote:
> > > > On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> > > > > From: "Kahola, Mika" 
> > > > > 
> > > > > Display Voltage and Frequency Switching (DVFS) is used to adjust the
> > > > > display voltage to match the display clock frequencies. If voltage is
> > > > > set too low, it will break functionality. If voltage is set too high,
> > > > > it will waste power. Voltage level is selected based on CD clock and
> > > > > DDI clock.
> > > > > 
> > > > > The sequence before frequency change is the following and it requests
> > > > > the power controller to raise voltage to maximum
> > > > > 
> > > > > - Ensure any previous GT Driver Mailbox transaction is complete.
> > > > > - Write GT Driver Mailbox Data Low = 0x3.
> > > > > - Write GT Driver Mailbox Data High = 0x0.
> > > > > - Write GT Driver Mailbox Interface = 0x8007.
> > > > > - Poll GT Driver Mailbox Interface for Run/Busy indication cleared 
> > > > > (bit 31 = 0).
> > > > > - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else 
> > > > > restart the sequence.
> > > > >   Timeout after 3ms
> > > > > 
> > > > > The sequence after frequency change is the following and it requests
> > > > > the port controller to raise voltage to the requested level.
> > > > > 
> > > > > - Write GT Driver Mailbox Data Low
> > > > >  * For level 0, write 0x0
> > > > >  * For level 1, write 0x1
> > > > >  * For level 2, write 0x2
> > > > >  * For level 3, write 0x3
> > > > >- Write GT Driver Mailbox Data High = 0x0.
> > > > >- Write GT Driver Mailbox Interface = 0x8007.
> > > > > 
> > > > > For Cannonlake, the level 3 is not used and it aliases to level 2.
> > > > > 
> > > > > v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
> > > > > [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
> > > > > v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's 
> > > > > name.
> > > > > v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock 
> > > > > calculation"
> > > > > The port clock calculation here was only addressing DP, so let's 
> > > > > reuse
> > > > > the current port calculation that is already in place without any 
> > > > > duplication.
> > > > > Alos fix portclk <= 594000 instead of portclk < 594000.
> > > > > 
> > > > > Cc: Paulo Zanoni 
> > > > > Cc: Ville Syrjälä 
> > > > > Signed-off-by: Kahola, Mika 
> > > > > Signed-off-by: Rodrigo Vivi 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 
> > > > > ++
> > > > >  1 file changed, 22 insertions(+), 8 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> > > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > index a2a3d93d67bd..6030fbafa580 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr 
> > > > > bxt_pll_mgr = {
> > > > >   .dump_hw_state = bxt_dump_hw_state,
> > > > >  };
> > > > >  
> > > > > +static int cnl_get_dvfs_level(int cdclk, int portclk)
> > > > > +{
> > > > > + if (cdclk == 168000 && portclk <= 594000)
> > > > > + return 0;
> > > > > + else if (cdclk == 336000 && portclk <= 594000)
> > > > > + return 1;
> > > > > + else
> > > > > + return 2;
> > > > > +}
> > > > > +
> > > > >  static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
> > > > >  struct intel_shared_dpll *pll)
> > > > >  {
> > > > >   uint32_t val;
> > > > > + int ret;
> > > > > + int level;
> > > > > + int cdclk, portclk;
> > > > >  
> > > > >   /* 1. Enable DPLL power in DPLL_ENABLE. */
> > > > >   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > > > > @@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct 
> > > > > drm_i915_private *dev_priv,
> > > > >   /*
> > > > >* 5. If the frequency will result in a change to the voltage
> > > > >* requirement, follow the Display Voltage Frequency Switching
> > > > > -  * Sequence Before Frequency Change
> > > > > -  *
> > > > > -  * FIXME: (DVFS) is used to adjust the display voltage to match 
> > > > > the
> > > > > -  * display clock frequencies
> > > > > +  * (DVFS) Sequence Before Frequency Change
> > > > >*/
> > > > > + ret = cnl_dvfs_pre_change(dev_priv);
> > > > >  
> > > > >   /* 6. Enable DPLL in DPLL_ENABLE. */
> > > > >   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > > > > @@ -2028,11 +2039,14 @@ static void cnl_ddi_pll_enable(struct 
> > > > > drm_i915_private *dev_priv,
> > > > >   /*
> > > > >* 8. If the fre

Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-17 Thread Rodrigo Vivi 
On Tue, Oct 17, 2017 at 05:23:20PM +, Ville Syrjälä wrote:
> On Tue, Oct 17, 2017 at 09:47:05AM -0700, Rodrigo Vivi wrote:
> > On Tue, Oct 17, 2017 at 03:44:21PM +, Ville Syrjälä wrote:
> > > On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> > > > From: "Kahola, Mika" 
> > > > 
> > > > Display Voltage and Frequency Switching (DVFS) is used to adjust the
> > > > display voltage to match the display clock frequencies. If voltage is
> > > > set too low, it will break functionality. If voltage is set too high,
> > > > it will waste power. Voltage level is selected based on CD clock and
> > > > DDI clock.
> > > > 
> > > > The sequence before frequency change is the following and it requests
> > > > the power controller to raise voltage to maximum
> > > > 
> > > > - Ensure any previous GT Driver Mailbox transaction is complete.
> > > > - Write GT Driver Mailbox Data Low = 0x3.
> > > > - Write GT Driver Mailbox Data High = 0x0.
> > > > - Write GT Driver Mailbox Interface = 0x8007.
> > > > - Poll GT Driver Mailbox Interface for Run/Busy indication cleared (bit 
> > > > 31 = 0).
> > > > - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else 
> > > > restart the sequence.
> > > >   Timeout after 3ms
> > > > 
> > > > The sequence after frequency change is the following and it requests
> > > > the port controller to raise voltage to the requested level.
> > > > 
> > > > - Write GT Driver Mailbox Data Low
> > > >  * For level 0, write 0x0
> > > >  * For level 1, write 0x1
> > > >  * For level 2, write 0x2
> > > >  * For level 3, write 0x3
> > > >- Write GT Driver Mailbox Data High = 0x0.
> > > >- Write GT Driver Mailbox Interface = 0x8007.
> > > > 
> > > > For Cannonlake, the level 3 is not used and it aliases to level 2.
> > > > 
> > > > v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
> > > > [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
> > > > v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's 
> > > > name.
> > > > v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock 
> > > > calculation"
> > > > The port clock calculation here was only addressing DP, so let's 
> > > > reuse
> > > > the current port calculation that is already in place without any 
> > > > duplication.
> > > > Alos fix portclk <= 594000 instead of portclk < 594000.
> > > > 
> > > > Cc: Paulo Zanoni 
> > > > Cc: Ville Syrjälä 
> > > > Signed-off-by: Kahola, Mika 
> > > > Signed-off-by: Rodrigo Vivi 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 
> > > > ++
> > > >  1 file changed, 22 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > index a2a3d93d67bd..6030fbafa580 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr bxt_pll_mgr 
> > > > = {
> > > > .dump_hw_state = bxt_dump_hw_state,
> > > >  };
> > > >  
> > > > +static int cnl_get_dvfs_level(int cdclk, int portclk)
> > > > +{
> > > > +   if (cdclk == 168000 && portclk <= 594000)
> > > > +   return 0;
> > > > +   else if (cdclk == 336000 && portclk <= 594000)
> > > > +   return 1;
> > > > +   else
> > > > +   return 2;
> > > > +}
> > > > +
> > > >  static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
> > > >struct intel_shared_dpll *pll)
> > > >  {
> > > > uint32_t val;
> > > > +   int ret;
> > > > +   int level;
> > > > +   int cdclk, portclk;
> > > >  
> > > > /* 1. Enable DPLL power in DPLL_ENABLE. */
> > > > val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > > > @@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct 
> > > > drm_i915_private *dev_priv,
> > > > /*
> > > >  * 5. If the frequency will result in a change to the voltage
> > > >  * requirement, follow the Display Voltage Frequency Switching
> > > > -* Sequence Before Frequency Change
> > > > -*
> > > > -* FIXME: (DVFS) is used to adjust the display voltage to match 
> > > > the
> > > > -* display clock frequencies
> > > > +* (DVFS) Sequence Before Frequency Change
> > > >  */
> > > > +   ret = cnl_dvfs_pre_change(dev_priv);
> > > >  
> > > > /* 6. Enable DPLL in DPLL_ENABLE. */
> > > > val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > > > @@ -2028,11 +2039,14 @@ static void cnl_ddi_pll_enable(struct 
> > > > drm_i915_private *dev_priv,
> > > > /*
> > > >  * 8. If the frequency will result in a change to the voltage
> > > >  * requirement, follow the Display Voltage Frequency Switching
> > > > -* Sequence After Frequency Change
> > > > -*
> > > > -* FIXME: (DV

Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-17 Thread Ville Syrjälä 
On Tue, Oct 17, 2017 at 09:47:05AM -0700, Rodrigo Vivi wrote:
> On Tue, Oct 17, 2017 at 03:44:21PM +, Ville Syrjälä wrote:
> > On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> > > From: "Kahola, Mika" 
> > > 
> > > Display Voltage and Frequency Switching (DVFS) is used to adjust the
> > > display voltage to match the display clock frequencies. If voltage is
> > > set too low, it will break functionality. If voltage is set too high,
> > > it will waste power. Voltage level is selected based on CD clock and
> > > DDI clock.
> > > 
> > > The sequence before frequency change is the following and it requests
> > > the power controller to raise voltage to maximum
> > > 
> > > - Ensure any previous GT Driver Mailbox transaction is complete.
> > > - Write GT Driver Mailbox Data Low = 0x3.
> > > - Write GT Driver Mailbox Data High = 0x0.
> > > - Write GT Driver Mailbox Interface = 0x8007.
> > > - Poll GT Driver Mailbox Interface for Run/Busy indication cleared (bit 
> > > 31 = 0).
> > > - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else 
> > > restart the sequence.
> > >   Timeout after 3ms
> > > 
> > > The sequence after frequency change is the following and it requests
> > > the port controller to raise voltage to the requested level.
> > > 
> > > - Write GT Driver Mailbox Data Low
> > >  * For level 0, write 0x0
> > >  * For level 1, write 0x1
> > >  * For level 2, write 0x2
> > >  * For level 3, write 0x3
> > >- Write GT Driver Mailbox Data High = 0x0.
> > >- Write GT Driver Mailbox Interface = 0x8007.
> > > 
> > > For Cannonlake, the level 3 is not used and it aliases to level 2.
> > > 
> > > v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
> > > [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
> > > v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's name.
> > > v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock 
> > > calculation"
> > > The port clock calculation here was only addressing DP, so let's reuse
> > > the current port calculation that is already in place without any 
> > > duplication.
> > > Alos fix portclk <= 594000 instead of portclk < 594000.
> > > 
> > > Cc: Paulo Zanoni 
> > > Cc: Ville Syrjälä 
> > > Signed-off-by: Kahola, Mika 
> > > Signed-off-by: Rodrigo Vivi 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 ++
> > >  1 file changed, 22 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > index a2a3d93d67bd..6030fbafa580 100644
> > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr bxt_pll_mgr = {
> > >   .dump_hw_state = bxt_dump_hw_state,
> > >  };
> > >  
> > > +static int cnl_get_dvfs_level(int cdclk, int portclk)
> > > +{
> > > + if (cdclk == 168000 && portclk <= 594000)
> > > + return 0;
> > > + else if (cdclk == 336000 && portclk <= 594000)
> > > + return 1;
> > > + else
> > > + return 2;
> > > +}
> > > +
> > >  static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
> > >  struct intel_shared_dpll *pll)
> > >  {
> > >   uint32_t val;
> > > + int ret;
> > > + int level;
> > > + int cdclk, portclk;
> > >  
> > >   /* 1. Enable DPLL power in DPLL_ENABLE. */
> > >   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > > @@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct 
> > > drm_i915_private *dev_priv,
> > >   /*
> > >* 5. If the frequency will result in a change to the voltage
> > >* requirement, follow the Display Voltage Frequency Switching
> > > -  * Sequence Before Frequency Change
> > > -  *
> > > -  * FIXME: (DVFS) is used to adjust the display voltage to match the
> > > -  * display clock frequencies
> > > +  * (DVFS) Sequence Before Frequency Change
> > >*/
> > > + ret = cnl_dvfs_pre_change(dev_priv);
> > >  
> > >   /* 6. Enable DPLL in DPLL_ENABLE. */
> > >   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > > @@ -2028,11 +2039,14 @@ static void cnl_ddi_pll_enable(struct 
> > > drm_i915_private *dev_priv,
> > >   /*
> > >* 8. If the frequency will result in a change to the voltage
> > >* requirement, follow the Display Voltage Frequency Switching
> > > -  * Sequence After Frequency Change
> > > -  *
> > > -  * FIXME: (DVFS) is used to adjust the display voltage to match the
> > > -  * display clock frequencies
> > > +  * (DVFS) Sequence After Frequency Change
> > >*/
> > > + if (ret == 0) {
> > > + cdclk = dev_priv->cdclk.hw.cdclk;
> > > + portclk = intel_ddi_port_clock(dev_priv, pll->id);
> > > + level = cnl_get_dvfs_level(cdclk, portclk);
> > > + cnl_dvfs_post_change(dev_priv, level);
> > 
> > This isn't how I imagined we'd handle this. What I was after is
> > pre-computing the correct "

Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-17 Thread Rodrigo Vivi 
On Tue, Oct 17, 2017 at 03:44:21PM +, Ville Syrjälä wrote:
> On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> > From: "Kahola, Mika" 
> > 
> > Display Voltage and Frequency Switching (DVFS) is used to adjust the
> > display voltage to match the display clock frequencies. If voltage is
> > set too low, it will break functionality. If voltage is set too high,
> > it will waste power. Voltage level is selected based on CD clock and
> > DDI clock.
> > 
> > The sequence before frequency change is the following and it requests
> > the power controller to raise voltage to maximum
> > 
> > - Ensure any previous GT Driver Mailbox transaction is complete.
> > - Write GT Driver Mailbox Data Low = 0x3.
> > - Write GT Driver Mailbox Data High = 0x0.
> > - Write GT Driver Mailbox Interface = 0x8007.
> > - Poll GT Driver Mailbox Interface for Run/Busy indication cleared (bit 31 
> > = 0).
> > - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else restart 
> > the sequence.
> >   Timeout after 3ms
> > 
> > The sequence after frequency change is the following and it requests
> > the port controller to raise voltage to the requested level.
> > 
> > - Write GT Driver Mailbox Data Low
> >  * For level 0, write 0x0
> >  * For level 1, write 0x1
> >  * For level 2, write 0x2
> >  * For level 3, write 0x3
> >- Write GT Driver Mailbox Data High = 0x0.
> >- Write GT Driver Mailbox Interface = 0x8007.
> > 
> > For Cannonlake, the level 3 is not used and it aliases to level 2.
> > 
> > v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
> > [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
> > v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's name.
> > v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock 
> > calculation"
> > The port clock calculation here was only addressing DP, so let's reuse
> > the current port calculation that is already in place without any 
> > duplication.
> > Alos fix portclk <= 594000 instead of portclk < 594000.
> > 
> > Cc: Paulo Zanoni 
> > Cc: Ville Syrjälä 
> > Signed-off-by: Kahola, Mika 
> > Signed-off-by: Rodrigo Vivi 
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 ++
> >  1 file changed, 22 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index a2a3d93d67bd..6030fbafa580 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr bxt_pll_mgr = {
> > .dump_hw_state = bxt_dump_hw_state,
> >  };
> >  
> > +static int cnl_get_dvfs_level(int cdclk, int portclk)
> > +{
> > +   if (cdclk == 168000 && portclk <= 594000)
> > +   return 0;
> > +   else if (cdclk == 336000 && portclk <= 594000)
> > +   return 1;
> > +   else
> > +   return 2;
> > +}
> > +
> >  static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
> >struct intel_shared_dpll *pll)
> >  {
> > uint32_t val;
> > +   int ret;
> > +   int level;
> > +   int cdclk, portclk;
> >  
> > /* 1. Enable DPLL power in DPLL_ENABLE. */
> > val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > @@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct 
> > drm_i915_private *dev_priv,
> > /*
> >  * 5. If the frequency will result in a change to the voltage
> >  * requirement, follow the Display Voltage Frequency Switching
> > -* Sequence Before Frequency Change
> > -*
> > -* FIXME: (DVFS) is used to adjust the display voltage to match the
> > -* display clock frequencies
> > +* (DVFS) Sequence Before Frequency Change
> >  */
> > +   ret = cnl_dvfs_pre_change(dev_priv);
> >  
> > /* 6. Enable DPLL in DPLL_ENABLE. */
> > val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> > @@ -2028,11 +2039,14 @@ static void cnl_ddi_pll_enable(struct 
> > drm_i915_private *dev_priv,
> > /*
> >  * 8. If the frequency will result in a change to the voltage
> >  * requirement, follow the Display Voltage Frequency Switching
> > -* Sequence After Frequency Change
> > -*
> > -* FIXME: (DVFS) is used to adjust the display voltage to match the
> > -* display clock frequencies
> > +* (DVFS) Sequence After Frequency Change
> >  */
> > +   if (ret == 0) {
> > +   cdclk = dev_priv->cdclk.hw.cdclk;
> > +   portclk = intel_ddi_port_clock(dev_priv, pll->id);
> > +   level = cnl_get_dvfs_level(cdclk, portclk);
> > +   cnl_dvfs_post_change(dev_priv, level);
> 
> This isn't how I imagined we'd handle this. What I was after is
> pre-computing the correct "dfvfs level" (or what I'd probably just call
> "voltage" or something like that), and then we'd just let the normal
> cdclk programming sequence do its thing.
> 
> Is it even safe to do this with other pipes/ports en

Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-17 Thread Ville Syrjälä 
On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> From: "Kahola, Mika" 
> 
> Display Voltage and Frequency Switching (DVFS) is used to adjust the
> display voltage to match the display clock frequencies. If voltage is
> set too low, it will break functionality. If voltage is set too high,
> it will waste power. Voltage level is selected based on CD clock and
> DDI clock.
> 
> The sequence before frequency change is the following and it requests
> the power controller to raise voltage to maximum
> 
> - Ensure any previous GT Driver Mailbox transaction is complete.
> - Write GT Driver Mailbox Data Low = 0x3.
> - Write GT Driver Mailbox Data High = 0x0.
> - Write GT Driver Mailbox Interface = 0x8007.
> - Poll GT Driver Mailbox Interface for Run/Busy indication cleared (bit 31 = 
> 0).
> - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else restart 
> the sequence.
>   Timeout after 3ms
> 
> The sequence after frequency change is the following and it requests
> the port controller to raise voltage to the requested level.
> 
> - Write GT Driver Mailbox Data Low
>  * For level 0, write 0x0
>  * For level 1, write 0x1
>  * For level 2, write 0x2
>  * For level 3, write 0x3
>- Write GT Driver Mailbox Data High = 0x0.
>- Write GT Driver Mailbox Interface = 0x8007.
> 
> For Cannonlake, the level 3 is not used and it aliases to level 2.
> 
> v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
> [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
> v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's name.
> v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock 
> calculation"
> The port clock calculation here was only addressing DP, so let's reuse
> the current port calculation that is already in place without any 
> duplication.
> Alos fix portclk <= 594000 instead of portclk < 594000.
> 
> Cc: Paulo Zanoni 
> Cc: Ville Syrjälä 
> Signed-off-by: Kahola, Mika 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 ++
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index a2a3d93d67bd..6030fbafa580 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr bxt_pll_mgr = {
>   .dump_hw_state = bxt_dump_hw_state,
>  };
>  
> +static int cnl_get_dvfs_level(int cdclk, int portclk)
> +{
> + if (cdclk == 168000 && portclk <= 594000)
> + return 0;
> + else if (cdclk == 336000 && portclk <= 594000)
> + return 1;
> + else
> + return 2;
> +}
> +
>  static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
>  struct intel_shared_dpll *pll)
>  {
>   uint32_t val;
> + int ret;
> + int level;
> + int cdclk, portclk;
>  
>   /* 1. Enable DPLL power in DPLL_ENABLE. */
>   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> @@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct drm_i915_private 
> *dev_priv,
>   /*
>* 5. If the frequency will result in a change to the voltage
>* requirement, follow the Display Voltage Frequency Switching
> -  * Sequence Before Frequency Change
> -  *
> -  * FIXME: (DVFS) is used to adjust the display voltage to match the
> -  * display clock frequencies
> +  * (DVFS) Sequence Before Frequency Change
>*/
> + ret = cnl_dvfs_pre_change(dev_priv);
>  
>   /* 6. Enable DPLL in DPLL_ENABLE. */
>   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> @@ -2028,11 +2039,14 @@ static void cnl_ddi_pll_enable(struct 
> drm_i915_private *dev_priv,
>   /*
>* 8. If the frequency will result in a change to the voltage
>* requirement, follow the Display Voltage Frequency Switching
> -  * Sequence After Frequency Change
> -  *
> -  * FIXME: (DVFS) is used to adjust the display voltage to match the
> -  * display clock frequencies
> +  * (DVFS) Sequence After Frequency Change
>*/
> + if (ret == 0) {
> + cdclk = dev_priv->cdclk.hw.cdclk;
> + portclk = intel_ddi_port_clock(dev_priv, pll->id);
> + level = cnl_get_dvfs_level(cdclk, portclk);
> + cnl_dvfs_post_change(dev_priv, level);

This isn't how I imagined we'd handle this. What I was after is
pre-computing the correct "dfvfs level" (or what I'd probably just call
"voltage" or something like that), and then we'd just let the normal
cdclk programming sequence do its thing.

Is it even safe to do this with other pipes/ports enabled? I would
have assumed no, but maybe I'm mistaken?


> + }
>  
>   /*
>* 9. turn on the clock for the DDI and map the DPLL to the DDI
> -- 
> 2.13.5

-- 
Ville Syrjälä
Intel OTC

Re: [Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-04 Thread Manasi Navare 
On Tue, Oct 03, 2017 at 12:06:08AM -0700, Rodrigo Vivi wrote:
> From: "Kahola, Mika" 
> 
> Display Voltage and Frequency Switching (DVFS) is used to adjust the
> display voltage to match the display clock frequencies. If voltage is
> set too low, it will break functionality. If voltage is set too high,
> it will waste power. Voltage level is selected based on CD clock and
> DDI clock.
> 
> The sequence before frequency change is the following and it requests
> the power controller to raise voltage to maximum
> 
> - Ensure any previous GT Driver Mailbox transaction is complete.
> - Write GT Driver Mailbox Data Low = 0x3.
> - Write GT Driver Mailbox Data High = 0x0.
> - Write GT Driver Mailbox Interface = 0x8007.
> - Poll GT Driver Mailbox Interface for Run/Busy indication cleared (bit 31 = 
> 0).
> - Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else restart 
> the sequence.
>   Timeout after 3ms
> 
> The sequence after frequency change is the following and it requests
> the port controller to raise voltage to the requested level.
> 
> - Write GT Driver Mailbox Data Low
>  * For level 0, write 0x0
>  * For level 1, write 0x1
>  * For level 2, write 0x2
>  * For level 3, write 0x3
>- Write GT Driver Mailbox Data High = 0x0.
>- Write GT Driver Mailbox Interface = 0x8007.
> 
> For Cannonlake, the level 3 is not used and it aliases to level 2.
> 
> v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
> [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
> v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's name.
> v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock 
> calculation"
> The port clock calculation here was only addressing DP, so let's reuse
> the current port calculation that is already in place without any 
> duplication.
> Alos fix portclk <= 594000 instead of portclk < 594000.
> 
> Cc: Paulo Zanoni 
> Cc: Ville Syrjälä 
> Signed-off-by: Kahola, Mika 
> Signed-off-by: Rodrigo Vivi 

Looks good, dowble checked with the voltage levels from the spec.

Reviewed-by: Manasi Navare 

> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 ++
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index a2a3d93d67bd..6030fbafa580 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr bxt_pll_mgr = {
>   .dump_hw_state = bxt_dump_hw_state,
>  };
>  
> +static int cnl_get_dvfs_level(int cdclk, int portclk)
> +{
> + if (cdclk == 168000 && portclk <= 594000)
> + return 0;
> + else if (cdclk == 336000 && portclk <= 594000)
> + return 1;
> + else
> + return 2;
> +}
> +
>  static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
>  struct intel_shared_dpll *pll)
>  {
>   uint32_t val;
> + int ret;
> + int level;
> + int cdclk, portclk;
>  
>   /* 1. Enable DPLL power in DPLL_ENABLE. */
>   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> @@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct drm_i915_private 
> *dev_priv,
>   /*
>* 5. If the frequency will result in a change to the voltage
>* requirement, follow the Display Voltage Frequency Switching
> -  * Sequence Before Frequency Change
> -  *
> -  * FIXME: (DVFS) is used to adjust the display voltage to match the
> -  * display clock frequencies
> +  * (DVFS) Sequence Before Frequency Change
>*/
> + ret = cnl_dvfs_pre_change(dev_priv);
>  
>   /* 6. Enable DPLL in DPLL_ENABLE. */
>   val = I915_READ(CNL_DPLL_ENABLE(pll->id));
> @@ -2028,11 +2039,14 @@ static void cnl_ddi_pll_enable(struct 
> drm_i915_private *dev_priv,
>   /*
>* 8. If the frequency will result in a change to the voltage
>* requirement, follow the Display Voltage Frequency Switching
> -  * Sequence After Frequency Change
> -  *
> -  * FIXME: (DVFS) is used to adjust the display voltage to match the
> -  * display clock frequencies
> +  * (DVFS) Sequence After Frequency Change
>*/
> + if (ret == 0) {
> + cdclk = dev_priv->cdclk.hw.cdclk;
> + portclk = intel_ddi_port_clock(dev_priv, pll->id);
> + level = cnl_get_dvfs_level(cdclk, portclk);
> + cnl_dvfs_post_change(dev_priv, level);
> + }
>  
>   /*
>* 9. turn on the clock for the DDI and map the DPLL to the DDI
> -- 
> 2.13.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/li

[Intel-gfx] [PATCH 07/13] drm/i915/cnl: DVFS for PLL enabling

2017-10-03 Thread Rodrigo Vivi 
From: "Kahola, Mika" 

Display Voltage and Frequency Switching (DVFS) is used to adjust the
display voltage to match the display clock frequencies. If voltage is
set too low, it will break functionality. If voltage is set too high,
it will waste power. Voltage level is selected based on CD clock and
DDI clock.

The sequence before frequency change is the following and it requests
the power controller to raise voltage to maximum

- Ensure any previous GT Driver Mailbox transaction is complete.
- Write GT Driver Mailbox Data Low = 0x3.
- Write GT Driver Mailbox Data High = 0x0.
- Write GT Driver Mailbox Interface = 0x8007.
- Poll GT Driver Mailbox Interface for Run/Busy indication cleared (bit 31 = 0).
- Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else restart the 
sequence.
  Timeout after 3ms

The sequence after frequency change is the following and it requests
the port controller to raise voltage to the requested level.

- Write GT Driver Mailbox Data Low
 * For level 0, write 0x0
 * For level 1, write 0x1
 * For level 2, write 0x2
 * For level 3, write 0x3
   - Write GT Driver Mailbox Data High = 0x0.
   - Write GT Driver Mailbox Interface = 0x8007.

For Cannonlake, the level 3 is not used and it aliases to level 2.

v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
[PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's name.
v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock calculation"
The port clock calculation here was only addressing DP, so let's reuse
the current port calculation that is already in place without any 
duplication.
Alos fix portclk <= 594000 instead of portclk < 594000.

Cc: Paulo Zanoni 
Cc: Ville Syrjälä 
Signed-off-by: Kahola, Mika 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c 
b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index a2a3d93d67bd..6030fbafa580 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr bxt_pll_mgr = {
.dump_hw_state = bxt_dump_hw_state,
 };
 
+static int cnl_get_dvfs_level(int cdclk, int portclk)
+{
+   if (cdclk == 168000 && portclk <= 594000)
+   return 0;
+   else if (cdclk == 336000 && portclk <= 594000)
+   return 1;
+   else
+   return 2;
+}
+
 static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
   struct intel_shared_dpll *pll)
 {
uint32_t val;
+   int ret;
+   int level;
+   int cdclk, portclk;
 
/* 1. Enable DPLL power in DPLL_ENABLE. */
val = I915_READ(CNL_DPLL_ENABLE(pll->id));
@@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct drm_i915_private 
*dev_priv,
/*
 * 5. If the frequency will result in a change to the voltage
 * requirement, follow the Display Voltage Frequency Switching
-* Sequence Before Frequency Change
-*
-* FIXME: (DVFS) is used to adjust the display voltage to match the
-* display clock frequencies
+* (DVFS) Sequence Before Frequency Change
 */
+   ret = cnl_dvfs_pre_change(dev_priv);
 
/* 6. Enable DPLL in DPLL_ENABLE. */
val = I915_READ(CNL_DPLL_ENABLE(pll->id));
@@ -2028,11 +2039,14 @@ static void cnl_ddi_pll_enable(struct drm_i915_private 
*dev_priv,
/*
 * 8. If the frequency will result in a change to the voltage
 * requirement, follow the Display Voltage Frequency Switching
-* Sequence After Frequency Change
-*
-* FIXME: (DVFS) is used to adjust the display voltage to match the
-* display clock frequencies
+* (DVFS) Sequence After Frequency Change
 */
+   if (ret == 0) {
+   cdclk = dev_priv->cdclk.hw.cdclk;
+   portclk = intel_ddi_port_clock(dev_priv, pll->id);
+   level = cnl_get_dvfs_level(cdclk, portclk);
+   cnl_dvfs_post_change(dev_priv, level);
+   }
 
/*
 * 9. turn on the clock for the DDI and map the DPLL to the DDI
-- 
2.13.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx