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.