[PATCH] drm/i915: dp aux irq support for g4x/vlv

[PATCH] drm/i915: dp aux irq support for g4x/vlv

[Daniel Vetter]

Now we have this everywhere. Next up would be to wire up the DP
hotplug pin to speed up panel power sequencing for eDP panels ...

I've decided to leave the has_aux_irq logic in the code, it should
come handy for hw bringup.

For testing/fail-safety the dp aux code already has a timeout when
waiting for interrupts to signal completion and screams rather loud if
they don't arrive in time. Given that we need a real piece of hw to
talk to anyway this is probably as good as it gets.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 6 ++++++
 drivers/gpu/drm/i915/i915_reg.h | 4 ++++
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2a44816..7c075a2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1375,6 +1375,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 			intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
 
+			if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
+				dp_aux_irq_handler(dev);
+
 			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
@@ -3256,6 +3259,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 			intel_hpd_irq_handler(dev, hotplug_trigger,
 					      IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i915);
 
+			if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
+				dp_aux_irq_handler(dev);
+
 			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4d2db59..447fd83 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2116,6 +2116,10 @@
 #define   CRT_HOTPLUG_MONITOR_COLOR		(3 << 8)
 #define   CRT_HOTPLUG_MONITOR_MONO		(2 << 8)
 #define   CRT_HOTPLUG_MONITOR_NONE		(0 << 8)
+#define   DP_AUX_CHANNEL_D_INT_STATUS_G4X	(1 << 6)
+#define   DP_AUX_CHANNEL_C_INT_STATUS_G4X	(1 << 5)
+#define   DP_AUX_CHANNEL_B_INT_STATUS_G4X	(1 << 4)
+#define   DP_AUX_CHANNEL_MASK_INT_STATUS_G4X	(1 << 4)
 /* SDVO is different across gen3/4 */
 #define   SDVOC_HOTPLUG_INT_STATUS_G4X		(1 << 3)
 #define   SDVOB_HOTPLUG_INT_STATUS_G4X		(1 << 2)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b3cc333..7fa4518 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -404,7 +404,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	int i, ret, recv_bytes;
 	uint32_t status;
 	int try, precharge, clock = 0;
