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