Re: [PATCH] drm/i915/tv: After disabling the pipe, use wait_for_vblank_off()

Previous thread: linux-next: manual merge of the block tree with the s390 tree by Stephen Rothwell on Monday, August 23, 2010 - 5:53 pm. (3 messages)

Next thread: [RFC -v2] kfifo writer side lock-less support by Huang Ying on Monday, August 23, 2010 - 6:42 pm. (8 messages)
From: Ivan Bulatovic
Date: Monday, August 23, 2010 - 6:00 pm

While in init3, resolution is native, KMS works fine, no problems at
all. As soon as gdm starts it seems that TV1 output is recognized as
connected even if it isn't so resolution on VGA output is degraded from
native 1280x1024 to 1024x768 on startup.

I've disabled LVDS output with video=LVDS-1:d because the cable looses
connection in certain lid positions.

2.6.36-rc1 didn't have this issue, 2.6.36-rc2 did so I've bisected and
it came down to this commit:

9d0498a2bf7455159b317f19531a3e5db2ecc9c4 is the first bad commit
commit 9d0498a2bf7455159b317f19531a3e5db2ecc9c4
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Wed Aug 18 13:20:54 2010 -0700

    drm/i915: wait for actual vblank, not just 20ms
    
Waiting for a hard coded 20ms isn't always enough to make sure a vblank
period has actually occurred, so add code to make sure we really have
passed through a vblank period (or that the pipe is off when disabling).
    
This prevents problems with mode setting and link training, and seems to
fix a bug like https://bugs.freedesktop.org/show_bug.cgi?id=29278, but
on an HP 8440p instead.  Hopefully also fixes
https://bugs.freedesktop.org/show_bug.cgi?id=29141.
    
    Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
2e407ad1d03b60f0e5b020313a8a63bd176b4005 M drivers

I'm sending you dmesg, xorg and xrandr logs as attachements. GM965 on
DELL Vostro 1310 is in question. Thanks,

Ivan
From: Chris Wilson
Date: Tuesday, August 24, 2010 - 12:49 am

Interesting. I am chasing a spurious TV connection on SDVO that has been
present since time immemorial. How certain are you that you've never have a
false detection in earlier kernels? It may not have affected the choice of
outputs, except sporadically, but it should be recorded in the debug logs
with drm.debug=0x6.

Does this make any difference?

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo
index ea2f4ab..e7ff378 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1454,7 +1454,7 @@ intel_sdvo_detect(struct drm_connector *connector,
                return connector_status_unknown;
        if (intel_sdvo->is_tv) {
                /* add 30ms delay when the output type is SDVO-TV */
-               mdelay(30);
+               mdelay(100);
        }
        if (!intel_sdvo_read_response(intel_sdvo, &response, 2))
                return connector_status_unknown;

Or (more likely):

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo
index ea2f4ab..77c455b 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1216,8 +1216,8 @@ static int intel_sdvo_dpms(struct drm_encoder
*encoder, in
                temp = I915_READ(intel_sdvo->sdvo_reg);
                if ((temp & SDVO_ENABLE) == 0)
                        intel_sdvo_write_sdvox(intel_sdvo, temp |
SDVO_ENABLE);
-               for (i = 0; i < 2; i++)
-                       intel_wait_for_vblank(dev, intel_crtc->pipe);
+
+               msleep(100);
 
                status = intel_sdvo_get_trained_inputs(intel_sdvo,
                                                       &input1, &input2);

-- 
Chris Wilson, Intel Open Source Technology Centre
--

From: Ivan Bulatovic
Date: Tuesday, August 24, 2010 - 1:35 am

Hi Chris. Well, I think that I had these problems before, around
2.6.28/2.6.29 but all I remember is that it was fixed relatively fast.
Everything worked well untill 2.6.36-rc2 popped up. Not even sporadical
events like these. What was strange about this that I wasn't able to
kill TV connection with xrandr, so I was stuck with 1024x768 and all of
a sudden upon subsequent restarts it somehow remembered that tv is
disconnected and VGA resolution is native.

xrandr -q

TV1 disconnected (normal left inverted right x axis y axis)
  848x480 (0x55)   14.5MHz
        h: width   848 start  849 end  912 total  944 skew    0 clock
15.4KHz
        v: height  480 start  481 end  512 total  513           clock
30.0Hz


cat /var/log/Xorg.0.log | grep TV

[    27.733] (II) intel(0): Output TV1 has no monitor section
[    27.988] (II) intel(0): EDID for output TV1
[    27.991] (II) intel(0): Printing probed modes for output TV1
[    27.991] (II) intel(0): Output TV1 connected
[    27.991] (II) intel(0): Output TV1 using initial mode 1024x768

Above patches didn't help when applied to 2.6.36-rc2. 

I'm going to compare dmesg between 2.6.35.3 and 2.6.36-rc2 with
drm.debug=0x6. Thanks,

Ivan

--

From: Chris Wilson
Date: Tuesday, August 24, 2010 - 1:50 am

Aye that would be a useful check.

-- 
Chris Wilson, Intel Open Source Technology Centre
--

From: Ivan Bulatovic
Date: Tuesday, August 24, 2010 - 2:12 am

Here are both dmesg outputs with drm.debug=0x6 in the attachements

cat dmesg.drm.35 | grep TV
[    1.006007] [drm:intel_tv_detect_type], No TV connection detected
[   26.391167] [drm:intel_tv_detect_type], No TV connection detected
[   26.539169] [drm:intel_tv_detect_type], No TV connection detected
[   26.805170] [drm:intel_tv_detect_type], No TV connection detected
[   26.953169] [drm:intel_tv_detect_type], No TV connection detected
[   31.935291] [drm:intel_tv_detect_type], No TV connection detected
[   32.083153] [drm:intel_tv_detect_type], No TV connection detected
[   32.352162] [drm:intel_tv_detect_type], No TV connection detected
[   32.500166] [drm:intel_tv_detect_type], No TV connection detected
[   32.819162] [drm:intel_tv_detect_type], No TV connection detected
[   32.967161] [drm:intel_tv_detect_type], No TV connection detected
[   42.666163] [drm:intel_tv_detect_type], No TV connection detected
[   42.814164] [drm:intel_tv_detect_type], No TV connection detected
[   43.083165] [drm:intel_tv_detect_type], No TV connection detected
[   43.231164] [drm:intel_tv_detect_type], No TV connection detected
[   43.500165] [drm:intel_tv_detect_type], No TV connection detected
[   43.648165] [drm:intel_tv_detect_type], No TV connection detected
[   47.134054] [drm:intel_tv_detect_type], No TV connection detected
[   47.281076] [drm:intel_tv_detect_type], No TV connection detected

cat dmesg.drm.36 | grep TV
[    1.487022] [drm:drm_crtc_helper_set_mode], [ENCODER:15:TV-15] set
[MODE:0:NTSC 480i]
[    1.691016] [drm:intel_tv_detect_type], No TV connection detected
[   25.939358] [drm:drm_crtc_helper_set_mode], [ENCODER:15:TV-15] set
[MODE:0:NTSC 480i]
[   26.143015] [drm:intel_tv_detect_type], No TV connection detected
[   26.264155] [drm:drm_crtc_helper_set_mode], [ENCODER:15:TV-15] set
[MODE:0:NTSC 480i]
[   26.269980] [drm:intel_tv_detect_type], Detected S-Video TV
connection
[   26.406164] [drm:drm_crtc_helper_set_mode], [ENCODER:15:TV-15] set
[MODE:0:NTSC 480i]
[   ...
From: Chris Wilson
Date: Tuesday, August 24, 2010 - 2:21 am

Ivan, can you try this patch?

Linus is going to make some more snide comments if he sees just how many
of these trivial-ish patches that I have pending...

---

If we need to wait until the next vblank for the register to be updated
and to take effect, make sure the write is actually flushed to the register
prior to sleeping.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_tv.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index d2029ef..19b9739 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1157,10 +1157,13 @@ intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		I915_WRITE(dspbase_reg, I915_READ(dspbase_reg));
 
 		/* Wait for vblank for the disable to take effect */
-		if (!IS_I9XX(dev))
+		if (!IS_I9XX(dev)) {
+			POSTING_READ(dspbase_reg);
 			intel_wait_for_vblank(dev, intel_crtc->pipe);
+		}
 
 		I915_WRITE(pipeconf_reg, pipeconf & ~PIPEACONF_ENABLE);
+		POSTING_READ(pipeconf_reg);
 		/* Wait for vblank for the disable to take effect. */
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
 
@@ -1268,11 +1271,15 @@ intel_tv_detect_type (struct intel_tv *intel_tv)
 		   DAC_C_0_7_V);
 	I915_WRITE(TV_CTL, tv_ctl);
 	I915_WRITE(TV_DAC, tv_dac);
+	POSTING_READ(TV_DAC);
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
 	tv_dac = I915_READ(TV_DAC);
 	I915_WRITE(TV_DAC, save_tv_dac);
 	I915_WRITE(TV_CTL, save_tv_ctl);
+	POSTING_READ(TV_CTL);
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
+
 	/*
 	 *  A B C
 	 *  0 1 1 Composite
-- 
1.7.1

--

From: Ivan Bulatovic
Date: Tuesday, August 24, 2010 - 4:03 am

I've patched it, unfortunately it doesn't help. :( Dmesg is in
I don't think so, this is fun, I haven't had problems with intel drivers


From: Chris Wilson
Date: Tuesday, August 24, 2010 - 8:11 am

Hopefully this is a contributing factor to the spurious TV detection
repoted by Ivan Bulatovic and others.

References:

  Bug 16871 - "TV1 connected" with no tv
  https://bugzilla.kernel.org/show_bug.cgi?id=16871

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reported-by: Ivan Bulatovic <combuster@gmx.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_tv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index f70bc76..f96289d 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1171,7 +1171,7 @@ intel_tv_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		I915_WRITE(pipeconf_reg, pipeconf & ~PIPEACONF_ENABLE);
 		POSTING_READ(pipeconf_reg);
 		/* Wait for vblank for the disable to take effect. */
-		intel_wait_for_vblank(dev, intel_crtc->pipe);
+		intel_wait_for_vblank_off(dev, intel_crtc->pipe);
 
 		/* Filter ctl must be set before TV_WIN_SIZE */
 		I915_WRITE(TV_FILTER_CTL_1, TV_AUTO_SCALE);
-- 
1.7.1

--

From: Ivan Bulatovic
Date: Tuesday, August 24, 2010 - 2:08 pm

Unfotunately I still get TV connection detected :(
From: Chris Wilson
Date: Tuesday, August 24, 2010 - 2:52 pm

Jesse spotted the root cause and Linus has now pushed that fix for
intel_wait_for_vblank() out. This is just a very minor bug in comparison.
Can you please test against linus/master and check that resolves the
spurious TV detection for you?
Thanks.

-- 
Chris Wilson, Intel Open Source Technology Centre
--

From: Ivan Bulatovic
Date: Tuesday, August 24, 2010 - 4:00 pm

Spurious TV detection is still here and + I have a new problem with
resolution in init3 when KMS fires up, it occupies only 1/3rd of the
screen when booting.

I've tried patching the kernel I used for testing so far with that one
liner and I've tried without patches we tried so far (git pull on a
clean linux-git) and the symptoms are the same on both of them.

So
- PIPE_VBLANK_INTERRUPT_STATUS) == 0,
+ PIPE_VBLANK_INTERRUPT_STATUS),
in intel_display.c doesn't fix the issue, plus it brings a new one in my
case.

--

From: Ivan Bulatovic
Date: Tuesday, August 24, 2010 - 4:04 pm

Spurious TV detection is still here and + I have a new problem with
resolution in init3 when KMS fires up, it occupies only 1/3rd of the
screen when booting.

I've tried patching the kernel I used for testing so far with that one
liner and I've tried without patches we tried so far (git pull on a
clean linux-git) and the symptoms are the same on both of them.

So
- PIPE_VBLANK_INTERRUPT_STATUS) == 0,
+ PIPE_VBLANK_INTERRUPT_STATUS),
in intel_display.c doesn't fix the issue, plus it brings a new one in my
case.


--

From: Jesse Barnes
Date: Tuesday, August 24, 2010 - 4:05 pm

On Wed, 25 Aug 2010 01:04:31 +0200

Ugg, one step forward, two steps back.  Working on it.

-- 
Jesse Barnes, Intel Open Source Technology Center
--

From: Ivan Bulatovic
Date: Tuesday, August 24, 2010 - 4:06 pm

Spurious TV detection is still here and + I have a new problem with
resolution in init3 when KMS fires up, it occupies only 1/3rd of the
screen when booting.

I've tried patching the kernel I used for testing so far with that one
liner and I've tried without patches we tried so far (git pull on a
clean linux-git) and the symptoms are the same on both of them.

So
- PIPE_VBLANK_INTERRUPT_STATUS) == 0,
+ PIPE_VBLANK_INTERRUPT_STATUS),
in intel_display.c doesn't fix the issue, plus it brings a new one in my
case.

/edit: had to remove linux-gfx from cc or mail wouldn't send otherwise

--

From: Sitsofe Wheeler
Date: Wednesday, August 25, 2010 - 1:15 am

Does the patch in http://lkml.org/lkml/2010/8/24/280 change anything for
you? If so I'm guessing we are seeing the same issue...

-- 
Sitsofe | http://sucs.org/~sits/
--

From: Ivan Bulatovic
Date: Wednesday, August 25, 2010 - 2:17 am

Ok, gonna try it out now. As for Jesse's question about whether there
are timeouts on the wait_for_vblank function, I can see at least 30
[drm_intel_wait_for_vblank], vblank wait timed out printks all over my
dmesg.

--

From: Ivan Bulatovic
Date: Wednesday, August 25, 2010 - 2:31 am

No, that doesn't help, I still get TV connection detected. But the wait
for vblank timeout printks are gone as expected.

--

From: Kan-Ru Chen
Date: Wednesday, August 25, 2010 - 2:51 am

Same here with my Acer 3810T, Intel GM45, the kernel was built from git pulled today.

v2.6.36-rc2-203-g502adf5

TV1 connected (normal left inverted right x axis y axis)
   848x480        30.0 +
   640x480        30.0 +
   1024x768       30.0  
   800x600        30.0  

and the fb only occupies 1/3rd of the screen, the fbset output

mode "848x480"
    geometry 848 480 1366 768 32
    timings 0 0 0 0 0 0 0
    rgba 8/16,8/8,8/0,0/0
endmode


--

From: Pekka Enberg
Date: Sunday, August 29, 2010 - 4:29 am

Hi all,


I'm still seeing wrong resolution with my i915 Macbook with latest
Linus' git. Is there a fix in the pipeline or should we start looking
for a commit to revert? As a slab maintainer, I'm supposed to be able
to work with -rc kernels and this particular bug is pretty damn
annoying.

                           Pekka
--

From: Sitsofe Wheeler
Date: Sunday, August 29, 2010 - 6:37 am

I've been seeing a variety of strange behaviour since
9d0498a2bf7455159b317f19531a3e5db2ecc9c4 too (I found I could narrow it
down in http://lkml.org/lkml/2010/8/24/280 but the change mentioned
there doesn't seem to help others). I've filed
https://bugs.freedesktop.org/show_bug.cgi?id=29857 which summarises the
different strange behaviour that I've seen generated and the steps I've
used to reproduce them. Unfortunately this week I'm not going to have
much time for the tests I was able to do last week.

-- 
Sitsofe | http://sucs.org/~sits/
--

From: Pekka Enberg
Date: Tuesday, August 24, 2010 - 1:54 am

FWIW, I saw this on my MacBook as well. I was debugging a boot time
oops at the time so I just changed screen resolution in Gnome to fix
it.
--

From: Maciej Rutecki
Date: Sunday, August 29, 2010 - 5:59 am

I created a Bugzilla entry at 
https://bugzilla.kernel.org/show_bug.cgi?id=17301
for your bug report, please add your address to the CC list in there, thanks!

-- 
Maciej Rutecki
http://www.maciek.unixy.pl
--

Previous thread: linux-next: manual merge of the block tree with the s390 tree by Stephen Rothwell on Monday, August 23, 2010 - 5:53 pm. (3 messages)

Next thread: [RFC -v2] kfifo writer side lock-less support by Huang Ying on Monday, August 23, 2010 - 6:42 pm. (8 messages)