-	bool has_aux_irq = INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev);
+	bool has_aux_irq = true;
 
 	/* dp aux is extremely sensitive to irq latency, hence request the
 	 * lowest possible wakeup latency and so prevent the cpu from going into
-- 
1.8.4.rc3

Re: [PATCH] drm/i915: dp aux irq support for g4x/vlv

[Jani Nikula]

On Wed, 30 Oct 2013, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Now we have this everywhere. Next up would be to wire up the DP
> hotplug pin to speed up panel power sequencing for eDP panels ...
>
> I've decided to leave the has_aux_irq logic in the code, it should
> come handy for hw bringup.
>
> For testing/fail-safety the dp aux code already has a timeout when
> waiting for interrupts to signal completion and screams rather loud if
> they don't arrive in time. Given that we need a real piece of hw to
> talk to anyway this is probably as good as it gets.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 6 ++++++
>  drivers/gpu/drm/i915/i915_reg.h | 4 ++++
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2a44816..7c075a2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1375,6 +1375,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  
>  			intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
>  
> +			if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> +				dp_aux_irq_handler(dev);
> +
>  			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
>  			I915_READ(PORT_HOTPLUG_STAT);
>  		}
> @@ -3256,6 +3259,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
>  			intel_hpd_irq_handler(dev, hotplug_trigger,
>  					      IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i915);
>  
> +			if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> +				dp_aux_irq_handler(dev);

Should the condition have && IS_G4X(dev) there too? Bits 4:3 are sync
polarity on 965 per my spec. Not that it matters much since that won't
have DP anyway.

Otherwise looks okay as far as I can tell. The above fixed or not,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +
>  			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
>  			I915_READ(PORT_HOTPLUG_STAT);
>  		}
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4d2db59..447fd83 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2116,6 +2116,10 @@
>  #define   CRT_HOTPLUG_MONITOR_COLOR		(3 << 8)
>  #define   CRT_HOTPLUG_MONITOR_MONO		(2 << 8)
>  #define   CRT_HOTPLUG_MONITOR_NONE		(0 << 8)
> +#define   DP_AUX_CHANNEL_D_INT_STATUS_G4X	(1 << 6)
> +#define   DP_AUX_CHANNEL_C_INT_STATUS_G4X	(1 << 5)
> +#define   DP_AUX_CHANNEL_B_INT_STATUS_G4X	(1 << 4)
> +#define   DP_AUX_CHANNEL_MASK_INT_STATUS_G4X	(1 << 4)
>  /* SDVO is different across gen3/4 */
>  #define   SDVOC_HOTPLUG_INT_STATUS_G4X		(1 << 3)
>  #define   SDVOB_HOTPLUG_INT_STATUS_G4X		(1 << 2)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b3cc333..7fa4518 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -404,7 +404,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	int i, ret, recv_bytes;
>  	uint32_t status;
>  	int try, precharge, clock = 0;
> -	bool has_aux_irq = INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev);
> +	bool has_aux_irq = true;
>  
>  	/* dp aux is extremely sensitive to irq latency, hence request the
>  	 * lowest possible wakeup latency and so prevent the cpu from going into
> -- 
> 1.8.4.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH] drm/i915: dp aux irq support for g4x/vlv

[Daniel Vetter]

Now we have this everywhere. Next up would be to wire up the DP
hotplug pin to speed up panel power sequencing for eDP panels ...

I've decided to leave the has_aux_irq logic in the code, it should
come handy for hw bringup.

For testing/fail-safety the dp aux code already has a timeout when
waiting for interrupts to signal completion and screams rather loud if
they don't arrive in time. Given that we need a real piece of hw to
talk to anyway this is probably as good as it gets.

v2: Don't check the dp aux channel bits on i965 machines, they have a
different meaning there. Yay for reusing bits at will! Spotted by
Jani.

Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 7 +++++++
 drivers/gpu/drm/i915/i915_reg.h | 4 ++++
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2a44816..a2bfcdb 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1375,6 +1375,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 			intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
 
+			if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
+				dp_aux_irq_handler(dev);
+
 			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
@@ -3256,6 +3259,10 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
 			intel_hpd_irq_handler(dev, hotplug_trigger,
 					      IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i915);
 
+			if (IS_G4X(dev) &&
+			    (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X))
+				dp_aux_irq_handler(dev);
+
 			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4d2db59..447fd83 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2116,6 +2116,10 @@
 #define   CRT_HOTPLUG_MONITOR_COLOR		(3 << 8)
 #define   CRT_HOTPLUG_MONITOR_MONO		(2 << 8)
 #define   CRT_HOTPLUG_MONITOR_NONE		(0 << 8)
+#define   DP_AUX_CHANNEL_D_INT_STATUS_G4X	(1 << 6)
+#define   DP_AUX_CHANNEL_C_INT_STATUS_G4X	(1 << 5)
+#define   DP_AUX_CHANNEL_B_INT_STATUS_G4X	(1 << 4)
+#define   DP_AUX_CHANNEL_MASK_INT_STATUS_G4X	(1 << 4)
 /* SDVO is different across gen3/4 */
 #define   SDVOC_HOTPLUG_INT_STATUS_G4X		(1 << 3)
 #define   SDVOB_HOTPLUG_INT_STATUS_G4X		(1 << 2)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b3cc333..7fa4518 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -404,7 +404,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	int i, ret, recv_bytes;
 	uint32_t status;
 	int try, precharge, clock = 0;
