[PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v3

[PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v3

[Jesse Barnes]

But we need to get the right stolen base and make pre-allocated objects
for BIOS stuff so we don't clobber it.  If the BIOS hasn't allocated a
power context, we allocate one here too, from stolen space as required
by the docs.

v2: fix stolen to phys if ladder (Ben)
    keep BIOS reserved space out of allocator altogether (Ben)
v3: fix mask of stolen base (Ben)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h        |    2 +
 drivers/gpu/drm/i915/i915_gem_stolen.c |   69 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h        |    1 +
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c81100c..2fe5fd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1073,6 +1073,8 @@ typedef struct drm_i915_private {
 
 	struct i915_gpu_error gpu_error;
 
+	struct drm_i915_gem_object *vlv_pctx;
+
 	/* list of fbdev register on this device */
 	struct intel_fbdev *fbdev;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 67d3510..918fcab 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 	 * its value of TOLUD.
 	 */
 	base = 0;
-	if (INTEL_INFO(dev)->gen >= 6) {
+	if (IS_VALLEYVIEW(dev)) {
+		pci_read_config_dword(pdev, 0x5c, &base);
+		base &= ((1<<20) - 1);
+	} else if (INTEL_INFO(dev)->gen >= 6) {
 		/* Read Base Data of Stolen Memory Register (BDSM) directly.
 		 * Note that there is also a MCHBAR miror at 0x1080c0 or
 		 * we could use device 2:0x5c instead.
@@ -172,17 +175,72 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
 	dev_priv->cfb_size = 0;
 }
 
+static void valleyview_setup_pctx(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *pctx;
+	unsigned long pctx_paddr;
+	u32 pcbr;
+	int pctx_size = 24*1024;
+
+	pcbr = I915_READ(VLV_PCBR);
+	if (pcbr) {
+		/* BIOS set it up already, grab the pre-alloc'd space */
+		int pcbr_offset;
+
+		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
+		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
+								      pcbr_offset,
+								      pcbr_offset,
+								      pctx_size);
+		/* We don't need to track it since we don't own it */
+		return;
+	}
+
+	/*
+	 * From the Gunit register HAS:
+	 * The Gfx driver is expected to program this register and ensure
+	 * proper allocation within Gfx stolen memory.  For example, this
+	 * register should be programmed such than the PCBR range does not
+	 * overlap with other ranges, such as the frame buffer, protected
+	 * memory, or any other relevant ranges.
+	 */
+	pctx = i915_gem_object_create_stolen(dev, pctx_size);
+	if (!pctx) {
+		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
+		return;
+	}
+
+	dev_priv->vlv_pctx = pctx;
+	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
+	I915_WRITE(VLV_PCBR, pctx_paddr);
+}
+
+static void valleyview_cleanup_pctx(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (!dev_priv->vlv_pctx)
+		return;
+
+	i915_gem_object_release_stolen(dev_priv->vlv_pctx);
+	I915_WRITE(VLV_PCBR, 0);
+}
+
 void i915_gem_cleanup_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	i915_gem_stolen_cleanup_compression(dev);
+	if (IS_VALLEYVIEW(dev))
+		valleyview_cleanup_pctx(dev);
 	drm_mm_takedown(&dev_priv->mm.stolen);
 }
 
 int i915_gem_init_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int bios_reserved = 0;
 
 	dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
 	if (dev_priv->mm.stolen_base == 0)
@@ -191,8 +249,15 @@ int i915_gem_init_stolen(struct drm_device *dev)
 	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
 		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
 
+	if (IS_VALLEYVIEW(dev))
+		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
+
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size);
+	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
+		    bios_reserved);
+
+	if (IS_VALLEYVIEW(dev))
+		valleyview_setup_pctx(dev);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a470103..a809a56 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -700,6 +700,7 @@
 #define VLV_IIR		(VLV_DISPLAY_BASE + 0x20a4)
 #define VLV_IMR		(VLV_DISPLAY_BASE + 0x20a8)
 #define VLV_ISR		(VLV_DISPLAY_BASE + 0x20ac)
+#define VLV_PCBR	(VLV_DISPLAY_BASE + 0x2120)
 #define   I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
 #define   I915_DISPLAY_PORT_INTERRUPT			(1<<17)
 #define   I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
-- 
1.7.10.4

[PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v4

[Jesse Barnes]

But we need to get the right stolen base and make pre-allocated objects
for BIOS stuff so we don't clobber it.  If the BIOS hasn't allocated a
power context, we allocate one here too, from stolen space as required
by the docs.

v2: fix stolen to phys if ladder (Ben)
    keep BIOS reserved space out of allocator altogether (Ben)
v3: fix mask of stolen base (Ben)
v4: clean up preallocated object on unload (Ben)
    don't zero reg on unload (Jesse)
    fix mask harder (Jesse)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h        |    2 +
 drivers/gpu/drm/i915/i915_gem_stolen.c |   69 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h        |    1 +
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c81100c..2fe5fd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1073,6 +1073,8 @@ typedef struct drm_i915_private {
 
 	struct i915_gpu_error gpu_error;
 
+	struct drm_i915_gem_object *vlv_pctx;
+
 	/* list of fbdev register on this device */
 	struct intel_fbdev *fbdev;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 67d3510..02950bb 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 	 * its value of TOLUD.
 	 */
 	base = 0;
-	if (INTEL_INFO(dev)->gen >= 6) {
+	if (IS_VALLEYVIEW(dev)) {
+		pci_read_config_dword(pdev, 0x5c, &base);
+		base &= ~((1<<20) - 1);
+	} else if (INTEL_INFO(dev)->gen >= 6) {
 		/* Read Base Data of Stolen Memory Register (BDSM) directly.
 		 * Note that there is also a MCHBAR miror at 0x1080c0 or
 		 * we could use device 2:0x5c instead.
@@ -172,17 +175,72 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
 	dev_priv->cfb_size = 0;
 }
 
+static void valleyview_setup_pctx(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *pctx;
+	unsigned long pctx_paddr;
+	u32 pcbr;
+	int pctx_size = 24*1024;
+
+	pcbr = I915_READ(VLV_PCBR);
+	if (pcbr) {
+		/* BIOS set it up already, grab the pre-alloc'd space */
+		int pcbr_offset;
+
+		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
+		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
+								      pcbr_offset,
+								      pcbr_offset,
+								      pctx_size);
+		goto out;
+	}
+
+	/*
+	 * From the Gunit register HAS:
+	 * The Gfx driver is expected to program this register and ensure
+	 * proper allocation within Gfx stolen memory.  For example, this
+	 * register should be programmed such than the PCBR range does not
+	 * overlap with other ranges, such as the frame buffer, protected
+	 * memory, or any other relevant ranges.
+	 */
+	pctx = i915_gem_object_create_stolen(dev, pctx_size);
+	if (!pctx) {
+		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
+		return;
+	}
+
+	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
+	I915_WRITE(VLV_PCBR, pctx_paddr);
+
+out:
+	dev_priv->vlv_pctx = pctx;
+}
+
+static void valleyview_cleanup_pctx(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (!dev_priv->vlv_pctx)
+		return;
+
+	i915_gem_object_release_stolen(dev_priv->vlv_pctx);
+}
+
 void i915_gem_cleanup_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	i915_gem_stolen_cleanup_compression(dev);
+	if (IS_VALLEYVIEW(dev))
+		valleyview_cleanup_pctx(dev);
 	drm_mm_takedown(&dev_priv->mm.stolen);
 }
 
 int i915_gem_init_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int bios_reserved = 0;
 
 	dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
 	if (dev_priv->mm.stolen_base == 0)
@@ -191,8 +249,15 @@ int i915_gem_init_stolen(struct drm_device *dev)
 	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
 		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
 
+	if (IS_VALLEYVIEW(dev))
+		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
+
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size);
+	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
+		    bios_reserved);
+
+	if (IS_VALLEYVIEW(dev))
+		valleyview_setup_pctx(dev);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a470103..a809a56 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -700,6 +700,7 @@
 #define VLV_IIR		(VLV_DISPLAY_BASE + 0x20a4)
 #define VLV_IMR		(VLV_DISPLAY_BASE + 0x20a8)
 #define VLV_ISR		(VLV_DISPLAY_BASE + 0x20ac)
+#define VLV_PCBR	(VLV_DISPLAY_BASE + 0x2120)
 #define   I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
 #define   I915_DISPLAY_PORT_INTERRUPT			(1<<17)
 #define   I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
-- 
1.7.10.4

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v3

[Chris Wilson]

On Tue, May 07, 2013 at 11:50:47AM -0700, Jesse Barnes wrote:
> @@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>  	 * its value of TOLUD.
>  	 */
>  	base = 0;
> -	if (INTEL_INFO(dev)->gen >= 6) {
> +	if (IS_VALLEYVIEW(dev)) {
> +		pci_read_config_dword(pdev, 0x5c, &base);
> +		base &= ((1<<20) - 1);

Woah, only the low 1 MiB of the physical address is valid?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v4

[Chris Wilson]

On Tue, May 07, 2013 at 12:34:00PM -0700, Jesse Barnes wrote:
> But we need to get the right stolen base and make pre-allocated objects
> for BIOS stuff so we don't clobber it.  If the BIOS hasn't allocated a
> power context, we allocate one here too, from stolen space as required
> by the docs.
> 
> v2: fix stolen to phys if ladder (Ben)
>     keep BIOS reserved space out of allocator altogether (Ben)
> v3: fix mask of stolen base (Ben)
> v4: clean up preallocated object on unload (Ben)
>     don't zero reg on unload (Jesse)
>     fix mask harder (Jesse)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h        |    2 +
>  drivers/gpu/drm/i915/i915_gem_stolen.c |   69 +++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h        |    1 +
>  3 files changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c81100c..2fe5fd4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1073,6 +1073,8 @@ typedef struct drm_i915_private {
>  
>  	struct i915_gpu_error gpu_error;
>  
> +	struct drm_i915_gem_object *vlv_pctx;
> +
>  	/* list of fbdev register on this device */
>  	struct intel_fbdev *fbdev;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 67d3510..02950bb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>  	 * its value of TOLUD.
>  	 */
>  	base = 0;
> -	if (INTEL_INFO(dev)->gen >= 6) {
> +	if (IS_VALLEYVIEW(dev)) {
> +		pci_read_config_dword(pdev, 0x5c, &base);
> +		base &= ~((1<<20) - 1);
> +	} else if (INTEL_INFO(dev)->gen >= 6) {
>  		/* Read Base Data of Stolen Memory Register (BDSM) directly.
>  		 * Note that there is also a MCHBAR miror at 0x1080c0 or
>  		 * we could use device 2:0x5c instead.
> @@ -172,17 +175,72 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
>  	dev_priv->cfb_size = 0;
>  }
>  
> +static void valleyview_setup_pctx(struct drm_device *dev)
> +{

This doesn't belong in i915_gem_stolen.c. It has nothing to do with the
management of stolen memory, it is just a user. It should be closer to
the other power contexts.

> +static void valleyview_cleanup_pctx(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (!dev_priv->vlv_pctx)
> +		return;
> +
> +	i915_gem_object_release_stolen(dev_priv->vlv_pctx);

Cleanup should just be drm_gem_object_unreference();

> +}
> +
>  void i915_gem_cleanup_stolen(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	i915_gem_stolen_cleanup_compression(dev);
> +	if (IS_VALLEYVIEW(dev))
> +		valleyview_cleanup_pctx(dev);
Not here, not like this.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v5

[Jesse Barnes]

But we need to get the right stolen base and make pre-allocated objects
for BIOS stuff so we don't clobber it.  If the BIOS hasn't allocated a
power context, we allocate one here too, from stolen space as required
by the docs.

v2: fix stolen to phys if ladder (Ben)
    keep BIOS reserved space out of allocator altogether (Ben)
v3: fix mask of stolen base (Ben)
v4: clean up preallocated object on unload (Ben)
    don't zero reg on unload (Jesse)
    fix mask harder (Jesse)
v5: use unref for freeing stolen bits (Chris)
    move alloc/free to intel_pm.c (Chris)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h        |    2 ++
 drivers/gpu/drm/i915/i915_gem_stolen.c |   12 ++++++--
 drivers/gpu/drm/i915/i915_reg.h        |    1 +
 drivers/gpu/drm/i915/intel_pm.c        |   47 ++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c81100c..2fe5fd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1073,6 +1073,8 @@ typedef struct drm_i915_private {
 
 	struct i915_gpu_error gpu_error;
 
+	struct drm_i915_gem_object *vlv_pctx;
+
 	/* list of fbdev register on this device */
 	struct intel_fbdev *fbdev;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 67d3510..9137fa4c 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 	 * its value of TOLUD.
 	 */
 	base = 0;
-	if (INTEL_INFO(dev)->gen >= 6) {
+	if (IS_VALLEYVIEW(dev)) {
+		pci_read_config_dword(pdev, 0x5c, &base);
+		base &= ~((1<<20) - 1);
+	} else if (INTEL_INFO(dev)->gen >= 6) {
 		/* Read Base Data of Stolen Memory Register (BDSM) directly.
 		 * Note that there is also a MCHBAR miror at 0x1080c0 or
 		 * we could use device 2:0x5c instead.
@@ -183,6 +186,7 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
 int i915_gem_init_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int bios_reserved = 0;
 
 	dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
 	if (dev_priv->mm.stolen_base == 0)
@@ -191,8 +195,12 @@ int i915_gem_init_stolen(struct drm_device *dev)
 	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
 		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
 
+	if (IS_VALLEYVIEW(dev))
+		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
+
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size);
+	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
+		    bios_reserved);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a470103..a809a56 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -700,6 +700,7 @@
 #define VLV_IIR		(VLV_DISPLAY_BASE + 0x20a4)
 #define VLV_IMR		(VLV_DISPLAY_BASE + 0x20a8)
 #define VLV_ISR		(VLV_DISPLAY_BASE + 0x20ac)
+#define VLV_PCBR	(VLV_DISPLAY_BASE + 0x2120)
 #define   I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
 #define   I915_DISPLAY_PORT_INTERRUPT			(1<<17)
 #define   I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9b3e90e..581c513 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2562,6 +2562,9 @@ static void valleyview_disable_rps(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->rps.lock);
 
 	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
+
+	if (dev_priv->vlv_pctx)
+		drm_gem_object_unreference(&dev_priv->vlv_pctx->base);
 }
 
 int intel_enable_rc6(const struct drm_device *dev)
@@ -2856,6 +2859,48 @@ static void vlv_rps_timer_work(struct work_struct *work)
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+static void valleyview_setup_pctx(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *pctx;
+	unsigned long pctx_paddr;
+	u32 pcbr;
+	int pctx_size = 24*1024;
+
+	pcbr = I915_READ(VLV_PCBR);
+	if (pcbr) {
+		/* BIOS set it up already, grab the pre-alloc'd space */
+		int pcbr_offset;
+
+		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
+		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
+								      pcbr_offset,
+								      pcbr_offset,
+								      pctx_size);
+		goto out;
+	}
+
+	/*
+	 * From the Gunit register HAS:
+	 * The Gfx driver is expected to program this register and ensure
+	 * proper allocation within Gfx stolen memory.  For example, this
+	 * register should be programmed such than the PCBR range does not
+	 * overlap with other ranges, such as the frame buffer, protected
+	 * memory, or any other relevant ranges.
+	 */
+	pctx = i915_gem_object_create_stolen(dev, pctx_size);
+	if (!pctx) {
+		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
+		return;
+	}
+
+	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
+	I915_WRITE(VLV_PCBR, pctx_paddr);
+
+out:
+	dev_priv->vlv_pctx = pctx;
+}
+
 static void valleyview_enable_rps(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2870,6 +2915,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
 		I915_WRITE(GTFIFODBG, gtfifodbg);
 	}
 
+	valleyview_setup_pctx(dev);
+
 	gen6_gt_force_wake_get(dev_priv);
 
 	I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
-- 
1.7.10.4

[PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Jesse Barnes]

But we need to get the right stolen base and make pre-allocated objects
for BIOS stuff so we don't clobber it.  If the BIOS hasn't allocated a
power context, we allocate one here too, from stolen space as required
by the docs.

v2: fix stolen to phys if ladder (Ben)
    keep BIOS reserved space out of allocator altogether (Ben)
v3: fix mask of stolen base (Ben)
v4: clean up preallocated object on unload (Ben)
    don't zero reg on unload (Jesse)
    fix mask harder (Jesse)
v5: use unref for freeing stolen bits (Chris)
    move alloc/free to intel_pm.c (Chris)
v6: NULL pctx at disable time so error paths work (Ben)

Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h        |    2 ++
 drivers/gpu/drm/i915/i915_gem_stolen.c |   12 ++++++--
 drivers/gpu/drm/i915/i915_reg.h        |    1 +
 drivers/gpu/drm/i915/intel_pm.c        |   49 ++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c81100c..2fe5fd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1073,6 +1073,8 @@ typedef struct drm_i915_private {
 
 	struct i915_gpu_error gpu_error;
 
+	struct drm_i915_gem_object *vlv_pctx;
+
 	/* list of fbdev register on this device */
 	struct intel_fbdev *fbdev;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 67d3510..9137fa4c 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 	 * its value of TOLUD.
 	 */
 	base = 0;
-	if (INTEL_INFO(dev)->gen >= 6) {
+	if (IS_VALLEYVIEW(dev)) {
+		pci_read_config_dword(pdev, 0x5c, &base);
+		base &= ~((1<<20) - 1);
+	} else if (INTEL_INFO(dev)->gen >= 6) {
 		/* Read Base Data of Stolen Memory Register (BDSM) directly.
 		 * Note that there is also a MCHBAR miror at 0x1080c0 or
 		 * we could use device 2:0x5c instead.
@@ -183,6 +186,7 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
 int i915_gem_init_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int bios_reserved = 0;
 
 	dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
 	if (dev_priv->mm.stolen_base == 0)
@@ -191,8 +195,12 @@ int i915_gem_init_stolen(struct drm_device *dev)
 	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
 		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
 
+	if (IS_VALLEYVIEW(dev))
+		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
+
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size);
+	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
+		    bios_reserved);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a470103..a809a56 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -700,6 +700,7 @@
 #define VLV_IIR		(VLV_DISPLAY_BASE + 0x20a4)
 #define VLV_IMR		(VLV_DISPLAY_BASE + 0x20a8)
 #define VLV_ISR		(VLV_DISPLAY_BASE + 0x20ac)
+#define VLV_PCBR	(VLV_DISPLAY_BASE + 0x2120)
 #define   I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
 #define   I915_DISPLAY_PORT_INTERRUPT			(1<<17)
 #define   I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9b3e90e..e60cd3e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2562,6 +2562,11 @@ static void valleyview_disable_rps(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->rps.lock);
 
 	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
+
+	if (dev_priv->vlv_pctx) {
+		drm_gem_object_unreference(&dev_priv->vlv_pctx->base);
+		dev_priv->vlv_pctx = NULL;
+	}
 }
 
 int intel_enable_rc6(const struct drm_device *dev)
@@ -2856,6 +2861,48 @@ static void vlv_rps_timer_work(struct work_struct *work)
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+static void valleyview_setup_pctx(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *pctx;
+	unsigned long pctx_paddr;
+	u32 pcbr;
+	int pctx_size = 24*1024;
+
+	pcbr = I915_READ(VLV_PCBR);
+	if (pcbr) {
+		/* BIOS set it up already, grab the pre-alloc'd space */
+		int pcbr_offset;
+
+		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
+		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
+								      pcbr_offset,
+								      pcbr_offset,
+								      pctx_size);
+		goto out;
+	}
+
+	/*
+	 * From the Gunit register HAS:
+	 * The Gfx driver is expected to program this register and ensure
+	 * proper allocation within Gfx stolen memory.  For example, this
+	 * register should be programmed such than the PCBR range does not
+	 * overlap with other ranges, such as the frame buffer, protected
+	 * memory, or any other relevant ranges.
+	 */
+	pctx = i915_gem_object_create_stolen(dev, pctx_size);
+	if (!pctx) {
+		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
+		return;
+	}
+
+	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
+	I915_WRITE(VLV_PCBR, pctx_paddr);
+
+out:
+	dev_priv->vlv_pctx = pctx;
+}
+
 static void valleyview_enable_rps(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2870,6 +2917,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
 		I915_WRITE(GTFIFODBG, gtfifodbg);
 	}
 
+	valleyview_setup_pctx(dev);
+
 	gen6_gt_force_wake_get(dev_priv);
 
 	I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
-- 
1.7.10.4

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Chris Wilson]

On Tue, May 07, 2013 at 02:34:31PM -0700, Jesse Barnes wrote:
> +static void valleyview_setup_pctx(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *pctx;
> +	unsigned long pctx_paddr;
> +	u32 pcbr;
> +	int pctx_size = 24*1024;
> +
> +	pcbr = I915_READ(VLV_PCBR);
> +	if (pcbr) {
> +		/* BIOS set it up already, grab the pre-alloc'd space */
> +		int pcbr_offset;
> +
> +		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
> +		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> +								      pcbr_offset,
> +								      pcbr_offset,
> +								      pctx_size);

We're reserving global GTT space here for someting that just looks like
it requires contiguous physical memory. This may cause issues if
something else is already bound to that GTT range. I think we want to
extend the interface to not reserve GTT space if gtt_offset == -1.

Much happier with the placement of functions and the comments help a
lot.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Jesse Barnes]

On Tue, 7 May 2013 22:57:37 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, May 07, 2013 at 02:34:31PM -0700, Jesse Barnes wrote:
> > +static void valleyview_setup_pctx(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_i915_gem_object *pctx;
> > +	unsigned long pctx_paddr;
> > +	u32 pcbr;
> > +	int pctx_size = 24*1024;
> > +
> > +	pcbr = I915_READ(VLV_PCBR);
> > +	if (pcbr) {
> > +		/* BIOS set it up already, grab the pre-alloc'd space */
> > +		int pcbr_offset;
> > +
> > +		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
> > +		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> > +								      pcbr_offset,
> > +								      pcbr_offset,
> > +								      pctx_size);
> 
> We're reserving global GTT space here for someting that just looks like
> it requires contiguous physical memory. This may cause issues if
> something else is already bound to that GTT range. I think we want to
> extend the interface to not reserve GTT space if gtt_offset == -1.

I should have added a comment for this too.  The physical range for the
power context must reside in stolen space.  So while it needs
contiguous physical memory, it also has a relationship to the stolen
handling.  I'd prefer to see a failure here than to try to do something
fancy with funky GTT vs stolen setups...  (Still waiting on the BIOS
guys to promise the identity map going forward.)

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Chris Wilson]

On Tue, May 07, 2013 at 03:08:42PM -0700, Jesse Barnes wrote:
> On Tue, 7 May 2013 22:57:37 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > On Tue, May 07, 2013 at 02:34:31PM -0700, Jesse Barnes wrote:
> > > +static void valleyview_setup_pctx(struct drm_device *dev)
> > > +{
> > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > +	struct drm_i915_gem_object *pctx;
> > > +	unsigned long pctx_paddr;
> > > +	u32 pcbr;
> > > +	int pctx_size = 24*1024;
> > > +
> > > +	pcbr = I915_READ(VLV_PCBR);
> > > +	if (pcbr) {
> > > +		/* BIOS set it up already, grab the pre-alloc'd space */
> > > +		int pcbr_offset;
> > > +
> > > +		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
> > > +		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> > > +								      pcbr_offset,
> > > +								      pcbr_offset,
> > > +								      pctx_size);
> > 
> > We're reserving global GTT space here for someting that just looks like
> > it requires contiguous physical memory. This may cause issues if
> > something else is already bound to that GTT range. I think we want to
> > extend the interface to not reserve GTT space if gtt_offset == -1.
> 
> I should have added a comment for this too.  The physical range for the
> power context must reside in stolen space.  So while it needs
> contiguous physical memory, it also has a relationship to the stolen
> handling.  I'd prefer to see a failure here than to try to do something
> fancy with funky GTT vs stolen setups...  (Still waiting on the BIOS
> guys to promise the identity map going forward.)

Hmm, I am not understanding exactly what you need here. The register you
are reading looks to be a physical address - so why do we need to also
reserve a virtual address with the same offset? And if that is required,
shouldn't the else branch also reserve the corresponding virtual
address for its stolen allocation?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Jesse Barnes]

On Tue, 7 May 2013 23:14:40 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, May 07, 2013 at 03:08:42PM -0700, Jesse Barnes wrote:
> > On Tue, 7 May 2013 22:57:37 +0100
> > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > 
> > > On Tue, May 07, 2013 at 02:34:31PM -0700, Jesse Barnes wrote:
> > > > +static void valleyview_setup_pctx(struct drm_device *dev)
> > > > +{
> > > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > > +	struct drm_i915_gem_object *pctx;
> > > > +	unsigned long pctx_paddr;
> > > > +	u32 pcbr;
> > > > +	int pctx_size = 24*1024;
> > > > +
> > > > +	pcbr = I915_READ(VLV_PCBR);
> > > > +	if (pcbr) {
> > > > +		/* BIOS set it up already, grab the pre-alloc'd space */
> > > > +		int pcbr_offset;
> > > > +
> > > > +		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
> > > > +		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> > > > +								      pcbr_offset,
> > > > +								      pcbr_offset,
> > > > +								      pctx_size);
> > > 
> > > We're reserving global GTT space here for someting that just looks like
> > > it requires contiguous physical memory. This may cause issues if
> > > something else is already bound to that GTT range. I think we want to
> > > extend the interface to not reserve GTT space if gtt_offset == -1.
> > 
> > I should have added a comment for this too.  The physical range for the
> > power context must reside in stolen space.  So while it needs
> > contiguous physical memory, it also has a relationship to the stolen
> > handling.  I'd prefer to see a failure here than to try to do something
> > fancy with funky GTT vs stolen setups...  (Still waiting on the BIOS
> > guys to promise the identity map going forward.)
> 
> Hmm, I am not understanding exactly what you need here. The register you
> are reading looks to be a physical address - so why do we need to also
> reserve a virtual address with the same offset? And if that is required,
> shouldn't the else branch also reserve the corresponding virtual
> address for its stolen allocation?

I thought the stolen create code would do that for me; on looking I see
I'm wrong.

I'm conflating the stolen physically contiguous range here with the
first stolen_size portion of the GTT space.  I don't know if anything
depends on that or not, but given the way things are set up at boot
time I thought it would be best not to break that assumption.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Jesse Barnes]

Alan, is this something your team can test since your BIOS doesn't
allocate the power context?

Thanks,
Jesse

On Tue,  7 May 2013 14:34:31 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> But we need to get the right stolen base and make pre-allocated objects
> for BIOS stuff so we don't clobber it.  If the BIOS hasn't allocated a
> power context, we allocate one here too, from stolen space as required
> by the docs.
> 
> v2: fix stolen to phys if ladder (Ben)
>     keep BIOS reserved space out of allocator altogether (Ben)
> v3: fix mask of stolen base (Ben)
> v4: clean up preallocated object on unload (Ben)
>     don't zero reg on unload (Jesse)
>     fix mask harder (Jesse)
> v5: use unref for freeing stolen bits (Chris)
>     move alloc/free to intel_pm.c (Chris)
> v6: NULL pctx at disable time so error paths work (Ben)
> 
> Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h        |    2 ++
>  drivers/gpu/drm/i915/i915_gem_stolen.c |   12 ++++++--
>  drivers/gpu/drm/i915/i915_reg.h        |    1 +
>  drivers/gpu/drm/i915/intel_pm.c        |   49 ++++++++++++++++++++++++++++++++
>  4 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c81100c..2fe5fd4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1073,6 +1073,8 @@ typedef struct drm_i915_private {
>  
>  	struct i915_gpu_error gpu_error;
>  
> +	struct drm_i915_gem_object *vlv_pctx;
> +
>  	/* list of fbdev register on this device */
>  	struct intel_fbdev *fbdev;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 67d3510..9137fa4c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>  	 * its value of TOLUD.
>  	 */
>  	base = 0;
> -	if (INTEL_INFO(dev)->gen >= 6) {
> +	if (IS_VALLEYVIEW(dev)) {
> +		pci_read_config_dword(pdev, 0x5c, &base);
> +		base &= ~((1<<20) - 1);
> +	} else if (INTEL_INFO(dev)->gen >= 6) {
>  		/* Read Base Data of Stolen Memory Register (BDSM) directly.
>  		 * Note that there is also a MCHBAR miror at 0x1080c0 or
>  		 * we could use device 2:0x5c instead.
> @@ -183,6 +186,7 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
>  int i915_gem_init_stolen(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int bios_reserved = 0;
>  
>  	dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
>  	if (dev_priv->mm.stolen_base == 0)
> @@ -191,8 +195,12 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
>  		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
>  
> +	if (IS_VALLEYVIEW(dev))
> +		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
> +
>  	/* Basic memrange allocator for stolen space */
> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size);
> +	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> +		    bios_reserved);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a470103..a809a56 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -700,6 +700,7 @@
>  #define VLV_IIR		(VLV_DISPLAY_BASE + 0x20a4)
>  #define VLV_IMR		(VLV_DISPLAY_BASE + 0x20a8)
>  #define VLV_ISR		(VLV_DISPLAY_BASE + 0x20ac)
> +#define VLV_PCBR	(VLV_DISPLAY_BASE + 0x2120)
>  #define   I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
>  #define   I915_DISPLAY_PORT_INTERRUPT			(1<<17)
>  #define   I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9b3e90e..e60cd3e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2562,6 +2562,11 @@ static void valleyview_disable_rps(struct drm_device *dev)
>  	spin_unlock_irq(&dev_priv->rps.lock);
>  
>  	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
> +
> +	if (dev_priv->vlv_pctx) {
> +		drm_gem_object_unreference(&dev_priv->vlv_pctx->base);
> +		dev_priv->vlv_pctx = NULL;
> +	}
>  }
>  
>  int intel_enable_rc6(const struct drm_device *dev)
> @@ -2856,6 +2861,48 @@ static void vlv_rps_timer_work(struct work_struct *work)
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> +static void valleyview_setup_pctx(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *pctx;
> +	unsigned long pctx_paddr;
> +	u32 pcbr;
> +	int pctx_size = 24*1024;
> +
> +	pcbr = I915_READ(VLV_PCBR);
> +	if (pcbr) {
> +		/* BIOS set it up already, grab the pre-alloc'd space */
> +		int pcbr_offset;
> +
> +		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
> +		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> +								      pcbr_offset,
> +								      pcbr_offset,
> +								      pctx_size);
> +		goto out;
> +	}
> +
> +	/*
> +	 * From the Gunit register HAS:
> +	 * The Gfx driver is expected to program this register and ensure
> +	 * proper allocation within Gfx stolen memory.  For example, this
> +	 * register should be programmed such than the PCBR range does not
> +	 * overlap with other ranges, such as the frame buffer, protected
> +	 * memory, or any other relevant ranges.
> +	 */
> +	pctx = i915_gem_object_create_stolen(dev, pctx_size);
> +	if (!pctx) {
> +		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
> +		return;
> +	}
> +
> +	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
> +	I915_WRITE(VLV_PCBR, pctx_paddr);
> +
> +out:
> +	dev_priv->vlv_pctx = pctx;
> +}
> +
>  static void valleyview_enable_rps(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2870,6 +2917,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  		I915_WRITE(GTFIFODBG, gtfifodbg);
>  	}
>  
> +	valleyview_setup_pctx(dev);
> +
>  	gen6_gt_force_wake_get(dev_priv);
>  
>  	I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);


-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Jesse Barnes]

On Tue, 7 May 2013 23:14:40 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, May 07, 2013 at 03:08:42PM -0700, Jesse Barnes wrote:
> > On Tue, 7 May 2013 22:57:37 +0100
> > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > 
> > > On Tue, May 07, 2013 at 02:34:31PM -0700, Jesse Barnes wrote:
> > > > +static void valleyview_setup_pctx(struct drm_device *dev)
> > > > +{
> > > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > > +	struct drm_i915_gem_object *pctx;
> > > > +	unsigned long pctx_paddr;
> > > > +	u32 pcbr;
> > > > +	int pctx_size = 24*1024;
> > > > +
> > > > +	pcbr = I915_READ(VLV_PCBR);
> > > > +	if (pcbr) {
> > > > +		/* BIOS set it up already, grab the pre-alloc'd space */
> > > > +		int pcbr_offset;
> > > > +
> > > > +		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
> > > > +		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> > > > +								      pcbr_offset,
> > > > +								      pcbr_offset,
> > > > +								      pctx_size);
> > > 
> > > We're reserving global GTT space here for someting that just looks like
> > > it requires contiguous physical memory. This may cause issues if
> > > something else is already bound to that GTT range. I think we want to
> > > extend the interface to not reserve GTT space if gtt_offset == -1.
> > 
> > I should have added a comment for this too.  The physical range for the
> > power context must reside in stolen space.  So while it needs
> > contiguous physical memory, it also has a relationship to the stolen
> > handling.  I'd prefer to see a failure here than to try to do something
> > fancy with funky GTT vs stolen setups...  (Still waiting on the BIOS
> > guys to promise the identity map going forward.)
> 
> Hmm, I am not understanding exactly what you need here. The register you
> are reading looks to be a physical address - so why do we need to also
> reserve a virtual address with the same offset? And if that is required,
> shouldn't the else branch also reserve the corresponding virtual
> address for its stolen allocation?

Ok ok I'm sold, at least until we find otherwise or Alan's team reports
back that the GTT space is needed.

Extending the interface to not reserve GTT space makes sense; I can do
that as a follow up, if you're ok with the existing code.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Teres Alexis, Alan Previn]

I think we could.

Joshua, do u still have access to the CrestViewHills platform with Soo Keong - I need to make some changes to our GEM and try some new code. 
Lets take this offline and try to get a reply back to Jesse later.

...alan

-----Original Message-----
From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org] 
Sent: Wednesday, May 08, 2013 7:19 AM
To: Jesse Barnes
Cc: intel-gfx@lists.freedesktop.org; Teres Alexis, Alan Previn
Subject: Re: [Intel-gfx] [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

Alan, is this something your team can test since your BIOS doesn't allocate the power context?

Thanks,
Jesse

On Tue,  7 May 2013 14:34:31 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> But we need to get the right stolen base and make pre-allocated 
> objects for BIOS stuff so we don't clobber it.  If the BIOS hasn't 
> allocated a power context, we allocate one here too, from stolen space 
> as required by the docs.
> 
> v2: fix stolen to phys if ladder (Ben)
>     keep BIOS reserved space out of allocator altogether (Ben)
> v3: fix mask of stolen base (Ben)
> v4: clean up preallocated object on unload (Ben)
>     don't zero reg on unload (Jesse)
>     fix mask harder (Jesse)
> v5: use unref for freeing stolen bits (Chris)
>     move alloc/free to intel_pm.c (Chris)
> v6: NULL pctx at disable time so error paths work (Ben)
> 
> Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h        |    2 ++
>  drivers/gpu/drm/i915/i915_gem_stolen.c |   12 ++++++--
>  drivers/gpu/drm/i915/i915_reg.h        |    1 +
>  drivers/gpu/drm/i915/intel_pm.c        |   49 ++++++++++++++++++++++++++++++++
>  4 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> b/drivers/gpu/drm/i915/i915_drv.h index c81100c..2fe5fd4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1073,6 +1073,8 @@ typedef struct drm_i915_private {
>  
>  	struct i915_gpu_error gpu_error;
>  
> +	struct drm_i915_gem_object *vlv_pctx;
> +
>  	/* list of fbdev register on this device */
>  	struct intel_fbdev *fbdev;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 67d3510..9137fa4c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>  	 * its value of TOLUD.
>  	 */
>  	base = 0;
> -	if (INTEL_INFO(dev)->gen >= 6) {
> +	if (IS_VALLEYVIEW(dev)) {
> +		pci_read_config_dword(pdev, 0x5c, &base);
> +		base &= ~((1<<20) - 1);
> +	} else if (INTEL_INFO(dev)->gen >= 6) {
>  		/* Read Base Data of Stolen Memory Register (BDSM) directly.
>  		 * Note that there is also a MCHBAR miror at 0x1080c0 or
>  		 * we could use device 2:0x5c instead.
> @@ -183,6 +186,7 @@ void i915_gem_cleanup_stolen(struct drm_device 
> *dev)  int i915_gem_init_stolen(struct drm_device *dev)  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int bios_reserved = 0;
>  
>  	dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
>  	if (dev_priv->mm.stolen_base == 0)
> @@ -191,8 +195,12 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
>  		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
>  
> +	if (IS_VALLEYVIEW(dev))
> +		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
> +
>  	/* Basic memrange allocator for stolen space */
> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size);
> +	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> +		    bios_reserved);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> b/drivers/gpu/drm/i915/i915_reg.h index a470103..a809a56 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -700,6 +700,7 @@
>  #define VLV_IIR		(VLV_DISPLAY_BASE + 0x20a4)
>  #define VLV_IMR		(VLV_DISPLAY_BASE + 0x20a8)
>  #define VLV_ISR		(VLV_DISPLAY_BASE + 0x20ac)
> +#define VLV_PCBR	(VLV_DISPLAY_BASE + 0x2120)
>  #define   I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
>  #define   I915_DISPLAY_PORT_INTERRUPT			(1<<17)
>  #define   I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> b/drivers/gpu/drm/i915/intel_pm.c index 9b3e90e..e60cd3e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2562,6 +2562,11 @@ static void valleyview_disable_rps(struct drm_device *dev)
>  	spin_unlock_irq(&dev_priv->rps.lock);
>  
>  	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
> +
> +	if (dev_priv->vlv_pctx) {
> +		drm_gem_object_unreference(&dev_priv->vlv_pctx->base);
> +		dev_priv->vlv_pctx = NULL;
> +	}
>  }
>  
>  int intel_enable_rc6(const struct drm_device *dev) @@ -2856,6 
> +2861,48 @@ static void vlv_rps_timer_work(struct work_struct *work)
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
> +static void valleyview_setup_pctx(struct drm_device *dev) {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *pctx;
> +	unsigned long pctx_paddr;
> +	u32 pcbr;
> +	int pctx_size = 24*1024;
> +
> +	pcbr = I915_READ(VLV_PCBR);
> +	if (pcbr) {
> +		/* BIOS set it up already, grab the pre-alloc'd space */
> +		int pcbr_offset;
> +
> +		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
> +		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> +								      pcbr_offset,
> +								      pcbr_offset,
> +								      pctx_size);
> +		goto out;
> +	}
> +
> +	/*
> +	 * From the Gunit register HAS:
> +	 * The Gfx driver is expected to program this register and ensure
> +	 * proper allocation within Gfx stolen memory.  For example, this
> +	 * register should be programmed such than the PCBR range does not
> +	 * overlap with other ranges, such as the frame buffer, protected
> +	 * memory, or any other relevant ranges.
> +	 */
> +	pctx = i915_gem_object_create_stolen(dev, pctx_size);
> +	if (!pctx) {
> +		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
> +		return;
> +	}
> +
> +	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
> +	I915_WRITE(VLV_PCBR, pctx_paddr);
> +
> +out:
> +	dev_priv->vlv_pctx = pctx;
> +}
> +
>  static void valleyview_enable_rps(struct drm_device *dev)  {
>  	struct drm_i915_private *dev_priv = dev->dev_private; @@ -2870,6 
> +2917,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
>  		I915_WRITE(GTFIFODBG, gtfifodbg);
>  	}
>  
> +	valleyview_setup_pctx(dev);
> +
>  	gen6_gt_force_wake_get(dev_priv);
>  
>  	I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);


--
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: BIOS and power context stolen mem handling for VLV v6

[Chris Wilson]

On Tue, May 07, 2013 at 04:23:06PM -0700, Jesse Barnes wrote:
> Ok ok I'm sold, at least until we find otherwise or Alan's team reports
> back that the GTT space is needed.
> 
> Extending the interface to not reserve GTT space makes sense; I can do
> that as a follow up, if you're ok with the existing code.

I'm happy with the rest of the patch - just wishing that the size was
expressed in pages (6 * GTT_PAGE_SIZE) and similarly the mask
~(GTT_PAGE_SIZE-1).

Bring on the change to avoid reserving GTT space and lets hope it
sticks! :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre