public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/i915: Cdclk related fixes and polish
@ 2016-04-26 16:46 ville.syrjala
  2016-04-26 16:46 ` [PATCH 1/3] drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency ville.syrjala
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: ville.syrjala @ 2016-04-26 16:46 UTC (permalink / raw)
  To: intel-gfx

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

While looking at bug [1] I noticed that we're not updating the CDCLK_FREQ
register on BDW like we should. Turns out that wasn't the problem in the bug
but we should program it anyway, so that the's main fix in this series.
And I tossed in a few other cdclk related things.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=91410

Ville Syrjälä (3):
  drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk
    frequency
  drm/i915: Use cached cdclk value in
    i915_audio_component_get_cdclk_freq()
  drm/i915: Fix comments about GMBUSFREQ register

 drivers/gpu/drm/i915/i915_reg.h      |  2 ++
 drivers/gpu/drm/i915/intel_audio.c   |  8 +-------
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++----------
 3 files changed, 10 insertions(+), 17 deletions(-)

-- 
2.7.4

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

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

* [PATCH 1/3] drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency
  2016-04-26 16:46 [PATCH 0/3] drm/i915: Cdclk related fixes and polish ville.syrjala
@ 2016-04-26 16:46 ` ville.syrjala
  2016-04-27  8:19   ` Mika Kahola
  2016-04-26 16:46 ` [PATCH 2/3] drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq() ville.syrjala
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: ville.syrjala @ 2016-04-26 16:46 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Mika Kahola

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

Update CDCLK_FREQ on BDW after changing the cdclk frequency. Not sure
if this is a late addition to the spec, or if I simply overlooked this
step when writing the original code.

This is what Bspec has to say about CDCLK_FREQ:
"Program this field to the CD clock frequency minus one. This is used to
 generate a divided down clock for miscellaneous timers in display."

And the "Broadwell Sequences for Changing CD Clock Frequency" section
clarifies this further:
"For CD clock 337.5 MHz, program 337 decimal.
 For CD clock 450 MHz, program 449 decimal.
 For CD clock 540 MHz, program 539 decimal.
 For CD clock 675 MHz, program 674 decimal."

Cc: stable@vger.kernel.org
Cc: Mika Kahola <mika.kahola@intel.com>
Fixes: b432e5cfd5e9 ("drm/i915: BDW clock change support")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 2 ++
 drivers/gpu/drm/i915/intel_display.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 03264fd30fdd..41c9ae03652b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7484,6 +7484,8 @@ enum skl_disp_power_wells {
 #define  TRANS_CLK_SEL_DISABLED		(0x0<<29)
 #define  TRANS_CLK_SEL_PORT(x)		(((x)+1)<<29)
 
+#define CDCLK_FREQ			_MMIO(0x46200)
+
 #define _TRANSA_MSA_MISC		0x60410
 #define _TRANSB_MSA_MISC		0x61410
 #define _TRANSC_MSA_MISC		0x62410
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 929fd93b3e5d..ea55dd331fac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9647,6 +9647,8 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
 	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
+	I915_WRITE(CDCLK_FREQ, DIV_ROUND_CLOSEST(cdclk, 1000) - 1);
+
 	intel_update_cdclk(dev);
 
 	WARN(cdclk != dev_priv->cdclk_freq,
-- 
2.7.4

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

* [PATCH 2/3] drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq()
  2016-04-26 16:46 [PATCH 0/3] drm/i915: Cdclk related fixes and polish ville.syrjala
  2016-04-26 16:46 ` [PATCH 1/3] drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency ville.syrjala
@ 2016-04-26 16:46 ` ville.syrjala
  2016-04-27 13:06   ` Mika Kahola
  2016-04-26 16:46 ` [PATCH 3/3] drm/i915: Fix comments about GMBUSFREQ register ville.syrjala
  2016-04-27  6:19 ` ✗ Fi.CI.BAT: warning for drm/i915: Cdclk related fixes and polish Patchwork
  3 siblings, 1 reply; 12+ messages in thread
From: ville.syrjala @ 2016-04-26 16:46 UTC (permalink / raw)
  To: intel-gfx

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

No point in reading the cdclk out from the hardware every single time
since we have it cached already. Just return the cached value to the
audio driver.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 56ba8765816e..1063108a9bab 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -624,17 +624,11 @@ static void i915_audio_component_codec_wake_override(struct device *dev,
 static int i915_audio_component_get_cdclk_freq(struct device *dev)
 {
 	struct drm_i915_private *dev_priv = dev_to_i915(dev);
-	int ret;
 
 	if (WARN_ON_ONCE(!HAS_DDI(dev_priv)))
 		return -ENODEV;
 
-	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
-	ret = dev_priv->display.get_display_clock_speed(dev_priv->dev);
-
-	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
-
-	return ret;
+	return dev_priv->cdclk_freq;
 }
 
 static int i915_audio_component_sync_audio_rate(struct device *dev,
-- 
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] 12+ messages in thread

* [PATCH 3/3] drm/i915: Fix comments about GMBUSFREQ register
  2016-04-26 16:46 [PATCH 0/3] drm/i915: Cdclk related fixes and polish ville.syrjala
  2016-04-26 16:46 ` [PATCH 1/3] drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency ville.syrjala
  2016-04-26 16:46 ` [PATCH 2/3] drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq() ville.syrjala
@ 2016-04-26 16:46 ` ville.syrjala
  2016-04-27 13:30   ` Mika Kahola
  2016-04-27  6:19 ` ✗ Fi.CI.BAT: warning for drm/i915: Cdclk related fixes and polish Patchwork
  3 siblings, 1 reply; 12+ messages in thread
From: ville.syrjala @ 2016-04-26 16:46 UTC (permalink / raw)
  To: intel-gfx

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

The comment about GMBUSFREQ is confused. The spec actually explains
the 4MHz thing perfectly by noting that the 4MHz divider values is
actually just bits [9:2] not [9:0], hence the divide by 1000 correct.
Replace the confused note with a quote from the spec, and eliminate
the duplicated comment that snuck in.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ea55dd331fac..ec9144ded255 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5311,18 +5311,13 @@ static void intel_update_cdclk(struct drm_device *dev)
 			 dev_priv->cdclk_freq);
 
 	/*
-	 * Program the gmbus_freq based on the cdclk frequency.
-	 * BSpec erroneously claims we should aim for 4MHz, but
-	 * in fact 1MHz is the correct frequency.
+	 * 9:0 CMBUS [sic] CDCLK frequency (cdfreq):
+	 * Programmng [sic] note: bit[9:2] should be programmed to the number
+	 * of cdclk that generates 4MHz reference clock freq which is used to
+	 * generate GMBus clock. This will vary with the cdclk freq.
 	 */
-	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
-		/*
-		 * Program the gmbus_freq based on the cdclk frequency.
-		 * BSpec erroneously claims we should aim for 4MHz, but
-		 * in fact 1MHz is the correct frequency.
-		 */
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
-	}
 
 	if (dev_priv->max_cdclk_freq == 0)
 		intel_update_max_cdclk(dev);
-- 
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] 12+ messages in thread

* ✗ Fi.CI.BAT: warning for drm/i915: Cdclk related fixes and polish
  2016-04-26 16:46 [PATCH 0/3] drm/i915: Cdclk related fixes and polish ville.syrjala
                   ` (2 preceding siblings ...)
  2016-04-26 16:46 ` [PATCH 3/3] drm/i915: Fix comments about GMBUSFREQ register ville.syrjala
@ 2016-04-27  6:19 ` Patchwork
  2016-04-27 17:28   ` Ville Syrjälä
  3 siblings, 1 reply; 12+ messages in thread
From: Patchwork @ 2016-04-27  6:19 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Cdclk related fixes and polish
URL   : https://patchwork.freedesktop.org/series/6354/
State : warning

== Summary ==

Series 6354v1 drm/i915: Cdclk related fixes and polish
http://patchwork.freedesktop.org/api/1.0/series/6354/revisions/1/mbox/

Test drv_module_reload_basic:
                pass       -> SKIP       (skl-nuci5)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-b:
                incomplete -> 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 
skl-i7k-2        total:200  pass:173  dwarn:0   dfail:0   fail:0   skip:27 
skl-nuci5        total:200  pass:188  dwarn:0   dfail:0   fail:0   skip:12 
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_2080/

e005db1cb2c60d18abe837ac683d8993ea77b239 drm-intel-nightly: 2016y-04m-26d-12h-51m-57s UTC integration manifest
b85d9d7 drm/i915: Fix comments about GMBUSFREQ register
f897835 drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq()
4e443d4 drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency

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

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

* Re: [PATCH 1/3] drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency
  2016-04-26 16:46 ` [PATCH 1/3] drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency ville.syrjala
@ 2016-04-27  8:19   ` Mika Kahola
  0 siblings, 0 replies; 12+ messages in thread
From: Mika Kahola @ 2016-04-27  8:19 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, stable

Indeed, BSpec says that this register should be programmed by CD clock
minus one.

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

On Tue, 2016-04-26 at 19:46 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Update CDCLK_FREQ on BDW after changing the cdclk frequency. Not sure
> if this is a late addition to the spec, or if I simply overlooked this
> step when writing the original code.
> 
> This is what Bspec has to say about CDCLK_FREQ:
> "Program this field to the CD clock frequency minus one. This is used to
>  generate a divided down clock for miscellaneous timers in display."
> 
> And the "Broadwell Sequences for Changing CD Clock Frequency" section
> clarifies this further:
> "For CD clock 337.5 MHz, program 337 decimal.
>  For CD clock 450 MHz, program 449 decimal.
>  For CD clock 540 MHz, program 539 decimal.
>  For CD clock 675 MHz, program 674 decimal."
> 
> Cc: stable@vger.kernel.org
> Cc: Mika Kahola <mika.kahola@intel.com>
> Fixes: b432e5cfd5e9 ("drm/i915: BDW clock change support")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 2 ++
>  drivers/gpu/drm/i915/intel_display.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 03264fd30fdd..41c9ae03652b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7484,6 +7484,8 @@ enum skl_disp_power_wells {
>  #define  TRANS_CLK_SEL_DISABLED		(0x0<<29)
>  #define  TRANS_CLK_SEL_PORT(x)		(((x)+1)<<29)
>  
> +#define CDCLK_FREQ			_MMIO(0x46200)
> +
>  #define _TRANSA_MSA_MISC		0x60410
>  #define _TRANSB_MSA_MISC		0x61410
>  #define _TRANSC_MSA_MISC		0x62410
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 929fd93b3e5d..ea55dd331fac 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9647,6 +9647,8 @@ static void broadwell_set_cdclk(struct drm_device *dev, int cdclk)
>  	sandybridge_pcode_write(dev_priv, HSW_PCODE_DE_WRITE_FREQ_REQ, data);
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> +	I915_WRITE(CDCLK_FREQ, DIV_ROUND_CLOSEST(cdclk, 1000) - 1);
> +
>  	intel_update_cdclk(dev);
>  
>  	WARN(cdclk != dev_priv->cdclk_freq,

-- 
Mika Kahola - Intel OTC

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

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

* Re: [PATCH 2/3] drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq()
  2016-04-26 16:46 ` [PATCH 2/3] drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq() ville.syrjala
@ 2016-04-27 13:06   ` Mika Kahola
  0 siblings, 0 replies; 12+ messages in thread
From: Mika Kahola @ 2016-04-27 13:06 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

Looks reasonable.

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

On Tue, 2016-04-26 at 19:46 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> No point in reading the cdclk out from the hardware every single time
> since we have it cached already. Just return the cached value to the
> audio driver.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 56ba8765816e..1063108a9bab 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -624,17 +624,11 @@ static void i915_audio_component_codec_wake_override(struct device *dev,
>  static int i915_audio_component_get_cdclk_freq(struct device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev_to_i915(dev);
> -	int ret;
>  
>  	if (WARN_ON_ONCE(!HAS_DDI(dev_priv)))
>  		return -ENODEV;
>  
> -	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> -	ret = dev_priv->display.get_display_clock_speed(dev_priv->dev);
> -
> -	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> -
> -	return ret;
> +	return dev_priv->cdclk_freq;
>  }
>  
>  static int i915_audio_component_sync_audio_rate(struct device *dev,

-- 
Mika Kahola - Intel OTC

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

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

* Re: [PATCH 3/3] drm/i915: Fix comments about GMBUSFREQ register
  2016-04-26 16:46 ` [PATCH 3/3] drm/i915: Fix comments about GMBUSFREQ register ville.syrjala
@ 2016-04-27 13:30   ` Mika Kahola
  2016-04-27 13:56     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Mika Kahola @ 2016-04-27 13:30 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

s/Programmng/Programming

With this nitpick fixed, this is

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

On Tue, 2016-04-26 at 19:46 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The comment about GMBUSFREQ is confused. The spec actually explains
> the 4MHz thing perfectly by noting that the 4MHz divider values is
> actually just bits [9:2] not [9:0], hence the divide by 1000 correct.
> Replace the confused note with a quote from the spec, and eliminate
> the duplicated comment that snuck in.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ea55dd331fac..ec9144ded255 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5311,18 +5311,13 @@ static void intel_update_cdclk(struct drm_device *dev)
>  			 dev_priv->cdclk_freq);
>  
>  	/*
> -	 * Program the gmbus_freq based on the cdclk frequency.
> -	 * BSpec erroneously claims we should aim for 4MHz, but
> -	 * in fact 1MHz is the correct frequency.
> +	 * 9:0 CMBUS [sic] CDCLK frequency (cdfreq):
> +	 * Programmng [sic] note: bit[9:2] should be programmed to the number
> +	 * of cdclk that generates 4MHz reference clock freq which is used to
> +	 * generate GMBus clock. This will vary with the cdclk freq.
>  	 */
> -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> -		/*
> -		 * Program the gmbus_freq based on the cdclk frequency.
> -		 * BSpec erroneously claims we should aim for 4MHz, but
> -		 * in fact 1MHz is the correct frequency.
> -		 */
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> -	}
>  
>  	if (dev_priv->max_cdclk_freq == 0)
>  		intel_update_max_cdclk(dev);

-- 
Mika Kahola - Intel OTC

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

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

* Re: [PATCH 3/3] drm/i915: Fix comments about GMBUSFREQ register
  2016-04-27 13:30   ` Mika Kahola
@ 2016-04-27 13:56     ` Ville Syrjälä
  2016-04-28 18:40       ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2016-04-27 13:56 UTC (permalink / raw)
  To: Mika Kahola; +Cc: intel-gfx

On Wed, Apr 27, 2016 at 04:30:14PM +0300, Mika Kahola wrote:
> s/Programmng/Programming

It's straight from the spec, hence the [sic]

> 
> With this nitpick fixed, this is
> 
> Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> 
> On Tue, 2016-04-26 at 19:46 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The comment about GMBUSFREQ is confused. The spec actually explains
> > the 4MHz thing perfectly by noting that the 4MHz divider values is
> > actually just bits [9:2] not [9:0], hence the divide by 1000 correct.
> > Replace the confused note with a quote from the spec, and eliminate
> > the duplicated comment that snuck in.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 15 +++++----------
> >  1 file changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ea55dd331fac..ec9144ded255 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5311,18 +5311,13 @@ static void intel_update_cdclk(struct drm_device *dev)
> >  			 dev_priv->cdclk_freq);
> >  
> >  	/*
> > -	 * Program the gmbus_freq based on the cdclk frequency.
> > -	 * BSpec erroneously claims we should aim for 4MHz, but
> > -	 * in fact 1MHz is the correct frequency.
> > +	 * 9:0 CMBUS [sic] CDCLK frequency (cdfreq):
> > +	 * Programmng [sic] note: bit[9:2] should be programmed to the number
> > +	 * of cdclk that generates 4MHz reference clock freq which is used to
> > +	 * generate GMBus clock. This will vary with the cdclk freq.
> >  	 */
> > -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > -		/*
> > -		 * Program the gmbus_freq based on the cdclk frequency.
> > -		 * BSpec erroneously claims we should aim for 4MHz, but
> > -		 * in fact 1MHz is the correct frequency.
> > -		 */
> > +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> >  		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> > -	}
> >  
> >  	if (dev_priv->max_cdclk_freq == 0)
> >  		intel_update_max_cdclk(dev);
> 
> -- 
> Mika Kahola - Intel OTC

-- 
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] 12+ messages in thread

* Re: ✗ Fi.CI.BAT: warning for drm/i915: Cdclk related fixes and polish
  2016-04-27  6:19 ` ✗ Fi.CI.BAT: warning for drm/i915: Cdclk related fixes and polish Patchwork
@ 2016-04-27 17:28   ` Ville Syrjälä
  2016-04-27 17:34     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2016-04-27 17:28 UTC (permalink / raw)
  To: intel-gfx

On Wed, Apr 27, 2016 at 06:19:15AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Cdclk related fixes and polish
> URL   : https://patchwork.freedesktop.org/series/6354/
> State : warning
> 
> == Summary ==
> 
> Series 6354v1 drm/i915: Cdclk related fixes and polish
> http://patchwork.freedesktop.org/api/1.0/series/6354/revisions/1/mbox/
> 
> Test drv_module_reload_basic:
>                 pass       -> SKIP       (skl-nuci5)

Reloading i915.ko with
unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
module successfully unloaded
module successfully loaded again
Reloading i915.ko with inject_load_failure=1
unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
Reloading i915.ko with inject_load_failure=2
unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
Reloading i915.ko with inject_load_failure=3
unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
Reloading i915.ko with inject_load_failure=4
unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
Reloading i915.ko with
unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device

rmmod: ERROR: Module i915 is in use
rmmod: ERROR: Module i915 is in use
rmmod: ERROR: Module i915 is in use
rmmod: ERROR: Module i915 is in use
rmmod: ERROR: Module i915 is in use

So it managed to unload once, and after that it was no longer possible
to unload.

[  436.902443] Console: switching to colour dummy device 80x25
...
[  439.185816] [drm] Module unloaded
[  439.285377] [drm:intel_detect_pch] Found SunrisePoint LP PCH
...
[  440.671701] [drm] Initialized i915 1.6.0 20160425 for 0000:00:02.0 on minor 0
[  440.690772] snd_hda_intel 0000:00:1f.3: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915])
[  440.719344] snd_hda_codec_realtek hdaudioC0D0: autoconfig for ALC283: line_outs=1 (0x21/0x0/0x0/0x0/0x0) type:hp
[  440.719352] snd_hda_codec_realtek hdaudioC0D0:    speaker_outs=0 (0x0/0x0/0x0/0x0/0x0)
[  440.719356] snd_hda_codec_realtek hdaudioC0D0:    hp_outs=0 (0x0/0x0/0x0/0x0/0x0)
[  440.719359] snd_hda_codec_realtek hdaudioC0D0:    mono: mono_out=0x0
[  440.719362] snd_hda_codec_realtek hdaudioC0D0:    inputs:
[  440.719366] snd_hda_codec_realtek hdaudioC0D0:      Mic=0x19
[  440.798836] input: HDA Intel PCH Mic as /devices/pci0000:00/0000:00:1f.3/sound/card0/input13
[  440.801278] input: HDA Intel PCH Headphone as /devices/pci0000:00/0000:00:1f.3/sound/card0/input14
[  440.802905] input: HDA Intel PCH HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input15
[  440.804662] input: HDA Intel PCH HDMI/DP,pcm=7 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input16
[  440.806351] input: HDA Intel PCH HDMI/DP,pcm=8 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input17

after that nothing related. No real idea who/what was supposedly using
the module afterwards. Could have been snd-hda I suppose. It doesn't
seem to leave any trace in dmesg when it unloads, so can't tell from the dmesg.

> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-b:
>                 incomplete -> 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 
> skl-i7k-2        total:200  pass:173  dwarn:0   dfail:0   fail:0   skip:27 
> skl-nuci5        total:200  pass:188  dwarn:0   dfail:0   fail:0   skip:12 
> 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_2080/
> 
> e005db1cb2c60d18abe837ac683d8993ea77b239 drm-intel-nightly: 2016y-04m-26d-12h-51m-57s UTC integration manifest
> b85d9d7 drm/i915: Fix comments about GMBUSFREQ register
> f897835 drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq()
> 4e443d4 drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency

-- 
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] 12+ messages in thread

* Re: ✗ Fi.CI.BAT:  warning for drm/i915: Cdclk related fixes and polish
  2016-04-27 17:28   ` Ville Syrjälä
@ 2016-04-27 17:34     ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2016-04-27 17:34 UTC (permalink / raw)
  To: intel-gfx

On Wed, Apr 27, 2016 at 08:28:17PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 27, 2016 at 06:19:15AM -0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: drm/i915: Cdclk related fixes and polish
> > URL   : https://patchwork.freedesktop.org/series/6354/
> > State : warning
> > 
> > == Summary ==
> > 
> > Series 6354v1 drm/i915: Cdclk related fixes and polish
> > http://patchwork.freedesktop.org/api/1.0/series/6354/revisions/1/mbox/
> > 
> > Test drv_module_reload_basic:
> >                 pass       -> SKIP       (skl-nuci5)
> 
> Reloading i915.ko with
> unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
> module successfully unloaded
> module successfully loaded again
> Reloading i915.ko with inject_load_failure=1
> unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
> Reloading i915.ko with inject_load_failure=2
> unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
> Reloading i915.ko with inject_load_failure=3
> unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
> Reloading i915.ko with inject_load_failure=4
> unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
> Reloading i915.ko with
> unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
> 
> rmmod: ERROR: Module i915 is in use
> rmmod: ERROR: Module i915 is in use
> rmmod: ERROR: Module i915 is in use
> rmmod: ERROR: Module i915 is in use
> rmmod: ERROR: Module i915 is in use
> 
> So it managed to unload once, and after that it was no longer possible
> to unload.
> 
> [  436.902443] Console: switching to colour dummy device 80x25
> ...
> [  439.185816] [drm] Module unloaded
> [  439.285377] [drm:intel_detect_pch] Found SunrisePoint LP PCH
> ...
> [  440.671701] [drm] Initialized i915 1.6.0 20160425 for 0000:00:02.0 on minor 0
> [  440.690772] snd_hda_intel 0000:00:1f.3: bound 0000:00:02.0 (ops i915_audio_component_bind_ops [i915])
> [  440.719344] snd_hda_codec_realtek hdaudioC0D0: autoconfig for ALC283: line_outs=1 (0x21/0x0/0x0/0x0/0x0) type:hp
> [  440.719352] snd_hda_codec_realtek hdaudioC0D0:    speaker_outs=0 (0x0/0x0/0x0/0x0/0x0)
> [  440.719356] snd_hda_codec_realtek hdaudioC0D0:    hp_outs=0 (0x0/0x0/0x0/0x0/0x0)
> [  440.719359] snd_hda_codec_realtek hdaudioC0D0:    mono: mono_out=0x0
> [  440.719362] snd_hda_codec_realtek hdaudioC0D0:    inputs:
> [  440.719366] snd_hda_codec_realtek hdaudioC0D0:      Mic=0x19
> [  440.798836] input: HDA Intel PCH Mic as /devices/pci0000:00/0000:00:1f.3/sound/card0/input13
> [  440.801278] input: HDA Intel PCH Headphone as /devices/pci0000:00/0000:00:1f.3/sound/card0/input14
> [  440.802905] input: HDA Intel PCH HDMI/DP,pcm=3 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input15
> [  440.804662] input: HDA Intel PCH HDMI/DP,pcm=7 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input16
> [  440.806351] input: HDA Intel PCH HDMI/DP,pcm=8 as /devices/pci0000:00/0000:00:1f.3/sound/card0/input17
> 
> after that nothing related. No real idea who/what was supposedly using
> the module afterwards. Could have been snd-hda I suppose. It doesn't
> seem to leave any trace in dmesg when it unloads, so can't tell from the dmesg.

Oh there was actually 
[  440.936260] Console: switching to colour dummy device 80x25
afterwards, so at least the console unbind worked a second time.

> 
> > Test kms_pipe_crc_basic:
> >         Subgroup hang-read-crc-pipe-b:
> >                 incomplete -> 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 
> > skl-i7k-2        total:200  pass:173  dwarn:0   dfail:0   fail:0   skip:27 
> > skl-nuci5        total:200  pass:188  dwarn:0   dfail:0   fail:0   skip:12 
> > 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_2080/
> > 
> > e005db1cb2c60d18abe837ac683d8993ea77b239 drm-intel-nightly: 2016y-04m-26d-12h-51m-57s UTC integration manifest
> > b85d9d7 drm/i915: Fix comments about GMBUSFREQ register
> > f897835 drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq()
> > 4e443d4 drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] 12+ messages in thread

* Re: [PATCH 3/3] drm/i915: Fix comments about GMBUSFREQ register
  2016-04-27 13:56     ` Ville Syrjälä
@ 2016-04-28 18:40       ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2016-04-28 18:40 UTC (permalink / raw)
  To: Mika Kahola; +Cc: intel-gfx

On Wed, Apr 27, 2016 at 04:56:42PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 27, 2016 at 04:30:14PM +0300, Mika Kahola wrote:
> > s/Programmng/Programming
> 
> It's straight from the spec, hence the [sic]

Didn't get any further objections, so pushed the series to dinq, with the typoes
and [sic]s intact.

Thans for the reviews.

> 
> > 
> > With this nitpick fixed, this is
> > 
> > Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> > 
> > On Tue, 2016-04-26 at 19:46 +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > The comment about GMBUSFREQ is confused. The spec actually explains
> > > the 4MHz thing perfectly by noting that the 4MHz divider values is
> > > actually just bits [9:2] not [9:0], hence the divide by 1000 correct.
> > > Replace the confused note with a quote from the spec, and eliminate
> > > the duplicated comment that snuck in.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 15 +++++----------
> > >  1 file changed, 5 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index ea55dd331fac..ec9144ded255 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5311,18 +5311,13 @@ static void intel_update_cdclk(struct drm_device *dev)
> > >  			 dev_priv->cdclk_freq);
> > >  
> > >  	/*
> > > -	 * Program the gmbus_freq based on the cdclk frequency.
> > > -	 * BSpec erroneously claims we should aim for 4MHz, but
> > > -	 * in fact 1MHz is the correct frequency.
> > > +	 * 9:0 CMBUS [sic] CDCLK frequency (cdfreq):
> > > +	 * Programmng [sic] note: bit[9:2] should be programmed to the number
> > > +	 * of cdclk that generates 4MHz reference clock freq which is used to
> > > +	 * generate GMBus clock. This will vary with the cdclk freq.
> > >  	 */
> > > -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > > -		/*
> > > -		 * Program the gmbus_freq based on the cdclk frequency.
> > > -		 * BSpec erroneously claims we should aim for 4MHz, but
> > > -		 * in fact 1MHz is the correct frequency.
> > > -		 */
> > > +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > >  		I915_WRITE(GMBUSFREQ_VLV, DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> > > -	}
> > >  
> > >  	if (dev_priv->max_cdclk_freq == 0)
> > >  		intel_update_max_cdclk(dev);
> > 
> > -- 
> > Mika Kahola - Intel OTC
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
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] 12+ messages in thread

end of thread, other threads:[~2016-04-28 18:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-26 16:46 [PATCH 0/3] drm/i915: Cdclk related fixes and polish ville.syrjala
2016-04-26 16:46 ` [PATCH 1/3] drm/i915: Update CDCLK_FREQ register on BDW after changing cdclk frequency ville.syrjala
2016-04-27  8:19   ` Mika Kahola
2016-04-26 16:46 ` [PATCH 2/3] drm/i915: Use cached cdclk value in i915_audio_component_get_cdclk_freq() ville.syrjala
2016-04-27 13:06   ` Mika Kahola
2016-04-26 16:46 ` [PATCH 3/3] drm/i915: Fix comments about GMBUSFREQ register ville.syrjala
2016-04-27 13:30   ` Mika Kahola
2016-04-27 13:56     ` Ville Syrjälä
2016-04-28 18:40       ` Ville Syrjälä
2016-04-27  6:19 ` ✗ Fi.CI.BAT: warning for drm/i915: Cdclk related fixes and polish Patchwork
2016-04-27 17:28   ` Ville Syrjälä
2016-04-27 17:34     ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox