[PATCH 1/2] drm/i915: Reorganize uncore_init

[PATCH 1/2] drm/i915: Reorganize uncore_init

[Ben Widawsky]

Right now we have two separate tables for doing platform specific init.
With one table it's a lot clearer exactly where new things need to go
(which I'll be doing in a subsequent patch).

There is no intended functional change here. Only cleanup.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_uncore.c | 134 ++++++++++++++++++++----------------
 1 file changed, 74 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 76dc185..e206c41 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -739,6 +739,47 @@ __gen4_write(64)
 #undef REG_WRITE_FOOTER
 #undef REG_WRITE_HEADER
 
+
+static void ivybridge_setup_forcewake(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 ecobus;
+
+	/* IVB configs may use multi-threaded forcewake */
+
+	/* A small trick here - if the bios hasn't configured MT forcewake, and
+	 * if the device is in RC6, then force_wake_mt_get will not wake the
+	 * device and the ECOBUS read will return zero. Which will be
+	 * (correctly) interpreted by the test below as MT forcewake being
+	 * disabled.
+	 */
+	mutex_lock(&dev->struct_mutex);
+	__gen7_gt_force_wake_mt_get(dev_priv, FORCEWAKE_ALL);
+	ecobus = __raw_i915_read32(dev_priv, ECOBUS);
+	__gen7_gt_force_wake_mt_put(dev_priv, FORCEWAKE_ALL);
+	mutex_unlock(&dev->struct_mutex);
+
+	if (ecobus & FORCEWAKE_MT_ENABLE) {
+		dev_priv->uncore.funcs.force_wake_get = __gen7_gt_force_wake_mt_get;
+		dev_priv->uncore.funcs.force_wake_put = __gen7_gt_force_wake_mt_put;
+	} else {
+		DRM_INFO("No MT forcewake available on Ivybridge, this can result in issues\n");
+		DRM_INFO("when using vblank-synced partial screen updates.\n");
+		dev_priv->uncore.funcs.force_wake_get = __gen6_gt_force_wake_get;
+		dev_priv->uncore.funcs.force_wake_put = __gen6_gt_force_wake_put;
+	}
+}
+
+#define gen6_setup_read_mmio(dev_priv) do {				\
+		(dev_priv)->uncore.funcs.mmio_readb  = gen6_read8;	\
+		(dev_priv)->uncore.funcs.mmio_readw  = gen6_read16;	\
+		(dev_priv)->uncore.funcs.mmio_readl  = gen6_read32;	\
+		(dev_priv)->uncore.funcs.mmio_readq  = gen6_read64;	\
+} while (0)
+
+/* You can use this function to initialize gen specific function pointers
+ * that need to be initialized early.
+ */
 void intel_uncore_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -748,87 +789,60 @@ void intel_uncore_init(struct drm_device *dev)
 
 	intel_uncore_early_sanitize(dev);
 
-	if (IS_VALLEYVIEW(dev)) {
-		dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
-		dev_priv->uncore.funcs.force_wake_put = __vlv_force_wake_put;
-	} else if (IS_HASWELL(dev) || IS_GEN8(dev)) {
+	switch (INTEL_INFO(dev)->gen) {
+	case 8:
 		dev_priv->uncore.funcs.force_wake_get = __gen7_gt_force_wake_mt_get;
 		dev_priv->uncore.funcs.force_wake_put = __gen7_gt_force_wake_mt_put;
-	} else if (IS_IVYBRIDGE(dev)) {
-		u32 ecobus;
-
-		/* IVB configs may use multi-threaded forcewake */
-
-		/* A small trick here - if the bios hasn't configured
-		 * MT forcewake, and if the device is in RC6, then
-		 * force_wake_mt_get will not wake the device and the
-		 * ECOBUS read will return zero. Which will be
-		 * (correctly) interpreted by the test below as MT
-		 * forcewake being disabled.
-		 */
-		mutex_lock(&dev->struct_mutex);
-		__gen7_gt_force_wake_mt_get(dev_priv, FORCEWAKE_ALL);
-		ecobus = __raw_i915_read32(dev_priv, ECOBUS);
-		__gen7_gt_force_wake_mt_put(dev_priv, FORCEWAKE_ALL);
-		mutex_unlock(&dev->struct_mutex);
-
-		if (ecobus & FORCEWAKE_MT_ENABLE) {
-			dev_priv->uncore.funcs.force_wake_get =
-				__gen7_gt_force_wake_mt_get;
-			dev_priv->uncore.funcs.force_wake_put =
-				__gen7_gt_force_wake_mt_put;
-		} else {
-			DRM_INFO("No MT forcewake available on Ivybridge, this can result in issues\n");
-			DRM_INFO("when using vblank-synced partial screen updates.\n");
-			dev_priv->uncore.funcs.force_wake_get =
-				__gen6_gt_force_wake_get;
-			dev_priv->uncore.funcs.force_wake_put =
-				__gen6_gt_force_wake_put;
-		}
-	} else if (IS_GEN6(dev)) {
-		dev_priv->uncore.funcs.force_wake_get =
-			__gen6_gt_force_wake_get;
-		dev_priv->uncore.funcs.force_wake_put =
-			__gen6_gt_force_wake_put;
-	}
 
-	switch (INTEL_INFO(dev)->gen) {
-	default:
+		gen6_setup_read_mmio(dev_priv);
 		dev_priv->uncore.funcs.mmio_writeb  = gen8_write8;
 		dev_priv->uncore.funcs.mmio_writew  = gen8_write16;
 		dev_priv->uncore.funcs.mmio_writel  = gen8_write32;
 		dev_priv->uncore.funcs.mmio_writeq  = gen8_write64;
-		dev_priv->uncore.funcs.mmio_readb  = gen6_read8;
-		dev_priv->uncore.funcs.mmio_readw  = gen6_read16;
-		dev_priv->uncore.funcs.mmio_readl  = gen6_read32;
-		dev_priv->uncore.funcs.mmio_readq  = gen6_read64;
+
 		break;
 	case 7:
-	case 6:
 		if (IS_HASWELL(dev)) {
+			dev_priv->uncore.funcs.force_wake_get = __gen7_gt_force_wake_mt_get;
+			dev_priv->uncore.funcs.force_wake_put = __gen7_gt_force_wake_mt_put;
+
+			gen6_setup_read_mmio(dev_priv);
 			dev_priv->uncore.funcs.mmio_writeb  = hsw_write8;
 			dev_priv->uncore.funcs.mmio_writew  = hsw_write16;
 			dev_priv->uncore.funcs.mmio_writel  = hsw_write32;
 			dev_priv->uncore.funcs.mmio_writeq  = hsw_write64;
-		} else {
-			dev_priv->uncore.funcs.mmio_writeb  = gen6_write8;
-			dev_priv->uncore.funcs.mmio_writew  = gen6_write16;
-			dev_priv->uncore.funcs.mmio_writel  = gen6_write32;
-			dev_priv->uncore.funcs.mmio_writeq  = gen6_write64;
-		}
+		} else if (IS_VALLEYVIEW(dev)) {
+			dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
+			dev_priv->uncore.funcs.force_wake_put = __vlv_force_wake_put;
 
-		if (IS_VALLEYVIEW(dev)) {
+			gen6_setup_read_mmio(dev_priv);
 			dev_priv->uncore.funcs.mmio_readb  = vlv_read8;
 			dev_priv->uncore.funcs.mmio_readw  = vlv_read16;
 			dev_priv->uncore.funcs.mmio_readl  = vlv_read32;
 			dev_priv->uncore.funcs.mmio_readq  = vlv_read64;
-		} else {
-			dev_priv->uncore.funcs.mmio_readb  = gen6_read8;
-			dev_priv->uncore.funcs.mmio_readw  = gen6_read16;
-			dev_priv->uncore.funcs.mmio_readl  = gen6_read32;
-			dev_priv->uncore.funcs.mmio_readq  = gen6_read64;
+		} else if (IS_IVYBRIDGE(dev)) {
+			ivybridge_setup_forcewake(dev);
+			BUG_ON(!dev_priv->uncore.funcs.force_wake_get);
+			BUG_ON(!dev_priv->uncore.funcs.force_wake_put);
+
+			gen6_setup_read_mmio(dev_priv);
+			dev_priv->uncore.funcs.mmio_writeb  = gen6_write8;
+			dev_priv->uncore.funcs.mmio_writew  = gen6_write16;
+			dev_priv->uncore.funcs.mmio_writel  = gen6_write32;
+			dev_priv->uncore.funcs.mmio_writeq  = gen6_write64;
 		}
 		break;
+	case 6:
+		dev_priv->uncore.funcs.force_wake_get = __gen6_gt_force_wake_get;
+		dev_priv->uncore.funcs.force_wake_put = __gen6_gt_force_wake_put;
+
+		gen6_setup_read_mmio(dev_priv);
+		dev_priv->uncore.funcs.mmio_writeb  = gen6_write8;
+		dev_priv->uncore.funcs.mmio_writew  = gen6_write16;
+		dev_priv->uncore.funcs.mmio_writel  = gen6_write32;
+		dev_priv->uncore.funcs.mmio_writeq  = gen6_write64;
+
+		break;
 	case 5:
 		dev_priv->uncore.funcs.mmio_writeb  = gen5_write8;
 		dev_priv->uncore.funcs.mmio_writew  = gen5_write16;
-- 
1.9.1

[PATCH 2/2] drm/i915/bdw: Create a state setup hook

[Ben Widawsky]

Mika Kuoppala <miku@iki.fi>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h         | 2 ++
 drivers/gpu/drm/i915/i915_gem_context.c | 3 +++
 drivers/gpu/drm/i915/intel_uncore.c     | 8 ++++++++
 3 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 952fdba..9f6a8c1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -501,6 +501,8 @@ struct intel_uncore_funcs {
 				uint32_t val, bool trace);
 	void (*mmio_writeq)(struct drm_i915_private *dev_priv, off_t offset,
 				uint64_t val, bool trace);
+
+	int (*hw_state_setup)(struct drm_device *dev);
 };
 
 struct intel_uncore {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index fdf1736..513beea 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -491,6 +491,9 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv)
 	struct intel_ring_buffer *ring;
 	int ret, i;
 
+        if (dev_priv->uncore.funcs.hw_state_setup(dev_priv->dev))
+		dev_priv->uncore.funcs.hw_state_setup(dev_priv->dev);
+
 	if (!HAS_HW_CONTEXTS(dev_priv->dev))
 		return 0;
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index e206c41..9241901 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -40,6 +40,8 @@
 
 #define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32(dev_priv__, reg__)
 
+static int gen8_hw_state_setup(struct drm_device *dev);
+
 static void
 assert_device_not_suspended(struct drm_i915_private *dev_priv)
 {
@@ -800,6 +802,7 @@ void intel_uncore_init(struct drm_device *dev)
 		dev_priv->uncore.funcs.mmio_writel  = gen8_write32;
 		dev_priv->uncore.funcs.mmio_writeq  = gen8_write64;
 
+		dev_priv->uncore.funcs.hw_state_setup = gen8_hw_state_setup;
 		break;
 	case 7:
 		if (IS_HASWELL(dev)) {
@@ -1068,3 +1071,8 @@ void intel_uncore_check_errors(struct drm_device *dev)
 		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
 	}
 }
+
+static int gen8_hw_state_setup(struct drm_device *dev)
+{
+	return 0;
+}
-- 
1.9.1

Re: [PATCH 2/2] drm/i915/bdw: Create a state setup hook

[Ben Widawsky]

Screwed up the CC... my bad. Basically, I started doing my own state
setup thing, and thought Mika could benefit. In the end my thing didn't
solve the problem I was trying to solve.

On Sun, Apr 06, 2014 at 12:54:10PM -0700, Ben Widawsky wrote:
> Mika Kuoppala <miku@iki.fi>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 2 ++
>  drivers/gpu/drm/i915/i915_gem_context.c | 3 +++
>  drivers/gpu/drm/i915/intel_uncore.c     | 8 ++++++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 952fdba..9f6a8c1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -501,6 +501,8 @@ struct intel_uncore_funcs {
>  				uint32_t val, bool trace);
>  	void (*mmio_writeq)(struct drm_i915_private *dev_priv, off_t offset,
>  				uint64_t val, bool trace);
> +
> +	int (*hw_state_setup)(struct drm_device *dev);
>  };
>  
>  struct intel_uncore {
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index fdf1736..513beea 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -491,6 +491,9 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv)
>  	struct intel_ring_buffer *ring;
>  	int ret, i;
>  
> +        if (dev_priv->uncore.funcs.hw_state_setup(dev_priv->dev))
> +		dev_priv->uncore.funcs.hw_state_setup(dev_priv->dev);
> +
>  	if (!HAS_HW_CONTEXTS(dev_priv->dev))
>  		return 0;
>  
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index e206c41..9241901 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -40,6 +40,8 @@
>  
>  #define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32(dev_priv__, reg__)
>  
> +static int gen8_hw_state_setup(struct drm_device *dev);
> +
>  static void
>  assert_device_not_suspended(struct drm_i915_private *dev_priv)
>  {
> @@ -800,6 +802,7 @@ void intel_uncore_init(struct drm_device *dev)
>  		dev_priv->uncore.funcs.mmio_writel  = gen8_write32;
>  		dev_priv->uncore.funcs.mmio_writeq  = gen8_write64;
>  
> +		dev_priv->uncore.funcs.hw_state_setup = gen8_hw_state_setup;
>  		break;
>  	case 7:
>  		if (IS_HASWELL(dev)) {
> @@ -1068,3 +1071,8 @@ void intel_uncore_check_errors(struct drm_device *dev)
>  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
>  	}
>  }
> +
> +static int gen8_hw_state_setup(struct drm_device *dev)
> +{
> +	return 0;
> +}
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 2/2] drm/i915/bdw: Create a state setup hook

[Ben Widawsky]

On Sun, Apr 06, 2014 at 12:58:56PM -0700, Ben Widawsky wrote:
> Screwed up the CC... my bad. Basically, I started doing my own state
> setup thing, and thought Mika could benefit. In the end my thing didn't
> solve the problem I was trying to solve.

Okay, I botched the patch too. See below, if you use it.

> 
> On Sun, Apr 06, 2014 at 12:54:10PM -0700, Ben Widawsky wrote:
> > Mika Kuoppala <miku@iki.fi>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         | 2 ++
> >  drivers/gpu/drm/i915/i915_gem_context.c | 3 +++
> >  drivers/gpu/drm/i915/intel_uncore.c     | 8 ++++++++
> >  3 files changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 952fdba..9f6a8c1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -501,6 +501,8 @@ struct intel_uncore_funcs {
> >  				uint32_t val, bool trace);
> >  	void (*mmio_writeq)(struct drm_i915_private *dev_priv, off_t offset,
> >  				uint64_t val, bool trace);
> > +
> > +	int (*hw_state_setup)(struct drm_device *dev);
> >  };
> >  
> >  struct intel_uncore {
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index fdf1736..513beea 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -491,6 +491,9 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv)
> >  	struct intel_ring_buffer *ring;
> >  	int ret, i;
> >  
> > +        if (dev_priv->uncore.funcs.hw_state_setup(dev_priv->dev))
> > +		dev_priv->uncore.funcs.hw_state_setup(dev_priv->dev);
> > +

+        if (dev_priv->uncore.funcs.hw_state_setup)

> >  	if (!HAS_HW_CONTEXTS(dev_priv->dev))
> >  		return 0;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index e206c41..9241901 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -40,6 +40,8 @@
> >  
> >  #define __raw_posting_read(dev_priv__, reg__) (void)__raw_i915_read32(dev_priv__, reg__)
> >  
> > +static int gen8_hw_state_setup(struct drm_device *dev);
> > +
> >  static void
> >  assert_device_not_suspended(struct drm_i915_private *dev_priv)
> >  {
> > @@ -800,6 +802,7 @@ void intel_uncore_init(struct drm_device *dev)
> >  		dev_priv->uncore.funcs.mmio_writel  = gen8_write32;
> >  		dev_priv->uncore.funcs.mmio_writeq  = gen8_write64;
> >  
> > +		dev_priv->uncore.funcs.hw_state_setup = gen8_hw_state_setup;
> >  		break;
> >  	case 7:
> >  		if (IS_HASWELL(dev)) {
> > @@ -1068,3 +1071,8 @@ void intel_uncore_check_errors(struct drm_device *dev)
> >  		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> >  	}
> >  }
> > +
> > +static int gen8_hw_state_setup(struct drm_device *dev)
> > +{
> > +	return 0;
> > +}
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center