All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Update RAWCLK_FREQ register on VLV/CHV
@ 2016-04-27 14:43 ville.syrjala
  2016-04-27 15:23 ` ✓ Fi.CI.BAT: success for " Patchwork
  2016-04-27 15:54 ` [PATCH] " Vivi, Rodrigo
  0 siblings, 2 replies; 6+ messages in thread
From: ville.syrjala @ 2016-04-27 14:43 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I just noticed that VLV/CHV have a RAWCLK_FREQ register just like PCH
platforms. It lives in the display power well, so we should update it
when enabling the power well.

Interestingly the BIOS seems to leave it at the reset value (125) which
doesn't match the rawclk frequency on VLV/CHV (200 MHz). As always with
these register, the spec is extremely vague what the register does. All
it says is: "This is used to generate a divided down clock for
miscellaneous timers in display." Based on a quick test, at least AUX
and PWM appear to be unaffected by this.

But since the register is there, let's configure it in accordance with
the spec.

Note that we have to move intel_update_rawclk() to occur before we
touch the power wells, so that the dev_priv->rawclk_freq is already
populated when the disp2 enable hook gets called for the first time.
I think this should be safe to do on other platforms as well.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c         | 3 +++
 drivers/gpu/drm/i915/i915_reg.h         | 2 ++
 drivers/gpu/drm/i915/intel_display.c    | 4 ++--
 drivers/gpu/drm/i915/intel_drv.h        | 1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +++++
 5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5c7615041b31..fd1260d2b6ec 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -454,6 +454,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_vga_client;
 
+	/* must happen before intel_power_domains_init_hw() on VLV/CHV */
+	intel_update_rawclk(dev_priv);
+
 	intel_power_domains_init_hw(dev_priv, false);
 
 	intel_csr_ucode_init(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 25e229b609a7..0a2fd3000f94 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2449,6 +2449,8 @@ enum skl_disp_power_wells {
 #define   DPLL_MD_VGA_UDI_MULTIPLIER_MASK	0x0000003f
 #define   DPLL_MD_VGA_UDI_MULTIPLIER_SHIFT	0
 
+#define RAWCLK_FREQ_VLV		_MMIO(VLV_DISPLAY_BASE + 0x6024)
+
 #define _FPA0	0x6040
 #define _FPA1	0x6044
 #define _FPB0	0x6048
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b7cb632a2a31..eb7cb9431209 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -185,6 +185,7 @@ intel_pch_rawclk(struct drm_i915_private *dev_priv)
 static int
 intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
 {
+	/* RAWCLK_FREQ_VLV register updated from power well code */
 	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
 				      CCK_DISPLAY_REF_CLOCK_CONTROL);
 }
@@ -218,7 +219,7 @@ intel_g4x_hrawclk(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void intel_update_rawclk(struct drm_i915_private *dev_priv)
+void intel_update_rawclk(struct drm_i915_private *dev_priv)
 {
 	if (HAS_PCH_SPLIT(dev_priv))
 		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
@@ -15455,7 +15456,6 @@ void intel_modeset_init(struct drm_device *dev)
 	}
 
 	intel_update_czclk(dev_priv);
-	intel_update_rawclk(dev_priv);
 	intel_update_cdclk(dev);
 
 	intel_shared_dpll_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cb89a35a6755..ce78afefe3cd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1110,6 +1110,7 @@ void i915_audio_component_init(struct drm_i915_private *dev_priv);
 void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
 
 /* intel_display.c */
+void intel_update_rawclk(struct drm_i915_private *dev_priv);
 int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
 		      const char *name, u32 reg, int ref_freq);
 extern const struct drm_plane_funcs intel_plane_funcs;
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 7fb1da4e7fc3..b69b935516fb 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -948,6 +948,11 @@ static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
 	 */
 	I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
 	I915_WRITE(CBR1_VLV, 0);
+
+	WARN_ON(dev_priv->rawclk_freq == 0);
+
+	I915_WRITE(RAWCLK_FREQ_VLV,
+		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 1000));
 }
 
 static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Update RAWCLK_FREQ register on VLV/CHV
  2016-04-27 14:43 [PATCH] drm/i915: Update RAWCLK_FREQ register on VLV/CHV ville.syrjala
@ 2016-04-27 15:23 ` Patchwork
  2016-04-27 15:54 ` [PATCH] " Vivi, Rodrigo
  1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-04-27 15:23 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Update RAWCLK_FREQ register on VLV/CHV
URL   : https://patchwork.freedesktop.org/series/6410/
State : success

== Summary ==

Series 6410v1 drm/i915: Update RAWCLK_FREQ register on VLV/CHV
http://patchwork.freedesktop.org/api/1.0/series/6410/revisions/1/mbox/

Test drv_module_reload_basic:
                skip       -> PASS       (bdw-nuci7)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (bdw-ultra)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-b:
                dmesg-warn -> PASS       (snb-dellxps)

bdw-nuci7        total:200  pass:188  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:200  pass:175  dwarn:0   dfail:0   fail:0   skip:25 
bsw-nuc-2        total:199  pass:158  dwarn:0   dfail:0   fail:0   skip:41 
byt-nuc          total:199  pass:158  dwarn:0   dfail:0   fail:0   skip:41 
hsw-brixbox      total:200  pass:174  dwarn:0   dfail:0   fail:0   skip:26 
hsw-gt2          total:200  pass:178  dwarn:0   dfail:0   fail:1   skip:21 
ilk-hp8440p      total:200  pass:139  dwarn:0   dfail:0   fail:0   skip:61 
ivb-t430s        total:200  pass:169  dwarn:0   dfail:0   fail:0   skip:31 
skl-i7k-2        total:200  pass:173  dwarn:0   dfail:0   fail:0   skip:27 
skl-nuci5        total:200  pass:189  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:200  pass:158  dwarn:0   dfail:0   fail:0   skip:42 
snb-x220t        total:200  pass:158  dwarn:0   dfail:0   fail:1   skip:41 

Results at /archive/results/CI_IGT_test/Patchwork_2093/

e6c43160f35ad64114e1156e3f995e5dabe74835 drm-intel-nightly: 2016y-04m-27d-13h-17m-05s UTC integration manifest
db80506 drm/i915: Update RAWCLK_FREQ register on VLV/CHV

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Update RAWCLK_FREQ register on VLV/CHV
  2016-04-27 14:43 [PATCH] drm/i915: Update RAWCLK_FREQ register on VLV/CHV ville.syrjala
  2016-04-27 15:23 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2016-04-27 15:54 ` Vivi, Rodrigo
  2016-04-27 16:08   ` Ville Syrjälä
  1 sibling, 1 reply; 6+ messages in thread
From: Vivi, Rodrigo @ 2016-04-27 15:54 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Wed, 2016-04-27 at 17:43 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I just noticed that VLV/CHV have a RAWCLK_FREQ register just like PCH
> platforms. It lives in the display power well, so we should update it
> when enabling the power well.
> 
> Interestingly the BIOS seems to leave it at the reset value (125)
> which
> doesn't match the rawclk frequency on VLV/CHV (200 MHz). As always
> with
> these register, the spec is extremely vague what the register does.
> All
> it says is: "This is used to generate a divided down clock for
> miscellaneous timers in display." Based on a quick test, at least AUX
> and PWM appear to be unaffected by this.
> 
> But since the register is there, let's configure it in accordance
> with
> the spec.
> 
> Note that we have to move intel_update_rawclk() to occur before we
> touch the power wells, so that the dev_priv->rawclk_freq is already
> populated when the disp2 enable hook gets called for the first time.
> I think this should be safe to do on other platforms as well.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c         | 3 +++
>  drivers/gpu/drm/i915/i915_reg.h         | 2 ++
>  drivers/gpu/drm/i915/intel_display.c    | 4 ++--
>  drivers/gpu/drm/i915/intel_drv.h        | 1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +++++
>  5 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c
> index 5c7615041b31..fd1260d2b6ec 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -454,6 +454,9 @@ static int i915_load_modeset_init(struct
> drm_device *dev)
>  	if (ret)
>  		goto cleanup_vga_client;
>  
> +	/* must happen before intel_power_domains_init_hw() on
> VLV/CHV */
> +	intel_update_rawclk(dev_priv);

separated patches since this affects all platforms?!

anyways I checked CHV spec here so feel free to ignore me and use
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> +
>  	intel_power_domains_init_hw(dev_priv, false);
>  
>  	intel_csr_ucode_init(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 25e229b609a7..0a2fd3000f94 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2449,6 +2449,8 @@ enum skl_disp_power_wells {
>  #define   DPLL_MD_VGA_UDI_MULTIPLIER_MASK	0x0000003f
>  #define   DPLL_MD_VGA_UDI_MULTIPLIER_SHIFT	0
>  
> +#define RAWCLK_FREQ_VLV		_MMIO(VLV_DISPLAY_BASE +
> 0x6024)
> +
>  #define _FPA0	0x6040
>  #define _FPA1	0x6044
>  #define _FPB0	0x6048
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b7cb632a2a31..eb7cb9431209 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -185,6 +185,7 @@ intel_pch_rawclk(struct drm_i915_private
> *dev_priv)
>  static int
>  intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
>  {
> +	/* RAWCLK_FREQ_VLV register updated from power well code */
>  	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
>  				     
>  CCK_DISPLAY_REF_CLOCK_CONTROL);
>  }
> @@ -218,7 +219,7 @@ intel_g4x_hrawclk(struct drm_i915_private
> *dev_priv)
>  	}
>  }
>  
> -static void intel_update_rawclk(struct drm_i915_private *dev_priv)
> +void intel_update_rawclk(struct drm_i915_private *dev_priv)
>  {
>  	if (HAS_PCH_SPLIT(dev_priv))
>  		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
> @@ -15455,7 +15456,6 @@ void intel_modeset_init(struct drm_device
> *dev)
>  	}
>  
>  	intel_update_czclk(dev_priv);
> -	intel_update_rawclk(dev_priv);
>  	intel_update_cdclk(dev);
>  
>  	intel_shared_dpll_init(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index cb89a35a6755..ce78afefe3cd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1110,6 +1110,7 @@ void i915_audio_component_init(struct
> drm_i915_private *dev_priv);
>  void i915_audio_component_cleanup(struct drm_i915_private
> *dev_priv);
>  
>  /* intel_display.c */
> +void intel_update_rawclk(struct drm_i915_private *dev_priv);
>  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
>  		      const char *name, u32 reg, int ref_freq);
>  extern const struct drm_plane_funcs intel_plane_funcs;
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 7fb1da4e7fc3..b69b935516fb 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -948,6 +948,11 @@ static void vlv_init_display_clock_gating(struct
> drm_i915_private *dev_priv)
>  	 */
>  	I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
>  	I915_WRITE(CBR1_VLV, 0);
> +
> +	WARN_ON(dev_priv->rawclk_freq == 0);
> +
> +	I915_WRITE(RAWCLK_FREQ_VLV,
> +		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 1000));
>  }
>  
>  static void vlv_display_power_well_init(struct drm_i915_private
> *dev_priv)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Update RAWCLK_FREQ register on VLV/CHV
  2016-04-27 15:54 ` [PATCH] " Vivi, Rodrigo
@ 2016-04-27 16:08   ` Ville Syrjälä
  2016-04-27 16:09     ` Vivi, Rodrigo
  0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2016-04-27 16:08 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx

On Wed, Apr 27, 2016 at 03:54:12PM +0000, Vivi, Rodrigo wrote:
> On Wed, 2016-04-27 at 17:43 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I just noticed that VLV/CHV have a RAWCLK_FREQ register just like PCH
> > platforms. It lives in the display power well, so we should update it
> > when enabling the power well.
> > 
> > Interestingly the BIOS seems to leave it at the reset value (125)
> > which
> > doesn't match the rawclk frequency on VLV/CHV (200 MHz). As always
> > with
> > these register, the spec is extremely vague what the register does.
> > All
> > it says is: "This is used to generate a divided down clock for
> > miscellaneous timers in display." Based on a quick test, at least AUX
> > and PWM appear to be unaffected by this.
> > 
> > But since the register is there, let's configure it in accordance
> > with
> > the spec.
> > 
> > Note that we have to move intel_update_rawclk() to occur before we
> > touch the power wells, so that the dev_priv->rawclk_freq is already
> > populated when the disp2 enable hook gets called for the first time.
> > I think this should be safe to do on other platforms as well.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c         | 3 +++
> >  drivers/gpu/drm/i915/i915_reg.h         | 2 ++
> >  drivers/gpu/drm/i915/intel_display.c    | 4 ++--
> >  drivers/gpu/drm/i915/intel_drv.h        | 1 +
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +++++
> >  5 files changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 5c7615041b31..fd1260d2b6ec 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -454,6 +454,9 @@ static int i915_load_modeset_init(struct
> > drm_device *dev)
> >  	if (ret)
> >  		goto cleanup_vga_client;
> >  
> > +	/* must happen before intel_power_domains_init_hw() on
> > VLV/CHV */
> > +	intel_update_rawclk(dev_priv);
> 
> separated patches since this affects all platforms?!

I was going to do that originally, but then I thought that if we have to
revert due to some other platform, it's likely VLV/CHV would be
forgotten and only later someone would start to complain about the
WARN_ON() I added. So if this change needs to reverted, the VLV/CHV stuff
needs to be redone anyway, so having it in the same commit seems like
the slightly better option to me.

> 
> anyways I checked CHV spec here so feel free to ignore me and use
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> > +
> >  	intel_power_domains_init_hw(dev_priv, false);
> >  
> >  	intel_csr_ucode_init(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 25e229b609a7..0a2fd3000f94 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2449,6 +2449,8 @@ enum skl_disp_power_wells {
> >  #define   DPLL_MD_VGA_UDI_MULTIPLIER_MASK	0x0000003f
> >  #define   DPLL_MD_VGA_UDI_MULTIPLIER_SHIFT	0
> >  
> > +#define RAWCLK_FREQ_VLV		_MMIO(VLV_DISPLAY_BASE +
> > 0x6024)
> > +
> >  #define _FPA0	0x6040
> >  #define _FPA1	0x6044
> >  #define _FPB0	0x6048
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index b7cb632a2a31..eb7cb9431209 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -185,6 +185,7 @@ intel_pch_rawclk(struct drm_i915_private
> > *dev_priv)
> >  static int
> >  intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> >  {
> > +	/* RAWCLK_FREQ_VLV register updated from power well code */
> >  	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
> >  				     
> >  CCK_DISPLAY_REF_CLOCK_CONTROL);
> >  }
> > @@ -218,7 +219,7 @@ intel_g4x_hrawclk(struct drm_i915_private
> > *dev_priv)
> >  	}
> >  }
> >  
> > -static void intel_update_rawclk(struct drm_i915_private *dev_priv)
> > +void intel_update_rawclk(struct drm_i915_private *dev_priv)
> >  {
> >  	if (HAS_PCH_SPLIT(dev_priv))
> >  		dev_priv->rawclk_freq = intel_pch_rawclk(dev_priv);
> > @@ -15455,7 +15456,6 @@ void intel_modeset_init(struct drm_device
> > *dev)
> >  	}
> >  
> >  	intel_update_czclk(dev_priv);
> > -	intel_update_rawclk(dev_priv);
> >  	intel_update_cdclk(dev);
> >  
> >  	intel_shared_dpll_init(dev);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index cb89a35a6755..ce78afefe3cd 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1110,6 +1110,7 @@ void i915_audio_component_init(struct
> > drm_i915_private *dev_priv);
> >  void i915_audio_component_cleanup(struct drm_i915_private
> > *dev_priv);
> >  
> >  /* intel_display.c */
> > +void intel_update_rawclk(struct drm_i915_private *dev_priv);
> >  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> >  		      const char *name, u32 reg, int ref_freq);
> >  extern const struct drm_plane_funcs intel_plane_funcs;
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 7fb1da4e7fc3..b69b935516fb 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -948,6 +948,11 @@ static void vlv_init_display_clock_gating(struct
> > drm_i915_private *dev_priv)
> >  	 */
> >  	I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> >  	I915_WRITE(CBR1_VLV, 0);
> > +
> > +	WARN_ON(dev_priv->rawclk_freq == 0);
> > +
> > +	I915_WRITE(RAWCLK_FREQ_VLV,
> > +		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 1000));
> >  }
> >  
> >  static void vlv_display_power_well_init(struct drm_i915_private
> > *dev_priv)

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Update RAWCLK_FREQ register on VLV/CHV
  2016-04-27 16:08   ` Ville Syrjälä
@ 2016-04-27 16:09     ` Vivi, Rodrigo
  2016-04-27 19:38       ` Ville Syrjälä
  0 siblings, 1 reply; 6+ messages in thread
From: Vivi, Rodrigo @ 2016-04-27 16:09 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, 2016-04-27 at 19:08 +0300, Ville Syrjälä wrote:
> On Wed, Apr 27, 2016 at 03:54:12PM +0000, Vivi, Rodrigo wrote:
> > On Wed, 2016-04-27 at 17:43 +0300, ville.syrjala@linux.intel.com wr
> > ote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > I just noticed that VLV/CHV have a RAWCLK_FREQ register just like
> > > PCH
> > > platforms. It lives in the display power well, so we should
> > > update it
> > > when enabling the power well.
> > > 
> > > Interestingly the BIOS seems to leave it at the reset value (125)
> > > which
> > > doesn't match the rawclk frequency on VLV/CHV (200 MHz). As
> > > always
> > > with
> > > these register, the spec is extremely vague what the register
> > > does.
> > > All
> > > it says is: "This is used to generate a divided down clock for
> > > miscellaneous timers in display." Based on a quick test, at least
> > > AUX
> > > and PWM appear to be unaffected by this.
> > > 
> > > But since the register is there, let's configure it in accordance
> > > with
> > > the spec.
> > > 
> > > Note that we have to move intel_update_rawclk() to occur before
> > > we
> > > touch the power wells, so that the dev_priv->rawclk_freq is
> > > already
> > > populated when the disp2 enable hook gets called for the first
> > > time.
> > > I think this should be safe to do on other platforms as well.
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_dma.c         | 3 +++
> > >  drivers/gpu/drm/i915/i915_reg.h         | 2 ++
> > >  drivers/gpu/drm/i915/intel_display.c    | 4 ++--
> > >  drivers/gpu/drm/i915/intel_drv.h        | 1 +
> > >  drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +++++
> > >  5 files changed, 13 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > > b/drivers/gpu/drm/i915/i915_dma.c
> > > index 5c7615041b31..fd1260d2b6ec 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -454,6 +454,9 @@ static int i915_load_modeset_init(struct
> > > drm_device *dev)
> > >  	if (ret)
> > >  		goto cleanup_vga_client;
> > >  
> > > +	/* must happen before intel_power_domains_init_hw() on
> > > VLV/CHV */
> > > +	intel_update_rawclk(dev_priv);
> > 
> > separated patches since this affects all platforms?!
> 
> I was going to do that originally, but then I thought that if we have
> to
> revert due to some other platform, it's likely VLV/CHV would be
> forgotten and only later someone would start to complain about the
> WARN_ON() I added. So if this change needs to reverted, the VLV/CHV
> stuff
> needs to be redone anyway, so having it in the same commit seems like
> the slightly better option to me.

oh! makes a lot of sense indeed!

> 
> > 
> > anyways I checked CHV spec here so feel free to ignore me and use
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > 
> > > +
> > >  	intel_power_domains_init_hw(dev_priv, false);
> > >  
> > >  	intel_csr_ucode_init(dev_priv);
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 25e229b609a7..0a2fd3000f94 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -2449,6 +2449,8 @@ enum skl_disp_power_wells {
> > >  #define   DPLL_MD_VGA_UDI_MULTIPLIER_MASK	0x0000003f
> > >  #define   DPLL_MD_VGA_UDI_MULTIPLIER_SHIFT	0
> > >  
> > > +#define RAWCLK_FREQ_VLV		_MMIO(VLV_DISPLAY_BASE +
> > > 0x6024)
> > > +
> > >  #define _FPA0	0x6040
> > >  #define _FPA1	0x6044
> > >  #define _FPB0	0x6048
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index b7cb632a2a31..eb7cb9431209 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -185,6 +185,7 @@ intel_pch_rawclk(struct drm_i915_private
> > > *dev_priv)
> > >  static int
> > >  intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> > >  {
> > > +	/* RAWCLK_FREQ_VLV register updated from power well code
> > > */
> > >  	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
> > >  				     
> > >  CCK_DISPLAY_REF_CLOCK_CONTROL);
> > >  }
> > > @@ -218,7 +219,7 @@ intel_g4x_hrawclk(struct drm_i915_private
> > > *dev_priv)
> > >  	}
> > >  }
> > >  
> > > -static void intel_update_rawclk(struct drm_i915_private
> > > *dev_priv)
> > > +void intel_update_rawclk(struct drm_i915_private *dev_priv)
> > >  {
> > >  	if (HAS_PCH_SPLIT(dev_priv))
> > >  		dev_priv->rawclk_freq =
> > > intel_pch_rawclk(dev_priv);
> > > @@ -15455,7 +15456,6 @@ void intel_modeset_init(struct drm_device
> > > *dev)
> > >  	}
> > >  
> > >  	intel_update_czclk(dev_priv);
> > > -	intel_update_rawclk(dev_priv);
> > >  	intel_update_cdclk(dev);
> > >  
> > >  	intel_shared_dpll_init(dev);
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index cb89a35a6755..ce78afefe3cd 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1110,6 +1110,7 @@ void i915_audio_component_init(struct
> > > drm_i915_private *dev_priv);
> > >  void i915_audio_component_cleanup(struct drm_i915_private
> > > *dev_priv);
> > >  
> > >  /* intel_display.c */
> > > +void intel_update_rawclk(struct drm_i915_private *dev_priv);
> > >  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> > >  		      const char *name, u32 reg, int ref_freq);
> > >  extern const struct drm_plane_funcs intel_plane_funcs;
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 7fb1da4e7fc3..b69b935516fb 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -948,6 +948,11 @@ static void
> > > vlv_init_display_clock_gating(struct
> > > drm_i915_private *dev_priv)
> > >  	 */
> > >  	I915_WRITE(MI_ARB_VLV,
> > > MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> > >  	I915_WRITE(CBR1_VLV, 0);
> > > +
> > > +	WARN_ON(dev_priv->rawclk_freq == 0);
> > > +
> > > +	I915_WRITE(RAWCLK_FREQ_VLV,
> > > +		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq,
> > > 1000));
> > >  }
> > >  
> > >  static void vlv_display_power_well_init(struct drm_i915_private
> > > *dev_priv)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Update RAWCLK_FREQ register on VLV/CHV
  2016-04-27 16:09     ` Vivi, Rodrigo
@ 2016-04-27 19:38       ` Ville Syrjälä
  0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2016-04-27 19:38 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx

On Wed, Apr 27, 2016 at 04:09:53PM +0000, Vivi, Rodrigo wrote:
> On Wed, 2016-04-27 at 19:08 +0300, Ville Syrjälä wrote:
> > On Wed, Apr 27, 2016 at 03:54:12PM +0000, Vivi, Rodrigo wrote:
> > > On Wed, 2016-04-27 at 17:43 +0300, ville.syrjala@linux.intel.com wr
> > > ote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > I just noticed that VLV/CHV have a RAWCLK_FREQ register just like
> > > > PCH
> > > > platforms. It lives in the display power well, so we should
> > > > update it
> > > > when enabling the power well.
> > > > 
> > > > Interestingly the BIOS seems to leave it at the reset value (125)
> > > > which
> > > > doesn't match the rawclk frequency on VLV/CHV (200 MHz). As
> > > > always
> > > > with
> > > > these register, the spec is extremely vague what the register
> > > > does.
> > > > All
> > > > it says is: "This is used to generate a divided down clock for
> > > > miscellaneous timers in display." Based on a quick test, at least
> > > > AUX
> > > > and PWM appear to be unaffected by this.
> > > > 
> > > > But since the register is there, let's configure it in accordance
> > > > with
> > > > the spec.
> > > > 
> > > > Note that we have to move intel_update_rawclk() to occur before
> > > > we
> > > > touch the power wells, so that the dev_priv->rawclk_freq is
> > > > already
> > > > populated when the disp2 enable hook gets called for the first
> > > > time.
> > > > I think this should be safe to do on other platforms as well.
> > > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_dma.c         | 3 +++
> > > >  drivers/gpu/drm/i915/i915_reg.h         | 2 ++
> > > >  drivers/gpu/drm/i915/intel_display.c    | 4 ++--
> > > >  drivers/gpu/drm/i915/intel_drv.h        | 1 +
> > > >  drivers/gpu/drm/i915/intel_runtime_pm.c | 5 +++++
> > > >  5 files changed, 13 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > > > b/drivers/gpu/drm/i915/i915_dma.c
> > > > index 5c7615041b31..fd1260d2b6ec 100644
> > > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > > @@ -454,6 +454,9 @@ static int i915_load_modeset_init(struct
> > > > drm_device *dev)
> > > >  	if (ret)
> > > >  		goto cleanup_vga_client;
> > > >  
> > > > +	/* must happen before intel_power_domains_init_hw() on
> > > > VLV/CHV */
> > > > +	intel_update_rawclk(dev_priv);
> > > 
> > > separated patches since this affects all platforms?!
> > 
> > I was going to do that originally, but then I thought that if we have
> > to
> > revert due to some other platform, it's likely VLV/CHV would be
> > forgotten and only later someone would start to complain about the
> > WARN_ON() I added. So if this change needs to reverted, the VLV/CHV
> > stuff
> > needs to be redone anyway, so having it in the same commit seems like
> > the slightly better option to me.
> 
> oh! makes a lot of sense indeed!
> 
> > 
> > > 
> > > anyways I checked CHV spec here so feel free to ignore me and use
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Pushed to dinq. Thanks for the review.

> > > 
> > > 
> > > > +
> > > >  	intel_power_domains_init_hw(dev_priv, false);
> > > >  
> > > >  	intel_csr_ucode_init(dev_priv);
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 25e229b609a7..0a2fd3000f94 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -2449,6 +2449,8 @@ enum skl_disp_power_wells {
> > > >  #define   DPLL_MD_VGA_UDI_MULTIPLIER_MASK	0x0000003f
> > > >  #define   DPLL_MD_VGA_UDI_MULTIPLIER_SHIFT	0
> > > >  
> > > > +#define RAWCLK_FREQ_VLV		_MMIO(VLV_DISPLAY_BASE +
> > > > 0x6024)
> > > > +
> > > >  #define _FPA0	0x6040
> > > >  #define _FPA1	0x6044
> > > >  #define _FPB0	0x6048
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index b7cb632a2a31..eb7cb9431209 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -185,6 +185,7 @@ intel_pch_rawclk(struct drm_i915_private
> > > > *dev_priv)
> > > >  static int
> > > >  intel_vlv_hrawclk(struct drm_i915_private *dev_priv)
> > > >  {
> > > > +	/* RAWCLK_FREQ_VLV register updated from power well code
> > > > */
> > > >  	return vlv_get_cck_clock_hpll(dev_priv, "hrawclk",
> > > >  				     
> > > >  CCK_DISPLAY_REF_CLOCK_CONTROL);
> > > >  }
> > > > @@ -218,7 +219,7 @@ intel_g4x_hrawclk(struct drm_i915_private
> > > > *dev_priv)
> > > >  	}
> > > >  }
> > > >  
> > > > -static void intel_update_rawclk(struct drm_i915_private
> > > > *dev_priv)
> > > > +void intel_update_rawclk(struct drm_i915_private *dev_priv)
> > > >  {
> > > >  	if (HAS_PCH_SPLIT(dev_priv))
> > > >  		dev_priv->rawclk_freq =
> > > > intel_pch_rawclk(dev_priv);
> > > > @@ -15455,7 +15456,6 @@ void intel_modeset_init(struct drm_device
> > > > *dev)
> > > >  	}
> > > >  
> > > >  	intel_update_czclk(dev_priv);
> > > > -	intel_update_rawclk(dev_priv);
> > > >  	intel_update_cdclk(dev);
> > > >  
> > > >  	intel_shared_dpll_init(dev);
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > index cb89a35a6755..ce78afefe3cd 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -1110,6 +1110,7 @@ void i915_audio_component_init(struct
> > > > drm_i915_private *dev_priv);
> > > >  void i915_audio_component_cleanup(struct drm_i915_private
> > > > *dev_priv);
> > > >  
> > > >  /* intel_display.c */
> > > > +void intel_update_rawclk(struct drm_i915_private *dev_priv);
> > > >  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> > > >  		      const char *name, u32 reg, int ref_freq);
> > > >  extern const struct drm_plane_funcs intel_plane_funcs;
> > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > index 7fb1da4e7fc3..b69b935516fb 100644
> > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > @@ -948,6 +948,11 @@ static void
> > > > vlv_init_display_clock_gating(struct
> > > > drm_i915_private *dev_priv)
> > > >  	 */
> > > >  	I915_WRITE(MI_ARB_VLV,
> > > > MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> > > >  	I915_WRITE(CBR1_VLV, 0);
> > > > +
> > > > +	WARN_ON(dev_priv->rawclk_freq == 0);
> > > > +
> > > > +	I915_WRITE(RAWCLK_FREQ_VLV,
> > > > +		   DIV_ROUND_CLOSEST(dev_priv->rawclk_freq,
> > > > 1000));
> > > >  }
> > > >  
> > > >  static void vlv_display_power_well_init(struct drm_i915_private
> > > > *dev_priv)
> > 

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-04-27 19:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-27 14:43 [PATCH] drm/i915: Update RAWCLK_FREQ register on VLV/CHV ville.syrjala
2016-04-27 15:23 ` ✓ Fi.CI.BAT: success for " Patchwork
2016-04-27 15:54 ` [PATCH] " Vivi, Rodrigo
2016-04-27 16:08   ` Ville Syrjälä
2016-04-27 16:09     ` Vivi, Rodrigo
2016-04-27 19:38       ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.