On 14.07.2015 17:13, Michel Dänzer wrote: > 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 >>>>>>> Signed-off-by: Daniel Vetter >>>>>> >>>>>> 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 Recently, I'm occasionally (about once a week) getting a hang on DPMS off, see the attached dmesg snippet. I'm having trouble making sense of the backtraces; any opinions on whether this looks like an issue in the driver, DRM core or somewhere else? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer