diff mbox series

[v2,2/2] drm/i915/tgl: Read SAGV block time from PCODE

Message ID 20190927222427.5491-2-james.ausmus@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] drm/i915: Move SAGV block time to dev_priv | expand

Commit Message

James Ausmus Sept. 27, 2019, 10:24 p.m. UTC 
Starting from TGL, we now need to read the SAGV block time via a PCODE
mailbox, rather than having a static value.

BSpec: 49326

v2: Fix up pcode val data type (Ville), tighten variable scope (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

Comments

James Ausmus Oct. 4, 2019, 9:51 p.m. UTC | #1 
On Fri, Oct 04, 2019 at 01:55:46PM -0700, Lucas De Marchi wrote:
> On Fri, Sep 27, 2019 at 03:24:27PM -0700, James Ausmus wrote:
> >Starting from TGL, we now need to read the SAGV block time via a PCODE
> >mailbox, rather than having a static value.
> >
> >BSpec: 49326
> >
> >v2: Fix up pcode val data type (Ville), tighten variable scope (Ville)
> >
> >Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >Signed-off-by: James Ausmus <james.ausmus@intel.com>
> >Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> > drivers/gpu/drm/i915/i915_reg.h |  1 +
> > drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++++-
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >index 058aa5ca8b73..6a45df9dad9c 100644
> >--- a/drivers/gpu/drm/i915/i915_reg.h
> >+++ b/drivers/gpu/drm/i915/i915_reg.h
> >@@ -8869,6 +8869,7 @@ enum {
> > #define     GEN9_SAGV_DISABLE			0x0
> > #define     GEN9_SAGV_IS_DISABLED		0x1
> > #define     GEN9_SAGV_ENABLE			0x3
> >+#define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US	0x23
> > #define GEN6_PCODE_DATA				_MMIO(0x138128)
> > #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
> > #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >index b413a7f3bc5d..13721ba44013 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -3645,7 +3645,20 @@ intel_has_sagv(struct drm_i915_private *dev_priv)
> > static void
> > skl_setup_sagv_block_time(struct drm_i915_private *dev_priv)
> > {
> >-	if (IS_GEN(dev_priv, 11)) {
> >+	if (INTEL_GEN(dev_priv) >= 12) {
> 
> sagv will still never be enabled for TGL. Are you going to revert 
> 8ffa4392a32e ("drm/i915/tgl: disable SAGV temporarily")
> in a separete patch?

Yes, that's the idea - we land these two patches, then once HSD
1409542895 gets resolved, we revert 8ffa4392a32e and everything Just
Works. ;)

-James

> 
> Lucas De Marchi
> 
> >+		u32 val = 0;
> >+		int ret;
> >+
> >+		ret = sandybridge_pcode_read(dev_priv,
> >+					     GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
> >+					     &val, NULL);
> >+		if (!ret) {
> >+			dev_priv->sagv_block_time_us = val;
> >+			return;
> >+		}
> >+
> >+		DRM_DEBUG_DRIVER("Couldn't read SAGV block time!\n");
> >+	} else if (IS_GEN(dev_priv, 11)) {
> > 		dev_priv->sagv_block_time_us = 10;
> > 		return;
> > 	} else if (IS_GEN(dev_priv, 10)) {
> >-- 
> >2.22.1
> >
Ville Syrjala Oct. 7, 2019, 10:15 a.m. UTC | #2 
On Fri, Oct 04, 2019 at 02:51:34PM -0700, James Ausmus wrote:
> On Fri, Oct 04, 2019 at 01:55:46PM -0700, Lucas De Marchi wrote:
> > On Fri, Sep 27, 2019 at 03:24:27PM -0700, James Ausmus wrote:
> > >Starting from TGL, we now need to read the SAGV block time via a PCODE
> > >mailbox, rather than having a static value.
> > >
> > >BSpec: 49326
> > >
> > >v2: Fix up pcode val data type (Ville), tighten variable scope (Ville)
> > >
> > >Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > >Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > >Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >---
> > > drivers/gpu/drm/i915/i915_reg.h |  1 +
> > > drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++++-
> > > 2 files changed, 15 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > >index 058aa5ca8b73..6a45df9dad9c 100644
> > >--- a/drivers/gpu/drm/i915/i915_reg.h
> > >+++ b/drivers/gpu/drm/i915/i915_reg.h
> > >@@ -8869,6 +8869,7 @@ enum {
> > > #define     GEN9_SAGV_DISABLE			0x0
> > > #define     GEN9_SAGV_IS_DISABLED		0x1
> > > #define     GEN9_SAGV_ENABLE			0x3
> > >+#define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US	0x23
> > > #define GEN6_PCODE_DATA				_MMIO(0x138128)
> > > #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
> > > #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
> > >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > >index b413a7f3bc5d..13721ba44013 100644
> > >--- a/drivers/gpu/drm/i915/intel_pm.c
> > >+++ b/drivers/gpu/drm/i915/intel_pm.c
> > >@@ -3645,7 +3645,20 @@ intel_has_sagv(struct drm_i915_private *dev_priv)
> > > static void
> > > skl_setup_sagv_block_time(struct drm_i915_private *dev_priv)
> > > {
> > >-	if (IS_GEN(dev_priv, 11)) {
> > >+	if (INTEL_GEN(dev_priv) >= 12) {
> > 
> > sagv will still never be enabled for TGL. Are you going to revert 
> > 8ffa4392a32e ("drm/i915/tgl: disable SAGV temporarily")
> > in a separete patch?
> 
> Yes, that's the idea - we land these two patches, then once HSD
> 1409542895 gets resolved, we revert 8ffa4392a32e and everything Just
> Works. ;)

The whole sagv stuff is wrong for icl+. Stan is attempting to remedy
that.

> 
> -James
> 
> > 
> > Lucas De Marchi
> > 
> > >+		u32 val = 0;
> > >+		int ret;
> > >+
> > >+		ret = sandybridge_pcode_read(dev_priv,
> > >+					     GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
> > >+					     &val, NULL);
> > >+		if (!ret) {
> > >+			dev_priv->sagv_block_time_us = val;
> > >+			return;
> > >+		}
> > >+
> > >+		DRM_DEBUG_DRIVER("Couldn't read SAGV block time!\n");
> > >+	} else if (IS_GEN(dev_priv, 11)) {
> > > 		dev_priv->sagv_block_time_us = 10;
> > > 		return;
> > > 	} else if (IS_GEN(dev_priv, 10)) {
> > >-- 
> > >2.22.1
> > >
James Ausmus Oct. 7, 2019, 11:25 p.m. UTC | #3 
On Mon, Oct 07, 2019 at 01:15:24PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 04, 2019 at 02:51:34PM -0700, James Ausmus wrote:
> > On Fri, Oct 04, 2019 at 01:55:46PM -0700, Lucas De Marchi wrote:
> > > On Fri, Sep 27, 2019 at 03:24:27PM -0700, James Ausmus wrote:
> > > >Starting from TGL, we now need to read the SAGV block time via a PCODE
> > > >mailbox, rather than having a static value.
> > > >
> > > >BSpec: 49326
> > > >
> > > >v2: Fix up pcode val data type (Ville), tighten variable scope (Ville)
> > > >
> > > >Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > >Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > > >Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >---
> > > > drivers/gpu/drm/i915/i915_reg.h |  1 +
> > > > drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++++-
> > > > 2 files changed, 15 insertions(+), 1 deletion(-)
> > > >
> > > >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > >index 058aa5ca8b73..6a45df9dad9c 100644
> > > >--- a/drivers/gpu/drm/i915/i915_reg.h
> > > >+++ b/drivers/gpu/drm/i915/i915_reg.h
> > > >@@ -8869,6 +8869,7 @@ enum {
> > > > #define     GEN9_SAGV_DISABLE			0x0
> > > > #define     GEN9_SAGV_IS_DISABLED		0x1
> > > > #define     GEN9_SAGV_ENABLE			0x3
> > > >+#define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US	0x23
> > > > #define GEN6_PCODE_DATA				_MMIO(0x138128)
> > > > #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
> > > > #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
> > > >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > >index b413a7f3bc5d..13721ba44013 100644
> > > >--- a/drivers/gpu/drm/i915/intel_pm.c
> > > >+++ b/drivers/gpu/drm/i915/intel_pm.c
> > > >@@ -3645,7 +3645,20 @@ intel_has_sagv(struct drm_i915_private *dev_priv)
> > > > static void
> > > > skl_setup_sagv_block_time(struct drm_i915_private *dev_priv)
> > > > {
> > > >-	if (IS_GEN(dev_priv, 11)) {
> > > >+	if (INTEL_GEN(dev_priv) >= 12) {
> > > 
> > > sagv will still never be enabled for TGL. Are you going to revert 
> > > 8ffa4392a32e ("drm/i915/tgl: disable SAGV temporarily")
> > > in a separete patch?
> > 
> > Yes, that's the idea - we land these two patches, then once HSD
> > 1409542895 gets resolved, we revert 8ffa4392a32e and everything Just
> > Works. ;)
> 
> The whole sagv stuff is wrong for icl+. Stan is attempting to remedy
> that.

Well, we'll at least need to do this read of the block time - do you
think these two patches can land in the meantime, to help prep the TGL
path for actually working when Stan's work lands?

Thanks!

-James

> 
> > 
> > -James
> > 
> > > 
> > > Lucas De Marchi
> > > 
> > > >+		u32 val = 0;
> > > >+		int ret;
> > > >+
> > > >+		ret = sandybridge_pcode_read(dev_priv,
> > > >+					     GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
> > > >+					     &val, NULL);
> > > >+		if (!ret) {
> > > >+			dev_priv->sagv_block_time_us = val;
> > > >+			return;
> > > >+		}
> > > >+
> > > >+		DRM_DEBUG_DRIVER("Couldn't read SAGV block time!\n");
> > > >+	} else if (IS_GEN(dev_priv, 11)) {
> > > > 		dev_priv->sagv_block_time_us = 10;
> > > > 		return;
> > > > 	} else if (IS_GEN(dev_priv, 10)) {
> > > >-- 
> > > >2.22.1
> > > >
> 
> -- 
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 058aa5ca8b73..6a45df9dad9c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8869,6 +8869,7 @@  enum {
 #define     GEN9_SAGV_DISABLE			0x0
 #define     GEN9_SAGV_IS_DISABLED		0x1
 #define     GEN9_SAGV_ENABLE			0x3
+#define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US	0x23
 #define GEN6_PCODE_DATA				_MMIO(0x138128)
 #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
 #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b413a7f3bc5d..13721ba44013 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3645,7 +3645,20 @@  intel_has_sagv(struct drm_i915_private *dev_priv)
 static void
 skl_setup_sagv_block_time(struct drm_i915_private *dev_priv)
 {
-	if (IS_GEN(dev_priv, 11)) {
+	if (INTEL_GEN(dev_priv) >= 12) {
+		u32 val = 0;
+		int ret;
+
+		ret = sandybridge_pcode_read(dev_priv,
+					     GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
+					     &val, NULL);
+		if (!ret) {
+			dev_priv->sagv_block_time_us = val;
+			return;
+		}
+
+		DRM_DEBUG_DRIVER("Couldn't read SAGV block time!\n");
+	} else if (IS_GEN(dev_priv, 11)) {
 		dev_priv->sagv_block_time_us = 10;
 		return;
 	} else if (IS_GEN(dev_priv, 10)) {