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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tue, 22 Mar 2011, Michel [ISO-8859-1] Däer 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.

c) the change the way I made it is safer wrt. introducing new bugs; you can't as easily fake out the sequence number and timestamp as you may think.

I explained all of the above in my followup posts and although I didn't want to repeat them now, I now find myself repeating it anyway ;-)



+	} else {
+	    if (cap_value) {
+		info->high_crtc_works = TRUE;
+	    } else {
+		xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "Your kernel does not handle VBLANKs on CRTC>1\n");
+		info->high_crtc_works = FALSE;

Is there any point in having two different warning messages? I think
'CRTC > 1' could use spaces.

There is a point: one warning tells you that the kernel is old and you
have to upgrade. The other warning tells you that the kernel is new enough
(it has the GET_CAP ioctl), but for some other reason refused to handle
high-crtcs (which at this time doesn't exist, but it should not be the
reason to "destroy" the information).

Fair enough.


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).

-- Ilija
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux