[PATCH] drm/i915: fix errata for sync flush enable on 845G/830M

[PATCH] drm/i915: fix errata for sync flush enable on 845G/830M

[Chris Wilson]

From: Zhenyu Wang <zhenyuw@linux.intel.com>

The sync flush enable mask on 845G and 830M is flipped.
So driver should invert it usage.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f19ffe8..4b6c2f8 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1356,6 +1356,7 @@ static int i915_load_modeset_init(struct drm_device *dev,
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int fb_bar = IS_I9XX(dev) ? 2 : 0;
+	u32 instpm_val;
 	int ret = 0;
 
 	dev->mode_config.fb_base = pci_resource_start(dev->pdev, fb_bar) &
@@ -1432,9 +1433,14 @@ static int i915_load_modeset_init(struct drm_device *dev,
 
 	/*
 	 * Initialize the hardware status page IRQ location.
+	 * errata: on 845G and 830M, bit 21's polarity is reversed,
+	 * so the usage is inverted.
 	 */
-
-	I915_WRITE(INSTPM, (1 << 5) | (1 << 21));
+	if (IS_845G(dev) || IS_I830(dev))
+	    instpm_val = (1 << 5) | (0 << 21);
+	else
+	    instpm_val = (1 << 5) | (1 << 21);
+	I915_WRITE(INSTPM, instpm_val);
 
 	ret = intel_fbdev_init(dev);
 	if (ret)
-- 
1.7.1

Re: [PATCH] drm/i915: fix errata for sync flush enable on 845G/830M

[Jesse Barnes]

On Sun,  8 Aug 2010 19:39:40 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> From: Zhenyu Wang <zhenyuw@linux.intel.com>
> 
> The sync flush enable mask on 845G and 830M is flipped.
> So driver should invert it usage.
> 
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_dma.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index f19ffe8..4b6c2f8 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1356,6 +1356,7 @@ static int i915_load_modeset_init(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int fb_bar = IS_I9XX(dev) ? 2 : 0;
> +	u32 instpm_val;
>  	int ret = 0;
>  
>  	dev->mode_config.fb_base = pci_resource_start(dev->pdev, fb_bar) &
> @@ -1432,9 +1433,14 @@ static int i915_load_modeset_init(struct drm_device *dev,
>  
>  	/*
>  	 * Initialize the hardware status page IRQ location.
> +	 * errata: on 845G and 830M, bit 21's polarity is reversed,
> +	 * so the usage is inverted.
>  	 */
> -
> -	I915_WRITE(INSTPM, (1 << 5) | (1 << 21));
> +	if (IS_845G(dev) || IS_I830(dev))
> +	    instpm_val = (1 << 5) | (0 << 21);
> +	else
> +	    instpm_val = (1 << 5) | (1 << 21);
> +	I915_WRITE(INSTPM, instpm_val);
>  
>  	ret = intel_fbdev_init(dev);
>  	if (ret)

Is this hack even needed?  I noticed on my 945GM that INSTDONE has a
stuck bit when I use this code, removing it didn't seem to have any
negative effects...  If it really is necessary we should probably
document it better and ideally move it to somewhere better like where
we program the clock gating workarounds.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm/i915: fix errata for sync flush enable on 845G/830M

[Chris Wilson]

On Mon, 9 Aug 2010 09:45:38 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Is this hack even needed?  I noticed on my 945GM that INSTDONE has a
> stuck bit when I use this code, removing it didn't seem to have any
> negative effects...

I'm not sure why we need to trigger a Sync on the GPU at this point, I
just assumed it was one of those magic bits of code required to bring up
the hardware.

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: fix errata for sync flush enable on 845G/830M

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 707 bytes --]

On Mon, 09 Aug 2010 18:04:50 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, 9 Aug 2010 09:45:38 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > Is this hack even needed?  I noticed on my 945GM that INSTDONE has a
> > stuck bit when I use this code, removing it didn't seem to have any
> > negative effects...
> 
> I'm not sure why we need to trigger a Sync on the GPU at this point, I
> just assumed it was one of those magic bits of code required to bring up
> the hardware.

Beginning a sync flush and continuing without waiting for the sync flush
to actually complete looks awfully bogus to me.  In the absence of
justification, I'd lean towards nuking this code.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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