-	bool has_aux_irq = INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev);
+	bool has_aux_irq = true;
 
 	/* dp aux is extremely sensitive to irq latency, hence request the
 	 * lowest possible wakeup latency and so prevent the cpu from going into
-- 
1.8.4.rc3

Re: [PATCH] drm/i915: dp aux irq support for g4x/vlv

[Daniel Vetter]

On Thu, Oct 31, 2013 at 09:53:36AM +0100, Daniel Vetter wrote:
> Now we have this everywhere. Next up would be to wire up the DP
> hotplug pin to speed up panel power sequencing for eDP panels ...
> 
> I've decided to leave the has_aux_irq logic in the code, it should
> come handy for hw bringup.
> 
> For testing/fail-safety the dp aux code already has a timeout when
> waiting for interrupts to signal completion and screams rather loud if
> they don't arrive in time. Given that we need a real piece of hw to
> talk to anyway this is probably as good as it gets.
> 
> v2: Don't check the dp aux channel bits on i965 machines, they have a
> different meaning there. Yay for reusing bits at will! Spotted by
> Jani.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Oops, I've seemed to have missed this one here. Merged to dinq.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 7 +++++++
>  drivers/gpu/drm/i915/i915_reg.h | 4 ++++
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2a44816..a2bfcdb 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1375,6 +1375,9 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  
>  			intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
>  
> +			if (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> +				dp_aux_irq_handler(dev);
> +
>  			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
>  			I915_READ(PORT_HOTPLUG_STAT);
>  		}
> @@ -3256,6 +3259,10 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
>  			intel_hpd_irq_handler(dev, hotplug_trigger,
>  					      IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i915);
>  
> +			if (IS_G4X(dev) &&
> +			    (hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X))
> +				dp_aux_irq_handler(dev);
> +
>  			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
>  			I915_READ(PORT_HOTPLUG_STAT);
>  		}
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4d2db59..447fd83 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2116,6 +2116,10 @@
>  #define   CRT_HOTPLUG_MONITOR_COLOR		(3 << 8)
>  #define   CRT_HOTPLUG_MONITOR_MONO		(2 << 8)
>  #define   CRT_HOTPLUG_MONITOR_NONE		(0 << 8)
> +#define   DP_AUX_CHANNEL_D_INT_STATUS_G4X	(1 << 6)
> +#define   DP_AUX_CHANNEL_C_INT_STATUS_G4X	(1 << 5)
> +#define   DP_AUX_CHANNEL_B_INT_STATUS_G4X	(1 << 4)
> +#define   DP_AUX_CHANNEL_MASK_INT_STATUS_G4X	(1 << 4)
>  /* SDVO is different across gen3/4 */
>  #define   SDVOC_HOTPLUG_INT_STATUS_G4X		(1 << 3)
>  #define   SDVOB_HOTPLUG_INT_STATUS_G4X		(1 << 2)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b3cc333..7fa4518 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -404,7 +404,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	int i, ret, recv_bytes;
>  	uint32_t status;
>  	int try, precharge, clock = 0;
> -	bool has_aux_irq = INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev);
> +	bool has_aux_irq = true;
>  
>  	/* dp aux is extremely sensitive to irq latency, hence request the
>  	 * lowest possible wakeup latency and so prevent the cpu from going into
> -- 
> 1.8.4.rc3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] Watermark level workaround for i830

[Thomas Richter]

[-- Attachment #1: Type: text/plain, Size: 899 bytes --]

Hi Daniel, hi intel experts,

please find a patch attached concerning the watermark levels on the i830 
chipsets.

I did a couple of experiments this morning and found that the watermark 
on i830 may neither be
too small (i.e. the FW_BLC register values may not be too high) as 
otherwise the FIFO runs try, but
for some strange reasons, the watermark may neither be too high (the 
FW_BLC register must not
be too small) as otherwise the display flickers. Reasons for this are 
unclear at this moment, though
the attached patch seems to remove flickering quite reliably on linear 
and tiled displays.

What I should probably also report is that the watermark is set quite 
too low (i.e. much too conservative)
on the R31. The computed values are 1 and 5 (for VGA and LVDS, 
respectively), though the minimum
required levels are much higher, somewhere in the ballpark of 32.

Greetings,
     Thomas


[-- Attachment #2: 0003-Watermark-configuration-workaround-for-i830-chipsets.patch --]
[-- Type: text/x-patch, Size: 1889 bytes --]

>From f535e532f3279e43a7f20bc96d4e62b24a9af684 Mon Sep 17 00:00:00 2001
From: Thomas Richter <thor@math.tu-berlin.de>
Date: Mon, 18 Nov 2013 10:38:27 +0100
Subject: [PATCH 3/3] Watermark configuration workaround for i830 chipsets.

For unclear reasons, the watermark level on i830 and related
chipsets must not grow above 6 as otherwise display flickering
will occurr, specifically on panning.

Signed-off-by: Thomas Richter <thor@math.tu-berlin.de>
---
 drivers/gpu/drm/i915/intel_pm.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 365545f..43c65f0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1648,6 +1648,21 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
 			I915_WRITE(FW_BLC_SELF, srwm & 0x3f);
 	}
 
+	if (IS_I830(dev)) {
+		/* For unknown reasons, i830 chipsets run havok
+		 * on panning if the watermark is below 6,
+		 * thus adjust it accordingly.
+		 */
+		if (planea_wm < 6) {
+			planea_wm = 6;
+			DRM_DEBUG_KMS("i9xx plane A wm workaround enabled\n");
+		}
+		if (planeb_wm < 6) {
+			planeb_wm = 6;
+			DRM_DEBUG_KMS("i9xx plane B wm workaround enabled\n");
+		}
+	}
+
 	DRM_DEBUG_KMS("Setting FIFO watermarks - A: %d, B: %d, C: %d, SR %d\n",
 		      planea_wm, planeb_wm, cwm, srwm);
 
@@ -1692,6 +1707,16 @@ static void i830_update_wm(struct drm_crtc *unused_crtc)
 				       &i830_wm_info,
 				       dev_priv->display.get_fifo_size(dev, 0),
 				       4, latency_ns);
+
+	/* For unknown reasons, i830 chipsets run havok
+	 * on panning if the watermark is below 6,
+	 * thus adjust it accordingly.
+	 */
+	if (planea_wm < 6) {
+		planea_wm = 6;
+		DRM_DEBUG_KMS("i830 plane A wm workaround enabled\n");
+	}
+
 	fwater_lo = I915_READ(FW_BLC) & ~0xfff;
 	fwater_lo |= (3<<8) | planea_wm;
 
-- 
1.7.10.4


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

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

Fwd: [PATCH] Watermark level workaround for i830

[Thomas Richter]

[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]

Hi Daniel,

did you get this? (See below)?

In the meantime, I also checked with video overlays, and the minimum 
value of 6 is not yet quite optimal, higher values (lower watermarks) 
seem to do even better. From the values I get, I also estimate a minimum 
latency of about 1100 ns, the default value of 5000 is much too high for 
the i830.

There are two alternative possibilities to fix this:

a) Include in the watermark structure not only a maximum value, but also 
a minimum value and modify calculate_wm accordingly,

-and/or-

c) include a latency value in the watermark structure, setting it to
a lower value on i830.

What do you think?

Greetings,
	Thomas


-------- Original-Nachricht --------
Betreff: [PATCH] Watermark level workaround for i830
Datum: Mon, 18 Nov 2013 10:42:31 +0100
Von: Thomas Richter <thor@math.tu-berlin.de>
An: Daniel Vetter <daniel@ffwll.ch>
Kopie (CC): intel-gfx@lists.freedesktop.org

Hi Daniel, hi intel experts,

please find a patch attached concerning the watermark levels on the i830
chipsets.

I did a couple of experiments this morning and found that the watermark
on i830 may neither be
too small (i.e. the FW_BLC register values may not be too high) as
otherwise the FIFO runs try, but
for some strange reasons, the watermark may neither be too high (the
FW_BLC register must not
be too small) as otherwise the display flickers. Reasons for this are
unclear at this moment, though
the attached patch seems to remove flickering quite reliably on linear
and tiled displays.

What I should probably also report is that the watermark is set quite
too low (i.e. much too conservative)
on the R31. The computed values are 1 and 5 (for VGA and LVDS,
respectively), though the minimum
required levels are much higher, somewhere in the ballpark of 32.

Greetings,
      Thomas



[-- Attachment #2: 0003-Watermark-configuration-workaround-for-i830-chipsets.patch --]
[-- Type: text/x-patch, Size: 1890 bytes --]

>From f535e532f3279e43a7f20bc96d4e62b24a9af684 Mon Sep 17 00:00:00 2001
From: Thomas Richter <thor@math.tu-berlin.de>
Date: Mon, 18 Nov 2013 10:38:27 +0100
Subject: [PATCH 3/3] Watermark configuration workaround for i830 chipsets.

For unclear reasons, the watermark level on i830 and related
chipsets must not grow above 6 as otherwise display flickering
will occurr, specifically on panning.

Signed-off-by: Thomas Richter <thor@math.tu-berlin.de>
---
 drivers/gpu/drm/i915/intel_pm.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 365545f..43c65f0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1648,6 +1648,21 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
 			I915_WRITE(FW_BLC_SELF, srwm & 0x3f);
 	}
 
+	if (IS_I830(dev)) {
+		/* For unknown reasons, i830 chipsets run havok
+		 * on panning if the watermark is below 6,
+		 * thus adjust it accordingly.
+		 */
+		if (planea_wm < 6) {
+			planea_wm = 6;
+			DRM_DEBUG_KMS("i9xx plane A wm workaround enabled\n");
+		}
+		if (planeb_wm < 6) {
+			planeb_wm = 6;
+			DRM_DEBUG_KMS("i9xx plane B wm workaround enabled\n");
+		}
+	}
+
 	DRM_DEBUG_KMS("Setting FIFO watermarks - A: %d, B: %d, C: %d, SR %d\n",
 		      planea_wm, planeb_wm, cwm, srwm);
 
@@ -1692,6 +1707,16 @@ static void i830_update_wm(struct drm_crtc *unused_crtc)
 				       &i830_wm_info,
 				       dev_priv->display.get_fifo_size(dev, 0),
 				       4, latency_ns);
+
+	/* For unknown reasons, i830 chipsets run havok
+	 * on panning if the watermark is below 6,
+	 * thus adjust it accordingly.
+	 */
+	if (planea_wm < 6) {
+		planea_wm = 6;
+		DRM_DEBUG_KMS("i830 plane A wm workaround enabled\n");
+	}
+
 	fwater_lo = I915_READ(FW_BLC) & ~0xfff;
 	fwater_lo |= (3<<8) | planea_wm;
 
-- 
1.7.10.4



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

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

Re: [PATCH] Watermark level workaround for i830

[Daniel Vetter]

On Tue, Nov 19, 2013 at 10:55 AM, Thomas Richter <thor@math.tu-berlin.de> wrote:
> In the meantime, I also checked with video overlays, and the minimum value
> of 6 is not yet quite optimal, higher values (lower watermarks) seem to do
> even better. From the values I get, I also estimate a minimum latency of
> about 1100 ns, the default value of 5000 is much too high for the i830.
>
> There are two alternative possibilities to fix this:
>
> a) Include in the watermark structure not only a maximum value, but also a
> minimum value and modify calculate_wm accordingly,

I think this is what we need - Bspec explicitly mentions that setting
the watermark so that the fifo could overrun with the given burst
length is a bad idea. Atm we set the brust length to 8, so my standing
bet is that this is the limit where things start to get fully stable.

I've started to work on patches for this last w/e but got distracted a
bit with my real job, hence the silence.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch