[PATCH 00/16] drm/i915: 830M/ns201 fixes again

[PATCH 00/16] drm/i915: 830M/ns201 fixes again

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thomas asked me to repost my 830/ns2501 patches. So here they are. I added
a few more patches (trickle feed and unused ring init) to fix some post-resume
issues. The primary plane rmw elimination patches and some locking/load detect
fixes already got merged.

Apart from these we still lack the minimum watermark check. I guess
we could just take the patch Thomas posted [1]. Doesn't look like a
more advanced solution is coming any time soon. Though the commit
message of that patch needs work and it lacks a s-o-b.

The VGACNTR patch might not be necessary any longer since Daniel's
vga/dummycon stuff. I don't recall if I tested without it, but my
gut feeling is that it's no longer needed. But I included the patch
here anyway.

[1] http://patchwork.freedesktop.org/patch/27318/

Ville Syrjälä (16):
  drm/i915: Fix gen2 planes B and C max watermark value
  drm/i915: Disable trickle feed for gen2/3
  drm/i915: Idle unused rings on gen2/3 during init/resume
  drm/i915: Pass intel_crtc to intel_disable_pipe() and
    intel_wait_for_pipe_off()
  drm/i915: Disable double wide even when leaving the pipe on
  drm/i915: ns2501 is on DVOB
  drm/i915: Enable DVO between mode_set and dpms hooks
  drm/i915: Don't call DVO mode_set hook on DPMS changes
  drm/i915: Kill useless ns2501_dump_regs
  drm/i915: Rewrite ns2501 driver a bit
  drm/i915: Init important ns2501 registers
  drm/i915: Check pixel clock in ns2501 mode_valid hook
  drm/i915: Fix DVO 2x clock enable on 830M
  Revert "drm/i915: Nuke pipe A quirk on i830M"
  drm/i915: Add pipe B force quirk for 830M
  drm/i915: Preserve VGACNTR bits from the BIOS

 drivers/gpu/drm/i915/dvo_ns2501.c    | 560 +++++++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_drv.h      |   6 +
 drivers/gpu/drm/i915/i915_gem.c      |  35 +++
 drivers/gpu/drm/i915/i915_reg.h      |  11 +
 drivers/gpu/drm/i915/intel_display.c | 150 +++++++---
 drivers/gpu/drm/i915/intel_drv.h     |   1 -
 drivers/gpu/drm/i915/intel_dvo.c     |  11 +-
 drivers/gpu/drm/i915/intel_pm.c      |  34 ++-
 8 files changed, 532 insertions(+), 276 deletions(-)

-- 
1.8.5.5

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

[PATCH 01/16] drm/i915: Fix gen2 planes B and C max watermark value

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The max watermark value for gen2 planes B and C is 0x1f, instead of
the 0x3f that plane A uses.

Also check against the max even if the pipe is disabled since the
FIFO size exceeds the plane B and C max watermark value.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 12f4e14..f696b7f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -985,13 +985,20 @@ static const struct intel_watermark_params i915_wm_info = {
 	.guard_size = 2,
 	.cacheline_size = I915_FIFO_LINE_SIZE,
 };
-static const struct intel_watermark_params i830_wm_info = {
+static const struct intel_watermark_params i830_a_wm_info = {
 	.fifo_size = I855GM_FIFO_SIZE,
 	.max_wm = I915_MAX_WM,
 	.default_wm = 1,
 	.guard_size = 2,
 	.cacheline_size = I830_FIFO_LINE_SIZE,
 };
+static const struct intel_watermark_params i830_bc_wm_info = {
+	.fifo_size = I855GM_FIFO_SIZE,
+	.max_wm = I915_MAX_WM/2,
+	.default_wm = 1,
+	.guard_size = 2,
+	.cacheline_size = I830_FIFO_LINE_SIZE,
+};
 static const struct intel_watermark_params i845_wm_info = {
 	.fifo_size = I830_FIFO_SIZE,
 	.max_wm = I915_MAX_WM,
@@ -1673,7 +1680,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
 	else if (!IS_GEN2(dev))
 		wm_info = &i915_wm_info;
 	else
-		wm_info = &i830_wm_info;
+		wm_info = &i830_a_wm_info;
 
 	fifo_size = dev_priv->display.get_fifo_size(dev, 0);
 	crtc = intel_get_crtc_for_plane(dev, 0);
@@ -1688,8 +1695,14 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
 					       wm_info, fifo_size, cpp,
 					       latency_ns);
 		enabled = crtc;
-	} else
+	} else {
 		planea_wm = fifo_size - wm_info->guard_size;
+		if (planea_wm > (long)wm_info->max_wm)
+			planea_wm = wm_info->max_wm;
+	}
+
+	if (IS_GEN2(dev))
+		wm_info = &i830_bc_wm_info;
 
 	fifo_size = dev_priv->display.get_fifo_size(dev, 1);
 	crtc = intel_get_crtc_for_plane(dev, 1);
@@ -1707,8 +1720,11 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
 			enabled = crtc;
 		else
 			enabled = NULL;
-	} else
+	} else {
 		planeb_wm = fifo_size - wm_info->guard_size;
+		if (planeb_wm > (long)wm_info->max_wm)
+			planeb_wm = wm_info->max_wm;
+	}
 
 	DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
 
-- 
1.8.5.5

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

Re: [PATCH 01/16] drm/i915: Fix gen2 planes B and C max watermark value

[Thomas Richter]

On 15.08.2014 00:21, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The max watermark value for gen2 planes B and C is 0x1f, instead of
> the 0x3f that plane A uses.
>
> Also check against the max even if the pipe is disabled since the
> FIFO size exceeds the plane B and C max watermark value.
>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++++----
>   1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 12f4e14..f696b7f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -985,13 +985,20 @@ static const struct intel_watermark_params i915_wm_info = {
>   	.guard_size = 2,
>   	.cacheline_size = I915_FIFO_LINE_SIZE,
>   };
> -static const struct intel_watermark_params i830_wm_info = {
> +static const struct intel_watermark_params i830_a_wm_info = {
>   	.fifo_size = I855GM_FIFO_SIZE,
>   	.max_wm = I915_MAX_WM,
>   	.default_wm = 1,
>   	.guard_size = 2,
>   	.cacheline_size = I830_FIFO_LINE_SIZE,
>   };
> +static const struct intel_watermark_params i830_bc_wm_info = {
> +	.fifo_size = I855GM_FIFO_SIZE,
> +	.max_wm = I915_MAX_WM/2,
> +	.default_wm = 1,
> +	.guard_size = 2,
> +	.cacheline_size = I830_FIFO_LINE_SIZE,
> +};
>   static const struct intel_watermark_params i845_wm_info = {
>   	.fifo_size = I830_FIFO_SIZE,
>   	.max_wm = I915_MAX_WM,
> @@ -1673,7 +1680,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
>   	else if (!IS_GEN2(dev))
>   		wm_info = &i915_wm_info;
>   	else
> -		wm_info = &i830_wm_info;
> +		wm_info = &i830_a_wm_info;
>
>   	fifo_size = dev_priv->display.get_fifo_size(dev, 0);
>   	crtc = intel_get_crtc_for_plane(dev, 0);
> @@ -1688,8 +1695,14 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
>   					       wm_info, fifo_size, cpp,
>   					       latency_ns);
>   		enabled = crtc;
> -	} else
> +	} else {
>   		planea_wm = fifo_size - wm_info->guard_size;
> +		if (planea_wm > (long)wm_info->max_wm)
> +			planea_wm = wm_info->max_wm;
> +	}
> +
> +	if (IS_GEN2(dev))
> +		wm_info = &i830_bc_wm_info;
>
>   	fifo_size = dev_priv->display.get_fifo_size(dev, 1);
>   	crtc = intel_get_crtc_for_plane(dev, 1);
> @@ -1707,8 +1720,11 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
>   			enabled = crtc;
>   		else
>   			enabled = NULL;
> -	} else
> +	} else {
>   		planeb_wm = fifo_size - wm_info->guard_size;
> +		if (planeb_wm > (long)wm_info->max_wm)
> +			planeb_wm = wm_info->max_wm;
> +	}
>
>   	DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
>
>

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

[PATCH 02/16] drm/i915: Disable trickle feed for gen2/3

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

My 830 is unhappy with trickle feed enabled. The symptom is that
the image on the screen shifts a bit to right occasionally.

The BIOS initially disables trickle feed, but it gets reset during
suspend, so we need to re-disable it ourselves. Juse disable it
always.

Also disable it for all other gen2/3 platforms since we disable it
for all more recent platforms as well (until HSW that is). At least
my 855 doesn't seem to mind us doing this. I don't have gen3
hardware to test that.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 203062e..b5fdffc 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1272,6 +1272,10 @@ enum punit_power_well {
 #define   INSTPM_TLB_INVALIDATE	(1<<9)
 #define   INSTPM_SYNC_FLUSH	(1<<5)
 #define ACTHD	        0x020c8
+#define MEM_MODE	0x020cc
+#define   MEM_DISPLAY_B_TRICKLE_FEED_DISABLE (1<<3) /* 830 only */
+#define   MEM_DISPLAY_A_TRICKLE_FEED_DISABLE (1<<2) /* 830/845 only */
+#define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
 #define FW_BLC		0x020d8
 #define FW_BLC2		0x020dc
 #define FW_BLC_SELF	0x020e0 /* 915+ only */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f696b7f..2865948 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6020,6 +6020,9 @@ static void gen3_init_clock_gating(struct drm_device *dev)
 
 	/* On GEN3 we really need to make sure the ARB C3 LP bit is set */
 	I915_WRITE(MI_ARB_STATE, _MASKED_BIT_ENABLE(MI_ARB_C3_LP_WRITE_ENABLE));
+
+	I915_WRITE(MI_ARB_STATE,
+		   _MASKED_BIT_ENABLE(MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE));
 }
 
 static void i85x_init_clock_gating(struct drm_device *dev)
@@ -6031,6 +6034,9 @@ static void i85x_init_clock_gating(struct drm_device *dev)
 	/* interrupts should cause a wake up from C3 */
 	I915_WRITE(MI_STATE, _MASKED_BIT_ENABLE(MI_AGPBUSY_INT_EN) |
 		   _MASKED_BIT_DISABLE(MI_AGPBUSY_830_MODE));
+
+	I915_WRITE(MEM_MODE,
+		   _MASKED_BIT_ENABLE(MEM_DISPLAY_TRICKLE_FEED_DISABLE));
 }
 
 static void i830_init_clock_gating(struct drm_device *dev)
@@ -6038,6 +6044,10 @@ static void i830_init_clock_gating(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	I915_WRITE(DSPCLK_GATE_D, OVRUNIT_CLOCK_GATE_DISABLE);
+
+	I915_WRITE(MEM_MODE,
+		   _MASKED_BIT_ENABLE(MEM_DISPLAY_A_TRICKLE_FEED_DISABLE) |
+		   _MASKED_BIT_ENABLE(MEM_DISPLAY_B_TRICKLE_FEED_DISABLE));
 }
 
 void intel_init_clock_gating(struct drm_device *dev)
-- 
1.8.5.5

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

[PATCH 03/16] drm/i915: Idle unused rings on gen2/3 during init/resume

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

gen2/3 platforms have a boatload of rings we're not using. On my 830
the BIOS/hw can leave some of those "active" after resume which will
prevent c3 entry. The ring is apparently considered active whenever
head != tail even if the ring is disabled.

Disable and clear all such unused ringbuffers on init/resume.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h |  7 +++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6c2f0b8..6cfa13e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4618,11 +4618,46 @@ intel_enable_blt(struct drm_device *dev)
 	return true;
 }
 
+static void init_unused_ring(struct drm_device *dev, u32 base)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	I915_WRITE(RING_CTL(base), 0);
+	I915_WRITE(RING_HEAD(base), 0);
+	I915_WRITE(RING_TAIL(base), 0);
+	I915_WRITE(RING_START(base), 0);
+}
+
+static void init_unused_rings(struct drm_device *dev)
+{
+	if (IS_I830(dev)) {
+		init_unused_ring(dev, PRB1_BASE);
+		init_unused_ring(dev, SRB0_BASE);
+		init_unused_ring(dev, SRB1_BASE);
+		init_unused_ring(dev, SRB2_BASE);
+		init_unused_ring(dev, SRB3_BASE);
+	} else if (IS_GEN2(dev)) {
+		init_unused_ring(dev, SRB0_BASE);
+		init_unused_ring(dev, SRB1_BASE);
+	} else if (IS_GEN3(dev)) {
+		init_unused_ring(dev, PRB1_BASE);
+		init_unused_ring(dev, PRB2_BASE);
+	}
+}
+
 int i915_gem_init_rings(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
+	/*
+	 * At least 830 can leave some of the unused rings
+	 * "active" (ie. head != tail) after resume which
+	 * will prevent c3 entry. Makes sure all unused rings
+	 * are totally idle.
+	 */
+	init_unused_rings(dev);
+
 	ret = intel_init_render_ring_buffer(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b5fdffc..f80f9b0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1026,6 +1026,13 @@ enum punit_power_well {
 #define   PGTBL_ADDRESS_LO_MASK	0xfffff000 /* bits [31:12] */
 #define   PGTBL_ADDRESS_HI_MASK	0x000000f0 /* bits [35:32] (gen4) */
 #define PGTBL_ER	0x02024
+#define PRB0_BASE (0x2030-0x30)
+#define PRB1_BASE (0x2040-0x30) /* 830,gen3 */
+#define PRB2_BASE (0x2050-0x30) /* gen3 */
+#define SRB0_BASE (0x2100-0x30) /* gen2 */
+#define SRB1_BASE (0x2110-0x30) /* gen2 */
+#define SRB2_BASE (0x2120-0x30) /* 830 */
+#define SRB3_BASE (0x2130-0x30) /* 830 */
 #define RENDER_RING_BASE	0x02000
 #define BSD_RING_BASE		0x04000
 #define GEN6_BSD_RING_BASE	0x12000
-- 
1.8.5.5

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

[PATCH 04/16] drm/i915: Pass intel_crtc to intel_disable_pipe() and intel_wait_for_pipe_off()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just pass the intel_crtc around instead of dev_priv+pipe.

Also make intel_wait_for_pipe_off() static since it's only used in
intel_display.c.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3813526..e7175ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -913,8 +913,7 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
 
 /*
  * intel_wait_for_pipe_off - wait for pipe to turn off
- * @dev: drm device
- * @pipe: pipe to wait for
+ * @crtc: crtc whose pipe to wait for
  *
  * After disabling a pipe, we can't wait for vblank in the usual way,
  * spinning on the vblank interrupt status bit, since we won't actually
@@ -928,11 +927,12 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
  *   ends up stopping at the start of the next frame).
  *
  */
-void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
+static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
 {
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
-								      pipe);
+	enum transcoder cpu_transcoder = crtc->config.cpu_transcoder;
+	enum pipe pipe = crtc->pipe;
 
 	if (INTEL_INFO(dev)->gen >= 4) {
 		int reg = PIPECONF(cpu_transcoder);
@@ -1981,21 +1981,19 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 
 /**
  * intel_disable_pipe - disable a pipe, asserting requirements
- * @dev_priv: i915 private structure
- * @pipe: pipe to disable
- *
- * Disable @pipe, making sure that various hardware specific requirements
- * are met, if applicable, e.g. plane disabled, panel fitter off, etc.
+ * @crtc: crtc whose pipes is to be disabled
  *
- * @pipe should be %PIPE_A or %PIPE_B.
+ * Disable the pipe of @crtc, making sure that various hardware
+ * specific requirements are met, if applicable, e.g. plane
+ * disabled, panel fitter off, etc.
  *
  * Will wait until the pipe has shut down before returning.
  */
-static void intel_disable_pipe(struct drm_i915_private *dev_priv,
-			       enum pipe pipe)
+static void intel_disable_pipe(struct intel_crtc *crtc)
 {
-	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
-								      pipe);
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	enum transcoder cpu_transcoder = crtc->config.cpu_transcoder;
+	enum pipe pipe = crtc->pipe;
 	int reg;
 	u32 val;
 
@@ -2017,7 +2015,7 @@ static void intel_disable_pipe(struct drm_i915_private *dev_priv,
 		return;
 
 	I915_WRITE(reg, val & ~PIPECONF_ENABLE);
-	intel_wait_for_pipe_off(dev_priv->dev, pipe);
+	intel_wait_for_pipe_off(crtc);
 }
 
 /*
@@ -4115,7 +4113,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	if (intel_crtc->config.has_pch_encoder)
 		intel_set_pch_fifo_underrun_reporting(dev, pipe, false);
 
-	intel_disable_pipe(dev_priv, pipe);
+	intel_disable_pipe(intel_crtc);
 
 	if (intel_crtc->config.dp_encoder_is_mst)
 		intel_ddi_set_vc_payload_alloc(crtc, false);
@@ -4167,7 +4165,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
-	int pipe = intel_crtc->pipe;
 	enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
 
 	if (!intel_crtc->active)
@@ -4182,7 +4179,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	if (intel_crtc->config.has_pch_encoder)
 		intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, false);
-	intel_disable_pipe(dev_priv, pipe);
+	intel_disable_pipe(intel_crtc);
 
 	intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
 
@@ -4769,7 +4766,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	 */
 	intel_wait_for_vblank(dev, pipe);
 
-	intel_disable_pipe(dev_priv, pipe);
+	intel_disable_pipe(intel_crtc);
 
 	i9xx_pfit_disable(intel_crtc);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3abc915..6c8303e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -826,7 +826,6 @@ intel_wait_for_vblank(struct drm_device *dev, int pipe)
 {
 	drm_wait_one_vblank(dev, pipe);
 }
-void intel_wait_for_pipe_off(struct drm_device *dev, int pipe);
 int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
 void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
 			 struct intel_digital_port *dport);
-- 
1.8.5.5

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

Re: [PATCH 04/16] drm/i915: Pass intel_crtc to intel_disable_pipe() and intel_wait_for_pipe_off()

[Thomas Richter]

On 15.08.2014 00:21, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Just pass the intel_crtc around instead of dev_priv+pipe.
>
> Also make intel_wait_for_pipe_off() static since it's only used in
> intel_display.c.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++-------------------
>   drivers/gpu/drm/i915/intel_drv.h     |  1 -
>   2 files changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3813526..e7175ce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -913,8 +913,7 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
>
>   /*
>    * intel_wait_for_pipe_off - wait for pipe to turn off
> - * @dev: drm device
> - * @pipe: pipe to wait for
> + * @crtc: crtc whose pipe to wait for
>    *
>    * After disabling a pipe, we can't wait for vblank in the usual way,
>    * spinning on the vblank interrupt status bit, since we won't actually
> @@ -928,11 +927,12 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
>    *   ends up stopping at the start of the next frame).
>    *
>    */
> -void intel_wait_for_pipe_off(struct drm_device *dev, int pipe)
> +static void intel_wait_for_pipe_off(struct intel_crtc *crtc)
>   {
> +	struct drm_device *dev = crtc->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> -								      pipe);
> +	enum transcoder cpu_transcoder = crtc->config.cpu_transcoder;
> +	enum pipe pipe = crtc->pipe;
>
>   	if (INTEL_INFO(dev)->gen >= 4) {
>   		int reg = PIPECONF(cpu_transcoder);
> @@ -1981,21 +1981,19 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>
>   /**
>    * intel_disable_pipe - disable a pipe, asserting requirements
> - * @dev_priv: i915 private structure
> - * @pipe: pipe to disable
> - *
> - * Disable @pipe, making sure that various hardware specific requirements
> - * are met, if applicable, e.g. plane disabled, panel fitter off, etc.
> + * @crtc: crtc whose pipes is to be disabled
>    *
> - * @pipe should be %PIPE_A or %PIPE_B.
> + * Disable the pipe of @crtc, making sure that various hardware
> + * specific requirements are met, if applicable, e.g. plane
> + * disabled, panel fitter off, etc.
>    *
>    * Will wait until the pipe has shut down before returning.
>    */
> -static void intel_disable_pipe(struct drm_i915_private *dev_priv,
> -			       enum pipe pipe)
> +static void intel_disable_pipe(struct intel_crtc *crtc)
>   {
> -	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> -								      pipe);
> +	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +	enum transcoder cpu_transcoder = crtc->config.cpu_transcoder;
> +	enum pipe pipe = crtc->pipe;
>   	int reg;
>   	u32 val;
>
> @@ -2017,7 +2015,7 @@ static void intel_disable_pipe(struct drm_i915_private *dev_priv,
>   		return;
>
>   	I915_WRITE(reg, val & ~PIPECONF_ENABLE);
> -	intel_wait_for_pipe_off(dev_priv->dev, pipe);
> +	intel_wait_for_pipe_off(crtc);
>   }
>
>   /*
> @@ -4115,7 +4113,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>   	if (intel_crtc->config.has_pch_encoder)
>   		intel_set_pch_fifo_underrun_reporting(dev, pipe, false);
>
> -	intel_disable_pipe(dev_priv, pipe);
> +	intel_disable_pipe(intel_crtc);
>
>   	if (intel_crtc->config.dp_encoder_is_mst)
>   		intel_ddi_set_vc_payload_alloc(crtc, false);
> @@ -4167,7 +4165,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	struct intel_encoder *encoder;
> -	int pipe = intel_crtc->pipe;
>   	enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
>
>   	if (!intel_crtc->active)
> @@ -4182,7 +4179,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>
>   	if (intel_crtc->config.has_pch_encoder)
>   		intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, false);
> -	intel_disable_pipe(dev_priv, pipe);
> +	intel_disable_pipe(intel_crtc);
>
>   	intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
>
> @@ -4769,7 +4766,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>   	 */
>   	intel_wait_for_vblank(dev, pipe);
>
> -	intel_disable_pipe(dev_priv, pipe);
> +	intel_disable_pipe(intel_crtc);
>
>   	i9xx_pfit_disable(intel_crtc);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3abc915..6c8303e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -826,7 +826,6 @@ intel_wait_for_vblank(struct drm_device *dev, int pipe)
>   {
>   	drm_wait_one_vblank(dev, pipe);
>   }
> -void intel_wait_for_pipe_off(struct drm_device *dev, int pipe);
>   int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
>   void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
>   			 struct intel_digital_port *dport);
>

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

[PATCH v3 05/16] drm/i915: Disable double wide even when leaving the pipe on

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Disable double wide even if the pipe quirk compels us to leave the
pipe running. Double wide has certain implications for the plane
assignments so best keep it off.

Also helps resuming from S3 on the Fujitsu-Siemens Lifebook S6010
when double wide was enabled prior to suspend.

We do leave the pixel clock ticking at the original rate which would
require double wide to be enabled. But since the planes are all disabled
I'm hoping that the overly fast clock won't cause any problems. Seems
to be fine so far.

v2: Disable double wide also when turning the pipe off
v3: Reorder wrt. force pipe B quirk

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e7175ce..3eeb5ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2005,17 +2005,25 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
 	assert_cursor_disabled(dev_priv, pipe);
 	assert_sprites_disabled(dev_priv, pipe);
 
-	/* Don't disable pipe A or pipe A PLLs if needed */
-	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
-		return;
-
 	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
 	if ((val & PIPECONF_ENABLE) == 0)
 		return;
 
-	I915_WRITE(reg, val & ~PIPECONF_ENABLE);
-	intel_wait_for_pipe_off(crtc);
+	/*
+	 * Double wide has implications for planes
+	 * so best keep it disabled when not needed.
+	 */
+	if (crtc->config.double_wide)
+		val &= ~PIPECONF_DOUBLE_WIDE;
+
+	/* Don't disable pipe or pipe PLLs if needed */
+	if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE))
+		val &= ~PIPECONF_ENABLE;
+
+	I915_WRITE(reg, val);
+	if ((val & PIPECONF_ENABLE) == 0)
+		intel_wait_for_pipe_off(crtc);
 }
 
 /*
-- 
1.8.5.5

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

Re: [PATCH v3 05/16] drm/i915: Disable double wide even when leaving the pipe on

[Thomas Richter]

On 15.08.2014 00:21, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Disable double wide even if the pipe quirk compels us to leave the
> pipe running. Double wide has certain implications for the plane
> assignments so best keep it off.
>
> Also helps resuming from S3 on the Fujitsu-Siemens Lifebook S6010
> when double wide was enabled prior to suspend.
>
> We do leave the pixel clock ticking at the original rate which would
> require double wide to be enabled. But since the planes are all disabled
> I'm hoping that the overly fast clock won't cause any problems. Seems
> to be fine so far.
>
> v2: Disable double wide also when turning the pipe off
> v3: Reorder wrt. force pipe B quirk
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e7175ce..3eeb5ce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2005,17 +2005,25 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>   	assert_cursor_disabled(dev_priv, pipe);
>   	assert_sprites_disabled(dev_priv, pipe);
>
> -	/* Don't disable pipe A or pipe A PLLs if needed */
> -	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
> -		return;
> -
>   	reg = PIPECONF(cpu_transcoder);
>   	val = I915_READ(reg);
>   	if ((val & PIPECONF_ENABLE) == 0)
>   		return;
>
> -	I915_WRITE(reg, val & ~PIPECONF_ENABLE);
> -	intel_wait_for_pipe_off(crtc);
> +	/*
> +	 * Double wide has implications for planes
> +	 * so best keep it disabled when not needed.
> +	 */
> +	if (crtc->config.double_wide)
> +		val &= ~PIPECONF_DOUBLE_WIDE;
> +
> +	/* Don't disable pipe or pipe PLLs if needed */
> +	if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE))
> +		val &= ~PIPECONF_ENABLE;
> +
> +	I915_WRITE(reg, val);
> +	if ((val & PIPECONF_ENABLE) == 0)
> +		intel_wait_for_pipe_off(crtc);
>   }
>
>   /*
>

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

[PATCH 06/16] drm/i915: ns2501 is on DVOB

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On Fujitsu-Siememens S6010 the ns2501 chip is hooked up to DVOB instead
of DVOC.

FIXME: Maybe need to dig out the correct DVO port from VBT

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dvo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 56b47d2..d5ea393 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -85,7 +85,7 @@ static const struct intel_dvo_device intel_dvo_devices[] = {
 	{
 	        .type = INTEL_DVO_CHIP_TMDS,
 		.name = "ns2501",
-		.dvo_reg = DVOC,
+		.dvo_reg = DVOB,
 		.slave_addr = NS2501_ADDR,
 		.dev_ops = &ns2501_ops,
        }
-- 
1.8.5.5

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

Re: [PATCH 06/16] drm/i915: ns2501 is on DVOB

[Thomas Richter]

On 15.08.2014 00:21, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On Fujitsu-Siememens S6010 the ns2501 chip is hooked up to DVOB instead
> of DVOC.
>
> FIXME: Maybe need to dig out the correct DVO port from VBT
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/intel_dvo.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 56b47d2..d5ea393 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -85,7 +85,7 @@ static const struct intel_dvo_device intel_dvo_devices[] = {
>   	{
>   	        .type = INTEL_DVO_CHIP_TMDS,
>   		.name = "ns2501",
> -		.dvo_reg = DVOC,
> +		.dvo_reg = DVOB,
>   		.slave_addr = NS2501_ADDR,
>   		.dev_ops = &ns2501_ops,
>          }
>

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

[PATCH 07/16] drm/i915: Enable DVO between mode_set and dpms hooks

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

To more closely match the IEGD ns2501 driver behaviour, call the
mode_set hook while the DVO port is still disabled, then enable the DVO
port, and finally call the dpms hook.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dvo.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index d5ea393..4f115c1 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -185,12 +185,13 @@ static void intel_enable_dvo(struct intel_encoder *encoder)
 	u32 dvo_reg = intel_dvo->dev.dvo_reg;
 	u32 temp = I915_READ(dvo_reg);
 
-	I915_WRITE(dvo_reg, temp | DVO_ENABLE);
-	I915_READ(dvo_reg);
 	intel_dvo->dev.dev_ops->mode_set(&intel_dvo->dev,
 					 &crtc->config.requested_mode,
 					 &crtc->config.adjusted_mode);
 
+	I915_WRITE(dvo_reg, temp | DVO_ENABLE);
+	I915_READ(dvo_reg);
+
 	intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, true);
 }
 
-- 
1.8.5.5

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

Re: [PATCH 07/16] drm/i915: Enable DVO between mode_set and dpms hooks

[Thomas Richter]

On 15.08.2014 00:21, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> To more closely match the IEGD ns2501 driver behaviour, call the
> mode_set hook while the DVO port is still disabled, then enable the DVO
> port, and finally call the dpms hook.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/intel_dvo.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index d5ea393..4f115c1 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -185,12 +185,13 @@ static void intel_enable_dvo(struct intel_encoder *encoder)
>   	u32 dvo_reg = intel_dvo->dev.dvo_reg;
>   	u32 temp = I915_READ(dvo_reg);
>
> -	I915_WRITE(dvo_reg, temp | DVO_ENABLE);
> -	I915_READ(dvo_reg);
>   	intel_dvo->dev.dev_ops->mode_set(&intel_dvo->dev,
>   					 &crtc->config.requested_mode,
>   					 &crtc->config.adjusted_mode);
>
> +	I915_WRITE(dvo_reg, temp | DVO_ENABLE);
> +	I915_READ(dvo_reg);
> +
>   	intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, true);
>   }
>
>

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

[PATCH 08/16] drm/i915: Don't call DVO mode_set hook on DPMS changes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Calling the mode_set hook on DPMS changes doesn't seem to be necessary
for ns2501. Just drop it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dvo.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 4f115c1..e40e3df 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -227,10 +227,6 @@ static void intel_dvo_dpms(struct drm_connector *connector, int mode)
 
 		intel_crtc_update_dpms(crtc);
 
-		intel_dvo->dev.dev_ops->mode_set(&intel_dvo->dev,
-						 &config->requested_mode,
-						 &config->adjusted_mode);
-
 		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, true);
 	} else {
 		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, false);
-- 
1.8.5.5

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

Re: [PATCH 08/16] drm/i915: Don't call DVO mode_set hook on DPMS changes

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Calling the mode_set hook on DPMS changes doesn't seem to be necessary
> for ns2501. Just drop it.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/intel_dvo.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 4f115c1..e40e3df 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -227,10 +227,6 @@ static void intel_dvo_dpms(struct drm_connector *connector, int mode)
>
>   		intel_crtc_update_dpms(crtc);
>
> -		intel_dvo->dev.dev_ops->mode_set(&intel_dvo->dev,
> -						 &config->requested_mode,
> -						 &config->adjusted_mode);
> -
>   		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, true);
>   	} else {
>   		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, false);
>

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

[PATCH 09/16] drm/i915: Kill useless ns2501_dump_regs

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/dvo_ns2501.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
index 74f2af7..85030d4 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -479,22 +479,6 @@ static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
 	}
 }
 
-static void ns2501_dump_regs(struct intel_dvo_device *dvo)
-{
-	uint8_t val;
-
-	ns2501_readb(dvo, NS2501_FREQ_LO, &val);
-	DRM_DEBUG_KMS("NS2501_FREQ_LO: 0x%02x\n", val);
-	ns2501_readb(dvo, NS2501_FREQ_HI, &val);
-	DRM_DEBUG_KMS("NS2501_FREQ_HI: 0x%02x\n", val);
-	ns2501_readb(dvo, NS2501_REG8, &val);
-	DRM_DEBUG_KMS("NS2501_REG8: 0x%02x\n", val);
-	ns2501_readb(dvo, NS2501_REG9, &val);
-	DRM_DEBUG_KMS("NS2501_REG9: 0x%02x\n", val);
-	ns2501_readb(dvo, NS2501_REGC, &val);
-	DRM_DEBUG_KMS("NS2501_REGC: 0x%02x\n", val);
-}
-
 static void ns2501_destroy(struct intel_dvo_device *dvo)
 {
 	struct ns2501_priv *ns = dvo->dev_priv;
@@ -512,6 +496,5 @@ struct intel_dvo_dev_ops ns2501_ops = {
 	.mode_set = ns2501_mode_set,
 	.dpms = ns2501_dpms,
 	.get_hw_state = ns2501_get_hw_state,
-	.dump_regs = ns2501_dump_regs,
 	.destroy = ns2501_destroy,
 };
-- 
1.8.5.5

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

Re: [PATCH 09/16] drm/i915: Kill useless ns2501_dump_regs

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>

Looks perfectly fine to me.

Signed-off-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/dvo_ns2501.c | 17 -----------------
>   1 file changed, 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index 74f2af7..85030d4 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -479,22 +479,6 @@ static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
>   	}
>   }
>
> -static void ns2501_dump_regs(struct intel_dvo_device *dvo)
> -{
> -	uint8_t val;
> -
> -	ns2501_readb(dvo, NS2501_FREQ_LO, &val);
> -	DRM_DEBUG_KMS("NS2501_FREQ_LO: 0x%02x\n", val);
> -	ns2501_readb(dvo, NS2501_FREQ_HI, &val);
> -	DRM_DEBUG_KMS("NS2501_FREQ_HI: 0x%02x\n", val);
> -	ns2501_readb(dvo, NS2501_REG8, &val);
> -	DRM_DEBUG_KMS("NS2501_REG8: 0x%02x\n", val);
> -	ns2501_readb(dvo, NS2501_REG9, &val);
> -	DRM_DEBUG_KMS("NS2501_REG9: 0x%02x\n", val);
> -	ns2501_readb(dvo, NS2501_REGC, &val);
> -	DRM_DEBUG_KMS("NS2501_REGC: 0x%02x\n", val);
> -}
> -
>   static void ns2501_destroy(struct intel_dvo_device *dvo)
>   {
>   	struct ns2501_priv *ns = dvo->dev_priv;
> @@ -512,6 +496,5 @@ struct intel_dvo_dev_ops ns2501_ops = {
>   	.mode_set = ns2501_mode_set,
>   	.dpms = ns2501_dpms,
>   	.get_hw_state = ns2501_get_hw_state,
> -	.dump_regs = ns2501_dump_regs,
>   	.destroy = ns2501_destroy,
>   };
>

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

Re: [PATCH 09/16] drm/i915: Kill useless ns2501_dump_regs

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/dvo_ns2501.c | 17 -----------------
>   1 file changed, 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index 74f2af7..85030d4 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -479,22 +479,6 @@ static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
>   	}
>   }
>
> -static void ns2501_dump_regs(struct intel_dvo_device *dvo)
> -{
> -	uint8_t val;
> -
> -	ns2501_readb(dvo, NS2501_FREQ_LO, &val);
> -	DRM_DEBUG_KMS("NS2501_FREQ_LO: 0x%02x\n", val);
> -	ns2501_readb(dvo, NS2501_FREQ_HI, &val);
> -	DRM_DEBUG_KMS("NS2501_FREQ_HI: 0x%02x\n", val);
> -	ns2501_readb(dvo, NS2501_REG8, &val);
> -	DRM_DEBUG_KMS("NS2501_REG8: 0x%02x\n", val);
> -	ns2501_readb(dvo, NS2501_REG9, &val);
> -	DRM_DEBUG_KMS("NS2501_REG9: 0x%02x\n", val);
> -	ns2501_readb(dvo, NS2501_REGC, &val);
> -	DRM_DEBUG_KMS("NS2501_REGC: 0x%02x\n", val);
> -}
> -
>   static void ns2501_destroy(struct intel_dvo_device *dvo)
>   {
>   	struct ns2501_priv *ns = dvo->dev_priv;
> @@ -512,6 +496,5 @@ struct intel_dvo_dev_ops ns2501_ops = {
>   	.mode_set = ns2501_mode_set,
>   	.dpms = ns2501_dpms,
>   	.get_hw_state = ns2501_get_hw_state,
> -	.dump_regs = ns2501_dump_regs,
>   	.destroy = ns2501_destroy,
>   };
>

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

[PATCH 10/16] drm/i915: Rewrite ns2501 driver a bit

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Try to use the same programming sequence as used by the IEGD driver.

Also shovel the magic register values into a big static const array.

The register values are actually the based on what the BIOS programs
on the Fujitsu-Siemens Lifebook S6010. IEGD seemed to have hardcoded
register values (which also enabled the scaler for 1024x768 mode).
However those didn't actually work so well on the S6010. Possibly the
pipe timings that got used didn't match the ns2501 configuration.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/dvo_ns2501.c | 529 +++++++++++++++++++++++---------------
 1 file changed, 325 insertions(+), 204 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
index 85030d4..b278571 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -60,16 +60,291 @@
 
 #define NS2501_REGC 0x0c
 
+enum {
+	MODE_640x480,
+	MODE_800x600,
+	MODE_1024x768,
+};
+
+struct ns2501_reg {
+	 uint8_t offset;
+	 uint8_t value;
+};
+
+/*
+ * Magic values based on what the BIOS on
+ * Fujitsu-Siemens Lifebook S6010 programs (1024x768 panel).
+ */
+static const struct ns2501_reg regs_1024x768[][86] = {
+	[MODE_640x480] = {
+		[0] = { .offset = 0x0a, .value = 0x81, },
+		[1] = { .offset = 0x18, .value = 0x07, },
+		[2] = { .offset = 0x19, .value = 0x00, },
+		[3] = { .offset = 0x1a, .value = 0x00, },
+		[4] = { .offset = 0x1b, .value = 0x11, },
+		[5] = { .offset = 0x1c, .value = 0x54, },
+		[6] = { .offset = 0x1d, .value = 0x03, },
+		[7] = { .offset = 0x1e, .value = 0x02, },
+		[8] = { .offset = 0xf3, .value = 0x90, },
+		[9] = { .offset = 0xf9, .value = 0x00, },
+		[10] = { .offset = 0xc1, .value = 0x90, },
+		[11] = { .offset = 0xc2, .value = 0x00, },
+		[12] = { .offset = 0xc3, .value = 0x0f, },
+		[13] = { .offset = 0xc4, .value = 0x03, },
+		[14] = { .offset = 0xc5, .value = 0x16, },
+		[15] = { .offset = 0xc6, .value = 0x00, },
+		[16] = { .offset = 0xc7, .value = 0x02, },
+		[17] = { .offset = 0xc8, .value = 0x02, },
+		[18] = { .offset = 0xf4, .value = 0x00, },
+		[19] = { .offset = 0x80, .value = 0xff, },
+		[20] = { .offset = 0x81, .value = 0x07, },
+		[21] = { .offset = 0x82, .value = 0x3d, },
+		[22] = { .offset = 0x83, .value = 0x05, },
+		[23] = { .offset = 0x94, .value = 0x00, },
+		[24] = { .offset = 0x95, .value = 0x00, },
+		[25] = { .offset = 0x96, .value = 0x05, },
+		[26] = { .offset = 0x97, .value = 0x00, },
+		[27] = { .offset = 0x9a, .value = 0x88, },
+		[28] = { .offset = 0x9b, .value = 0x00, },
+		[29] = { .offset = 0x98, .value = 0x00, },
+		[30] = { .offset = 0x99, .value = 0x00, },
+		[31] = { .offset = 0xf7, .value = 0x88, },
+		[32] = { .offset = 0xf8, .value = 0x0a, },
+		[33] = { .offset = 0x9c, .value = 0x24, },
+		[34] = { .offset = 0x9d, .value = 0x00, },
+		[35] = { .offset = 0x9e, .value = 0x25, },
+		[36] = { .offset = 0x9f, .value = 0x03, },
+		[37] = { .offset = 0xa0, .value = 0x28, },
+		[38] = { .offset = 0xa1, .value = 0x01, },
+		[39] = { .offset = 0xa2, .value = 0x28, },
+		[40] = { .offset = 0xa3, .value = 0x05, },
+		[41] = { .offset = 0xb6, .value = 0x09, },
+		[42] = { .offset = 0xb8, .value = 0x00, },
+		[43] = { .offset = 0xb9, .value = 0xa0, },
+		[44] = { .offset = 0xba, .value = 0x00, },
+		[45] = { .offset = 0xbb, .value = 0x20, },
+		[46] = { .offset = 0x10, .value = 0x00, },
+		[47] = { .offset = 0x11, .value = 0xa0, },
+		[48] = { .offset = 0x12, .value = 0x02, },
+		[49] = { .offset = 0x20, .value = 0x00, },
+		[50] = { .offset = 0x22, .value = 0x00, },
+		[51] = { .offset = 0x23, .value = 0x00, },
+		[52] = { .offset = 0x24, .value = 0x00, },
+		[53] = { .offset = 0x25, .value = 0x00, },
+		[54] = { .offset = 0x8c, .value = 0x10, },
+		[55] = { .offset = 0x8d, .value = 0x02, },
+		[56] = { .offset = 0x8e, .value = 0x10, },
+		[57] = { .offset = 0x8f, .value = 0x00, },
+		[58] = { .offset = 0x90, .value = 0xff, },
+		[59] = { .offset = 0x91, .value = 0x07, },
+		[60] = { .offset = 0x92, .value = 0xa0, },
+		[61] = { .offset = 0x93, .value = 0x02, },
+		[62] = { .offset = 0xa5, .value = 0x00, },
+		[63] = { .offset = 0xa6, .value = 0x00, },
+		[64] = { .offset = 0xa7, .value = 0x00, },
+		[65] = { .offset = 0xa8, .value = 0x00, },
+		[66] = { .offset = 0xa9, .value = 0x04, },
+		[67] = { .offset = 0xaa, .value = 0x70, },
+		[68] = { .offset = 0xab, .value = 0x4f, },
+		[69] = { .offset = 0xac, .value = 0x00, },
+		[70] = { .offset = 0xa4, .value = 0x84, },
+		[71] = { .offset = 0x7e, .value = 0x18, },
+		[72] = { .offset = 0x84, .value = 0x00, },
+		[73] = { .offset = 0x85, .value = 0x00, },
+		[74] = { .offset = 0x86, .value = 0x00, },
+		[75] = { .offset = 0x87, .value = 0x00, },
+		[76] = { .offset = 0x88, .value = 0x00, },
+		[77] = { .offset = 0x89, .value = 0x00, },
+		[78] = { .offset = 0x8a, .value = 0x00, },
+		[79] = { .offset = 0x8b, .value = 0x00, },
+		[80] = { .offset = 0x26, .value = 0x00, },
+		[81] = { .offset = 0x27, .value = 0x00, },
+		[82] = { .offset = 0xad, .value = 0x00, },
+		[83] = { .offset = 0x08, .value = 0x30, }, /* 0x31 */
+		[84] = { .offset = 0x41, .value = 0x00, },
+		[85] = { .offset = 0xc0, .value = 0x05, },
+	},
+	[MODE_800x600] = {
+		[0] = { .offset = 0x0a, .value = 0x81, },
+		[1] = { .offset = 0x18, .value = 0x07, },
+		[2] = { .offset = 0x19, .value = 0x00, },
+		[3] = { .offset = 0x1a, .value = 0x00, },
+		[4] = { .offset = 0x1b, .value = 0x19, },
+		[5] = { .offset = 0x1c, .value = 0x64, },
+		[6] = { .offset = 0x1d, .value = 0x02, },
+		[7] = { .offset = 0x1e, .value = 0x02, },
+		[8] = { .offset = 0xf3, .value = 0x90, },
+		[9] = { .offset = 0xf9, .value = 0x00, },
+		[10] = { .offset = 0xc1, .value = 0xd7, },
+		[11] = { .offset = 0xc2, .value = 0x00, },
+		[12] = { .offset = 0xc3, .value = 0xf8, },
+		[13] = { .offset = 0xc4, .value = 0x03, },
+		[14] = { .offset = 0xc5, .value = 0x1a, },
+		[15] = { .offset = 0xc6, .value = 0x00, },
+		[16] = { .offset = 0xc7, .value = 0x73, },
+		[17] = { .offset = 0xc8, .value = 0x02, },
+		[18] = { .offset = 0xf4, .value = 0x00, },
+		[19] = { .offset = 0x80, .value = 0x27, },
+		[20] = { .offset = 0x81, .value = 0x03, },
+		[21] = { .offset = 0x82, .value = 0x41, },
+		[22] = { .offset = 0x83, .value = 0x05, },
+		[23] = { .offset = 0x94, .value = 0x00, },
+		[24] = { .offset = 0x95, .value = 0x00, },
+		[25] = { .offset = 0x96, .value = 0x05, },
+		[26] = { .offset = 0x97, .value = 0x00, },
+		[27] = { .offset = 0x9a, .value = 0x88, },
+		[28] = { .offset = 0x9b, .value = 0x00, },
+		[29] = { .offset = 0x98, .value = 0x00, },
+		[30] = { .offset = 0x99, .value = 0x00, },
+		[31] = { .offset = 0xf7, .value = 0x88, },
+		[32] = { .offset = 0xf8, .value = 0x06, },
+		[33] = { .offset = 0x9c, .value = 0x23, },
+		[34] = { .offset = 0x9d, .value = 0x00, },
+		[35] = { .offset = 0x9e, .value = 0x25, },
+		[36] = { .offset = 0x9f, .value = 0x03, },
+		[37] = { .offset = 0xa0, .value = 0x28, },
+		[38] = { .offset = 0xa1, .value = 0x01, },
+		[39] = { .offset = 0xa2, .value = 0x28, },
+		[40] = { .offset = 0xa3, .value = 0x05, },
+		[41] = { .offset = 0xb6, .value = 0x09, },
+		[42] = { .offset = 0xb8, .value = 0x30, },
+		[43] = { .offset = 0xb9, .value = 0xc8, },
+		[44] = { .offset = 0xba, .value = 0x00, },
+		[45] = { .offset = 0xbb, .value = 0x20, },
+		[46] = { .offset = 0x10, .value = 0x20, },
+		[47] = { .offset = 0x11, .value = 0xc8, },
+		[48] = { .offset = 0x12, .value = 0x02, },
+		[49] = { .offset = 0x20, .value = 0x00, },
+		[50] = { .offset = 0x22, .value = 0x00, },
+		[51] = { .offset = 0x23, .value = 0x00, },
+		[52] = { .offset = 0x24, .value = 0x00, },
+		[53] = { .offset = 0x25, .value = 0x00, },
+		[54] = { .offset = 0x8c, .value = 0x10, },
+		[55] = { .offset = 0x8d, .value = 0x02, },
+		[56] = { .offset = 0x8e, .value = 0x04, },
+		[57] = { .offset = 0x8f, .value = 0x00, },
+		[58] = { .offset = 0x90, .value = 0xff, },
+		[59] = { .offset = 0x91, .value = 0x07, },
+		[60] = { .offset = 0x92, .value = 0xa0, },
+		[61] = { .offset = 0x93, .value = 0x02, },
+		[62] = { .offset = 0xa5, .value = 0x00, },
+		[63] = { .offset = 0xa6, .value = 0x00, },
+		[64] = { .offset = 0xa7, .value = 0x00, },
+		[65] = { .offset = 0xa8, .value = 0x00, },
+		[66] = { .offset = 0xa9, .value = 0x83, },
+		[67] = { .offset = 0xaa, .value = 0x40, },
+		[68] = { .offset = 0xab, .value = 0x32, },
+		[69] = { .offset = 0xac, .value = 0x00, },
+		[70] = { .offset = 0xa4, .value = 0x80, },
+		[71] = { .offset = 0x7e, .value = 0x18, },
+		[72] = { .offset = 0x84, .value = 0x00, },
+		[73] = { .offset = 0x85, .value = 0x00, },
+		[74] = { .offset = 0x86, .value = 0x00, },
+		[75] = { .offset = 0x87, .value = 0x00, },
+		[76] = { .offset = 0x88, .value = 0x00, },
+		[77] = { .offset = 0x89, .value = 0x00, },
+		[78] = { .offset = 0x8a, .value = 0x00, },
+		[79] = { .offset = 0x8b, .value = 0x00, },
+		[80] = { .offset = 0x26, .value = 0x00, },
+		[81] = { .offset = 0x27, .value = 0x00, },
+		[82] = { .offset = 0xad, .value = 0x00, },
+		[83] = { .offset = 0x08, .value = 0x30, }, /* 0x31 */
+		[84] = { .offset = 0x41, .value = 0x00, },
+		[85] = { .offset = 0xc0, .value = 0x07, },
+	},
+	[MODE_1024x768] = {
+		[0] = { .offset = 0x0a, .value = 0x81, },
+		[1] = { .offset = 0x18, .value = 0x07, },
+		[2] = { .offset = 0x19, .value = 0x00, },
+		[3] = { .offset = 0x1a, .value = 0x00, },
+		[4] = { .offset = 0x1b, .value = 0x11, },
+		[5] = { .offset = 0x1c, .value = 0x54, },
+		[6] = { .offset = 0x1d, .value = 0x03, },
+		[7] = { .offset = 0x1e, .value = 0x02, },
+		[8] = { .offset = 0xf3, .value = 0x90, },
+		[9] = { .offset = 0xf9, .value = 0x00, },
+		[10] = { .offset = 0xc1, .value = 0x90, },
+		[11] = { .offset = 0xc2, .value = 0x00, },
+		[12] = { .offset = 0xc3, .value = 0x0f, },
+		[13] = { .offset = 0xc4, .value = 0x03, },
+		[14] = { .offset = 0xc5, .value = 0x16, },
+		[15] = { .offset = 0xc6, .value = 0x00, },
+		[16] = { .offset = 0xc7, .value = 0x02, },
+		[17] = { .offset = 0xc8, .value = 0x02, },
+		[18] = { .offset = 0xf4, .value = 0x00, },
+		[19] = { .offset = 0x80, .value = 0xff, },
+		[20] = { .offset = 0x81, .value = 0x07, },
+		[21] = { .offset = 0x82, .value = 0x3d, },
+		[22] = { .offset = 0x83, .value = 0x05, },
+		[23] = { .offset = 0x94, .value = 0x00, },
+		[24] = { .offset = 0x95, .value = 0x00, },
+		[25] = { .offset = 0x96, .value = 0x05, },
+		[26] = { .offset = 0x97, .value = 0x00, },
+		[27] = { .offset = 0x9a, .value = 0x88, },
+		[28] = { .offset = 0x9b, .value = 0x00, },
+		[29] = { .offset = 0x98, .value = 0x00, },
+		[30] = { .offset = 0x99, .value = 0x00, },
+		[31] = { .offset = 0xf7, .value = 0x88, },
+		[32] = { .offset = 0xf8, .value = 0x0a, },
+		[33] = { .offset = 0x9c, .value = 0x24, },
+		[34] = { .offset = 0x9d, .value = 0x00, },
+		[35] = { .offset = 0x9e, .value = 0x25, },
+		[36] = { .offset = 0x9f, .value = 0x03, },
+		[37] = { .offset = 0xa0, .value = 0x28, },
+		[38] = { .offset = 0xa1, .value = 0x01, },
+		[39] = { .offset = 0xa2, .value = 0x28, },
+		[40] = { .offset = 0xa3, .value = 0x05, },
+		[41] = { .offset = 0xb6, .value = 0x09, },
+		[42] = { .offset = 0xb8, .value = 0x00, },
+		[43] = { .offset = 0xb9, .value = 0xa0, },
+		[44] = { .offset = 0xba, .value = 0x00, },
+		[45] = { .offset = 0xbb, .value = 0x20, },
+		[46] = { .offset = 0x10, .value = 0x00, },
+		[47] = { .offset = 0x11, .value = 0xa0, },
+		[48] = { .offset = 0x12, .value = 0x02, },
+		[49] = { .offset = 0x20, .value = 0x00, },
+		[50] = { .offset = 0x22, .value = 0x00, },
+		[51] = { .offset = 0x23, .value = 0x00, },
+		[52] = { .offset = 0x24, .value = 0x00, },
+		[53] = { .offset = 0x25, .value = 0x00, },
+		[54] = { .offset = 0x8c, .value = 0x10, },
+		[55] = { .offset = 0x8d, .value = 0x02, },
+		[56] = { .offset = 0x8e, .value = 0x10, },
+		[57] = { .offset = 0x8f, .value = 0x00, },
+		[58] = { .offset = 0x90, .value = 0xff, },
+		[59] = { .offset = 0x91, .value = 0x07, },
+		[60] = { .offset = 0x92, .value = 0xa0, },
+		[61] = { .offset = 0x93, .value = 0x02, },
+		[62] = { .offset = 0xa5, .value = 0x00, },
+		[63] = { .offset = 0xa6, .value = 0x00, },
+		[64] = { .offset = 0xa7, .value = 0x00, },
+		[65] = { .offset = 0xa8, .value = 0x00, },
+		[66] = { .offset = 0xa9, .value = 0x04, },
+		[67] = { .offset = 0xaa, .value = 0x70, },
+		[68] = { .offset = 0xab, .value = 0x4f, },
+		[69] = { .offset = 0xac, .value = 0x00, },
+		[70] = { .offset = 0xa4, .value = 0x84, },
+		[71] = { .offset = 0x7e, .value = 0x18, },
+		[72] = { .offset = 0x84, .value = 0x00, },
+		[73] = { .offset = 0x85, .value = 0x00, },
+		[74] = { .offset = 0x86, .value = 0x00, },
+		[75] = { .offset = 0x87, .value = 0x00, },
+		[76] = { .offset = 0x88, .value = 0x00, },
+		[77] = { .offset = 0x89, .value = 0x00, },
+		[78] = { .offset = 0x8a, .value = 0x00, },
+		[79] = { .offset = 0x8b, .value = 0x00, },
+		[80] = { .offset = 0x26, .value = 0x00, },
+		[81] = { .offset = 0x27, .value = 0x00, },
+		[82] = { .offset = 0xad, .value = 0x00, },
+		[83] = { .offset = 0x08, .value = 0x34, }, /* 0x35 */
+		[84] = { .offset = 0x41, .value = 0x00, },
+		[85] = { .offset = 0xc0, .value = 0x01, },
+	},
+};
+
 struct ns2501_priv {
-	//I2CDevRec d;
 	bool quiet;
-	int reg_8_shadow;
-	int reg_8_set;
-	// Shadow registers for i915
-	int dvoc;
-	int pll_a;
-	int srcdim;
-	int fw_blc;
+	const struct ns2501_reg *regs;
 };
 
 #define NSPTR(d) ((NS2501Ptr)(d->DriverPrivate.ptr))
@@ -205,11 +480,9 @@ static bool ns2501_init(struct intel_dvo_device *dvo,
 		goto out;
 	}
 	ns->quiet = false;
-	ns->reg_8_set = 0;
-	ns->reg_8_shadow =
-	    NS2501_8_PD | NS2501_8_BPAS | NS2501_8_VEN | NS2501_8_HEN;
 
 	DRM_DEBUG_KMS("init ns2501 dvo controller successfully!\n");
+
 	return true;
 
 out:
@@ -255,180 +528,26 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
 			    struct drm_display_mode *mode,
 			    struct drm_display_mode *adjusted_mode)
 {
-	bool ok;
-	int retries = 10;
 	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
+	int mode_idx, i;
 
 	DRM_DEBUG_KMS
 	    ("set mode (hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d).\n",
 	     mode->hdisplay, mode->htotal, mode->vdisplay, mode->vtotal);
 
-	/*
-	 * Where do I find the native resolution for which scaling is not required???
-	 *
-	 * First trigger the DVO on as otherwise the chip does not appear on the i2c
-	 * bus.
-	 */
-	do {
-		ok = true;
-
-		if (mode->hdisplay == 800 && mode->vdisplay == 600) {
-			/* mode 277 */
-			ns->reg_8_shadow &= ~NS2501_8_BPAS;
-			DRM_DEBUG_KMS("switching to 800x600\n");
-
-			/*
-			 * No, I do not know where this data comes from.
-			 * It is just what the video bios left in the DVO, so
-			 * I'm just copying it here over.
-			 * This also means that I cannot support any other modes
-			 * except the ones supported by the bios.
-			 */
-			ok &= ns2501_writeb(dvo, 0x11, 0xc8);	// 0xc7 also works.
-			ok &= ns2501_writeb(dvo, 0x1b, 0x19);
-			ok &= ns2501_writeb(dvo, 0x1c, 0x62);	// VBIOS left 0x64 here, but 0x62 works nicer
-			ok &= ns2501_writeb(dvo, 0x1d, 0x02);
-
-			ok &= ns2501_writeb(dvo, 0x34, 0x03);
-			ok &= ns2501_writeb(dvo, 0x35, 0xff);
-
-			ok &= ns2501_writeb(dvo, 0x80, 0x27);
-			ok &= ns2501_writeb(dvo, 0x81, 0x03);
-			ok &= ns2501_writeb(dvo, 0x82, 0x41);
-			ok &= ns2501_writeb(dvo, 0x83, 0x05);
+	if (mode->hdisplay == 640 && mode->vdisplay == 480)
+		mode_idx = MODE_640x480;
+	else if (mode->hdisplay == 800 && mode->vdisplay == 600)
+		mode_idx = MODE_800x600;
+	else if (mode->hdisplay == 1024 && mode->vdisplay == 768)
+		mode_idx = MODE_1024x768;
+	else
+		return;
 
-			ok &= ns2501_writeb(dvo, 0x8d, 0x02);
-			ok &= ns2501_writeb(dvo, 0x8e, 0x04);
-			ok &= ns2501_writeb(dvo, 0x8f, 0x00);
+	ns->regs = regs_1024x768[mode_idx];
 
-			ok &= ns2501_writeb(dvo, 0x90, 0xfe);	/* vertical. VBIOS left 0xff here, but 0xfe works better */
-			ok &= ns2501_writeb(dvo, 0x91, 0x07);
-			ok &= ns2501_writeb(dvo, 0x94, 0x00);
-			ok &= ns2501_writeb(dvo, 0x95, 0x00);
-
-			ok &= ns2501_writeb(dvo, 0x96, 0x00);
-
-			ok &= ns2501_writeb(dvo, 0x99, 0x00);
-			ok &= ns2501_writeb(dvo, 0x9a, 0x88);
-
-			ok &= ns2501_writeb(dvo, 0x9c, 0x23);	/* Looks like first and last line of the image. */
-			ok &= ns2501_writeb(dvo, 0x9d, 0x00);
-			ok &= ns2501_writeb(dvo, 0x9e, 0x25);
-			ok &= ns2501_writeb(dvo, 0x9f, 0x03);
-
-			ok &= ns2501_writeb(dvo, 0xa4, 0x80);
-
-			ok &= ns2501_writeb(dvo, 0xb6, 0x00);
-
-			ok &= ns2501_writeb(dvo, 0xb9, 0xc8);	/* horizontal? */
-			ok &= ns2501_writeb(dvo, 0xba, 0x00);	/* horizontal? */
-
-			ok &= ns2501_writeb(dvo, 0xc0, 0x05);	/* horizontal? */
-			ok &= ns2501_writeb(dvo, 0xc1, 0xd7);
-
-			ok &= ns2501_writeb(dvo, 0xc2, 0x00);
-			ok &= ns2501_writeb(dvo, 0xc3, 0xf8);
-
-			ok &= ns2501_writeb(dvo, 0xc4, 0x03);
-			ok &= ns2501_writeb(dvo, 0xc5, 0x1a);
-
-			ok &= ns2501_writeb(dvo, 0xc6, 0x00);
-			ok &= ns2501_writeb(dvo, 0xc7, 0x73);
-			ok &= ns2501_writeb(dvo, 0xc8, 0x02);
-
-		} else if (mode->hdisplay == 640 && mode->vdisplay == 480) {
-			/* mode 274 */
-			DRM_DEBUG_KMS("switching to 640x480\n");
-			/*
-			 * No, I do not know where this data comes from.
-			 * It is just what the video bios left in the DVO, so
-			 * I'm just copying it here over.
-			 * This also means that I cannot support any other modes
-			 * except the ones supported by the bios.
-			 */
-			ns->reg_8_shadow &= ~NS2501_8_BPAS;
-
-			ok &= ns2501_writeb(dvo, 0x11, 0xa0);
-			ok &= ns2501_writeb(dvo, 0x1b, 0x11);
-			ok &= ns2501_writeb(dvo, 0x1c, 0x54);
-			ok &= ns2501_writeb(dvo, 0x1d, 0x03);
-
-			ok &= ns2501_writeb(dvo, 0x34, 0x03);
-			ok &= ns2501_writeb(dvo, 0x35, 0xff);
-
-			ok &= ns2501_writeb(dvo, 0x80, 0xff);
-			ok &= ns2501_writeb(dvo, 0x81, 0x07);
-			ok &= ns2501_writeb(dvo, 0x82, 0x3d);
-			ok &= ns2501_writeb(dvo, 0x83, 0x05);
-
-			ok &= ns2501_writeb(dvo, 0x8d, 0x02);
-			ok &= ns2501_writeb(dvo, 0x8e, 0x10);
-			ok &= ns2501_writeb(dvo, 0x8f, 0x00);
-
-			ok &= ns2501_writeb(dvo, 0x90, 0xff);	/* vertical */
-			ok &= ns2501_writeb(dvo, 0x91, 0x07);
-			ok &= ns2501_writeb(dvo, 0x94, 0x00);
-			ok &= ns2501_writeb(dvo, 0x95, 0x00);
-
-			ok &= ns2501_writeb(dvo, 0x96, 0x05);
-
-			ok &= ns2501_writeb(dvo, 0x99, 0x00);
-			ok &= ns2501_writeb(dvo, 0x9a, 0x88);
-
-			ok &= ns2501_writeb(dvo, 0x9c, 0x24);
-			ok &= ns2501_writeb(dvo, 0x9d, 0x00);
-			ok &= ns2501_writeb(dvo, 0x9e, 0x25);
-			ok &= ns2501_writeb(dvo, 0x9f, 0x03);
-
-			ok &= ns2501_writeb(dvo, 0xa4, 0x84);
-
-			ok &= ns2501_writeb(dvo, 0xb6, 0x09);
-
-			ok &= ns2501_writeb(dvo, 0xb9, 0xa0);	/* horizontal? */
-			ok &= ns2501_writeb(dvo, 0xba, 0x00);	/* horizontal? */
-
-			ok &= ns2501_writeb(dvo, 0xc0, 0x05);	/* horizontal? */
-			ok &= ns2501_writeb(dvo, 0xc1, 0x90);
-
-			ok &= ns2501_writeb(dvo, 0xc2, 0x00);
-			ok &= ns2501_writeb(dvo, 0xc3, 0x0f);
-
-			ok &= ns2501_writeb(dvo, 0xc4, 0x03);
-			ok &= ns2501_writeb(dvo, 0xc5, 0x16);
-
-			ok &= ns2501_writeb(dvo, 0xc6, 0x00);
-			ok &= ns2501_writeb(dvo, 0xc7, 0x02);
-			ok &= ns2501_writeb(dvo, 0xc8, 0x02);
-
-		} else if (mode->hdisplay == 1024 && mode->vdisplay == 768) {
-			/* mode 280 */
-			DRM_DEBUG_KMS("switching to 1024x768\n");
-			/*
-			 * This might or might not work, actually. I'm silently
-			 * assuming here that the native panel resolution is
-			 * 1024x768. If not, then this leaves the scaler disabled
-			 * generating a picture that is likely not the expected.
-			 *
-			 * Problem is that I do not know where to take the panel
-			 * dimensions from.
-			 *
-			 * Enable the bypass, scaling not required.
-			 *
-			 * The scaler registers are irrelevant here....
-			 *
-			 */
-			ns->reg_8_shadow |= NS2501_8_BPAS;
-			ok &= ns2501_writeb(dvo, 0x37, 0x44);
-		} else {
-			/*
-			 * Data not known. Bummer!
-			 * Hopefully, the code should not go here
-			 * as mode_OK delivered no other modes.
-			 */
-			ns->reg_8_shadow |= NS2501_8_BPAS;
-		}
-		ok &= ns2501_writeb(dvo, NS2501_REG8, ns->reg_8_shadow);
-	} while (!ok && retries--);
+	for (i = 0; i < 84; i++)
+		ns2501_writeb(dvo, ns->regs[i].offset, ns->regs[i].value);
 }
 
 /* set the NS2501 power state */
@@ -439,43 +558,45 @@ static bool ns2501_get_hw_state(struct intel_dvo_device *dvo)
 	if (!ns2501_readb(dvo, NS2501_REG8, &ch))
 		return false;
 
-	if (ch & NS2501_8_PD)
-		return true;
-	else
-		return false;
+	return ch & NS2501_8_PD;
 }
 
 /* set the NS2501 power state */
 static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
 {
-	bool ok;
-	int retries = 10;
 	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
-	unsigned char ch;
 
 	DRM_DEBUG_KMS("Trying set the dpms of the DVO to %i\n", enable);
 
-	ch = ns->reg_8_shadow;
+	if (enable) {
+		if (WARN_ON(ns->regs[83].offset != 0x08 ||
+			    ns->regs[84].offset != 0x41 ||
+			    ns->regs[85].offset != 0xc0))
+			return;
 
-	if (enable)
-		ch |= NS2501_8_PD;
-	else
-		ch &= ~NS2501_8_PD;
-
-	if (ns->reg_8_set == 0 || ns->reg_8_shadow != ch) {
-		ns->reg_8_set = 1;
-		ns->reg_8_shadow = ch;
-
-		do {
-			ok = true;
-			ok &= ns2501_writeb(dvo, NS2501_REG8, ch);
-			ok &=
-			    ns2501_writeb(dvo, 0x34,
-					  enable ? 0x03 : 0x00);
-			ok &=
-			    ns2501_writeb(dvo, 0x35,
-					  enable ? 0xff : 0x00);
-		} while (!ok && retries--);
+		ns2501_writeb(dvo, 0xc0, ns->regs[85].value | 0x08);
+
+		ns2501_writeb(dvo, 0x41, ns->regs[84].value);
+
+		ns2501_writeb(dvo, 0x34, 0x01);
+		msleep(15);
+
+		ns2501_writeb(dvo, 0x08, 0x35);
+		if (!(ns->regs[83].value & NS2501_8_BPAS))
+			ns2501_writeb(dvo, 0x08, 0x31);
+		msleep(200);
+
+		ns2501_writeb(dvo, 0x34, 0x03);
+
+		ns2501_writeb(dvo, 0xc0, ns->regs[85].value);
+	} else {
+		ns2501_writeb(dvo, 0x34, 0x01);
+		msleep(200);
+
+		ns2501_writeb(dvo, 0x08, 0x34);
+		msleep(15);
+
+		ns2501_writeb(dvo, 0x34, 0x00);
 	}
 }
 
-- 
1.8.5.5

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

Re: [PATCH 10/16] drm/i915: Rewrite ns2501 driver a bit

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Try to use the same programming sequence as used by the IEGD driver.
>
> Also shovel the magic register values into a big static const array.
>
> The register values are actually the based on what the BIOS programs
> on the Fujitsu-Siemens Lifebook S6010. IEGD seemed to have hardcoded
> register values (which also enabled the scaler for 1024x768 mode).
> However those didn't actually work so well on the S6010. Possibly the
> pipe timings that got used didn't match the ns2501 configuration.

Looks fine. Hard to say what these values actually mean, I tried to find 
some reasonable algorithm to derive them, but had little success. The 
current set of values is working for the S6010, though probably not for 
other devices using the same DVO but having a different display size.

Signed-off-by: Thomas Richter <richter@rus.uni-stuttgart.de>




> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/dvo_ns2501.c | 529 +++++++++++++++++++++++---------------
>   1 file changed, 325 insertions(+), 204 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index 85030d4..b278571 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -60,16 +60,291 @@
>
>   #define NS2501_REGC 0x0c
>
> +enum {
> +	MODE_640x480,
> +	MODE_800x600,
> +	MODE_1024x768,
> +};
> +
> +struct ns2501_reg {
> +	 uint8_t offset;
> +	 uint8_t value;
> +};
> +
> +/*
> + * Magic values based on what the BIOS on
> + * Fujitsu-Siemens Lifebook S6010 programs (1024x768 panel).
> + */
> +static const struct ns2501_reg regs_1024x768[][86] = {
> +	[MODE_640x480] = {
> +		[0] = { .offset = 0x0a, .value = 0x81, },
> +		[1] = { .offset = 0x18, .value = 0x07, },
> +		[2] = { .offset = 0x19, .value = 0x00, },
> +		[3] = { .offset = 0x1a, .value = 0x00, },
> +		[4] = { .offset = 0x1b, .value = 0x11, },
> +		[5] = { .offset = 0x1c, .value = 0x54, },
> +		[6] = { .offset = 0x1d, .value = 0x03, },
> +		[7] = { .offset = 0x1e, .value = 0x02, },
> +		[8] = { .offset = 0xf3, .value = 0x90, },
> +		[9] = { .offset = 0xf9, .value = 0x00, },
> +		[10] = { .offset = 0xc1, .value = 0x90, },
> +		[11] = { .offset = 0xc2, .value = 0x00, },
> +		[12] = { .offset = 0xc3, .value = 0x0f, },
> +		[13] = { .offset = 0xc4, .value = 0x03, },
> +		[14] = { .offset = 0xc5, .value = 0x16, },
> +		[15] = { .offset = 0xc6, .value = 0x00, },
> +		[16] = { .offset = 0xc7, .value = 0x02, },
> +		[17] = { .offset = 0xc8, .value = 0x02, },
> +		[18] = { .offset = 0xf4, .value = 0x00, },
> +		[19] = { .offset = 0x80, .value = 0xff, },
> +		[20] = { .offset = 0x81, .value = 0x07, },
> +		[21] = { .offset = 0x82, .value = 0x3d, },
> +		[22] = { .offset = 0x83, .value = 0x05, },
> +		[23] = { .offset = 0x94, .value = 0x00, },
> +		[24] = { .offset = 0x95, .value = 0x00, },
> +		[25] = { .offset = 0x96, .value = 0x05, },
> +		[26] = { .offset = 0x97, .value = 0x00, },
> +		[27] = { .offset = 0x9a, .value = 0x88, },
> +		[28] = { .offset = 0x9b, .value = 0x00, },
> +		[29] = { .offset = 0x98, .value = 0x00, },
> +		[30] = { .offset = 0x99, .value = 0x00, },
> +		[31] = { .offset = 0xf7, .value = 0x88, },
> +		[32] = { .offset = 0xf8, .value = 0x0a, },
> +		[33] = { .offset = 0x9c, .value = 0x24, },
> +		[34] = { .offset = 0x9d, .value = 0x00, },
> +		[35] = { .offset = 0x9e, .value = 0x25, },
> +		[36] = { .offset = 0x9f, .value = 0x03, },
> +		[37] = { .offset = 0xa0, .value = 0x28, },
> +		[38] = { .offset = 0xa1, .value = 0x01, },
> +		[39] = { .offset = 0xa2, .value = 0x28, },
> +		[40] = { .offset = 0xa3, .value = 0x05, },
> +		[41] = { .offset = 0xb6, .value = 0x09, },
> +		[42] = { .offset = 0xb8, .value = 0x00, },
> +		[43] = { .offset = 0xb9, .value = 0xa0, },
> +		[44] = { .offset = 0xba, .value = 0x00, },
> +		[45] = { .offset = 0xbb, .value = 0x20, },
> +		[46] = { .offset = 0x10, .value = 0x00, },
> +		[47] = { .offset = 0x11, .value = 0xa0, },
> +		[48] = { .offset = 0x12, .value = 0x02, },
> +		[49] = { .offset = 0x20, .value = 0x00, },
> +		[50] = { .offset = 0x22, .value = 0x00, },
> +		[51] = { .offset = 0x23, .value = 0x00, },
> +		[52] = { .offset = 0x24, .value = 0x00, },
> +		[53] = { .offset = 0x25, .value = 0x00, },
> +		[54] = { .offset = 0x8c, .value = 0x10, },
> +		[55] = { .offset = 0x8d, .value = 0x02, },
> +		[56] = { .offset = 0x8e, .value = 0x10, },
> +		[57] = { .offset = 0x8f, .value = 0x00, },
> +		[58] = { .offset = 0x90, .value = 0xff, },
> +		[59] = { .offset = 0x91, .value = 0x07, },
> +		[60] = { .offset = 0x92, .value = 0xa0, },
> +		[61] = { .offset = 0x93, .value = 0x02, },
> +		[62] = { .offset = 0xa5, .value = 0x00, },
> +		[63] = { .offset = 0xa6, .value = 0x00, },
> +		[64] = { .offset = 0xa7, .value = 0x00, },
> +		[65] = { .offset = 0xa8, .value = 0x00, },
> +		[66] = { .offset = 0xa9, .value = 0x04, },
> +		[67] = { .offset = 0xaa, .value = 0x70, },
> +		[68] = { .offset = 0xab, .value = 0x4f, },
> +		[69] = { .offset = 0xac, .value = 0x00, },
> +		[70] = { .offset = 0xa4, .value = 0x84, },
> +		[71] = { .offset = 0x7e, .value = 0x18, },
> +		[72] = { .offset = 0x84, .value = 0x00, },
> +		[73] = { .offset = 0x85, .value = 0x00, },
> +		[74] = { .offset = 0x86, .value = 0x00, },
> +		[75] = { .offset = 0x87, .value = 0x00, },
> +		[76] = { .offset = 0x88, .value = 0x00, },
> +		[77] = { .offset = 0x89, .value = 0x00, },
> +		[78] = { .offset = 0x8a, .value = 0x00, },
> +		[79] = { .offset = 0x8b, .value = 0x00, },
> +		[80] = { .offset = 0x26, .value = 0x00, },
> +		[81] = { .offset = 0x27, .value = 0x00, },
> +		[82] = { .offset = 0xad, .value = 0x00, },
> +		[83] = { .offset = 0x08, .value = 0x30, }, /* 0x31 */
> +		[84] = { .offset = 0x41, .value = 0x00, },
> +		[85] = { .offset = 0xc0, .value = 0x05, },
> +	},
> +	[MODE_800x600] = {
> +		[0] = { .offset = 0x0a, .value = 0x81, },
> +		[1] = { .offset = 0x18, .value = 0x07, },
> +		[2] = { .offset = 0x19, .value = 0x00, },
> +		[3] = { .offset = 0x1a, .value = 0x00, },
> +		[4] = { .offset = 0x1b, .value = 0x19, },
> +		[5] = { .offset = 0x1c, .value = 0x64, },
> +		[6] = { .offset = 0x1d, .value = 0x02, },
> +		[7] = { .offset = 0x1e, .value = 0x02, },
> +		[8] = { .offset = 0xf3, .value = 0x90, },
> +		[9] = { .offset = 0xf9, .value = 0x00, },
> +		[10] = { .offset = 0xc1, .value = 0xd7, },
> +		[11] = { .offset = 0xc2, .value = 0x00, },
> +		[12] = { .offset = 0xc3, .value = 0xf8, },
> +		[13] = { .offset = 0xc4, .value = 0x03, },
> +		[14] = { .offset = 0xc5, .value = 0x1a, },
> +		[15] = { .offset = 0xc6, .value = 0x00, },
> +		[16] = { .offset = 0xc7, .value = 0x73, },
> +		[17] = { .offset = 0xc8, .value = 0x02, },
> +		[18] = { .offset = 0xf4, .value = 0x00, },
> +		[19] = { .offset = 0x80, .value = 0x27, },
> +		[20] = { .offset = 0x81, .value = 0x03, },
> +		[21] = { .offset = 0x82, .value = 0x41, },
> +		[22] = { .offset = 0x83, .value = 0x05, },
> +		[23] = { .offset = 0x94, .value = 0x00, },
> +		[24] = { .offset = 0x95, .value = 0x00, },
> +		[25] = { .offset = 0x96, .value = 0x05, },
> +		[26] = { .offset = 0x97, .value = 0x00, },
> +		[27] = { .offset = 0x9a, .value = 0x88, },
> +		[28] = { .offset = 0x9b, .value = 0x00, },
> +		[29] = { .offset = 0x98, .value = 0x00, },
> +		[30] = { .offset = 0x99, .value = 0x00, },
> +		[31] = { .offset = 0xf7, .value = 0x88, },
> +		[32] = { .offset = 0xf8, .value = 0x06, },
> +		[33] = { .offset = 0x9c, .value = 0x23, },
> +		[34] = { .offset = 0x9d, .value = 0x00, },
> +		[35] = { .offset = 0x9e, .value = 0x25, },
> +		[36] = { .offset = 0x9f, .value = 0x03, },
> +		[37] = { .offset = 0xa0, .value = 0x28, },
> +		[38] = { .offset = 0xa1, .value = 0x01, },
> +		[39] = { .offset = 0xa2, .value = 0x28, },
> +		[40] = { .offset = 0xa3, .value = 0x05, },
> +		[41] = { .offset = 0xb6, .value = 0x09, },
> +		[42] = { .offset = 0xb8, .value = 0x30, },
> +		[43] = { .offset = 0xb9, .value = 0xc8, },
> +		[44] = { .offset = 0xba, .value = 0x00, },
> +		[45] = { .offset = 0xbb, .value = 0x20, },
> +		[46] = { .offset = 0x10, .value = 0x20, },
> +		[47] = { .offset = 0x11, .value = 0xc8, },
> +		[48] = { .offset = 0x12, .value = 0x02, },
> +		[49] = { .offset = 0x20, .value = 0x00, },
> +		[50] = { .offset = 0x22, .value = 0x00, },
> +		[51] = { .offset = 0x23, .value = 0x00, },
> +		[52] = { .offset = 0x24, .value = 0x00, },
> +		[53] = { .offset = 0x25, .value = 0x00, },
> +		[54] = { .offset = 0x8c, .value = 0x10, },
> +		[55] = { .offset = 0x8d, .value = 0x02, },
> +		[56] = { .offset = 0x8e, .value = 0x04, },
> +		[57] = { .offset = 0x8f, .value = 0x00, },
> +		[58] = { .offset = 0x90, .value = 0xff, },
> +		[59] = { .offset = 0x91, .value = 0x07, },
> +		[60] = { .offset = 0x92, .value = 0xa0, },
> +		[61] = { .offset = 0x93, .value = 0x02, },
> +		[62] = { .offset = 0xa5, .value = 0x00, },
> +		[63] = { .offset = 0xa6, .value = 0x00, },
> +		[64] = { .offset = 0xa7, .value = 0x00, },
> +		[65] = { .offset = 0xa8, .value = 0x00, },
> +		[66] = { .offset = 0xa9, .value = 0x83, },
> +		[67] = { .offset = 0xaa, .value = 0x40, },
> +		[68] = { .offset = 0xab, .value = 0x32, },
> +		[69] = { .offset = 0xac, .value = 0x00, },
> +		[70] = { .offset = 0xa4, .value = 0x80, },
> +		[71] = { .offset = 0x7e, .value = 0x18, },
> +		[72] = { .offset = 0x84, .value = 0x00, },
> +		[73] = { .offset = 0x85, .value = 0x00, },
> +		[74] = { .offset = 0x86, .value = 0x00, },
> +		[75] = { .offset = 0x87, .value = 0x00, },
> +		[76] = { .offset = 0x88, .value = 0x00, },
> +		[77] = { .offset = 0x89, .value = 0x00, },
> +		[78] = { .offset = 0x8a, .value = 0x00, },
> +		[79] = { .offset = 0x8b, .value = 0x00, },
> +		[80] = { .offset = 0x26, .value = 0x00, },
> +		[81] = { .offset = 0x27, .value = 0x00, },
> +		[82] = { .offset = 0xad, .value = 0x00, },
> +		[83] = { .offset = 0x08, .value = 0x30, }, /* 0x31 */
> +		[84] = { .offset = 0x41, .value = 0x00, },
> +		[85] = { .offset = 0xc0, .value = 0x07, },
> +	},
> +	[MODE_1024x768] = {
> +		[0] = { .offset = 0x0a, .value = 0x81, },
> +		[1] = { .offset = 0x18, .value = 0x07, },
> +		[2] = { .offset = 0x19, .value = 0x00, },
> +		[3] = { .offset = 0x1a, .value = 0x00, },
> +		[4] = { .offset = 0x1b, .value = 0x11, },
> +		[5] = { .offset = 0x1c, .value = 0x54, },
> +		[6] = { .offset = 0x1d, .value = 0x03, },
> +		[7] = { .offset = 0x1e, .value = 0x02, },
> +		[8] = { .offset = 0xf3, .value = 0x90, },
> +		[9] = { .offset = 0xf9, .value = 0x00, },
> +		[10] = { .offset = 0xc1, .value = 0x90, },
> +		[11] = { .offset = 0xc2, .value = 0x00, },
> +		[12] = { .offset = 0xc3, .value = 0x0f, },
> +		[13] = { .offset = 0xc4, .value = 0x03, },
> +		[14] = { .offset = 0xc5, .value = 0x16, },
> +		[15] = { .offset = 0xc6, .value = 0x00, },
> +		[16] = { .offset = 0xc7, .value = 0x02, },
> +		[17] = { .offset = 0xc8, .value = 0x02, },
> +		[18] = { .offset = 0xf4, .value = 0x00, },
> +		[19] = { .offset = 0x80, .value = 0xff, },
> +		[20] = { .offset = 0x81, .value = 0x07, },
> +		[21] = { .offset = 0x82, .value = 0x3d, },
> +		[22] = { .offset = 0x83, .value = 0x05, },
> +		[23] = { .offset = 0x94, .value = 0x00, },
> +		[24] = { .offset = 0x95, .value = 0x00, },
> +		[25] = { .offset = 0x96, .value = 0x05, },
> +		[26] = { .offset = 0x97, .value = 0x00, },
> +		[27] = { .offset = 0x9a, .value = 0x88, },
> +		[28] = { .offset = 0x9b, .value = 0x00, },
> +		[29] = { .offset = 0x98, .value = 0x00, },
> +		[30] = { .offset = 0x99, .value = 0x00, },
> +		[31] = { .offset = 0xf7, .value = 0x88, },
> +		[32] = { .offset = 0xf8, .value = 0x0a, },
> +		[33] = { .offset = 0x9c, .value = 0x24, },
> +		[34] = { .offset = 0x9d, .value = 0x00, },
> +		[35] = { .offset = 0x9e, .value = 0x25, },
> +		[36] = { .offset = 0x9f, .value = 0x03, },
> +		[37] = { .offset = 0xa0, .value = 0x28, },
> +		[38] = { .offset = 0xa1, .value = 0x01, },
> +		[39] = { .offset = 0xa2, .value = 0x28, },
> +		[40] = { .offset = 0xa3, .value = 0x05, },
> +		[41] = { .offset = 0xb6, .value = 0x09, },
> +		[42] = { .offset = 0xb8, .value = 0x00, },
> +		[43] = { .offset = 0xb9, .value = 0xa0, },
> +		[44] = { .offset = 0xba, .value = 0x00, },
> +		[45] = { .offset = 0xbb, .value = 0x20, },
> +		[46] = { .offset = 0x10, .value = 0x00, },
> +		[47] = { .offset = 0x11, .value = 0xa0, },
> +		[48] = { .offset = 0x12, .value = 0x02, },
> +		[49] = { .offset = 0x20, .value = 0x00, },
> +		[50] = { .offset = 0x22, .value = 0x00, },
> +		[51] = { .offset = 0x23, .value = 0x00, },
> +		[52] = { .offset = 0x24, .value = 0x00, },
> +		[53] = { .offset = 0x25, .value = 0x00, },
> +		[54] = { .offset = 0x8c, .value = 0x10, },
> +		[55] = { .offset = 0x8d, .value = 0x02, },
> +		[56] = { .offset = 0x8e, .value = 0x10, },
> +		[57] = { .offset = 0x8f, .value = 0x00, },
> +		[58] = { .offset = 0x90, .value = 0xff, },
> +		[59] = { .offset = 0x91, .value = 0x07, },
> +		[60] = { .offset = 0x92, .value = 0xa0, },
> +		[61] = { .offset = 0x93, .value = 0x02, },
> +		[62] = { .offset = 0xa5, .value = 0x00, },
> +		[63] = { .offset = 0xa6, .value = 0x00, },
> +		[64] = { .offset = 0xa7, .value = 0x00, },
> +		[65] = { .offset = 0xa8, .value = 0x00, },
> +		[66] = { .offset = 0xa9, .value = 0x04, },
> +		[67] = { .offset = 0xaa, .value = 0x70, },
> +		[68] = { .offset = 0xab, .value = 0x4f, },
> +		[69] = { .offset = 0xac, .value = 0x00, },
> +		[70] = { .offset = 0xa4, .value = 0x84, },
> +		[71] = { .offset = 0x7e, .value = 0x18, },
> +		[72] = { .offset = 0x84, .value = 0x00, },
> +		[73] = { .offset = 0x85, .value = 0x00, },
> +		[74] = { .offset = 0x86, .value = 0x00, },
> +		[75] = { .offset = 0x87, .value = 0x00, },
> +		[76] = { .offset = 0x88, .value = 0x00, },
> +		[77] = { .offset = 0x89, .value = 0x00, },
> +		[78] = { .offset = 0x8a, .value = 0x00, },
> +		[79] = { .offset = 0x8b, .value = 0x00, },
> +		[80] = { .offset = 0x26, .value = 0x00, },
> +		[81] = { .offset = 0x27, .value = 0x00, },
> +		[82] = { .offset = 0xad, .value = 0x00, },
> +		[83] = { .offset = 0x08, .value = 0x34, }, /* 0x35 */
> +		[84] = { .offset = 0x41, .value = 0x00, },
> +		[85] = { .offset = 0xc0, .value = 0x01, },
> +	},
> +};
> +
>   struct ns2501_priv {
> -	//I2CDevRec d;
>   	bool quiet;
> -	int reg_8_shadow;
> -	int reg_8_set;
> -	// Shadow registers for i915
> -	int dvoc;
> -	int pll_a;
> -	int srcdim;
> -	int fw_blc;
> +	const struct ns2501_reg *regs;
>   };
>
>   #define NSPTR(d) ((NS2501Ptr)(d->DriverPrivate.ptr))
> @@ -205,11 +480,9 @@ static bool ns2501_init(struct intel_dvo_device *dvo,
>   		goto out;
>   	}
>   	ns->quiet = false;
> -	ns->reg_8_set = 0;
> -	ns->reg_8_shadow =
> -	    NS2501_8_PD | NS2501_8_BPAS | NS2501_8_VEN | NS2501_8_HEN;
>
>   	DRM_DEBUG_KMS("init ns2501 dvo controller successfully!\n");
> +
>   	return true;
>
>   out:
> @@ -255,180 +528,26 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
>   			    struct drm_display_mode *mode,
>   			    struct drm_display_mode *adjusted_mode)
>   {
> -	bool ok;
> -	int retries = 10;
>   	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
> +	int mode_idx, i;
>
>   	DRM_DEBUG_KMS
>   	    ("set mode (hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d).\n",
>   	     mode->hdisplay, mode->htotal, mode->vdisplay, mode->vtotal);
>
> -	/*
> -	 * Where do I find the native resolution for which scaling is not required???
> -	 *
> -	 * First trigger the DVO on as otherwise the chip does not appear on the i2c
> -	 * bus.
> -	 */
> -	do {
> -		ok = true;
> -
> -		if (mode->hdisplay == 800 && mode->vdisplay == 600) {
> -			/* mode 277 */
> -			ns->reg_8_shadow &= ~NS2501_8_BPAS;
> -			DRM_DEBUG_KMS("switching to 800x600\n");
> -
> -			/*
> -			 * No, I do not know where this data comes from.
> -			 * It is just what the video bios left in the DVO, so
> -			 * I'm just copying it here over.
> -			 * This also means that I cannot support any other modes
> -			 * except the ones supported by the bios.
> -			 */
> -			ok &= ns2501_writeb(dvo, 0x11, 0xc8);	// 0xc7 also works.
> -			ok &= ns2501_writeb(dvo, 0x1b, 0x19);
> -			ok &= ns2501_writeb(dvo, 0x1c, 0x62);	// VBIOS left 0x64 here, but 0x62 works nicer
> -			ok &= ns2501_writeb(dvo, 0x1d, 0x02);
> -
> -			ok &= ns2501_writeb(dvo, 0x34, 0x03);
> -			ok &= ns2501_writeb(dvo, 0x35, 0xff);
> -
> -			ok &= ns2501_writeb(dvo, 0x80, 0x27);
> -			ok &= ns2501_writeb(dvo, 0x81, 0x03);
> -			ok &= ns2501_writeb(dvo, 0x82, 0x41);
> -			ok &= ns2501_writeb(dvo, 0x83, 0x05);
> +	if (mode->hdisplay == 640 && mode->vdisplay == 480)
> +		mode_idx = MODE_640x480;
> +	else if (mode->hdisplay == 800 && mode->vdisplay == 600)
> +		mode_idx = MODE_800x600;
> +	else if (mode->hdisplay == 1024 && mode->vdisplay == 768)
> +		mode_idx = MODE_1024x768;
> +	else
> +		return;
>
> -			ok &= ns2501_writeb(dvo, 0x8d, 0x02);
> -			ok &= ns2501_writeb(dvo, 0x8e, 0x04);
> -			ok &= ns2501_writeb(dvo, 0x8f, 0x00);
> +	ns->regs = regs_1024x768[mode_idx];
>
> -			ok &= ns2501_writeb(dvo, 0x90, 0xfe);	/* vertical. VBIOS left 0xff here, but 0xfe works better */
> -			ok &= ns2501_writeb(dvo, 0x91, 0x07);
> -			ok &= ns2501_writeb(dvo, 0x94, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x95, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0x96, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0x99, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x9a, 0x88);
> -
> -			ok &= ns2501_writeb(dvo, 0x9c, 0x23);	/* Looks like first and last line of the image. */
> -			ok &= ns2501_writeb(dvo, 0x9d, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x9e, 0x25);
> -			ok &= ns2501_writeb(dvo, 0x9f, 0x03);
> -
> -			ok &= ns2501_writeb(dvo, 0xa4, 0x80);
> -
> -			ok &= ns2501_writeb(dvo, 0xb6, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0xb9, 0xc8);	/* horizontal? */
> -			ok &= ns2501_writeb(dvo, 0xba, 0x00);	/* horizontal? */
> -
> -			ok &= ns2501_writeb(dvo, 0xc0, 0x05);	/* horizontal? */
> -			ok &= ns2501_writeb(dvo, 0xc1, 0xd7);
> -
> -			ok &= ns2501_writeb(dvo, 0xc2, 0x00);
> -			ok &= ns2501_writeb(dvo, 0xc3, 0xf8);
> -
> -			ok &= ns2501_writeb(dvo, 0xc4, 0x03);
> -			ok &= ns2501_writeb(dvo, 0xc5, 0x1a);
> -
> -			ok &= ns2501_writeb(dvo, 0xc6, 0x00);
> -			ok &= ns2501_writeb(dvo, 0xc7, 0x73);
> -			ok &= ns2501_writeb(dvo, 0xc8, 0x02);
> -
> -		} else if (mode->hdisplay == 640 && mode->vdisplay == 480) {
> -			/* mode 274 */
> -			DRM_DEBUG_KMS("switching to 640x480\n");
> -			/*
> -			 * No, I do not know where this data comes from.
> -			 * It is just what the video bios left in the DVO, so
> -			 * I'm just copying it here over.
> -			 * This also means that I cannot support any other modes
> -			 * except the ones supported by the bios.
> -			 */
> -			ns->reg_8_shadow &= ~NS2501_8_BPAS;
> -
> -			ok &= ns2501_writeb(dvo, 0x11, 0xa0);
> -			ok &= ns2501_writeb(dvo, 0x1b, 0x11);
> -			ok &= ns2501_writeb(dvo, 0x1c, 0x54);
> -			ok &= ns2501_writeb(dvo, 0x1d, 0x03);
> -
> -			ok &= ns2501_writeb(dvo, 0x34, 0x03);
> -			ok &= ns2501_writeb(dvo, 0x35, 0xff);
> -
> -			ok &= ns2501_writeb(dvo, 0x80, 0xff);
> -			ok &= ns2501_writeb(dvo, 0x81, 0x07);
> -			ok &= ns2501_writeb(dvo, 0x82, 0x3d);
> -			ok &= ns2501_writeb(dvo, 0x83, 0x05);
> -
> -			ok &= ns2501_writeb(dvo, 0x8d, 0x02);
> -			ok &= ns2501_writeb(dvo, 0x8e, 0x10);
> -			ok &= ns2501_writeb(dvo, 0x8f, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0x90, 0xff);	/* vertical */
> -			ok &= ns2501_writeb(dvo, 0x91, 0x07);
> -			ok &= ns2501_writeb(dvo, 0x94, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x95, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0x96, 0x05);
> -
> -			ok &= ns2501_writeb(dvo, 0x99, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x9a, 0x88);
> -
> -			ok &= ns2501_writeb(dvo, 0x9c, 0x24);
> -			ok &= ns2501_writeb(dvo, 0x9d, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x9e, 0x25);
> -			ok &= ns2501_writeb(dvo, 0x9f, 0x03);
> -
> -			ok &= ns2501_writeb(dvo, 0xa4, 0x84);
> -
> -			ok &= ns2501_writeb(dvo, 0xb6, 0x09);
> -
> -			ok &= ns2501_writeb(dvo, 0xb9, 0xa0);	/* horizontal? */
> -			ok &= ns2501_writeb(dvo, 0xba, 0x00);	/* horizontal? */
> -
> -			ok &= ns2501_writeb(dvo, 0xc0, 0x05);	/* horizontal? */
> -			ok &= ns2501_writeb(dvo, 0xc1, 0x90);
> -
> -			ok &= ns2501_writeb(dvo, 0xc2, 0x00);
> -			ok &= ns2501_writeb(dvo, 0xc3, 0x0f);
> -
> -			ok &= ns2501_writeb(dvo, 0xc4, 0x03);
> -			ok &= ns2501_writeb(dvo, 0xc5, 0x16);
> -
> -			ok &= ns2501_writeb(dvo, 0xc6, 0x00);
> -			ok &= ns2501_writeb(dvo, 0xc7, 0x02);
> -			ok &= ns2501_writeb(dvo, 0xc8, 0x02);
> -
> -		} else if (mode->hdisplay == 1024 && mode->vdisplay == 768) {
> -			/* mode 280 */
> -			DRM_DEBUG_KMS("switching to 1024x768\n");
> -			/*
> -			 * This might or might not work, actually. I'm silently
> -			 * assuming here that the native panel resolution is
> -			 * 1024x768. If not, then this leaves the scaler disabled
> -			 * generating a picture that is likely not the expected.
> -			 *
> -			 * Problem is that I do not know where to take the panel
> -			 * dimensions from.
> -			 *
> -			 * Enable the bypass, scaling not required.
> -			 *
> -			 * The scaler registers are irrelevant here....
> -			 *
> -			 */
> -			ns->reg_8_shadow |= NS2501_8_BPAS;
> -			ok &= ns2501_writeb(dvo, 0x37, 0x44);
> -		} else {
> -			/*
> -			 * Data not known. Bummer!
> -			 * Hopefully, the code should not go here
> -			 * as mode_OK delivered no other modes.
> -			 */
> -			ns->reg_8_shadow |= NS2501_8_BPAS;
> -		}
> -		ok &= ns2501_writeb(dvo, NS2501_REG8, ns->reg_8_shadow);
> -	} while (!ok && retries--);
> +	for (i = 0; i < 84; i++)
> +		ns2501_writeb(dvo, ns->regs[i].offset, ns->regs[i].value);
>   }
>
>   /* set the NS2501 power state */
> @@ -439,43 +558,45 @@ static bool ns2501_get_hw_state(struct intel_dvo_device *dvo)
>   	if (!ns2501_readb(dvo, NS2501_REG8, &ch))
>   		return false;
>
> -	if (ch & NS2501_8_PD)
> -		return true;
> -	else
> -		return false;
> +	return ch & NS2501_8_PD;
>   }
>
>   /* set the NS2501 power state */
>   static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
>   {
> -	bool ok;
> -	int retries = 10;
>   	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
> -	unsigned char ch;
>
>   	DRM_DEBUG_KMS("Trying set the dpms of the DVO to %i\n", enable);
>
> -	ch = ns->reg_8_shadow;
> +	if (enable) {
> +		if (WARN_ON(ns->regs[83].offset != 0x08 ||
> +			    ns->regs[84].offset != 0x41 ||
> +			    ns->regs[85].offset != 0xc0))
> +			return;
>
> -	if (enable)
> -		ch |= NS2501_8_PD;
> -	else
> -		ch &= ~NS2501_8_PD;
> -
> -	if (ns->reg_8_set == 0 || ns->reg_8_shadow != ch) {
> -		ns->reg_8_set = 1;
> -		ns->reg_8_shadow = ch;
> -
> -		do {
> -			ok = true;
> -			ok &= ns2501_writeb(dvo, NS2501_REG8, ch);
> -			ok &=
> -			    ns2501_writeb(dvo, 0x34,
> -					  enable ? 0x03 : 0x00);
> -			ok &=
> -			    ns2501_writeb(dvo, 0x35,
> -					  enable ? 0xff : 0x00);
> -		} while (!ok && retries--);
> +		ns2501_writeb(dvo, 0xc0, ns->regs[85].value | 0x08);
> +
> +		ns2501_writeb(dvo, 0x41, ns->regs[84].value);
> +
> +		ns2501_writeb(dvo, 0x34, 0x01);
> +		msleep(15);
> +
> +		ns2501_writeb(dvo, 0x08, 0x35);
> +		if (!(ns->regs[83].value & NS2501_8_BPAS))
> +			ns2501_writeb(dvo, 0x08, 0x31);
> +		msleep(200);
> +
> +		ns2501_writeb(dvo, 0x34, 0x03);
> +
> +		ns2501_writeb(dvo, 0xc0, ns->regs[85].value);
> +	} else {
> +		ns2501_writeb(dvo, 0x34, 0x01);
> +		msleep(200);
> +
> +		ns2501_writeb(dvo, 0x08, 0x34);
> +		msleep(15);
> +
> +		ns2501_writeb(dvo, 0x34, 0x00);
>   	}
>   }
>
>

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

Re: [PATCH 10/16] drm/i915: Rewrite ns2501 driver a bit

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Try to use the same programming sequence as used by the IEGD driver.
>
> Also shovel the magic register values into a big static const array.
>
> The register values are actually the based on what the BIOS programs
> on the Fujitsu-Siemens Lifebook S6010. IEGD seemed to have hardcoded
> register values (which also enabled the scaler for 1024x768 mode).
> However those didn't actually work so well on the S6010. Possibly the
> pipe timings that got used didn't match the ns2501 configuration.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/dvo_ns2501.c | 529 +++++++++++++++++++++++---------------
>   1 file changed, 325 insertions(+), 204 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index 85030d4..b278571 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -60,16 +60,291 @@
>
>   #define NS2501_REGC 0x0c
>
> +enum {
> +	MODE_640x480,
> +	MODE_800x600,
> +	MODE_1024x768,
> +};
> +
> +struct ns2501_reg {
> +	 uint8_t offset;
> +	 uint8_t value;
> +};
> +
> +/*
> + * Magic values based on what the BIOS on
> + * Fujitsu-Siemens Lifebook S6010 programs (1024x768 panel).
> + */
> +static const struct ns2501_reg regs_1024x768[][86] = {
> +	[MODE_640x480] = {
> +		[0] = { .offset = 0x0a, .value = 0x81, },
> +		[1] = { .offset = 0x18, .value = 0x07, },
> +		[2] = { .offset = 0x19, .value = 0x00, },
> +		[3] = { .offset = 0x1a, .value = 0x00, },
> +		[4] = { .offset = 0x1b, .value = 0x11, },
> +		[5] = { .offset = 0x1c, .value = 0x54, },
> +		[6] = { .offset = 0x1d, .value = 0x03, },
> +		[7] = { .offset = 0x1e, .value = 0x02, },
> +		[8] = { .offset = 0xf3, .value = 0x90, },
> +		[9] = { .offset = 0xf9, .value = 0x00, },
> +		[10] = { .offset = 0xc1, .value = 0x90, },
> +		[11] = { .offset = 0xc2, .value = 0x00, },
> +		[12] = { .offset = 0xc3, .value = 0x0f, },
> +		[13] = { .offset = 0xc4, .value = 0x03, },
> +		[14] = { .offset = 0xc5, .value = 0x16, },
> +		[15] = { .offset = 0xc6, .value = 0x00, },
> +		[16] = { .offset = 0xc7, .value = 0x02, },
> +		[17] = { .offset = 0xc8, .value = 0x02, },
> +		[18] = { .offset = 0xf4, .value = 0x00, },
> +		[19] = { .offset = 0x80, .value = 0xff, },
> +		[20] = { .offset = 0x81, .value = 0x07, },
> +		[21] = { .offset = 0x82, .value = 0x3d, },
> +		[22] = { .offset = 0x83, .value = 0x05, },
> +		[23] = { .offset = 0x94, .value = 0x00, },
> +		[24] = { .offset = 0x95, .value = 0x00, },
> +		[25] = { .offset = 0x96, .value = 0x05, },
> +		[26] = { .offset = 0x97, .value = 0x00, },
> +		[27] = { .offset = 0x9a, .value = 0x88, },
> +		[28] = { .offset = 0x9b, .value = 0x00, },
> +		[29] = { .offset = 0x98, .value = 0x00, },
> +		[30] = { .offset = 0x99, .value = 0x00, },
> +		[31] = { .offset = 0xf7, .value = 0x88, },
> +		[32] = { .offset = 0xf8, .value = 0x0a, },
> +		[33] = { .offset = 0x9c, .value = 0x24, },
> +		[34] = { .offset = 0x9d, .value = 0x00, },
> +		[35] = { .offset = 0x9e, .value = 0x25, },
> +		[36] = { .offset = 0x9f, .value = 0x03, },
> +		[37] = { .offset = 0xa0, .value = 0x28, },
> +		[38] = { .offset = 0xa1, .value = 0x01, },
> +		[39] = { .offset = 0xa2, .value = 0x28, },
> +		[40] = { .offset = 0xa3, .value = 0x05, },
> +		[41] = { .offset = 0xb6, .value = 0x09, },
> +		[42] = { .offset = 0xb8, .value = 0x00, },
> +		[43] = { .offset = 0xb9, .value = 0xa0, },
> +		[44] = { .offset = 0xba, .value = 0x00, },
> +		[45] = { .offset = 0xbb, .value = 0x20, },
> +		[46] = { .offset = 0x10, .value = 0x00, },
> +		[47] = { .offset = 0x11, .value = 0xa0, },
> +		[48] = { .offset = 0x12, .value = 0x02, },
> +		[49] = { .offset = 0x20, .value = 0x00, },
> +		[50] = { .offset = 0x22, .value = 0x00, },
> +		[51] = { .offset = 0x23, .value = 0x00, },
> +		[52] = { .offset = 0x24, .value = 0x00, },
> +		[53] = { .offset = 0x25, .value = 0x00, },
> +		[54] = { .offset = 0x8c, .value = 0x10, },
> +		[55] = { .offset = 0x8d, .value = 0x02, },
> +		[56] = { .offset = 0x8e, .value = 0x10, },
> +		[57] = { .offset = 0x8f, .value = 0x00, },
> +		[58] = { .offset = 0x90, .value = 0xff, },
> +		[59] = { .offset = 0x91, .value = 0x07, },
> +		[60] = { .offset = 0x92, .value = 0xa0, },
> +		[61] = { .offset = 0x93, .value = 0x02, },
> +		[62] = { .offset = 0xa5, .value = 0x00, },
> +		[63] = { .offset = 0xa6, .value = 0x00, },
> +		[64] = { .offset = 0xa7, .value = 0x00, },
> +		[65] = { .offset = 0xa8, .value = 0x00, },
> +		[66] = { .offset = 0xa9, .value = 0x04, },
> +		[67] = { .offset = 0xaa, .value = 0x70, },
> +		[68] = { .offset = 0xab, .value = 0x4f, },
> +		[69] = { .offset = 0xac, .value = 0x00, },
> +		[70] = { .offset = 0xa4, .value = 0x84, },
> +		[71] = { .offset = 0x7e, .value = 0x18, },
> +		[72] = { .offset = 0x84, .value = 0x00, },
> +		[73] = { .offset = 0x85, .value = 0x00, },
> +		[74] = { .offset = 0x86, .value = 0x00, },
> +		[75] = { .offset = 0x87, .value = 0x00, },
> +		[76] = { .offset = 0x88, .value = 0x00, },
> +		[77] = { .offset = 0x89, .value = 0x00, },
> +		[78] = { .offset = 0x8a, .value = 0x00, },
> +		[79] = { .offset = 0x8b, .value = 0x00, },
> +		[80] = { .offset = 0x26, .value = 0x00, },
> +		[81] = { .offset = 0x27, .value = 0x00, },
> +		[82] = { .offset = 0xad, .value = 0x00, },
> +		[83] = { .offset = 0x08, .value = 0x30, }, /* 0x31 */
> +		[84] = { .offset = 0x41, .value = 0x00, },
> +		[85] = { .offset = 0xc0, .value = 0x05, },
> +	},
> +	[MODE_800x600] = {
> +		[0] = { .offset = 0x0a, .value = 0x81, },
> +		[1] = { .offset = 0x18, .value = 0x07, },
> +		[2] = { .offset = 0x19, .value = 0x00, },
> +		[3] = { .offset = 0x1a, .value = 0x00, },
> +		[4] = { .offset = 0x1b, .value = 0x19, },
> +		[5] = { .offset = 0x1c, .value = 0x64, },
> +		[6] = { .offset = 0x1d, .value = 0x02, },
> +		[7] = { .offset = 0x1e, .value = 0x02, },
> +		[8] = { .offset = 0xf3, .value = 0x90, },
> +		[9] = { .offset = 0xf9, .value = 0x00, },
> +		[10] = { .offset = 0xc1, .value = 0xd7, },
> +		[11] = { .offset = 0xc2, .value = 0x00, },
> +		[12] = { .offset = 0xc3, .value = 0xf8, },
> +		[13] = { .offset = 0xc4, .value = 0x03, },
> +		[14] = { .offset = 0xc5, .value = 0x1a, },
> +		[15] = { .offset = 0xc6, .value = 0x00, },
> +		[16] = { .offset = 0xc7, .value = 0x73, },
> +		[17] = { .offset = 0xc8, .value = 0x02, },
> +		[18] = { .offset = 0xf4, .value = 0x00, },
> +		[19] = { .offset = 0x80, .value = 0x27, },
> +		[20] = { .offset = 0x81, .value = 0x03, },
> +		[21] = { .offset = 0x82, .value = 0x41, },
> +		[22] = { .offset = 0x83, .value = 0x05, },
> +		[23] = { .offset = 0x94, .value = 0x00, },
> +		[24] = { .offset = 0x95, .value = 0x00, },
> +		[25] = { .offset = 0x96, .value = 0x05, },
> +		[26] = { .offset = 0x97, .value = 0x00, },
> +		[27] = { .offset = 0x9a, .value = 0x88, },
> +		[28] = { .offset = 0x9b, .value = 0x00, },
> +		[29] = { .offset = 0x98, .value = 0x00, },
> +		[30] = { .offset = 0x99, .value = 0x00, },
> +		[31] = { .offset = 0xf7, .value = 0x88, },
> +		[32] = { .offset = 0xf8, .value = 0x06, },
> +		[33] = { .offset = 0x9c, .value = 0x23, },
> +		[34] = { .offset = 0x9d, .value = 0x00, },
> +		[35] = { .offset = 0x9e, .value = 0x25, },
> +		[36] = { .offset = 0x9f, .value = 0x03, },
> +		[37] = { .offset = 0xa0, .value = 0x28, },
> +		[38] = { .offset = 0xa1, .value = 0x01, },
> +		[39] = { .offset = 0xa2, .value = 0x28, },
> +		[40] = { .offset = 0xa3, .value = 0x05, },
> +		[41] = { .offset = 0xb6, .value = 0x09, },
> +		[42] = { .offset = 0xb8, .value = 0x30, },
> +		[43] = { .offset = 0xb9, .value = 0xc8, },
> +		[44] = { .offset = 0xba, .value = 0x00, },
> +		[45] = { .offset = 0xbb, .value = 0x20, },
> +		[46] = { .offset = 0x10, .value = 0x20, },
> +		[47] = { .offset = 0x11, .value = 0xc8, },
> +		[48] = { .offset = 0x12, .value = 0x02, },
> +		[49] = { .offset = 0x20, .value = 0x00, },
> +		[50] = { .offset = 0x22, .value = 0x00, },
> +		[51] = { .offset = 0x23, .value = 0x00, },
> +		[52] = { .offset = 0x24, .value = 0x00, },
> +		[53] = { .offset = 0x25, .value = 0x00, },
> +		[54] = { .offset = 0x8c, .value = 0x10, },
> +		[55] = { .offset = 0x8d, .value = 0x02, },
> +		[56] = { .offset = 0x8e, .value = 0x04, },
> +		[57] = { .offset = 0x8f, .value = 0x00, },
> +		[58] = { .offset = 0x90, .value = 0xff, },
> +		[59] = { .offset = 0x91, .value = 0x07, },
> +		[60] = { .offset = 0x92, .value = 0xa0, },
> +		[61] = { .offset = 0x93, .value = 0x02, },
> +		[62] = { .offset = 0xa5, .value = 0x00, },
> +		[63] = { .offset = 0xa6, .value = 0x00, },
> +		[64] = { .offset = 0xa7, .value = 0x00, },
> +		[65] = { .offset = 0xa8, .value = 0x00, },
> +		[66] = { .offset = 0xa9, .value = 0x83, },
> +		[67] = { .offset = 0xaa, .value = 0x40, },
> +		[68] = { .offset = 0xab, .value = 0x32, },
> +		[69] = { .offset = 0xac, .value = 0x00, },
> +		[70] = { .offset = 0xa4, .value = 0x80, },
> +		[71] = { .offset = 0x7e, .value = 0x18, },
> +		[72] = { .offset = 0x84, .value = 0x00, },
> +		[73] = { .offset = 0x85, .value = 0x00, },
> +		[74] = { .offset = 0x86, .value = 0x00, },
> +		[75] = { .offset = 0x87, .value = 0x00, },
> +		[76] = { .offset = 0x88, .value = 0x00, },
> +		[77] = { .offset = 0x89, .value = 0x00, },
> +		[78] = { .offset = 0x8a, .value = 0x00, },
> +		[79] = { .offset = 0x8b, .value = 0x00, },
> +		[80] = { .offset = 0x26, .value = 0x00, },
> +		[81] = { .offset = 0x27, .value = 0x00, },
> +		[82] = { .offset = 0xad, .value = 0x00, },
> +		[83] = { .offset = 0x08, .value = 0x30, }, /* 0x31 */
> +		[84] = { .offset = 0x41, .value = 0x00, },
> +		[85] = { .offset = 0xc0, .value = 0x07, },
> +	},
> +	[MODE_1024x768] = {
> +		[0] = { .offset = 0x0a, .value = 0x81, },
> +		[1] = { .offset = 0x18, .value = 0x07, },
> +		[2] = { .offset = 0x19, .value = 0x00, },
> +		[3] = { .offset = 0x1a, .value = 0x00, },
> +		[4] = { .offset = 0x1b, .value = 0x11, },
> +		[5] = { .offset = 0x1c, .value = 0x54, },
> +		[6] = { .offset = 0x1d, .value = 0x03, },
> +		[7] = { .offset = 0x1e, .value = 0x02, },
> +		[8] = { .offset = 0xf3, .value = 0x90, },
> +		[9] = { .offset = 0xf9, .value = 0x00, },
> +		[10] = { .offset = 0xc1, .value = 0x90, },
> +		[11] = { .offset = 0xc2, .value = 0x00, },
> +		[12] = { .offset = 0xc3, .value = 0x0f, },
> +		[13] = { .offset = 0xc4, .value = 0x03, },
> +		[14] = { .offset = 0xc5, .value = 0x16, },
> +		[15] = { .offset = 0xc6, .value = 0x00, },
> +		[16] = { .offset = 0xc7, .value = 0x02, },
> +		[17] = { .offset = 0xc8, .value = 0x02, },
> +		[18] = { .offset = 0xf4, .value = 0x00, },
> +		[19] = { .offset = 0x80, .value = 0xff, },
> +		[20] = { .offset = 0x81, .value = 0x07, },
> +		[21] = { .offset = 0x82, .value = 0x3d, },
> +		[22] = { .offset = 0x83, .value = 0x05, },
> +		[23] = { .offset = 0x94, .value = 0x00, },
> +		[24] = { .offset = 0x95, .value = 0x00, },
> +		[25] = { .offset = 0x96, .value = 0x05, },
> +		[26] = { .offset = 0x97, .value = 0x00, },
> +		[27] = { .offset = 0x9a, .value = 0x88, },
> +		[28] = { .offset = 0x9b, .value = 0x00, },
> +		[29] = { .offset = 0x98, .value = 0x00, },
> +		[30] = { .offset = 0x99, .value = 0x00, },
> +		[31] = { .offset = 0xf7, .value = 0x88, },
> +		[32] = { .offset = 0xf8, .value = 0x0a, },
> +		[33] = { .offset = 0x9c, .value = 0x24, },
> +		[34] = { .offset = 0x9d, .value = 0x00, },
> +		[35] = { .offset = 0x9e, .value = 0x25, },
> +		[36] = { .offset = 0x9f, .value = 0x03, },
> +		[37] = { .offset = 0xa0, .value = 0x28, },
> +		[38] = { .offset = 0xa1, .value = 0x01, },
> +		[39] = { .offset = 0xa2, .value = 0x28, },
> +		[40] = { .offset = 0xa3, .value = 0x05, },
> +		[41] = { .offset = 0xb6, .value = 0x09, },
> +		[42] = { .offset = 0xb8, .value = 0x00, },
> +		[43] = { .offset = 0xb9, .value = 0xa0, },
> +		[44] = { .offset = 0xba, .value = 0x00, },
> +		[45] = { .offset = 0xbb, .value = 0x20, },
> +		[46] = { .offset = 0x10, .value = 0x00, },
> +		[47] = { .offset = 0x11, .value = 0xa0, },
> +		[48] = { .offset = 0x12, .value = 0x02, },
> +		[49] = { .offset = 0x20, .value = 0x00, },
> +		[50] = { .offset = 0x22, .value = 0x00, },
> +		[51] = { .offset = 0x23, .value = 0x00, },
> +		[52] = { .offset = 0x24, .value = 0x00, },
> +		[53] = { .offset = 0x25, .value = 0x00, },
> +		[54] = { .offset = 0x8c, .value = 0x10, },
> +		[55] = { .offset = 0x8d, .value = 0x02, },
> +		[56] = { .offset = 0x8e, .value = 0x10, },
> +		[57] = { .offset = 0x8f, .value = 0x00, },
> +		[58] = { .offset = 0x90, .value = 0xff, },
> +		[59] = { .offset = 0x91, .value = 0x07, },
> +		[60] = { .offset = 0x92, .value = 0xa0, },
> +		[61] = { .offset = 0x93, .value = 0x02, },
> +		[62] = { .offset = 0xa5, .value = 0x00, },
> +		[63] = { .offset = 0xa6, .value = 0x00, },
> +		[64] = { .offset = 0xa7, .value = 0x00, },
> +		[65] = { .offset = 0xa8, .value = 0x00, },
> +		[66] = { .offset = 0xa9, .value = 0x04, },
> +		[67] = { .offset = 0xaa, .value = 0x70, },
> +		[68] = { .offset = 0xab, .value = 0x4f, },
> +		[69] = { .offset = 0xac, .value = 0x00, },
> +		[70] = { .offset = 0xa4, .value = 0x84, },
> +		[71] = { .offset = 0x7e, .value = 0x18, },
> +		[72] = { .offset = 0x84, .value = 0x00, },
> +		[73] = { .offset = 0x85, .value = 0x00, },
> +		[74] = { .offset = 0x86, .value = 0x00, },
> +		[75] = { .offset = 0x87, .value = 0x00, },
> +		[76] = { .offset = 0x88, .value = 0x00, },
> +		[77] = { .offset = 0x89, .value = 0x00, },
> +		[78] = { .offset = 0x8a, .value = 0x00, },
> +		[79] = { .offset = 0x8b, .value = 0x00, },
> +		[80] = { .offset = 0x26, .value = 0x00, },
> +		[81] = { .offset = 0x27, .value = 0x00, },
> +		[82] = { .offset = 0xad, .value = 0x00, },
> +		[83] = { .offset = 0x08, .value = 0x34, }, /* 0x35 */
> +		[84] = { .offset = 0x41, .value = 0x00, },
> +		[85] = { .offset = 0xc0, .value = 0x01, },
> +	},
> +};
> +
>   struct ns2501_priv {
> -	//I2CDevRec d;
>   	bool quiet;
> -	int reg_8_shadow;
> -	int reg_8_set;
> -	// Shadow registers for i915
> -	int dvoc;
> -	int pll_a;
> -	int srcdim;
> -	int fw_blc;
> +	const struct ns2501_reg *regs;
>   };
>
>   #define NSPTR(d) ((NS2501Ptr)(d->DriverPrivate.ptr))
> @@ -205,11 +480,9 @@ static bool ns2501_init(struct intel_dvo_device *dvo,
>   		goto out;
>   	}
>   	ns->quiet = false;
> -	ns->reg_8_set = 0;
> -	ns->reg_8_shadow =
> -	    NS2501_8_PD | NS2501_8_BPAS | NS2501_8_VEN | NS2501_8_HEN;
>
>   	DRM_DEBUG_KMS("init ns2501 dvo controller successfully!\n");
> +
>   	return true;
>
>   out:
> @@ -255,180 +528,26 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
>   			    struct drm_display_mode *mode,
>   			    struct drm_display_mode *adjusted_mode)
>   {
> -	bool ok;
> -	int retries = 10;
>   	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
> +	int mode_idx, i;
>
>   	DRM_DEBUG_KMS
>   	    ("set mode (hdisplay=%d,htotal=%d,vdisplay=%d,vtotal=%d).\n",
>   	     mode->hdisplay, mode->htotal, mode->vdisplay, mode->vtotal);
>
> -	/*
> -	 * Where do I find the native resolution for which scaling is not required???
> -	 *
> -	 * First trigger the DVO on as otherwise the chip does not appear on the i2c
> -	 * bus.
> -	 */
> -	do {
> -		ok = true;
> -
> -		if (mode->hdisplay == 800 && mode->vdisplay == 600) {
> -			/* mode 277 */
> -			ns->reg_8_shadow &= ~NS2501_8_BPAS;
> -			DRM_DEBUG_KMS("switching to 800x600\n");
> -
> -			/*
> -			 * No, I do not know where this data comes from.
> -			 * It is just what the video bios left in the DVO, so
> -			 * I'm just copying it here over.
> -			 * This also means that I cannot support any other modes
> -			 * except the ones supported by the bios.
> -			 */
> -			ok &= ns2501_writeb(dvo, 0x11, 0xc8);	// 0xc7 also works.
> -			ok &= ns2501_writeb(dvo, 0x1b, 0x19);
> -			ok &= ns2501_writeb(dvo, 0x1c, 0x62);	// VBIOS left 0x64 here, but 0x62 works nicer
> -			ok &= ns2501_writeb(dvo, 0x1d, 0x02);
> -
> -			ok &= ns2501_writeb(dvo, 0x34, 0x03);
> -			ok &= ns2501_writeb(dvo, 0x35, 0xff);
> -
> -			ok &= ns2501_writeb(dvo, 0x80, 0x27);
> -			ok &= ns2501_writeb(dvo, 0x81, 0x03);
> -			ok &= ns2501_writeb(dvo, 0x82, 0x41);
> -			ok &= ns2501_writeb(dvo, 0x83, 0x05);
> +	if (mode->hdisplay == 640 && mode->vdisplay == 480)
> +		mode_idx = MODE_640x480;
> +	else if (mode->hdisplay == 800 && mode->vdisplay == 600)
> +		mode_idx = MODE_800x600;
> +	else if (mode->hdisplay == 1024 && mode->vdisplay == 768)
> +		mode_idx = MODE_1024x768;
> +	else
> +		return;
>
> -			ok &= ns2501_writeb(dvo, 0x8d, 0x02);
> -			ok &= ns2501_writeb(dvo, 0x8e, 0x04);
> -			ok &= ns2501_writeb(dvo, 0x8f, 0x00);
> +	ns->regs = regs_1024x768[mode_idx];
>
> -			ok &= ns2501_writeb(dvo, 0x90, 0xfe);	/* vertical. VBIOS left 0xff here, but 0xfe works better */
> -			ok &= ns2501_writeb(dvo, 0x91, 0x07);
> -			ok &= ns2501_writeb(dvo, 0x94, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x95, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0x96, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0x99, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x9a, 0x88);
> -
> -			ok &= ns2501_writeb(dvo, 0x9c, 0x23);	/* Looks like first and last line of the image. */
> -			ok &= ns2501_writeb(dvo, 0x9d, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x9e, 0x25);
> -			ok &= ns2501_writeb(dvo, 0x9f, 0x03);
> -
> -			ok &= ns2501_writeb(dvo, 0xa4, 0x80);
> -
> -			ok &= ns2501_writeb(dvo, 0xb6, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0xb9, 0xc8);	/* horizontal? */
> -			ok &= ns2501_writeb(dvo, 0xba, 0x00);	/* horizontal? */
> -
> -			ok &= ns2501_writeb(dvo, 0xc0, 0x05);	/* horizontal? */
> -			ok &= ns2501_writeb(dvo, 0xc1, 0xd7);
> -
> -			ok &= ns2501_writeb(dvo, 0xc2, 0x00);
> -			ok &= ns2501_writeb(dvo, 0xc3, 0xf8);
> -
> -			ok &= ns2501_writeb(dvo, 0xc4, 0x03);
> -			ok &= ns2501_writeb(dvo, 0xc5, 0x1a);
> -
> -			ok &= ns2501_writeb(dvo, 0xc6, 0x00);
> -			ok &= ns2501_writeb(dvo, 0xc7, 0x73);
> -			ok &= ns2501_writeb(dvo, 0xc8, 0x02);
> -
> -		} else if (mode->hdisplay == 640 && mode->vdisplay == 480) {
> -			/* mode 274 */
> -			DRM_DEBUG_KMS("switching to 640x480\n");
> -			/*
> -			 * No, I do not know where this data comes from.
> -			 * It is just what the video bios left in the DVO, so
> -			 * I'm just copying it here over.
> -			 * This also means that I cannot support any other modes
> -			 * except the ones supported by the bios.
> -			 */
> -			ns->reg_8_shadow &= ~NS2501_8_BPAS;
> -
> -			ok &= ns2501_writeb(dvo, 0x11, 0xa0);
> -			ok &= ns2501_writeb(dvo, 0x1b, 0x11);
> -			ok &= ns2501_writeb(dvo, 0x1c, 0x54);
> -			ok &= ns2501_writeb(dvo, 0x1d, 0x03);
> -
> -			ok &= ns2501_writeb(dvo, 0x34, 0x03);
> -			ok &= ns2501_writeb(dvo, 0x35, 0xff);
> -
> -			ok &= ns2501_writeb(dvo, 0x80, 0xff);
> -			ok &= ns2501_writeb(dvo, 0x81, 0x07);
> -			ok &= ns2501_writeb(dvo, 0x82, 0x3d);
> -			ok &= ns2501_writeb(dvo, 0x83, 0x05);
> -
> -			ok &= ns2501_writeb(dvo, 0x8d, 0x02);
> -			ok &= ns2501_writeb(dvo, 0x8e, 0x10);
> -			ok &= ns2501_writeb(dvo, 0x8f, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0x90, 0xff);	/* vertical */
> -			ok &= ns2501_writeb(dvo, 0x91, 0x07);
> -			ok &= ns2501_writeb(dvo, 0x94, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x95, 0x00);
> -
> -			ok &= ns2501_writeb(dvo, 0x96, 0x05);
> -
> -			ok &= ns2501_writeb(dvo, 0x99, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x9a, 0x88);
> -
> -			ok &= ns2501_writeb(dvo, 0x9c, 0x24);
> -			ok &= ns2501_writeb(dvo, 0x9d, 0x00);
> -			ok &= ns2501_writeb(dvo, 0x9e, 0x25);
> -			ok &= ns2501_writeb(dvo, 0x9f, 0x03);
> -
> -			ok &= ns2501_writeb(dvo, 0xa4, 0x84);
> -
> -			ok &= ns2501_writeb(dvo, 0xb6, 0x09);
> -
> -			ok &= ns2501_writeb(dvo, 0xb9, 0xa0);	/* horizontal? */
> -			ok &= ns2501_writeb(dvo, 0xba, 0x00);	/* horizontal? */
> -
> -			ok &= ns2501_writeb(dvo, 0xc0, 0x05);	/* horizontal? */
> -			ok &= ns2501_writeb(dvo, 0xc1, 0x90);
> -
> -			ok &= ns2501_writeb(dvo, 0xc2, 0x00);
> -			ok &= ns2501_writeb(dvo, 0xc3, 0x0f);
> -
> -			ok &= ns2501_writeb(dvo, 0xc4, 0x03);
> -			ok &= ns2501_writeb(dvo, 0xc5, 0x16);
> -
> -			ok &= ns2501_writeb(dvo, 0xc6, 0x00);
> -			ok &= ns2501_writeb(dvo, 0xc7, 0x02);
> -			ok &= ns2501_writeb(dvo, 0xc8, 0x02);
> -
> -		} else if (mode->hdisplay == 1024 && mode->vdisplay == 768) {
> -			/* mode 280 */
> -			DRM_DEBUG_KMS("switching to 1024x768\n");
> -			/*
> -			 * This might or might not work, actually. I'm silently
> -			 * assuming here that the native panel resolution is
> -			 * 1024x768. If not, then this leaves the scaler disabled
> -			 * generating a picture that is likely not the expected.
> -			 *
> -			 * Problem is that I do not know where to take the panel
> -			 * dimensions from.
> -			 *
> -			 * Enable the bypass, scaling not required.
> -			 *
> -			 * The scaler registers are irrelevant here....
> -			 *
> -			 */
> -			ns->reg_8_shadow |= NS2501_8_BPAS;
> -			ok &= ns2501_writeb(dvo, 0x37, 0x44);
> -		} else {
> -			/*
> -			 * Data not known. Bummer!
> -			 * Hopefully, the code should not go here
> -			 * as mode_OK delivered no other modes.
> -			 */
> -			ns->reg_8_shadow |= NS2501_8_BPAS;
> -		}
> -		ok &= ns2501_writeb(dvo, NS2501_REG8, ns->reg_8_shadow);
> -	} while (!ok && retries--);
> +	for (i = 0; i < 84; i++)
> +		ns2501_writeb(dvo, ns->regs[i].offset, ns->regs[i].value);
>   }
>
>   /* set the NS2501 power state */
> @@ -439,43 +558,45 @@ static bool ns2501_get_hw_state(struct intel_dvo_device *dvo)
>   	if (!ns2501_readb(dvo, NS2501_REG8, &ch))
>   		return false;
>
> -	if (ch & NS2501_8_PD)
> -		return true;
> -	else
> -		return false;
> +	return ch & NS2501_8_PD;
>   }
>
>   /* set the NS2501 power state */
>   static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
>   {
> -	bool ok;
> -	int retries = 10;
>   	struct ns2501_priv *ns = (struct ns2501_priv *)(dvo->dev_priv);
> -	unsigned char ch;
>
>   	DRM_DEBUG_KMS("Trying set the dpms of the DVO to %i\n", enable);
>
> -	ch = ns->reg_8_shadow;
> +	if (enable) {
> +		if (WARN_ON(ns->regs[83].offset != 0x08 ||
> +			    ns->regs[84].offset != 0x41 ||
> +			    ns->regs[85].offset != 0xc0))
> +			return;
>
> -	if (enable)
> -		ch |= NS2501_8_PD;
> -	else
> -		ch &= ~NS2501_8_PD;
> -
> -	if (ns->reg_8_set == 0 || ns->reg_8_shadow != ch) {
> -		ns->reg_8_set = 1;
> -		ns->reg_8_shadow = ch;
> -
> -		do {
> -			ok = true;
> -			ok &= ns2501_writeb(dvo, NS2501_REG8, ch);
> -			ok &=
> -			    ns2501_writeb(dvo, 0x34,
> -					  enable ? 0x03 : 0x00);
> -			ok &=
> -			    ns2501_writeb(dvo, 0x35,
> -					  enable ? 0xff : 0x00);
> -		} while (!ok && retries--);
> +		ns2501_writeb(dvo, 0xc0, ns->regs[85].value | 0x08);
> +
> +		ns2501_writeb(dvo, 0x41, ns->regs[84].value);
> +
> +		ns2501_writeb(dvo, 0x34, 0x01);
> +		msleep(15);
> +
> +		ns2501_writeb(dvo, 0x08, 0x35);
> +		if (!(ns->regs[83].value & NS2501_8_BPAS))
> +			ns2501_writeb(dvo, 0x08, 0x31);
> +		msleep(200);
> +
> +		ns2501_writeb(dvo, 0x34, 0x03);
> +
> +		ns2501_writeb(dvo, 0xc0, ns->regs[85].value);
> +	} else {
> +		ns2501_writeb(dvo, 0x34, 0x01);
> +		msleep(200);
> +
> +		ns2501_writeb(dvo, 0x08, 0x34);
> +		msleep(15);
> +
> +		ns2501_writeb(dvo, 0x34, 0x00);
>   	}
>   }
>
>

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

[PATCH 11/16] drm/i915: Init important ns2501 registers

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In my earlier rewrite I missed a few important registers. Thomas Richter
noticed that they're needed to make his machine resume correctly.

Looks like IEGD does a one time init of these three registers. We don't
have a good one time init place in the ns2501 driver, so let's just
stick them into the .mode_set() hook and see if that helps things along.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/dvo_ns2501.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
index b278571..345235b 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -342,6 +342,12 @@ static const struct ns2501_reg regs_1024x768[][86] = {
 	},
 };
 
+static const struct ns2501_reg regs_init[] = {
+	[0] = { .offset = 0x35, .value = 0xff, },
+	[1] = { .offset = 0x34, .value = 0x00, },
+	[2] = { .offset = 0x08, .value = 0x30, },
+};
+
 struct ns2501_priv {
 	bool quiet;
 	const struct ns2501_reg *regs;
@@ -544,6 +550,10 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
 	else
 		return;
 
+	/* Hopefully doing it every time won't hurt... */
+	for (i = 0; i < ARRAY_SIZE(regs_init); i++)
+		ns2501_writeb(dvo, regs_init[i].offset, regs_init[i].value);
+
 	ns->regs = regs_1024x768[mode_idx];
 
 	for (i = 0; i < 84; i++)
-- 
1.8.5.5

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

Re: [PATCH 11/16] drm/i915: Init important ns2501 registers

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> In my earlier rewrite I missed a few important registers. Thomas Richter
> noticed that they're needed to make his machine resume correctly.
>
> Looks like IEGD does a one time init of these three registers. We don't
> have a good one time init place in the ns2501 driver, so let's just
> stick them into the .mode_set() hook and see if that helps things along.

Looks good to me.

Signed-off-by: Thomas Richter <richter@rus.uni-stuttgart.de>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/dvo_ns2501.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index b278571..345235b 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -342,6 +342,12 @@ static const struct ns2501_reg regs_1024x768[][86] = {
>   	},
>   };
>
> +static const struct ns2501_reg regs_init[] = {
> +	[0] = { .offset = 0x35, .value = 0xff, },
> +	[1] = { .offset = 0x34, .value = 0x00, },
> +	[2] = { .offset = 0x08, .value = 0x30, },
> +};
> +
>   struct ns2501_priv {
>   	bool quiet;
>   	const struct ns2501_reg *regs;
> @@ -544,6 +550,10 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
>   	else
>   		return;
>
> +	/* Hopefully doing it every time won't hurt... */
> +	for (i = 0; i < ARRAY_SIZE(regs_init); i++)
> +		ns2501_writeb(dvo, regs_init[i].offset, regs_init[i].value);
> +
>   	ns->regs = regs_1024x768[mode_idx];
>
>   	for (i = 0; i < 84; i++)
>

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

Re: [PATCH 11/16] drm/i915: Init important ns2501 registers

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> In my earlier rewrite I missed a few important registers. Thomas Richter
> noticed that they're needed to make his machine resume correctly.
>
> Looks like IEGD does a one time init of these three registers. We don't
> have a good one time init place in the ns2501 driver, so let's just
> stick them into the .mode_set() hook and see if that helps things along.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/dvo_ns2501.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index b278571..345235b 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -342,6 +342,12 @@ static const struct ns2501_reg regs_1024x768[][86] = {
>   	},
>   };
>
> +static const struct ns2501_reg regs_init[] = {
> +	[0] = { .offset = 0x35, .value = 0xff, },
> +	[1] = { .offset = 0x34, .value = 0x00, },
> +	[2] = { .offset = 0x08, .value = 0x30, },
> +};
> +
>   struct ns2501_priv {
>   	bool quiet;
>   	const struct ns2501_reg *regs;
> @@ -544,6 +550,10 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
>   	else
>   		return;
>
> +	/* Hopefully doing it every time won't hurt... */
> +	for (i = 0; i < ARRAY_SIZE(regs_init); i++)
> +		ns2501_writeb(dvo, regs_init[i].offset, regs_init[i].value);
> +
>   	ns->regs = regs_1024x768[mode_idx];
>
>   	for (i = 0; i < 84; i++)
>

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

Re: [PATCH 11/16] drm/i915: Init important ns2501 registers

[Daniel Vetter]

On Fri, Aug 15, 2014 at 01:22:03AM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> In my earlier rewrite I missed a few important registers. Thomas Richter
> noticed that they're needed to make his machine resume correctly.
> 
> Looks like IEGD does a one time init of these three registers. We don't
> have a good one time init place in the ns2501 driver, so let's just
> stick them into the .mode_set() hook and see if that helps things along.

We have the encoder->reset hooks which are commonly used for such stuff.
Not worth to wire that up for dvo, just an fyi really.
-Daniel

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/dvo_ns2501.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index b278571..345235b 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -342,6 +342,12 @@ static const struct ns2501_reg regs_1024x768[][86] = {
>  	},
>  };
>  
> +static const struct ns2501_reg regs_init[] = {
> +	[0] = { .offset = 0x35, .value = 0xff, },
> +	[1] = { .offset = 0x34, .value = 0x00, },
> +	[2] = { .offset = 0x08, .value = 0x30, },
> +};
> +
>  struct ns2501_priv {
>  	bool quiet;
>  	const struct ns2501_reg *regs;
> @@ -544,6 +550,10 @@ static void ns2501_mode_set(struct intel_dvo_device *dvo,
>  	else
>  		return;
>  
> +	/* Hopefully doing it every time won't hurt... */
> +	for (i = 0; i < ARRAY_SIZE(regs_init); i++)
> +		ns2501_writeb(dvo, regs_init[i].offset, regs_init[i].value);
> +
>  	ns->regs = regs_1024x768[mode_idx];
>  
>  	for (i = 0; i < 84; i++)
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 12/16] drm/i915: Check pixel clock in ns2501 mode_valid hook

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The vbt on my Fujitsu-Siemens Lifebook S6010 provides two 800x600 modes,
60Hz and 56Hz. The magic register values we have correspond to the 60Hz
mode, and as I don't know how one would trick the VGA BIOS to set up
the 56Hz mode we can't get the magic values for the orther mode. So
when checking whether a mode is valid also check the pixel clock so that
we filter out the 56Hz variant.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/dvo_ns2501.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
index 345235b..4416304 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -521,9 +521,9 @@ static enum drm_mode_status ns2501_mode_valid(struct intel_dvo_device *dvo,
 	 * of the panel in here so we could always accept it
 	 * by disabling the scaler.
 	 */
-	if ((mode->hdisplay == 800 && mode->vdisplay == 600) ||
-	    (mode->hdisplay == 640 && mode->vdisplay == 480) ||
-	    (mode->hdisplay == 1024 && mode->vdisplay == 768)) {
+	if ((mode->hdisplay == 640 && mode->vdisplay == 480 && mode->clock == 25175) ||
+	    (mode->hdisplay == 800 && mode->vdisplay == 600 && mode->clock == 40000) ||
+	    (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock == 65000)) {
 		return MODE_OK;
 	} else {
 		return MODE_ONE_SIZE;	/* Is this a reasonable error? */
-- 
1.8.5.5

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

Re: [PATCH 12/16] drm/i915: Check pixel clock in ns2501 mode_valid hook

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The vbt on my Fujitsu-Siemens Lifebook S6010 provides two 800x600 modes,
> 60Hz and 56Hz. The magic register values we have correspond to the 60Hz
> mode, and as I don't know how one would trick the VGA BIOS to set up
> the 56Hz mode we can't get the magic values for the orther mode. So
> when checking whether a mode is valid also check the pixel clock so that
> we filter out the 56Hz variant.

The 56Hz mode works here as does the 60Hz mode, though it is really 
better to have only one 800x600 mode. Looks fine.

Signed-off-by: Thomas Richter <richter@rus.uni-stuttgart.de>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/dvo_ns2501.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index 345235b..4416304 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -521,9 +521,9 @@ static enum drm_mode_status ns2501_mode_valid(struct intel_dvo_device *dvo,
>   	 * of the panel in here so we could always accept it
>   	 * by disabling the scaler.
>   	 */
> -	if ((mode->hdisplay == 800 && mode->vdisplay == 600) ||
> -	    (mode->hdisplay == 640 && mode->vdisplay == 480) ||
> -	    (mode->hdisplay == 1024 && mode->vdisplay == 768)) {
> +	if ((mode->hdisplay == 640 && mode->vdisplay == 480 && mode->clock == 25175) ||
> +	    (mode->hdisplay == 800 && mode->vdisplay == 600 && mode->clock == 40000) ||
> +	    (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock == 65000)) {
>   		return MODE_OK;
>   	} else {
>   		return MODE_ONE_SIZE;	/* Is this a reasonable error? */
>

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

Re: [PATCH 12/16] drm/i915: Check pixel clock in ns2501 mode_valid hook

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The vbt on my Fujitsu-Siemens Lifebook S6010 provides two 800x600 modes,
> 60Hz and 56Hz. The magic register values we have correspond to the 60Hz
> mode, and as I don't know how one would trick the VGA BIOS to set up
> the 56Hz mode we can't get the magic values for the orther mode. So
> when checking whether a mode is valid also check the pixel clock so that
> we filter out the 56Hz variant.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/dvo_ns2501.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
> index 345235b..4416304 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -521,9 +521,9 @@ static enum drm_mode_status ns2501_mode_valid(struct intel_dvo_device *dvo,
>   	 * of the panel in here so we could always accept it
>   	 * by disabling the scaler.
>   	 */
> -	if ((mode->hdisplay == 800 && mode->vdisplay == 600) ||
> -	    (mode->hdisplay == 640 && mode->vdisplay == 480) ||
> -	    (mode->hdisplay == 1024 && mode->vdisplay == 768)) {
> +	if ((mode->hdisplay == 640 && mode->vdisplay == 480 && mode->clock == 25175) ||
> +	    (mode->hdisplay == 800 && mode->vdisplay == 600 && mode->clock == 40000) ||
> +	    (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock == 65000)) {
>   		return MODE_OK;
>   	} else {
>   		return MODE_ONE_SIZE;	/* Is this a reasonable error? */
>

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

[PATCH 13/16] drm/i915: Fix DVO 2x clock enable on 830M

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The spec says:
"For the correct operation of the muxed DVO pins (GDEVSELB/ I2Cdata,
GIRDBY/I2CClk) and (GFRAMEB/DVI_Data, GTRDYB/DVI_Clk): Bit 31
(DPLL VCO Enable) and Bit 30 (2X Clock Enable) must be set to “1” in
both the DPLL A Control Register (06014h-06017h) and DPLL B Control
Register (06018h-0601Bh)."

The pipe A and B force quirks take care of DPLL_VCO_ENABLE, so we
just need a bit of special care to handle DPLL_DVO_2X_MODE.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 541fb6f..54895a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1561,6 +1561,9 @@ struct drm_i915_private {
 
 	u16 orig_clock;
 
+	/* used to control DVO 2x clock enable on 830M */
+	uint8_t dvo_pipes;
+
 	bool mchbar_need_disable;
 
 	struct intel_l3_parity l3_parity;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3eeb5ce..6462bcf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1548,6 +1548,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe = crtc->pipe;
 	int reg = DPLL(crtc->pipe);
 	u32 dpll = crtc->config.dpll_hw_state.dpll;
 
@@ -1560,7 +1561,16 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
 	if (IS_MOBILE(dev) && !IS_I830(dev))
 		assert_panel_unlocked(dev_priv, crtc->pipe);
 
-	I915_WRITE(reg, dpll);
+	/* enable DVO 2x clock on both PLLs if necessary */
+	if (IS_I830(dev)) {
+		if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
+			dev_priv->dvo_pipes |= 1 << pipe;
+
+		if (dev_priv->dvo_pipes) {
+			dpll |= DPLL_DVO_2X_MODE;
+			I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe)) | DPLL_DVO_2X_MODE);
+		}
+	}
 
 	/* Wait for the clocks to stabilize. */
 	POSTING_READ(reg);
@@ -1599,8 +1609,23 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
  *
  * Note!  This is for pre-ILK only.
  */
-static void i9xx_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
+static void i9xx_disable_pll(struct intel_crtc *crtc)
 {
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe = crtc->pipe;
+
+	/* disable DVO 2x clock on both PLLs if necessary */
+	if (IS_I830(dev)) {
+		if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
+			dev_priv->dvo_pipes &= ~(1 << pipe);
+
+		if (!dev_priv->dvo_pipes) {
+			I915_WRITE(DPLL(pipe), I915_READ(DPLL(pipe)) & ~DPLL_DVO_2X_MODE);
+			I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe)) & ~DPLL_DVO_2X_MODE);
+		}
+	}
+
 	/* Don't disable pipe A or pipe A PLLs if needed */
 	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
 		return;
@@ -4788,7 +4813,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 		else if (IS_VALLEYVIEW(dev))
 			vlv_disable_pll(dev_priv, pipe);
 		else
-			i9xx_disable_pll(dev_priv, pipe);
+			i9xx_disable_pll(intel_crtc);
 	}
 
 	if (!IS_GEN2(dev))
@@ -5792,7 +5817,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
 			dpll |= PLL_P2_DIVIDE_BY_4;
 	}
 
-	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
+	if (!IS_I830(dev) && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
 		dpll |= DPLL_DVO_2X_MODE;
 
 	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
@@ -6298,6 +6323,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	}
 	pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(crtc->pipe));
 	if (!IS_VALLEYVIEW(dev)) {
+		if (IS_I830(dev))
+			pipe_config->dpll_hw_state.dpll &= ~DPLL_DVO_2X_MODE;
+
 		pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(crtc->pipe));
 		pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(crtc->pipe));
 	} else {
@@ -13021,6 +13049,17 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 		}
 	}
 
+	/* update dvo_pipes */
+	if (IS_I830(dev)) {
+		dev_priv->dvo_pipes = 0;
+
+		for_each_intel_crtc(dev, crtc) {
+			if (crtc->active &&
+			    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
+				dev_priv->dvo_pipes |= 1 << crtc->pipe;
+		}
+	}
+
 	/* HW state is read out, now we need to sanitize this mess. */
 	for_each_intel_encoder(dev, encoder) {
 		intel_sanitize_encoder(encoder);
-- 
1.8.5.5

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

Re: [PATCH 13/16] drm/i915: Fix DVO 2x clock enable on 830M

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The spec says:
> "For the correct operation of the muxed DVO pins (GDEVSELB/ I2Cdata,
> GIRDBY/I2CClk) and (GFRAMEB/DVI_Data, GTRDYB/DVI_Clk): Bit 31
> (DPLL VCO Enable) and Bit 30 (2X Clock Enable) must be set to “1” in
> both the DPLL A Control Register (06014h-06017h) and DPLL B Control
> Register (06018h-0601Bh)."
>
> The pipe A and B force quirks take care of DPLL_VCO_ENABLE, so we
> just need a bit of special care to handle DPLL_DVO_2X_MODE.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  3 +++
>   drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++++---
>   2 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 541fb6f..54895a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1561,6 +1561,9 @@ struct drm_i915_private {
>
>   	u16 orig_clock;
>
> +	/* used to control DVO 2x clock enable on 830M */
> +	uint8_t dvo_pipes;
> +
>   	bool mchbar_need_disable;
>
>   	struct intel_l3_parity l3_parity;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3eeb5ce..6462bcf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1548,6 +1548,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
>   {
>   	struct drm_device *dev = crtc->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = crtc->pipe;
>   	int reg = DPLL(crtc->pipe);
>   	u32 dpll = crtc->config.dpll_hw_state.dpll;
>
> @@ -1560,7 +1561,16 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
>   	if (IS_MOBILE(dev) && !IS_I830(dev))
>   		assert_panel_unlocked(dev_priv, crtc->pipe);
>
> -	I915_WRITE(reg, dpll);
> +	/* enable DVO 2x clock on both PLLs if necessary */
> +	if (IS_I830(dev)) {
> +		if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +			dev_priv->dvo_pipes |= 1 << pipe;
> +
> +		if (dev_priv->dvo_pipes) {
> +			dpll |= DPLL_DVO_2X_MODE;
> +			I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe)) | DPLL_DVO_2X_MODE);
> +		}
> +	}
>
>   	/* Wait for the clocks to stabilize. */
>   	POSTING_READ(reg);
> @@ -1599,8 +1609,23 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
>    *
>    * Note!  This is for pre-ILK only.
>    */
> -static void i9xx_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> +static void i9xx_disable_pll(struct intel_crtc *crtc)
>   {
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = crtc->pipe;
> +
> +	/* disable DVO 2x clock on both PLLs if necessary */
> +	if (IS_I830(dev)) {
> +		if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +			dev_priv->dvo_pipes &= ~(1 << pipe);
> +
> +		if (!dev_priv->dvo_pipes) {
> +			I915_WRITE(DPLL(pipe), I915_READ(DPLL(pipe)) & ~DPLL_DVO_2X_MODE);
> +			I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe)) & ~DPLL_DVO_2X_MODE);
> +		}
> +	}
> +
>   	/* Don't disable pipe A or pipe A PLLs if needed */
>   	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
>   		return;
> @@ -4788,7 +4813,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>   		else if (IS_VALLEYVIEW(dev))
>   			vlv_disable_pll(dev_priv, pipe);
>   		else
> -			i9xx_disable_pll(dev_priv, pipe);
> +			i9xx_disable_pll(intel_crtc);
>   	}
>
>   	if (!IS_GEN2(dev))
> @@ -5792,7 +5817,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
>   			dpll |= PLL_P2_DIVIDE_BY_4;
>   	}
>
> -	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +	if (!IS_I830(dev) && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
>   		dpll |= DPLL_DVO_2X_MODE;
>
>   	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> @@ -6298,6 +6323,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>   	}
>   	pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(crtc->pipe));
>   	if (!IS_VALLEYVIEW(dev)) {
> +		if (IS_I830(dev))
> +			pipe_config->dpll_hw_state.dpll &= ~DPLL_DVO_2X_MODE;
> +
>   		pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(crtc->pipe));
>   		pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(crtc->pipe));
>   	} else {
> @@ -13021,6 +13049,17 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>   		}
>   	}
>
> +	/* update dvo_pipes */
> +	if (IS_I830(dev)) {
> +		dev_priv->dvo_pipes = 0;
> +
> +		for_each_intel_crtc(dev, crtc) {
> +			if (crtc->active &&
> +			    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +				dev_priv->dvo_pipes |= 1 << crtc->pipe;
> +		}
> +	}
> +
>   	/* HW state is read out, now we need to sanitize this mess. */
>   	for_each_intel_encoder(dev, encoder) {
>   		intel_sanitize_encoder(encoder);
>

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

Re: [PATCH 13/16] drm/i915: Fix DVO 2x clock enable on 830M

[Daniel Vetter]

On Fri, Aug 15, 2014 at 01:22:05AM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The spec says:
> "For the correct operation of the muxed DVO pins (GDEVSELB/ I2Cdata,
> GIRDBY/I2CClk) and (GFRAMEB/DVI_Data, GTRDYB/DVI_Clk): Bit 31
> (DPLL VCO Enable) and Bit 30 (2X Clock Enable) must be set to “1” in
> both the DPLL A Control Register (06014h-06017h) and DPLL B Control
> Register (06018h-0601Bh)."
> 
> The pipe A and B force quirks take care of DPLL_VCO_ENABLE, so we
> just need a bit of special care to handle DPLL_DVO_2X_MODE.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  3 +++
>  drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++++---
>  2 files changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 541fb6f..54895a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1561,6 +1561,9 @@ struct drm_i915_private {
>  
>  	u16 orig_clock;
>  
> +	/* used to control DVO 2x clock enable on 830M */
> +	uint8_t dvo_pipes;
> +
>  	bool mchbar_need_disable;
>  
>  	struct intel_l3_parity l3_parity;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3eeb5ce..6462bcf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1548,6 +1548,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = crtc->pipe;
>  	int reg = DPLL(crtc->pipe);
>  	u32 dpll = crtc->config.dpll_hw_state.dpll;
>  
> @@ -1560,7 +1561,16 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
>  	if (IS_MOBILE(dev) && !IS_I830(dev))
>  		assert_panel_unlocked(dev_priv, crtc->pipe);
>  
> -	I915_WRITE(reg, dpll);
> +	/* enable DVO 2x clock on both PLLs if necessary */
> +	if (IS_I830(dev)) {
> +		if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +			dev_priv->dvo_pipes |= 1 << pipe;
> +
> +		if (dev_priv->dvo_pipes) {
> +			dpll |= DPLL_DVO_2X_MODE;
> +			I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe)) | DPLL_DVO_2X_MODE);
> +		}
> +	}
>  
>  	/* Wait for the clocks to stabilize. */
>  	POSTING_READ(reg);
> @@ -1599,8 +1609,23 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
>   *
>   * Note!  This is for pre-ILK only.
>   */
> -static void i9xx_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> +static void i9xx_disable_pll(struct intel_crtc *crtc)
>  {
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = crtc->pipe;
> +
> +	/* disable DVO 2x clock on both PLLs if necessary */
> +	if (IS_I830(dev)) {
> +		if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +			dev_priv->dvo_pipes &= ~(1 << pipe);
> +
> +		if (!dev_priv->dvo_pipes) {
> +			I915_WRITE(DPLL(pipe), I915_READ(DPLL(pipe)) & ~DPLL_DVO_2X_MODE);
> +			I915_WRITE(DPLL(!pipe), I915_READ(DPLL(!pipe)) & ~DPLL_DVO_2X_MODE);

Bikeshed: I'd just use explicit PIPE_A and PIPE_B here, except when we
really have to turn of our pipe first before the other.

> +		}
> +	}
> +
>  	/* Don't disable pipe A or pipe A PLLs if needed */
>  	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
>  		return;
> @@ -4788,7 +4813,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>  		else if (IS_VALLEYVIEW(dev))
>  			vlv_disable_pll(dev_priv, pipe);
>  		else
> -			i9xx_disable_pll(dev_priv, pipe);
> +			i9xx_disable_pll(intel_crtc);
>  	}
>  
>  	if (!IS_GEN2(dev))
> @@ -5792,7 +5817,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
>  			dpll |= PLL_P2_DIVIDE_BY_4;
>  	}
>  
> -	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +	if (!IS_I830(dev) && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
>  		dpll |= DPLL_DVO_2X_MODE;
>  
>  	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> @@ -6298,6 +6323,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  	}
>  	pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(crtc->pipe));
>  	if (!IS_VALLEYVIEW(dev)) {
> +		if (IS_I830(dev))
> +			pipe_config->dpll_hw_state.dpll &= ~DPLL_DVO_2X_MODE;
> +
>  		pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(crtc->pipe));
>  		pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(crtc->pipe));
>  	} else {
> @@ -13021,6 +13049,17 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  		}
>  	}
>  
> +	/* update dvo_pipes */
> +	if (IS_I830(dev)) {
> +		dev_priv->dvo_pipes = 0;
> +
> +		for_each_intel_crtc(dev, crtc) {
> +			if (crtc->active &&
> +			    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +				dev_priv->dvo_pipes |= 1 << crtc->pipe;
> +		}
> +	}

This puts gunk into the modeset hw state stuff. Imo just loop over all
pipes and recompute the i830_needs_dvo_2x everywhere you need it, instead
of adding a new dev_priv->dvo_pipes state which might get out of sync
eventually.

I'll punt on this for now.
-Daniel

> +
>  	/* HW state is read out, now we need to sanitize this mess. */
>  	for_each_intel_encoder(dev, encoder) {
>  		intel_sanitize_encoder(encoder);
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 13/16] drm/i915: Fix DVO 2x clock enable on 830M

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The spec says:
"For the correct operation of the muxed DVO pins (GDEVSELB/ I2Cdata,
GIRDBY/I2CClk) and (GFRAMEB/DVI_Data, GTRDYB/DVI_Clk): Bit 31
(DPLL VCO Enable) and Bit 30 (2X Clock Enable) must be set to “1” in
both the DPLL A Control Register (06014h-06017h) and DPLL B Control
Register (06018h-0601Bh)."

The pipe A and B force quirks take care of DPLL_VCO_ENABLE, so we
just need a bit of special care to handle DPLL_DVO_2X_MODE.

v2: Recompute num_dvo_pipes on the spot, use PIPE_A/PIPE_B instead
    of pipe/!pipe for the register offsets in disable (Daniel)
    Add a comment about the ordering in enable and another one
    about filtering out the DVO 2x bit in state readout

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 53 +++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 842a5e1..6d6214a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1612,6 +1612,18 @@ static void chv_enable_pll(struct intel_crtc *crtc)
 	mutex_unlock(&dev_priv->dpio_lock);
 }
 
+static int intel_num_dvo_pipes(struct drm_device *dev)
+{
+	struct intel_crtc *crtc;
+	int count = 0;
+
+	for_each_intel_crtc(dev, crtc)
+		count += crtc->active &&
+			intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO);
+
+	return count;
+}
+
 static void i9xx_enable_pll(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
@@ -1628,7 +1640,18 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
 	if (IS_MOBILE(dev) && !IS_I830(dev))
 		assert_panel_unlocked(dev_priv, crtc->pipe);
 
-	I915_WRITE(reg, dpll);
+	/* Enable DVO 2x clock on both PLLs if necessary */
+	if (IS_I830(dev) && intel_num_dvo_pipes(dev) > 0) {
+		/*
+		 * It appears to be important that we don't enable this
+		 * for the current pipe before otherwise configuring the
+		 * PLL. No idea how this should be handled if multiple
+		 * DVO outputs are enabled simultaneosly.
+		 */
+		dpll |= DPLL_DVO_2X_MODE;
+		I915_WRITE(DPLL(!crtc->pipe),
+			   I915_READ(DPLL(!crtc->pipe)) | DPLL_DVO_2X_MODE);
+	}
 
 	/* Wait for the clocks to stabilize. */
 	POSTING_READ(reg);
@@ -1667,8 +1690,22 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
  *
  * Note!  This is for pre-ILK only.
  */
-static void i9xx_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
+static void i9xx_disable_pll(struct intel_crtc *crtc)
 {
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe = crtc->pipe;
+
+	/* Disable DVO 2x clock on both PLLs if necessary */
+	if (IS_I830(dev) &&
+	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO) &&
+	    intel_num_dvo_pipes(dev) == 1) {
+		I915_WRITE(DPLL(PIPE_B),
+			   I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
+		I915_WRITE(DPLL(PIPE_A),
+			   I915_READ(DPLL(PIPE_A)) & ~DPLL_DVO_2X_MODE);
+	}
+
 	/* Don't disable pipe or pipe PLLs if needed */
 	if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
 	    (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
@@ -4940,7 +4977,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 		else if (IS_VALLEYVIEW(dev))
 			vlv_disable_pll(dev_priv, pipe);
 		else
-			i9xx_disable_pll(dev_priv, pipe);
+			i9xx_disable_pll(intel_crtc);
 	}
 
 	if (!IS_GEN2(dev))
@@ -5944,7 +5981,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
 			dpll |= PLL_P2_DIVIDE_BY_4;
 	}
 
-	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
+	if (!IS_I830(dev) && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
 		dpll |= DPLL_DVO_2X_MODE;
 
 	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
@@ -6450,6 +6487,14 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	}
 	pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(crtc->pipe));
 	if (!IS_VALLEYVIEW(dev)) {
+		/*
+		 * DPLL_DVO_2X_MODE must be enabled for both DPLLs
+		 * on 830. Filter it out here so that we don't
+		 * report errors due to that.
+		 */
+		if (IS_I830(dev))
+			pipe_config->dpll_hw_state.dpll &= ~DPLL_DVO_2X_MODE;
+
 		pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(crtc->pipe));
 		pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(crtc->pipe));
 	} else {
-- 
1.8.5.5

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

Re: [PATCH v2 13/16] drm/i915: Fix DVO 2x clock enable on 830M

[Daniel Vetter]

On Fri, Sep 05, 2014 at 09:52:42PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The spec says:
> "For the correct operation of the muxed DVO pins (GDEVSELB/ I2Cdata,
> GIRDBY/I2CClk) and (GFRAMEB/DVI_Data, GTRDYB/DVI_Clk): Bit 31
> (DPLL VCO Enable) and Bit 30 (2X Clock Enable) must be set to “1” in
> both the DPLL A Control Register (06014h-06017h) and DPLL B Control
> Register (06018h-0601Bh)."
> 
> The pipe A and B force quirks take care of DPLL_VCO_ENABLE, so we
> just need a bit of special care to handle DPLL_DVO_2X_MODE.
> 
> v2: Recompute num_dvo_pipes on the spot, use PIPE_A/PIPE_B instead
>     of pipe/!pipe for the register offsets in disable (Daniel)
>     Add a comment about the ordering in enable and another one
>     about filtering out the DVO 2x bit in state readout
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 53 +++++++++++++++++++++++++++++++++---
>  1 file changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 842a5e1..6d6214a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1612,6 +1612,18 @@ static void chv_enable_pll(struct intel_crtc *crtc)
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> +static int intel_num_dvo_pipes(struct drm_device *dev)
> +{
> +	struct intel_crtc *crtc;
> +	int count = 0;
> +
> +	for_each_intel_crtc(dev, crtc)
> +		count += crtc->active &&
> +			intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO);
> +
> +	return count;
> +}
> +
>  static void i9xx_enable_pll(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
> @@ -1628,7 +1640,18 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
>  	if (IS_MOBILE(dev) && !IS_I830(dev))
>  		assert_panel_unlocked(dev_priv, crtc->pipe);
>  
> -	I915_WRITE(reg, dpll);
> +	/* Enable DVO 2x clock on both PLLs if necessary */
> +	if (IS_I830(dev) && intel_num_dvo_pipes(dev) > 0) {
> +		/*
> +		 * It appears to be important that we don't enable this
> +		 * for the current pipe before otherwise configuring the
> +		 * PLL. No idea how this should be handled if multiple
> +		 * DVO outputs are enabled simultaneosly.
> +		 */
> +		dpll |= DPLL_DVO_2X_MODE;
> +		I915_WRITE(DPLL(!crtc->pipe),
> +			   I915_READ(DPLL(!crtc->pipe)) | DPLL_DVO_2X_MODE);
> +	}
>  
>  	/* Wait for the clocks to stabilize. */
>  	POSTING_READ(reg);
> @@ -1667,8 +1690,22 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
>   *
>   * Note!  This is for pre-ILK only.
>   */
> -static void i9xx_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> +static void i9xx_disable_pll(struct intel_crtc *crtc)
>  {
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = crtc->pipe;
> +
> +	/* Disable DVO 2x clock on both PLLs if necessary */
> +	if (IS_I830(dev) &&
> +	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO) &&
> +	    intel_num_dvo_pipes(dev) == 1) {
> +		I915_WRITE(DPLL(PIPE_B),
> +			   I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
> +		I915_WRITE(DPLL(PIPE_A),
> +			   I915_READ(DPLL(PIPE_A)) & ~DPLL_DVO_2X_MODE);
> +	}
> +
>  	/* Don't disable pipe or pipe PLLs if needed */
>  	if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
>  	    (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
> @@ -4940,7 +4977,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>  		else if (IS_VALLEYVIEW(dev))
>  			vlv_disable_pll(dev_priv, pipe);
>  		else
> -			i9xx_disable_pll(dev_priv, pipe);
> +			i9xx_disable_pll(intel_crtc);
>  	}
>  
>  	if (!IS_GEN2(dev))
> @@ -5944,7 +5981,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
>  			dpll |= PLL_P2_DIVIDE_BY_4;
>  	}
>  
> -	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> +	if (!IS_I830(dev) && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
>  		dpll |= DPLL_DVO_2X_MODE;
>  
>  	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> @@ -6450,6 +6487,14 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  	}
>  	pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(crtc->pipe));
>  	if (!IS_VALLEYVIEW(dev)) {
> +		/*
> +		 * DPLL_DVO_2X_MODE must be enabled for both DPLLs
> +		 * on 830. Filter it out here so that we don't
> +		 * report errors due to that.
> +		 */
> +		if (IS_I830(dev))
> +			pipe_config->dpll_hw_state.dpll &= ~DPLL_DVO_2X_MODE;
> +
>  		pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(crtc->pipe));
>  		pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(crtc->pipe));
>  	} else {
> -- 
> 1.8.5.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 14/16] Revert "drm/i915: Nuke pipe A quirk on i830M"

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

830 really does want the pipe A quirk. The planes and ports don't
react to any register writes unless the pipe currently attached
to them is running, so it's impossible to move them to the other
pipe unless both pipes are running.

Also it's documented that the DPLL must be enabled on both pipes
whenever it's needed.

This reverts commit ac6696d3236bd61503f89a1a99680fd7894d5d53.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6462bcf..e1c0c0b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12503,6 +12503,9 @@ static struct intel_quirk intel_quirks[] = {
 	/* ThinkPad T60 needs pipe A force quirk (bug #16494) */
 	{ 0x2782, 0x17aa, 0x201a, quirk_pipea_force },
 
+	/* 830 needs to leave pipe A & dpll A up */
+	{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
+
 	/* Lenovo U160 cannot use SSC on LVDS */
 	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
 
-- 
1.8.5.5

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

Re: [PATCH 14/16] Revert "drm/i915: Nuke pipe A quirk on i830M"

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 830 really does want the pipe A quirk. The planes and ports don't
> react to any register writes unless the pipe currently attached
> to them is running, so it's impossible to move them to the other
> pipe unless both pipes are running.
>
> Also it's documented that the DPLL must be enabled on both pipes
> whenever it's needed.
>
> This reverts commit ac6696d3236bd61503f89a1a99680fd7894d5d53.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Works on both the R31 and the S6010.

Reviewed-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/intel_display.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6462bcf..e1c0c0b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12503,6 +12503,9 @@ static struct intel_quirk intel_quirks[] = {
>   	/* ThinkPad T60 needs pipe A force quirk (bug #16494) */
>   	{ 0x2782, 0x17aa, 0x201a, quirk_pipea_force },
>
> +	/* 830 needs to leave pipe A & dpll A up */
> +	{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
> +
>   	/* Lenovo U160 cannot use SSC on LVDS */
>   	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
>
>

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

[PATCH v2 15/16] drm/i915: Add pipe B force quirk for 830M

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

830M has problems when some of the pipes are disabled. Namely if a
plane, DVO port etc. is currently assigned to a disabled pipe, it
can't moved to the other pipe until the current pipe is also enabled.
To keep things simple just leave both pipes running all the time.

Ideally I think should turn the pipes off if neither is active, and
when either becomes active we enable both. But that would reuquire
proper atomic modeset support, and probably a bit of extra care in
the order things get enabled.

v2: Reorder wrt. double wide handling changes

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 54895a6..b1ed71e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -706,6 +706,7 @@ enum intel_sbi_destination {
 #define QUIRK_LVDS_SSC_DISABLE (1<<1)
 #define QUIRK_INVERT_BRIGHTNESS (1<<2)
 #define QUIRK_BACKLIGHT_PRESENT (1<<3)
+#define QUIRK_PIPEB_FORCE (1<<4)
 
 struct intel_fbdev;
 struct intel_fbc_work;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e1c0c0b..92baf6f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1194,8 +1194,9 @@ void assert_pipe(struct drm_i915_private *dev_priv,
 	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
 								      pipe);
 
-	/* if we need the pipe A quirk it must be always on */
-	if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
+	/* if we need the pipe quirk it must be always on */
+	if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
+	    (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
 		state = true;
 
 	if (!intel_display_power_enabled(dev_priv,
@@ -1626,8 +1627,9 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
 		}
 	}
 
-	/* Don't disable pipe A or pipe A PLLs if needed */
-	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
+	/* Don't disable pipe or pipe PLLs if needed */
+	if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
+	    (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
 		return;
 
 	/* Make sure the pipe isn't still relying on us */
@@ -1995,8 +1997,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 	reg = PIPECONF(cpu_transcoder);
 	val = I915_READ(reg);
 	if (val & PIPECONF_ENABLE) {
-		WARN_ON(!(pipe == PIPE_A &&
-			  dev_priv->quirks & QUIRK_PIPEA_FORCE));
+		WARN_ON(!((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
+			  (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE)));
 		return;
 	}
 
@@ -2043,7 +2045,8 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
 		val &= ~PIPECONF_DOUBLE_WIDE;
 
 	/* Don't disable pipe or pipe PLLs if needed */
-	if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE))
+	if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) &&
+	    !(pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
 		val &= ~PIPECONF_ENABLE;
 
 	I915_WRITE(reg, val);
@@ -5968,9 +5971,9 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 
 	pipeconf = 0;
 
-	if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
-	    I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE)
-		pipeconf |= PIPECONF_ENABLE;
+	if ((intel_crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
+	    (intel_crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
+		pipeconf |= I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE;
 
 	if (intel_crtc->config.double_wide)
 		pipeconf |= PIPECONF_DOUBLE_WIDE;
@@ -10680,8 +10683,9 @@ check_crtc_state(struct drm_device *dev)
 		active = dev_priv->display.get_pipe_config(crtc,
 							   &pipe_config);
 
-		/* hw state is inconsistent with the pipe A quirk */
-		if (crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
+		/* hw state is inconsistent with the pipe quirk */
+		if ((crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
+		    (crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
 			active = crtc->active;
 
 		for_each_intel_encoder(dev, encoder) {
@@ -12429,6 +12433,14 @@ static void quirk_pipea_force(struct drm_device *dev)
 	DRM_INFO("applying pipe a force quirk\n");
 }
 
+static void quirk_pipeb_force(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	dev_priv->quirks |= QUIRK_PIPEB_FORCE;
+	DRM_INFO("applying pipe b force quirk\n");
+}
+
 /*
  * Some machines (Lenovo U160) do not work with SSC on LVDS for some reason
  */
@@ -12506,6 +12518,9 @@ static struct intel_quirk intel_quirks[] = {
 	/* 830 needs to leave pipe A & dpll A up */
 	{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
 
+	/* 830 needs to leave pipe B & dpll B up */
+	{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipeb_force },
+
 	/* Lenovo U160 cannot use SSC on LVDS */
 	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
 
-- 
1.8.5.5

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

Re: [PATCH v2 15/16] drm/i915: Add pipe B force quirk for 830M

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 830M has problems when some of the pipes are disabled. Namely if a
> plane, DVO port etc. is currently assigned to a disabled pipe, it
> can't moved to the other pipe until the current pipe is also enabled.
> To keep things simple just leave both pipes running all the time.
>
> Ideally I think should turn the pipes off if neither is active, and
> when either becomes active we enable both. But that would reuquire
> proper atomic modeset support, and probably a bit of extra care in
> the order things get enabled.
>
> v2: Reorder wrt. double wide handling changes
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  1 +
>   drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++-----------
>   2 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 54895a6..b1ed71e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -706,6 +706,7 @@ enum intel_sbi_destination {
>   #define QUIRK_LVDS_SSC_DISABLE (1<<1)
>   #define QUIRK_INVERT_BRIGHTNESS (1<<2)
>   #define QUIRK_BACKLIGHT_PRESENT (1<<3)
> +#define QUIRK_PIPEB_FORCE (1<<4)
>
>   struct intel_fbdev;
>   struct intel_fbc_work;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e1c0c0b..92baf6f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1194,8 +1194,9 @@ void assert_pipe(struct drm_i915_private *dev_priv,
>   	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
>   								      pipe);
>
> -	/* if we need the pipe A quirk it must be always on */
> -	if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
> +	/* if we need the pipe quirk it must be always on */
> +	if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +	    (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
>   		state = true;
>
>   	if (!intel_display_power_enabled(dev_priv,
> @@ -1626,8 +1627,9 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
>   		}
>   	}
>
> -	/* Don't disable pipe A or pipe A PLLs if needed */
> -	if (pipe == PIPE_A && (dev_priv->quirks & QUIRK_PIPEA_FORCE))
> +	/* Don't disable pipe or pipe PLLs if needed */
> +	if ((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +	    (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
>   		return;
>
>   	/* Make sure the pipe isn't still relying on us */
> @@ -1995,8 +1997,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>   	reg = PIPECONF(cpu_transcoder);
>   	val = I915_READ(reg);
>   	if (val & PIPECONF_ENABLE) {
> -		WARN_ON(!(pipe == PIPE_A &&
> -			  dev_priv->quirks & QUIRK_PIPEA_FORCE));
> +		WARN_ON(!((pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +			  (pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE)));
>   		return;
>   	}
>
> @@ -2043,7 +2045,8 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>   		val &= ~PIPECONF_DOUBLE_WIDE;
>
>   	/* Don't disable pipe or pipe PLLs if needed */
> -	if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE))
> +	if (!(pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) &&
> +	    !(pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
>   		val &= ~PIPECONF_ENABLE;
>
>   	I915_WRITE(reg, val);
> @@ -5968,9 +5971,9 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>
>   	pipeconf = 0;
>
> -	if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
> -	    I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE)
> -		pipeconf |= PIPECONF_ENABLE;
> +	if ((intel_crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +	    (intel_crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
> +		pipeconf |= I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE;
>
>   	if (intel_crtc->config.double_wide)
>   		pipeconf |= PIPECONF_DOUBLE_WIDE;
> @@ -10680,8 +10683,9 @@ check_crtc_state(struct drm_device *dev)
>   		active = dev_priv->display.get_pipe_config(crtc,
>   							   &pipe_config);
>
> -		/* hw state is inconsistent with the pipe A quirk */
> -		if (crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
> +		/* hw state is inconsistent with the pipe quirk */
> +		if ((crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
> +		    (crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
>   			active = crtc->active;
>
>   		for_each_intel_encoder(dev, encoder) {
> @@ -12429,6 +12433,14 @@ static void quirk_pipea_force(struct drm_device *dev)
>   	DRM_INFO("applying pipe a force quirk\n");
>   }
>
> +static void quirk_pipeb_force(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	dev_priv->quirks |= QUIRK_PIPEB_FORCE;
> +	DRM_INFO("applying pipe b force quirk\n");
> +}
> +
>   /*
>    * Some machines (Lenovo U160) do not work with SSC on LVDS for some reason
>    */
> @@ -12506,6 +12518,9 @@ static struct intel_quirk intel_quirks[] = {
>   	/* 830 needs to leave pipe A & dpll A up */
>   	{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
>
> +	/* 830 needs to leave pipe B & dpll B up */
> +	{ 0x3577, PCI_ANY_ID, PCI_ANY_ID, quirk_pipeb_force },
> +
>   	/* Lenovo U160 cannot use SSC on LVDS */
>   	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
>
>

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

[PATCH 16/16] drm/i915: Preserve VGACNTR bits from the BIOS

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

My Fujistsu-Siemens Lifebook S6010 doesn't like to resume from
S3 unless VGACNTR has been restore to the original value. The BIOS
value in this case was 0x0124008E. Setting the "VGA disable" bit
doesn't interfere with the S3 resume fortunately.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      | 2 ++
 drivers/gpu/drm/i915/intel_display.c | 8 +++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b1ed71e..e0f64e4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1638,6 +1638,8 @@ struct drm_i915_private {
 	 */
 	struct workqueue_struct *dp_wq;
 
+	uint32_t bios_vgacntr;
+
 	/* Old dri1 support infrastructure, beware the dragons ya fools entering
 	 * here! */
 	struct i915_dri1_state dri1;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 92baf6f..f154993 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12591,7 +12591,11 @@ static void i915_disable_vga(struct drm_device *dev)
 	vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
 	udelay(300);
 
-	I915_WRITE(vga_reg, VGA_DISP_DISABLE);
+	/*
+	 * Fujitsu-Siemens Lifebook S6010 (830) has problems resuming
+	 * from S3 without preserving (some of?) the other bits.
+	 */
+	I915_WRITE(vga_reg, dev_priv->bios_vgacntr | VGA_DISP_DISABLE);
 	POSTING_READ(vga_reg);
 }
 
@@ -12680,6 +12684,8 @@ void intel_modeset_init(struct drm_device *dev)
 
 	intel_shared_dpll_init(dev);
 
+	/* save the BIOS value before clobbering it */
+	dev_priv->bios_vgacntr = I915_READ(i915_vgacntrl_reg(dev));
 	/* Just disable it once at startup */
 	i915_disable_vga(dev);
 	intel_setup_outputs(dev);
-- 
1.8.5.5

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

Re: [PATCH 16/16] drm/i915: Preserve VGACNTR bits from the BIOS

[Thomas Richter]

On 15.08.2014 00:22, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> My Fujistsu-Siemens Lifebook S6010 doesn't like to resume from
> S3 unless VGACNTR has been restore to the original value. The BIOS
> value in this case was 0x0124008E. Setting the "VGA disable" bit
> doesn't interfere with the S3 resume fortunately.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Works on the S6010 as advertized, though cannot test on the R31 since it
does not resume from S3 - it does not even reach the real-mode entry 
hook of the kernel.

Tested-by: Thomas Richter <richter@rus.uni-stuttgart.de>

> ---
>   drivers/gpu/drm/i915/i915_drv.h      | 2 ++
>   drivers/gpu/drm/i915/intel_display.c | 8 +++++++-
>   2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b1ed71e..e0f64e4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1638,6 +1638,8 @@ struct drm_i915_private {
>   	 */
>   	struct workqueue_struct *dp_wq;
>
> +	uint32_t bios_vgacntr;
> +
>   	/* Old dri1 support infrastructure, beware the dragons ya fools entering
>   	 * here! */
>   	struct i915_dri1_state dri1;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 92baf6f..f154993 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12591,7 +12591,11 @@ static void i915_disable_vga(struct drm_device *dev)
>   	vga_put(dev->pdev, VGA_RSRC_LEGACY_IO);
>   	udelay(300);
>
> -	I915_WRITE(vga_reg, VGA_DISP_DISABLE);
> +	/*
> +	 * Fujitsu-Siemens Lifebook S6010 (830) has problems resuming
> +	 * from S3 without preserving (some of?) the other bits.
> +	 */
> +	I915_WRITE(vga_reg, dev_priv->bios_vgacntr | VGA_DISP_DISABLE);
>   	POSTING_READ(vga_reg);
>   }
>
> @@ -12680,6 +12684,8 @@ void intel_modeset_init(struct drm_device *dev)
>
>   	intel_shared_dpll_init(dev);
>
> +	/* save the BIOS value before clobbering it */
> +	dev_priv->bios_vgacntr = I915_READ(i915_vgacntrl_reg(dev));
>   	/* Just disable it once at startup */
>   	i915_disable_vga(dev);
>   	intel_setup_outputs(dev);
>

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

Re: [PATCH 00/16] drm/i915: 830M/ns201 fixes again

[Ville Syrjälä]

On Fri, Aug 15, 2014 at 01:21:52AM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Thomas asked me to repost my 830/ns2501 patches. So here they are. I added
> a few more patches (trickle feed and unused ring init) to fix some post-resume
> issues. The primary plane rmw elimination patches and some locking/load detect
> fixes already got merged.
> 
> Apart from these we still lack the minimum watermark check. I guess
> we could just take the patch Thomas posted [1]. Doesn't look like a
> more advanced solution is coming any time soon. Though the commit
> message of that patch needs work and it lacks a s-o-b.
> 
> The VGACNTR patch might not be necessary any longer since Daniel's
> vga/dummycon stuff. I don't recall if I tested without it, but my
> gut feeling is that it's no longer needed. But I included the patch
> here anyway.
> 
> [1] http://patchwork.freedesktop.org/patch/27318/

Here's the branch (includes my earlier watermark hack):

git://gitorious.org/vsyrjala/linux.git alm_fixes11

-- 
Ville Syrjälä
Intel OTC

Re: S6010 - brightness adjustment not available

[Thomas Richter]

Hi Ville

>> just testing your alm_fixes11 branch. So far, everything works fine,
>> including suspend2ram, for the first time! Yippiee!
>>
>> However, there is one thing that bothers me, namely that the brightness
>> adjustment is no longer working. Specifically, fujitsu_laptop fails with:
>>
>> Fujitsu laptop FUJ02B1 ACPI brightness driver: probe of FUJ02B1:00
>> failed with error -22
>
> My impression was that those were happening since forever. But I can't
> be sure.

Actually, it did work with the alm-fixes5 patch, based on 3.15.0 rc7+ IIRC.

>> The source of fujitsu_laptop does not seem to have changed, thus I
>> believe it must be somehow related to the modifications you made. Any
>> idea where to look?
>
> Hmm. My impression was that the backlight control is handled by some
> hardware/firmware magic and OS can't touch it. IIRC that's how it seemed
> to work on my 855 machine which is a slighly newer Fujitsu-Siemens
> Lifebook model. At least on that machine backlight buttons worked
> a while ago.

Oh, sorry, I should have clarified. Yes, the *buttons* work, those are 
handled by the firmware (via the Bios service management interrupt, I 
suppose), but usually the fujitsu-laptop module *also* creates a device 
which provides access to the brightness control such that tools like the 
Gnome brightness applet can pick the information up and allows 
user-control of the brightness. The corresponding device does no longer 
exist on 3.16.0. Not yet fully clear whether that's due to the patches 
you made or something else, but I was running out of time yesterday to 
start a bisection, and I'm currently on the road and cannot test 
(greetings from Newark airport, BTW.)

> I can re-test the s6010 backlight next week and see if it works for me.
>
> I guess one idea might be the "init important ns2501 registers" patch.

No, I don't think so. You had that in in alm_fixes5 already, and there 
it worked. So the DVO is innocent, at least this time. As said, I 
haven't had the chance to have a more detailed look, but it seems that 
the fujitsu laptop extras go through some scary ACPI access path to 
control the backlight, and somehow this seems to be broken now, for a 
reason I haven't tried to understand.

> Maybe the values I used weren't entirely correct for this machine and
> we need to use the exact same values as the BIOS. All the other register
> values I used were definitely the same that hte BIOS set up, but I have
> nagging feeling there was some slight difference with these few
> registers.

Probably try something else first if you get the chance. My next 
possibility to work on this is next Saturday when I'm back from San 
Diego (this is where I'm going right now) and then take out your patches 
one by one.

Greetings,
	Thomas

Re: S6010 - brightness adjustment not available

[Ville Syrjälä]

On Sat, Aug 16, 2014 at 08:25:42PM +0200, Thomas Richter wrote:
> Hi Ville
> 
> >> just testing your alm_fixes11 branch. So far, everything works fine,
> >> including suspend2ram, for the first time! Yippiee!
> >>
> >> However, there is one thing that bothers me, namely that the brightness
> >> adjustment is no longer working. Specifically, fujitsu_laptop fails with:
> >>
> >> Fujitsu laptop FUJ02B1 ACPI brightness driver: probe of FUJ02B1:00
> >> failed with error -22
> >
> > My impression was that those were happening since forever. But I can't
> > be sure.
> 
> Actually, it did work with the alm-fixes5 patch, based on 3.15.0 rc7+ IIRC.
> 
> >> The source of fujitsu_laptop does not seem to have changed, thus I
> >> believe it must be somehow related to the modifications you made. Any
> >> idea where to look?
> >
> > Hmm. My impression was that the backlight control is handled by some
> > hardware/firmware magic and OS can't touch it. IIRC that's how it seemed
> > to work on my 855 machine which is a slighly newer Fujitsu-Siemens
> > Lifebook model. At least on that machine backlight buttons worked
> > a while ago.
> 
> Oh, sorry, I should have clarified. Yes, the *buttons* work, those are 
> handled by the firmware (via the Bios service management interrupt, I 
> suppose), but usually the fujitsu-laptop module *also* creates a device 
> which provides access to the brightness control such that tools like the 
> Gnome brightness applet can pick the information up and allows 
> user-control of the brightness. The corresponding device does no longer 
> exist on 3.16.0. Not yet fully clear whether that's due to the patches 
> you made or something else, but I was running out of time yesterday to 
> start a bisection, and I'm currently on the road and cannot test 
> (greetings from Newark airport, BTW.)

I would suggest it's some acpi change then. There was some change to
break the acpi video button stuff from acpi backlight driver, but I'm
not sure this would have affected fujitsu_laptop, and I'm not sure when
that went in. Might also be some other acpi patch. Not sure if just
looking at why/how it fails could pintpoint the problem or if bisecting
is the only reasonable option.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 00/16] drm/i915: 830M/ns201 fixes again

[Daniel Vetter]

On Fri, Aug 15, 2014 at 10:57:21AM +0300, Ville Syrjälä wrote:
> On Fri, Aug 15, 2014 at 01:21:52AM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Thomas asked me to repost my 830/ns2501 patches. So here they are. I added
> > a few more patches (trickle feed and unused ring init) to fix some post-resume
> > issues. The primary plane rmw elimination patches and some locking/load detect
> > fixes already got merged.
> > 
> > Apart from these we still lack the minimum watermark check. I guess
> > we could just take the patch Thomas posted [1]. Doesn't look like a
> > more advanced solution is coming any time soon. Though the commit
> > message of that patch needs work and it lacks a s-o-b.
> > 
> > The VGACNTR patch might not be necessary any longer since Daniel's
> > vga/dummycon stuff. I don't recall if I tested without it, but my
> > gut feeling is that it's no longer needed. But I included the patch
> > here anyway.
> > 
> > [1] http://patchwork.freedesktop.org/patch/27318/
> 
> Here's the branch (includes my earlier watermark hack):
> 
> git://gitorious.org/vsyrjala/linux.git alm_fixes11

Ok, I've merged all the patches here except the dvo 2x mode thing. And imo
we can also pull in the burst size hack since imo that's what we want -
afaik the hw has some bits to select the burst size between 4, 8, ...
entries, and atm we always set it to 8. And Bspec explicitly says that
setting the fifo to something smaller than burst size will lead to tears.

So the patch with a pimped commit message + bit a better comment is good
to go. Originally I wanted to implement "proper" burst size selection, but
that ran afoul of the general gen2/3 wm mess.

Aside: We might want to give intel_calculate_wm some i9xx prefix, since
it's very much not generic.

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

[PATCH] drm/i915: Limit the watermark to at least 8 entries on gen2/3

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

830 is very unhappy of the watermark value is too low (indicating a very
high watermark in fact, ie. memory fetch will occur with an almost full
FIFO). Limit the watermark value to at least 8 cache lines.

That also matches the burst size we use on most platforms. BSpec seems
to indicate we should limit the watermark to 'burst size + 1'. But on
gen4 we already use a hardcoded 8 as the watermark value (as the spec
says we should), so just use 8 as the limit on gen2/3 as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 45f71e6..5b683e8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1070,6 +1070,17 @@ static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
 		wm_size = wm->max_wm;
 	if (wm_size <= 0)
 		wm_size = wm->default_wm;
+
+	/*
+	 * Bspec seems to indicate that the value shouldn't be lower than
+	 * 'burst size + 1'. Certainly 830 is quite unhappy with low values.
+	 * Lets go for 8 which is the burst size since certain platforms
+	 * already use a hardcoded 8 (which is what the spec says should be
+	 * done).
+	 */
+	if (wm_size <= 8)
+		wm_size = 8;
+
 	return wm_size;
 }
 
-- 
1.8.5.5

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

Re: [PATCH] drm/i915: Limit the watermark to at least 8 entries on gen2/3

[Thomas Richter]

On 05.09.2014 20:54, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 830 is very unhappy of the watermark value is too low (indicating a very
> high watermark in fact, ie. memory fetch will occur with an almost full
> FIFO). Limit the watermark value to at least 8 cache lines.
>
> That also matches the burst size we use on most platforms. BSpec seems
> to indicate we should limit the watermark to 'burst size + 1'. But on
> gen4 we already use a hardcoded 8 as the watermark value (as the spec
> says we should), so just use 8 as the limit on gen2/3 as well.

/* snip */

That's certainly one way to fix it. Nothing against it. It might be more 
conservative to make this fix only in the calling context of the 830 and 
845 specific functions as I don't know whether the other chipsets have 
similar issues. I.e. one level up in the call-chain.

This being said, I can certainly confirm that the suggested modification 
works on i830.

One way or another, it seems to me that drm-intel-nightly which I pulled 
had a couple of more serious issues. I didn't even get X running, and 
monitor detection did not seem to work reliable, too. I had the same 
kind of instability on the external monitor for the console Ville's 
patches fixed so nicely, so it seems to me that there is something else 
that is missing. The new DVO code was definitely there, I checked that.

Thanks,
	Thomas


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

Re: [PATCH] drm/i915: Limit the watermark to at least 8 entries on gen2/3

[Ville Syrjälä]

On Fri, Sep 05, 2014 at 09:03:11PM +0200, Thomas Richter wrote:
> On 05.09.2014 20:54, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > 830 is very unhappy of the watermark value is too low (indicating a very
> > high watermark in fact, ie. memory fetch will occur with an almost full
> > FIFO). Limit the watermark value to at least 8 cache lines.
> >
> > That also matches the burst size we use on most platforms. BSpec seems
> > to indicate we should limit the watermark to 'burst size + 1'. But on
> > gen4 we already use a hardcoded 8 as the watermark value (as the spec
> > says we should), so just use 8 as the limit on gen2/3 as well.
> 
> /* snip */
> 
> That's certainly one way to fix it. Nothing against it. It might be more 
> conservative to make this fix only in the calling context of the 830 and 
> 845 specific functions as I don't know whether the other chipsets have 
> similar issues. I.e. one level up in the call-chain.
> 
> This being said, I can certainly confirm that the suggested modification 
> works on i830.
> 
> One way or another, it seems to me that drm-intel-nightly which I pulled 
> had a couple of more serious issues. I didn't even get X running, and 
> monitor detection did not seem to work reliable, too. I had the same 
> kind of instability on the external monitor for the console Ville's 
> patches fixed so nicely, so it seems to me that there is something else 
> that is missing. The new DVO code was definitely there, I checked that.

The DVO 2x clock fix was missing. I posted a new version. But I think
that should only affect the DVO output and not the VGA output. Anyway on
my 830 nightly with those two patches is pretty damn good. I'm going to
assume this is the first time in history when 830 works properly in linux
(well, excluding all the features we still haven't even implemented in
the driver). Only took 14 years or so to get there :P

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Limit the watermark to at least 8 entries on gen2/3

[Thomas Richter]

On 06.09.2014 19:33, Ville Syrjälä wrote:

> The DVO 2x clock fix was missing. I posted a new version. But I think
> that should only affect the DVO output and not the VGA output. Anyway on
> my 830 nightly with those two patches is pretty damn good. I'm going to
> assume this is the first time in history when 830 works properly in linux
> (well, excluding all the features we still haven't even implemented in
> the driver). Only took 14 years or so to get there :P

Many thanks for all that, it's really appreciated. (-: Still, what I did 
last week is to clone drm-intel-nightly and tested what I got this way. 
Is this not the recommended way to test whether the patches are in?

Greetings and have a nice weekend,

Thomas

Re: [PATCH] drm/i915: Limit the watermark to at least 8 entries on gen2/3

[Daniel Vetter]

On Fri, Sep 05, 2014 at 09:54:13PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 830 is very unhappy of the watermark value is too low (indicating a very
> high watermark in fact, ie. memory fetch will occur with an almost full
> FIFO). Limit the watermark value to at least 8 cache lines.
> 
> That also matches the burst size we use on most platforms. BSpec seems
> to indicate we should limit the watermark to 'burst size + 1'. But on
> gen4 we already use a hardcoded 8 as the watermark value (as the spec
> says we should), so just use 8 as the limit on gen2/3 as well.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch. And since this goes in through
-next the potential risk to other platforms is imo manageable. And we've
had other people complaint about serious underrun troubles on e.g.
underpowered i915gm atoms with big screens. Maybe this helps there like
for the i830m.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Limit the watermark to at least 8 entries on gen2/3

[Thomas Richter]

Am 08.09.2014 09:39, schrieb Daniel Vetter:
> On Fri, Sep 05, 2014 at 09:54:13PM +0300, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> 830 is very unhappy of the watermark value is too low (indicating a very
>> high watermark in fact, ie. memory fetch will occur with an almost full
>> FIFO). Limit the watermark value to at least 8 cache lines.
>>
>> That also matches the burst size we use on most platforms. BSpec seems
>> to indicate we should limit the watermark to 'burst size + 1'. But on
>> gen4 we already use a hardcoded 8 as the watermark value (as the spec
>> says we should), so just use 8 as the limit on gen2/3 as well.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Queued for -next, thanks for the patch. And since this goes in through
> -next the potential risk to other platforms is imo manageable. And we've
> had other people complaint about serious underrun troubles on e.g.
> underpowered i915gm atoms with big screens. Maybe this helps there like
> for the i830m.

Thanks, I'll give -next a spin on my 830 and 945 based machines. I don't 
think I have a 845 here, though.

Greetings,
	Thomas