[PATCH] xf86-video-ati: vblank wait on crtc > 1

[Michel Dänzer]

On Die, 2011-03-22 at 09:53 -0500, Ilija Hadzic wrote: 
> 
> On Tue, 22 Mar 2011, Michel [ISO-8859-1] Dnzer wrote:
> 
> >
> > In the post I referenced above, you said:
> >
> >> [...] I'll add a hook to the DDX to check the version and not issue
> >> the ioctl at all if it is requested on a higher CRTC. I think that's
> >> better than falling back to the old style and issuing the system call
> >> on (potentially wrong) CRTC #1 because that can block the application
> >> (and I'd rather see it proceed without attempting vblank
> >> synchronization, then block).
> >
> > Which made sense then and still does now, in contrast to trying to
> > preserve ill-defined broken behaviour.
> >
> 
> ... and then as I started to write code I changed my mind (am I forgiven ? 
> ;-) ) because I realized that three things would happen:
> 
> a) just not issuing the ioctl will cause an application to "firehose" 
> the kernel with rendering commands and potentially impact the performance 
> of other processes.
> 
> b) both behaviors (not issuing the ioctl and thus causing application to 
> keep coming back after glxSwap or just blocking the application on
> bad CRTC) are broken anyway so introducing a wider variety of 
> broken behaviors was probably worse as far as user's experience is 
> concerned.

Not calling the ioctl doesn't imply returning immediately.

Your changes only fix the bug you found (the X radeon driver calls the
ioctl when that doesn't make sense) when both the kernel and X driver
are updated, but it would be possible to also fix it when only the X
driver is updated.


> >> I bet the change on my desk in my office that if I had the blankspace,
> >> someone would have responded with an opposite suggestion ;-).
> >
> > That's bold of you. I stand by my request.
> 
> Next time I touch this file, I'll "smuggle" the blankspace, but let's say 
> that this is a rock bottom of the priority list (just as fixing grammar, 
> style and spelling in any message would be  ... and there is plenty).

Then I suppose applying this patch is rock bottom of my priority list...
But, as it seems you'd rather argue in your changes than adjust them to
reviews, I can also just fix this part up after it goes in in the worst
case.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer