On 28.05.2015 18:03, Michel Dänzer wrote: > On 28.05.2015 17:38, Daniel Vetter wrote: >> On Thu, May 28, 2015 at 04:11:53PM +0900, Michel Dänzer wrote: >>> On 27.05.2015 18:41, Daniel Vetter wrote: >>>> On Wed, May 27, 2015 at 06:21:24PM +0900, Michel Dänzer wrote: >>>>> On 27.05.2015 18:04, Daniel Vetter wrote: >>>>>> These should be functionally equivalent to the older per/post modeset >>>>>> functions, except that they block out drm_vblank_get right away. >>>>>> There's only the clock adjusting code (outside of pageflips) in >>>>>> readone which uses drm_vblank_get. But that code doesn't synchronize >>>>>> against concurrent modesets and instead handles any such races by >>>>>> waiting for the right vblank to arrive with a short timetout. >>>>>> >>>>>> The longer-term plan here is to switch all kms drivers to >>>>>> drm_vblank_on/off so that common code like pending event cleanup can >>>>>> be done there, while drm_vblank_pre/post_modeset will be purely >>>>>> drm internal for the old UMS ioctl. >>>>>> >>>>>> Note that the kerneldoc for pre/post_modeset is wrong since as Michel >>>>>> Dänzer correctly pointed out it works if only using pre/post_modeset. >>>>>> The trouble that lead to this comment is the very old version of >>>>>> drm_vblank_off to clear out pending events when disabling a pipe, >>>>>> which did seem to wreak havoc with the trick used by pre/post_modeset. >>>>>> Michel also expressed dissatisfaction with intel folks pushing new >>>>>> interfaces with bogus justifications. I still maintain that having a >>>>>> consistent set of vblank behaviour across kms drivers, separate from >>>>>> any old UMS functions is a useful goal. >>>>>> >>>>>> Cc: Michel Dänzer <michel.daenzer@xxxxxxx> >>>>>> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> >>>>> >>>>> Can you describe at least one tangible benefit this change provides for >>>>> the radeon driver? >>>>> >>>>> Because I'm afraid that this might cause subtle breakage, and since we >>>>> don't have any rigorous tests for this like in intel-gpu-tools (yet?), >>>>> it might be painful to track it down. >>>>> >>>>> So, I'd like to have a good reason for taking the risk. >>>> >>>> right now at most a bit of code to clean out pending events on modeset >>>> disable, for somewhat consistent behaviour with other drivers. But in >>>> general it's fairly ill-defined what happens with vblank events. >>> >>> Yeah, while that's nice to have, I don't think it makes too much >>> difference in practice. >>> >>> Anyway, I'm giving this patch a spin, and it does indeed cause userspace >>> fallout, at least with DRI3/Present enabled, because the vblank and >>> pageflip ioctls now return -EINVAL while the CRTC is off. However, it >>> looks like fixing that up might not be too bad, so I'm cautiously >>> optimistic for this change. But I'd like some more time for testing and >>> fixing userspace. > > [...] > >> Otoh asking for a vblank event on a dead pipe smells like a userspace bug >> and could result in stuck compositors. Not sure what's best here really. > > Agreed, and we're already careful not to do that with DRI2, just not yet > with DRI3/Present (which isn't in any xf86-video-ati release yet). I've fixed up the DRI3/Present code as well. This patch (with maybe some cleanups to the commit log, in particular I'm not sure the third paragraph should be there) is Acked-and-Tested-by: Michel Dänzer <michel.daenzer@xxxxxxx> -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
