All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] drm/i915: Some VLV/CHV rc6/rps fixes/cleanups
@ 2014-11-07 19:33 ville.syrjala
  2014-11-07 19:33 ` [PATCH 1/5] drm/i915: Silence valleyview_set_rps() ville.syrjala
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: ville.syrjala @ 2014-11-07 19:33 UTC (permalink / raw)
  To: intel-gfx

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

Here's a random collection of patches touching the vlv/chv rc6/rps
code. The CCK fuse thing is the only fix here, the rest is mostly
just me getting annoyed at stuff.

Ville Syrjälä (5):
  drm/i915: Silence valleyview_set_rps()
  drm/i915: Read the CCK fuse register from CCK
  drm/i915: Add a name for the Punit GPLLENABLE bit
  drm/i915: Warn if GPLL isn't used on vlv/chv
  drm/i915: Improve PCBR debug information

 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 31 +++++++++++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

-- 
2.0.4

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

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

* [PATCH 1/5] drm/i915: Silence valleyview_set_rps()
  2014-11-07 19:33 [PATCH 0/5] drm/i915: Some VLV/CHV rc6/rps fixes/cleanups ville.syrjala
@ 2014-11-07 19:33 ` ville.syrjala
  2014-11-18  8:26   ` Deepak S
  2014-11-07 19:33 ` [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK ville.syrjala
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: ville.syrjala @ 2014-11-07 19:33 UTC (permalink / raw)
  To: intel-gfx

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

Even with the rps debug messages signficantly recuced  by
 commit 67956867aa07c59d6d83628bbc9ee4bd9799a1e1
 Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
 Date:   Tue Sep 2 15:12:17 2014 +0300

    drm/i915: Don't spam dmesg with rps messages on vlv/chv

we still get an inordinate amount of spam from this. Just kill the debug
print. If someone wants to observe it they can just use the tracepoint.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 300d7e5..9285dee 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4504,14 +4504,8 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 		      "Odd GPU freq value\n"))
 		val &= ~1;
 
-	if (val != dev_priv->rps.cur_freq) {
-		DRM_DEBUG_DRIVER("GPU freq request from %d MHz (%u) to %d MHz (%u)\n",
-				 vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
-				 dev_priv->rps.cur_freq,
-				 vlv_gpu_freq(dev_priv, val), val);
-
+	if (val != dev_priv->rps.cur_freq)
 		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
-	}
 
 	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
-- 
2.0.4

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

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

* [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK
  2014-11-07 19:33 [PATCH 0/5] drm/i915: Some VLV/CHV rc6/rps fixes/cleanups ville.syrjala
  2014-11-07 19:33 ` [PATCH 1/5] drm/i915: Silence valleyview_set_rps() ville.syrjala
@ 2014-11-07 19:33 ` ville.syrjala
  2014-11-12  3:10   ` Deepak S
  2014-11-07 19:33 ` [PATCH 3/5] drm/i915: Add a name for the Punit GPLLENABLE bit ville.syrjala
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: ville.syrjala @ 2014-11-07 19:33 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

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

When reading a CCK register we should obviously read it from CCK not
Punit. This problem has been present ever since this of code was
introduced in

 commit 67c3bf6f55a97a0915a0f9ea07278a3073cc9601
 Author: Deepak S <deepak.s@linux.intel.com>
 Date:   Thu Jul 10 13:16:24 2014 +0530

    drm/i915: populate mem_freq/cz_clock for chv

The problem was raised during review by Mika [1] but somehow slipped
through the cracks, and the patch got applied with the problem unfixed.

[1] http://lists.freedesktop.org/archives/intel-gfx/2014-July/048937.html

Cc: Deepak S <deepak.s@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9285dee..befad36 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5253,7 +5253,10 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
 
 	mutex_lock(&dev_priv->rps.hw_lock);
 
-	val = vlv_punit_read(dev_priv, CCK_FUSE_REG);
+	mutex_lock(&dev_priv->dpio_lock);
+	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
+	mutex_unlock(&dev_priv->dpio_lock);
+
 	switch ((val >> 2) & 0x7) {
 	case 0:
 	case 1:
-- 
2.0.4

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

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

* [PATCH 3/5] drm/i915: Add a name for the Punit GPLLENABLE bit
  2014-11-07 19:33 [PATCH 0/5] drm/i915: Some VLV/CHV rc6/rps fixes/cleanups ville.syrjala
  2014-11-07 19:33 ` [PATCH 1/5] drm/i915: Silence valleyview_set_rps() ville.syrjala
  2014-11-07 19:33 ` [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK ville.syrjala
@ 2014-11-07 19:33 ` ville.syrjala
  2014-11-18  9:20   ` Deepak S
  2014-11-07 19:33 ` [PATCH 4/5] drm/i915: Warn if GPLL isn't used on vlv/chv ville.syrjala
  2014-11-07 19:33 ` [PATCH 5/5] drm/i915: Improve PCBR debug information ville.syrjala
  4 siblings, 1 reply; 14+ messages in thread
From: ville.syrjala @ 2014-11-07 19:33 UTC (permalink / raw)
  To: intel-gfx

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

Remove the magic number for the GPLLENABLE bit by adding a name for it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_pm.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d43fa0e..ec4dc00 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -564,6 +564,7 @@ enum punit_power_well {
 #define PUNIT_REG_GPU_LFM			0xd3
 #define PUNIT_REG_GPU_FREQ_REQ			0xd4
 #define PUNIT_REG_GPU_FREQ_STS			0xd8
+#define   GPLLENABLE				(1<<4)
 #define   GENFREQSTATUS				(1<<0)
 #define PUNIT_REG_MEDIA_TURBO_FREQ_REQ		0xdc
 #define PUNIT_REG_CZ_TIMESTAMP			0xce
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index befad36..71eb377 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5397,7 +5397,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
-	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
+	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
 
 	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
@@ -5477,7 +5477,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
-	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
+	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
 
 	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
-- 
2.0.4

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

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

* [PATCH 4/5] drm/i915: Warn if GPLL isn't used on vlv/chv
  2014-11-07 19:33 [PATCH 0/5] drm/i915: Some VLV/CHV rc6/rps fixes/cleanups ville.syrjala
                   ` (2 preceding siblings ...)
  2014-11-07 19:33 ` [PATCH 3/5] drm/i915: Add a name for the Punit GPLLENABLE bit ville.syrjala
@ 2014-11-07 19:33 ` ville.syrjala
  2014-11-18  9:19   ` Deepak S
  2014-11-07 19:33 ` [PATCH 5/5] drm/i915: Improve PCBR debug information ville.syrjala
  4 siblings, 1 reply; 14+ messages in thread
From: ville.syrjala @ 2014-11-07 19:33 UTC (permalink / raw)
  To: intel-gfx

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

Our freq<->opcode conversions assume that GPLL is always used.
Apparently that should be the case always, but let's scream if we
ever encounter something different.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 71eb377..e8a6f92 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5397,6 +5397,9 @@ static void cherryview_enable_rps(struct drm_device *dev)
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
+	/* RPS code assumes GPLL is used */
+	WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
+
 	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
 
@@ -5477,6 +5480,9 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 
+	/* RPS code assumes GPLL is used */
+	WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
+
 	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
 
-- 
2.0.4

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

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

* [PATCH 5/5] drm/i915: Improve PCBR debug information
  2014-11-07 19:33 [PATCH 0/5] drm/i915: Some VLV/CHV rc6/rps fixes/cleanups ville.syrjala
                   ` (3 preceding siblings ...)
  2014-11-07 19:33 ` [PATCH 4/5] drm/i915: Warn if GPLL isn't used on vlv/chv ville.syrjala
@ 2014-11-07 19:33 ` ville.syrjala
  2014-11-18  9:18   ` Deepak S
  4 siblings, 1 reply; 14+ messages in thread
From: ville.syrjala @ 2014-11-07 19:33 UTC (permalink / raw)
  To: intel-gfx

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

Always print the final PCBR register value on both vlv and chv, and
also tell us whether the BIOS was a good citizen or not.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e8a6f92..ef8e055 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5126,12 +5126,15 @@ static void cherryview_setup_pctx(struct drm_device *dev)
 
 	pcbr = I915_READ(VLV_PCBR);
 	if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) {
+		DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
 		paddr = (dev_priv->mm.stolen_base +
 			 (gtt->stolen_size - pctx_size));
 
 		pctx_paddr = (paddr & (~4095));
 		I915_WRITE(VLV_PCBR, pctx_paddr);
 	}
+
+	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
 }
 
 static void valleyview_setup_pctx(struct drm_device *dev)
@@ -5157,6 +5160,8 @@ static void valleyview_setup_pctx(struct drm_device *dev)
 		goto out;
 	}
 
+	DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
+
 	/*
 	 * From the Gunit register HAS:
 	 * The Gfx driver is expected to program this register and ensure
@@ -5175,6 +5180,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
 	I915_WRITE(VLV_PCBR, pctx_paddr);
 
 out:
+	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
 	dev_priv->vlv_pctx = pctx;
 }
 
@@ -5366,8 +5372,6 @@ static void cherryview_enable_rps(struct drm_device *dev)
 	/* For now we assume BIOS is allocating and populating the PCBR  */
 	pcbr = I915_READ(VLV_PCBR);
 
-	DRM_DEBUG_DRIVER("PCBR offset : 0x%x\n", pcbr);
-
 	/* 3: Enable RC6 */
 	if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
 						(pcbr >> VLV_PCBR_ADDR_SHIFT))
-- 
2.0.4

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

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

* Re: [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK
  2014-11-12  3:10   ` Deepak S
@ 2014-11-11 15:28     ` Daniel Vetter
  2014-11-11 15:39       ` S, Deepak
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2014-11-11 15:28 UTC (permalink / raw)
  To: Deepak S; +Cc: intel-gfx, Mika Kuoppala

On Wed, Nov 12, 2014 at 08:40:47AM +0530, Deepak S wrote:
> 
> On Saturday 08 November 2014 01:03 AM, ville.syrjala@linux.intel.com wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >When reading a CCK register we should obviously read it from CCK not
> >Punit. This problem has been present ever since this of code was
> >introduced in
> >
> >  commit 67c3bf6f55a97a0915a0f9ea07278a3073cc9601
> >  Author: Deepak S <deepak.s@linux.intel.com>
> >  Date:   Thu Jul 10 13:16:24 2014 +0530
> >
> >     drm/i915: populate mem_freq/cz_clock for chv
> >
> >The problem was raised during review by Mika [1] but somehow slipped
> >through the cracks, and the patch got applied with the problem unfixed.
> >
> >[1] http://lists.freedesktop.org/archives/intel-gfx/2014-July/048937.html
> >
> >Cc: Deepak S <deepak.s@linux.intel.com>
> >Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >index 9285dee..befad36 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -5253,7 +5253,10 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
> >  	mutex_lock(&dev_priv->rps.hw_lock);
> >-	val = vlv_punit_read(dev_priv, CCK_FUSE_REG);
> >+	mutex_lock(&dev_priv->dpio_lock);
> >+	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
> >+	mutex_unlock(&dev_priv->dpio_lock);
> >+
> >  	switch ((val >> 2) & 0x7) {
> >  	case 0:
> >  	case 1:
> >
> >
> Oops i missed the  comment.
> Reviewed-by: Deepak S <deepak.s@linux.intel.com>

Queued for -next, thanks for the patch.

Deepak, can you please review the other patches in Ville's series here,
too?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK
  2014-11-11 15:28     ` Daniel Vetter
@ 2014-11-11 15:39       ` S, Deepak
  0 siblings, 0 replies; 14+ messages in thread
From: S, Deepak @ 2014-11-11 15:39 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, Mika Kuoppala


On 11/11/2014 8:58 PM, Daniel Vetter wrote:
> On Wed, Nov 12, 2014 at 08:40:47AM +0530, Deepak S wrote:
>> On Saturday 08 November 2014 01:03 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> When reading a CCK register we should obviously read it from CCK not
>>> Punit. This problem has been present ever since this of code was
>>> introduced in
>>>
>>>   commit 67c3bf6f55a97a0915a0f9ea07278a3073cc9601
>>>   Author: Deepak S <deepak.s@linux.intel.com>
>>>   Date:   Thu Jul 10 13:16:24 2014 +0530
>>>
>>>      drm/i915: populate mem_freq/cz_clock for chv
>>>
>>> The problem was raised during review by Mika [1] but somehow slipped
>>> through the cracks, and the patch got applied with the problem unfixed.
>>>
>>> [1] http://lists.freedesktop.org/archives/intel-gfx/2014-July/048937.html
>>>
>>> Cc: Deepak S <deepak.s@linux.intel.com>
>>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 9285dee..befad36 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -5253,7 +5253,10 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>>>   	mutex_lock(&dev_priv->rps.hw_lock);
>>> -	val = vlv_punit_read(dev_priv, CCK_FUSE_REG);
>>> +	mutex_lock(&dev_priv->dpio_lock);
>>> +	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
>>> +	mutex_unlock(&dev_priv->dpio_lock);
>>> +
>>>   	switch ((val >> 2) & 0x7) {
>>>   	case 0:
>>>   	case 1:
>>>
>>>
>> Oops i missed the  comment.
>> Reviewed-by: Deepak S <deepak.s@linux.intel.com>
> Queued for -next, thanks for the patch.
>
> Deepak, can you please review the other patches in Ville's series here,
> too?
Sure Daniel. Since CCK was a bug fix reviewed that first.
Other patches Just Started to review. I will send the comments. if any.
> Thanks, Daniel

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

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

* Re: [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK
  2014-11-07 19:33 ` [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK ville.syrjala
@ 2014-11-12  3:10   ` Deepak S
  2014-11-11 15:28     ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Deepak S @ 2014-11-12  3:10 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx; +Cc: Mika Kuoppala


On Saturday 08 November 2014 01:03 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> When reading a CCK register we should obviously read it from CCK not
> Punit. This problem has been present ever since this of code was
> introduced in
>
>   commit 67c3bf6f55a97a0915a0f9ea07278a3073cc9601
>   Author: Deepak S <deepak.s@linux.intel.com>
>   Date:   Thu Jul 10 13:16:24 2014 +0530
>
>      drm/i915: populate mem_freq/cz_clock for chv
>
> The problem was raised during review by Mika [1] but somehow slipped
> through the cracks, and the patch got applied with the problem unfixed.
>
> [1] http://lists.freedesktop.org/archives/intel-gfx/2014-July/048937.html
>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9285dee..befad36 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5253,7 +5253,10 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>   
>   	mutex_lock(&dev_priv->rps.hw_lock);
> -	val = vlv_punit_read(dev_priv, CCK_FUSE_REG);
> +	mutex_lock(&dev_priv->dpio_lock);
> +	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
> +	mutex_unlock(&dev_priv->dpio_lock);
> +
>   	switch ((val >> 2) & 0x7) {
>   	case 0:
>   	case 1:
>
>
Oops i missed the  comment.
Reviewed-by: Deepak S <deepak.s@linux.intel.com>

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

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

* Re: [PATCH 5/5] drm/i915: Improve PCBR debug information
  2014-11-18  9:18   ` Deepak S
@ 2014-11-17 14:31     ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2014-11-17 14:31 UTC (permalink / raw)
  To: Deepak S; +Cc: intel-gfx

On Tue, Nov 18, 2014 at 02:48:27PM +0530, Deepak S wrote:
> 
> On Saturday 08 November 2014 01:03 AM, ville.syrjala@linux.intel.com wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >Always print the final PCBR register value on both vlv and chv, and
> >also tell us whether the BIOS was a good citizen or not.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_pm.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >index e8a6f92..ef8e055 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -5126,12 +5126,15 @@ static void cherryview_setup_pctx(struct drm_device *dev)
> >  	pcbr = I915_READ(VLV_PCBR);
> >  	if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) {
> >+		DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
> >  		paddr = (dev_priv->mm.stolen_base +
> >  			 (gtt->stolen_size - pctx_size));
> >  		pctx_paddr = (paddr & (~4095));
> >  		I915_WRITE(VLV_PCBR, pctx_paddr);
> >  	}
> >+
> >+	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
> >  }
> >  static void valleyview_setup_pctx(struct drm_device *dev)
> >@@ -5157,6 +5160,8 @@ static void valleyview_setup_pctx(struct drm_device *dev)
> >  		goto out;
> >  	}
> >+	DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
> >+
> >  	/*
> >  	 * From the Gunit register HAS:
> >  	 * The Gfx driver is expected to program this register and ensure
> >@@ -5175,6 +5180,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
> >  	I915_WRITE(VLV_PCBR, pctx_paddr);
> >  out:
> >+	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
> >  	dev_priv->vlv_pctx = pctx;
> >  }
> >@@ -5366,8 +5372,6 @@ static void cherryview_enable_rps(struct drm_device *dev)
> >  	/* For now we assume BIOS is allocating and populating the PCBR  */
> >  	pcbr = I915_READ(VLV_PCBR);
> >-	DRM_DEBUG_DRIVER("PCBR offset : 0x%x\n", pcbr);
> >-
> >  	/* 3: Enable RC6 */
> >  	if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
> >  						(pcbr >> VLV_PCBR_ADDR_SHIFT))
> 
> Reviewed-by: Deepak S <deepak.s@linux.intel.com>

Thanks for the patches&review, all merged to dinq now.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/5] drm/i915: Silence valleyview_set_rps()
  2014-11-07 19:33 ` [PATCH 1/5] drm/i915: Silence valleyview_set_rps() ville.syrjala
@ 2014-11-18  8:26   ` Deepak S
  0 siblings, 0 replies; 14+ messages in thread
From: Deepak S @ 2014-11-18  8:26 UTC (permalink / raw)
  To: intel-gfx


On Saturday 08 November 2014 01:03 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Even with the rps debug messages signficantly recuced  by
>   commit 67956867aa07c59d6d83628bbc9ee4bd9799a1e1
>   Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
>   Date:   Tue Sep 2 15:12:17 2014 +0300
>
>      drm/i915: Don't spam dmesg with rps messages on vlv/chv
>
> we still get an inordinate amount of spam from this. Just kill the debug
> print. If someone wants to observe it they can just use the tracepoint.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 8 +-------
>   1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 300d7e5..9285dee 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4504,14 +4504,8 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
>   		      "Odd GPU freq value\n"))
>   		val &= ~1;
>   
> -	if (val != dev_priv->rps.cur_freq) {
> -		DRM_DEBUG_DRIVER("GPU freq request from %d MHz (%u) to %d MHz (%u)\n",
> -				 vlv_gpu_freq(dev_priv, dev_priv->rps.cur_freq),
> -				 dev_priv->rps.cur_freq,
> -				 vlv_gpu_freq(dev_priv, val), val);
> -
> +	if (val != dev_priv->rps.cur_freq)
>   		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> -	}
>   
>   	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
>   
:)

- Reviewed-by: Deepak S<deepak.s@linux.intel.com>

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

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

* Re: [PATCH 5/5] drm/i915: Improve PCBR debug information
  2014-11-07 19:33 ` [PATCH 5/5] drm/i915: Improve PCBR debug information ville.syrjala
@ 2014-11-18  9:18   ` Deepak S
  2014-11-17 14:31     ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Deepak S @ 2014-11-18  9:18 UTC (permalink / raw)
  To: intel-gfx


On Saturday 08 November 2014 01:03 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Always print the final PCBR register value on both vlv and chv, and
> also tell us whether the BIOS was a good citizen or not.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e8a6f92..ef8e055 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5126,12 +5126,15 @@ static void cherryview_setup_pctx(struct drm_device *dev)
>   
>   	pcbr = I915_READ(VLV_PCBR);
>   	if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) {
> +		DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
>   		paddr = (dev_priv->mm.stolen_base +
>   			 (gtt->stolen_size - pctx_size));
>   
>   		pctx_paddr = (paddr & (~4095));
>   		I915_WRITE(VLV_PCBR, pctx_paddr);
>   	}
> +
> +	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
>   }
>   
>   static void valleyview_setup_pctx(struct drm_device *dev)
> @@ -5157,6 +5160,8 @@ static void valleyview_setup_pctx(struct drm_device *dev)
>   		goto out;
>   	}
>   
> +	DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
> +
>   	/*
>   	 * From the Gunit register HAS:
>   	 * The Gfx driver is expected to program this register and ensure
> @@ -5175,6 +5180,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
>   	I915_WRITE(VLV_PCBR, pctx_paddr);
>   
>   out:
> +	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
>   	dev_priv->vlv_pctx = pctx;
>   }
>   
> @@ -5366,8 +5372,6 @@ static void cherryview_enable_rps(struct drm_device *dev)
>   	/* For now we assume BIOS is allocating and populating the PCBR  */
>   	pcbr = I915_READ(VLV_PCBR);
>   
> -	DRM_DEBUG_DRIVER("PCBR offset : 0x%x\n", pcbr);
> -
>   	/* 3: Enable RC6 */
>   	if ((intel_enable_rc6(dev) & INTEL_RC6_ENABLE) &&
>   						(pcbr >> VLV_PCBR_ADDR_SHIFT))

Reviewed-by: Deepak S <deepak.s@linux.intel.com>

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

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

* Re: [PATCH 4/5] drm/i915: Warn if GPLL isn't used on vlv/chv
  2014-11-07 19:33 ` [PATCH 4/5] drm/i915: Warn if GPLL isn't used on vlv/chv ville.syrjala
@ 2014-11-18  9:19   ` Deepak S
  0 siblings, 0 replies; 14+ messages in thread
From: Deepak S @ 2014-11-18  9:19 UTC (permalink / raw)
  To: intel-gfx


On Saturday 08 November 2014 01:03 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Our freq<->opcode conversions assume that GPLL is always used.
> Apparently that should be the case always, but let's scream if we
> ever encounter something different.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 71eb377..e8a6f92 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5397,6 +5397,9 @@ static void cherryview_enable_rps(struct drm_device *dev)
>   
>   	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>   
> +	/* RPS code assumes GPLL is used */
> +	WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
> +
>   	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
>   	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
>   
> @@ -5477,6 +5480,9 @@ static void valleyview_enable_rps(struct drm_device *dev)
>   
>   	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>   
> +	/* RPS code assumes GPLL is used */
> +	WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
> +
>   	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
>   	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
>   

Yup better to give warning if our assumption is wrong:)

Reviewed-by: Deepak S <deepak.s@linux.intel.com>

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

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

* Re: [PATCH 3/5] drm/i915: Add a name for the Punit GPLLENABLE bit
  2014-11-07 19:33 ` [PATCH 3/5] drm/i915: Add a name for the Punit GPLLENABLE bit ville.syrjala
@ 2014-11-18  9:20   ` Deepak S
  0 siblings, 0 replies; 14+ messages in thread
From: Deepak S @ 2014-11-18  9:20 UTC (permalink / raw)
  To: intel-gfx


On Saturday 08 November 2014 01:03 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Remove the magic number for the GPLLENABLE bit by adding a name for it.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h | 1 +
>   drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>   2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d43fa0e..ec4dc00 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -564,6 +564,7 @@ enum punit_power_well {
>   #define PUNIT_REG_GPU_LFM			0xd3
>   #define PUNIT_REG_GPU_FREQ_REQ			0xd4
>   #define PUNIT_REG_GPU_FREQ_STS			0xd8
> +#define   GPLLENABLE				(1<<4)
>   #define   GENFREQSTATUS				(1<<0)
>   #define PUNIT_REG_MEDIA_TURBO_FREQ_REQ		0xdc
>   #define PUNIT_REG_CZ_TIMESTAMP			0xce
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index befad36..71eb377 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5397,7 +5397,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
>   
>   	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>   
> -	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
> +	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
>   	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
>   
>   	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
> @@ -5477,7 +5477,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
>   
>   	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>   
> -	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & 0x10 ? "yes" : "no");
> +	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
>   	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
>   
:)

Reviewed-by: Deepak S <deepak.s@linux.intel.com>


>   	dev_priv->rps.cur_freq = (val >> 8) & 0xff;

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

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

end of thread, other threads:[~2014-11-17 14:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-07 19:33 [PATCH 0/5] drm/i915: Some VLV/CHV rc6/rps fixes/cleanups ville.syrjala
2014-11-07 19:33 ` [PATCH 1/5] drm/i915: Silence valleyview_set_rps() ville.syrjala
2014-11-18  8:26   ` Deepak S
2014-11-07 19:33 ` [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK ville.syrjala
2014-11-12  3:10   ` Deepak S
2014-11-11 15:28     ` Daniel Vetter
2014-11-11 15:39       ` S, Deepak
2014-11-07 19:33 ` [PATCH 3/5] drm/i915: Add a name for the Punit GPLLENABLE bit ville.syrjala
2014-11-18  9:20   ` Deepak S
2014-11-07 19:33 ` [PATCH 4/5] drm/i915: Warn if GPLL isn't used on vlv/chv ville.syrjala
2014-11-18  9:19   ` Deepak S
2014-11-07 19:33 ` [PATCH 5/5] drm/i915: Improve PCBR debug information ville.syrjala
2014-11-18  9:18   ` Deepak S
2014-11-17 14:31     ` Daniel Vetter

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.