Created attachment 24619 [details] Xorg.0.log I am using latest xorg and intel drives on 2.6.29-zen2. When I run video in mplayer with xv video output, I get very obvious tearing. Changing the value of XV_SYNC_TO_VBLANK does not affect it. I have tried running mplayer in just X (no window or compositing manager) and that doesn't help either.
Created attachment 24620 [details] xorg.conf
Created attachment 24621 [details] dmesg
Does this commit work for you? commit bc3312fd7c03d09a231dfebfe390fe668ad15d1e Author: Carl Worth <cworth@cworth.org> Date: Mon Apr 6 11:16:40 2009 -0700 Use WAIT_FOR_SCAN_LINE instead of WAIT_FOR_VBLANK
(In reply to comment #3) > Does this commit work for you? > > commit bc3312fd7c03d09a231dfebfe390fe668ad15d1e > Author: Carl Worth <cworth@cworth.org> > Date: Mon Apr 6 11:16:40 2009 -0700 > > Use WAIT_FOR_SCAN_LINE instead of WAIT_FOR_VBLANK > I have tried this commit on 2.7 branch, it turns out it's working on pipe 1, but not working on pipe 0. Here's the results: on 915GM, with a LVDS output on pipe 1, it's working on Q965, with a VGA output on pipe 0, it's not working on 945GME, with LVDS and VGA, it's working, if turn off LVDS, VGA is not working. And if I roll back to commit 5944f5e32511984b11decc0df6074600e1989934, both pipes are working. Carl, could you have a look at this?
(In reply to comment #4) > (In reply to comment #3) > > Does this commit work for you? > > > > commit bc3312fd7c03d09a231dfebfe390fe668ad15d1e > > Author: Carl Worth <cworth@cworth.org> > > Date: Mon Apr 6 11:16:40 2009 -0700 > > > > Use WAIT_FOR_SCAN_LINE instead of WAIT_FOR_VBLANK > > > > I have tried this commit on 2.7 branch, it turns out it's working on pipe 1, > but not working on pipe 0. Here's the results: > on 915GM, with a LVDS output on pipe 1, it's working > on Q965, with a VGA output on pipe 0, it's not working > on 945GME, with LVDS and VGA, it's working, if turn off LVDS, VGA is not > working. > > And if I roll back to commit 5944f5e32511984b11decc0df6074600e1989934, both > pipes are working. > > Carl, could you have a look at this? > Just found the MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW should be bit 1. diff --git a/src/i810_reg.h b/src/i810_reg.h index c964569..5211400 100644 --- a/src/i810_reg.h +++ b/src/i810_reg.h @@ -2442,7 +2442,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #define MI_WAIT_FOR_PIPEB_VBLANK (1<<7) #define MI_WAIT_FOR_PIPEB_SCAN_LINE_WINDOW (1<<5) #define MI_WAIT_FOR_PIPEA_VBLANK (1<<3) -#define MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW (1<<2) +#define MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW (1<<1) /* Set the scan line for MI_WAIT_FOR_PIPE?_SCAN_LINE_WINDOW */ #define MI_LOAD_SCAN_LINES_INCL
(In reply to comment #3) > Does this commit work for you? > > commit bc3312fd7c03d09a231dfebfe390fe668ad15d1e > Author: Carl Worth <cworth@cworth.org> > Date: Mon Apr 6 11:16:40 2009 -0700 > > Use WAIT_FOR_SCAN_LINE instead of WAIT_FOR_VBLANK > No. It didn't work before that commit and it isn't working after it either.
(In reply to comment #5) > Just found the MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW should be bit 1. > > diff --git a/src/i810_reg.h b/src/i810_reg.h > index c964569..5211400 100644 > --- a/src/i810_reg.h > +++ b/src/i810_reg.h > @@ -2442,7 +2442,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > #define MI_WAIT_FOR_PIPEB_VBLANK (1<<7) > #define MI_WAIT_FOR_PIPEB_SCAN_LINE_WINDOW (1<<5) > #define MI_WAIT_FOR_PIPEA_VBLANK (1<<3) > -#define MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW (1<<2) > +#define MI_WAIT_FOR_PIPEA_SCAN_LINE_WINDOW (1<<1) > > /* Set the scan line for MI_WAIT_FOR_PIPE?_SCAN_LINE_WINDOW */ > #define MI_LOAD_SCAN_LINES_INCL Thanks for the catch! I had also noticed that my fix wasn't working on both pipes, and I double-checked the values for MI_LOAD_SCAN_LINES_DISPLAY_PIPEA vs. MI_LOAD_SCAN_LINES_DISPLAY_PIPEB, but forgot to look at these values as well. I just tested with this fix, and I now get tear-free video on either pipe, (whether only a single pipe is enabled or both). Of course, if I have clone-mode enabled, then I only get tear-free video on one of the two pipes, but that is expected, (and is the same as the behavior before my changes). I've now pushed this patch out to both master and the 2.7 branch. Eric, can you see what you get with this change as well, (on top of my previous change mentioned earlier). -Carl
No, still no change. Is there any additional information I could provide that would help?
(In reply to comment #8) > No, still no change. Is there any additional information I could provide that > would help? Hi Eric, It's strange that things aren't working for you. From your Xorg.log file it looks like you have two displays enabled, (one LVDS---such as a laptop panel, and one VGA---such as an external monitor). Is that correct? If so, does the tearing occur on both displays? It's actually expected that tearing will occur on one of the displays if both are enabled. If you want tear-free video on a specific display, one option to get that is to turn the other display off, with a command such as: xrandr --output VGA --off or: xrandr --output LVDS --off Give those a try and see if you don't have any luck. Otherwise, I'm stumped as to what the difference might be that things are working for me but not for you. -Carl
(In reply to comment #9) > (In reply to comment #8) > > No, still no change. Is there any additional information I could provide that > > would help? > > Hi Eric, > > It's strange that things aren't working for you. > > From your Xorg.log file it looks like you have two displays enabled, (one > LVDS---such as a laptop panel, and one VGA---such as an external monitor). Is > that correct? If so, does the tearing occur on both displays? > > It's actually expected that tearing will occur on one of the displays if both > are enabled. If you want tear-free video on a specific display, one option to > get that is to turn the other display off, with a command such as: > > xrandr --output VGA --off > > or: > > xrandr --output LVDS --off > > Give those a try and see if you don't have any luck. > > Otherwise, I'm stumped as to what the difference might be that things are > working for me but not for you. > > -Carl > I only have a VGA connected, but an LVDS shows up as connected because of a bug in the detection code. (Bug #20089 had fixed this, but the patch was later removed because it caused other problems). I now manually disable it as you can see in my xorg.conf. Here is what my xrandr shows: gentoo ~ # xrandr --verbose Screen 0: minimum 320 x 200, current 1280 x 1024, maximum 2048 x 2048 VGA1 connected 1280x1024+0+0 (0x3c) normal (normal left inverted right x axis y axis) 338mm x 270mm Identifier: 0x3b Timestamp: 52520126 Subpixel: unknown Clones: CRTC: 0 CRTCs: 0 1 Transform: 1.000000 0.000000 0.000000 0.000000 1.000000 0.000000 0.000000 0.000000 1.000000 filter: 1280x1024 (0x3c) 108.0MHz +HSync +VSync *current +preferred h: width 1280 start 1328 end 1440 total 1688 skew 0 clock 64.0KHz v: height 1024 start 1025 end 1028 total 1066 clock 60.0Hz 1280x1024 (0x3d) 135.0MHz +HSync +VSync h: width 1280 start 1296 end 1440 total 1688 skew 0 clock 80.0KHz v: height 1024 start 1025 end 1028 total 1066 clock 75.0Hz ...
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > No, still no change. Is there any additional information I could provide that > > > would help? > I only have a VGA connected, but an LVDS shows up as connected because of a bug > in the detection code. (Bug #20089 had fixed this, but the patch was later > removed because it caused other problems). I now manually disable it as you can > see in my xorg.conf. OK. I was just taking a guess, which is all I can do from here, I'm afraid. I think your next best bet is to go sprinkling some ErrorF calls into src/i830_video.c to see if the logic is wrong and its not even attempting to sync, or if the logic is correct, but for some reason the sync commands aren't behaving on your GPU. In particular, it's the I830PutImage function that you'll want to instrument, and the critical code is happening in the "if (sync)" block. So you might even start with just: if (sync) { ErrorF ("In if (sync) block"); ... } and then look in your log file to see if that message is getting there. And then poke around similarly to ensure its syncing on the correct pipe, etc. -Carl
> OK. I was just taking a guess, which is all I can do from here, I'm afraid. > > I think your next best bet is to go sprinkling some ErrorF calls into > src/i830_video.c to see if the logic is wrong and its not even attempting to > sync, or if the logic is correct, but for some reason the sync commands aren't > behaving on your GPU. > > In particular, it's the I830PutImage function that you'll want to instrument, > and the critical code is happening in the "if (sync)" block. So you might even > start with just: > > if (sync) { > ErrorF ("In if (sync) block"); > ... > } > > and then look in your log file to see if that message is getting there. > > And then poke around similarly to ensure its syncing on the correct pipe, etc. > > -Carl > Ok, so I added some ErrorF calls and it does get inside of "if (sync)". I don't really understand how it works, but I was expecting pipe to be set to either 0 or 1 based on the output for crtc in xrandr --verbose. But crtc->pipe has value 8585296 (5 lines into "if (sync)"), and pipe is set to 1048576 (6 lines later). Could this indicate a problem?
(In reply to comment #12) > Ok, so I added some ErrorF calls and it does get inside of "if (sync)". I > don't really understand how it works, but I was expecting pipe to be set to > either 0 or 1 based on the output for crtc in xrandr --verbose. But crtc->pipe > has value 8585296 (5 lines into "if (sync)"), and pipe is set to 1048576 (6 > lines later). Could this indicate a problem? Interesting. So that value of 8585296 is something we need to figure out where it's coming from. The code in the driver is certainly written to expect that intel_crtc == 0 means pipe A, and any non-zero value means pipe B. Meanwhile, the value for "pipe" of 1048576 (== 1<<20) is expected for pipe B, (see the definition of MI_LOAD_SCAN_LINES_DISPLAY_PIPEB in i810_reg.h. So it could be that the problem is just that the driver is trying to sync on pipe B when it should be syncing on pipe A. Why don't you try changing the conditional inside the "if (sync)" block so that both "event" and "pipe" are set for pipe A, and see if that fixes your problem. If so, we'll next just need to figure out why the code isn't getting what it expects in intel_crtc->pipe. -Carl
(In reply to comment #13) > Interesting. So that value of 8585296 is something we need to figure out where > it's coming from. The code in the driver is certainly written to expect that > intel_crtc == 0 means pipe A, and any non-zero value means pipe B. > > Meanwhile, the value for "pipe" of 1048576 (== 1<<20) is expected for pipe B, > (see the definition of MI_LOAD_SCAN_LINES_DISPLAY_PIPEB in i810_reg.h. > > So it could be that the problem is just that the driver is trying to sync on > pipe B when it should be syncing on pipe A. Why don't you try changing the > conditional inside the "if (sync)" block so that both "event" and "pipe" are > set for pipe A, and see if that fixes your problem. > > If so, we'll next just need to figure out why the code isn't getting what it > expects in intel_crtc->pipe. > > -Carl > Yes, that seems to be the problem. Forcing "event" and "pipe" to be set to pipe A gives me beautiful tear-free video! :) I'll try to figure out where intel_crtc->pipe gets the bad value, but any hints on where to look would be be helpful! Thanks, Eric
(In reply to comment #14) > Yes, that seems to be the problem. Forcing "event" and "pipe" to be set to > pipe A gives me beautiful tear-free video! :) I'll try to figure out where > intel_crtc->pipe gets the bad value, but any hints on where to look would be be > helpful! Excellent news! You can start in i830_crtc_init where it should first be getting assigned. I don't see any other assignments to this value in a quick glance at the code. So if it's correct there, then you'll likely need to next attach a debugger and set a watch on the *(&intel_crtc->pipe) value to get the debugger to stop at the point where the value is getting smashed. Thanks again for your help here. Let me know if you need anything more. -Carl
(In reply to comment #15) > (In reply to comment #14) > > Yes, that seems to be the problem. Forcing "event" and "pipe" to be set to > > pipe A gives me beautiful tear-free video! :) I'll try to figure out where > > intel_crtc->pipe gets the bad value, but any hints on where to look would be be > > helpful! > > Excellent news! > > You can start in i830_crtc_init where it should first be getting assigned. I > don't see any other assignments to this value in a quick glance at the code. So > if it's correct there, then you'll likely need to next attach a debugger and > set a watch on the *(&intel_crtc->pipe) value to get the debugger to stop at > the point where the value is getting smashed. > > Thanks again for your help here. Let me know if you need anything more. > > -Carl > I finally had some time to look at this, but I'm a little confused. It seems like the only place that i830_crtc_init is actually called is inside of I830AccelMethodInit on line 1680 of i830_driver.c. But I830AccelMethodInit is only called if pI830->use_drm_mode is false (which it isn't, see line 1913 of i830_driver.c). A simple grep of the source shows that i830_crtc_init is the only place intel_crtc->pipe is assigned. So since i830_crtc_init doesn't seem to be called, where else should I look? Sorry if I'm missing something obvious because of my nonexistent programming skills.. Eric
(In reply to comment #16) > I finally had some time to look at this, but I'm a little confused. It seems > like the only place that i830_crtc_init is actually called is inside of > I830AccelMethodInit on line 1680 of i830_driver.c. But I830AccelMethodInit is > only called if pI830->use_drm_mode is false (which it isn't, see line 1913 of > i830_driver.c). > A simple grep of the source shows that i830_crtc_init is the only place > intel_crtc->pipe is assigned. So since i830_crtc_init doesn't seem to be > called, where else should I look? > Sorry if I'm missing something obvious because of my nonexistent programming > skills.. It's not a problem with your programming skills. I'm sorry I led you astray. So there are two different paths in the driver---one for kernel mode-setting (KMS) and one for user mode-setting (UMS), as controlled by the "use_drm_mode" flag, (which would be more clearly named use_kms, but there you have it). So, anyway, if you are using the KMS case, then the code that initializes intel_crtc->pipe is instead in the kernel source, at drivers/gpu/drm/i915/intel_display.c:intel_crtc_init. One thing you could do before diving into the kernel is to see if you get tear-free XV with KMS turned off. For this, before starting the X server, try the following: sudo rmmod i915 sudo modprobe i915 modeset=0 Then start X and see if intel_crtc->pipe is set correctly, and see if you get the desired tear-free behavior. -Carl
(In reply to comment #17) > One thing you could do before diving into the kernel is to see if you get > tear-free XV with KMS turned off. For this, before starting the X server, try > the following: I've done this now the opposite direction. I enabled KMS and have now successfully reproduced the bug (hurrah!). The bug is definitely KMS-specific, so I've re-titled it that way. Maybe the kernel and user-space code are seeing different definitions for the intel_crtc struct somehow? Anyway, I'll investigate and keep you posted. Thanks again for your debugging efforts that pointed us the right direction to find where the bug is at least. -Carl
I've talked with Jesse a bit about this bug. The code in i830_video.c is definitely very broken for the KMS case. It's misinterpreting some memory so getting effectively random data and acting on it. Also, what the code wants is the pipe value corresponding to a given CRTC, but in the KMS case, the kernel driver doesn't expose this information at all. So we're likely going to need some new interface in the kernel driver to do what we want. And we already knew that we wanted a way for arbitrary applications, (other than XV users---compositing managers for example), to ask for a tearing-free copyregion operation. This will be necessary to allow for tear-free XV while a compositing manager is running. So hopefulyl we can do one piece of work and eliminate XV tearing for both the KMS and the compositing-manager cases. I'll try to learn this week exactly what will be needed to pull that off. -Carl
Option "Device" "/dev/input/event2 ^^^ this line in the original xorg.conf has a syntax error, the missing '"', does it matter ?
(In reply to comment #20) > Option "Device" "/dev/input/event2 > ^^^ this line in the original xorg.conf has a syntax error, the missing '"', > does it matter ? > It's no problem. I must have accidentally deleted it while I was removing some commented lines.
(In reply to comment #19) > So we're likely going to need some new interface in the kernel driver to do > what we want. And we already knew that we wanted a way for arbitrary > applications, (other than XV users---compositing managers for example), to ask > for a tearing-free copyregion operation. This will be necessary to allow for > tear-free XV while a compositing manager is running. > > So hopefulyl we can do one piece of work and eliminate XV tearing for both the > KMS and the compositing-manager cases. > > I'll try to learn this week exactly what will be needed to pull that off. The code to enable tear-free syncing of video-playback with KMS has now all landed in the xf86-video-intel driver repository as well as in the upstream kernel repository, (ie. Linus' repository as of v2.6.30-rc6), and in the 2.4.11 libdrm (which has already been released as well). So getting the code for each of those from git will fix the bug. And once the following releases happen, they will contain the fix as well: Linux: 2.6.30 libdrm: 2.4.11 xf86-video-intel: 2.8.0 -Carl
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.