Daniel Vetter <daniel@xxxxxxxx> writes: > On Wed, Feb 01, 2017 at 11:58:16AM -0800, Eric Anholt wrote: >> Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> writes: >> >> > On Tue, 31 Jan 2017, Eric Anholt <eric@xxxxxxxxxx> wrote: >> >> Martin Peres <martin.peres@xxxxxxxxxxxxxxx> writes: >> >> >> >>> Despite all the careful planing of the kernel, a link may become >> >>> insufficient to handle the currently-set mode. At this point, the >> >>> kernel should mark this particular configuration as being broken >> >>> and potentially prune the mode before setting the offending connector's >> >>> link-status to BAD and send the userspace a hotplug event. This may >> >>> happen right after a modeset or later on. >> >>> >> >>> When available, we should use the link-status information to reset >> >>> the wanted mode. >> >>> >> >>> Signed-off-by: Martin Peres <martin.peres@xxxxxxxxxxxxxxx> >> >> >> >> If I understand this right, there are two failure modes being handled: >> >> >> >> 1) A mode that won't actually work because the link isn't good enough. >> >> >> >> 2) A mode that should work, but link parameters were too optimistic and >> >> if we just ask the kernel to set the mode again it'll use more >> >> conservative parameters that work. >> >> >> >> This patch seems good for 2). For 1), the drmmode_set_mode_major is >> >> going to set our old mode back. Won't the modeset then fail to link >> >> train again, and bring us back into this loop? The only escape that I >> >> see would be some other userspace responding to the advertised mode list >> >> changing, and then asking X to modeset to something new. >> >> >> >> To avoid that failure busy loop, should we re-fetching modes at this >> >> point, and only re-setting if our mode still exists? >> > >> > Disclaimer: I don't know anything about the internals of the modesetting >> > driver. >> > >> > Perhaps we can identify the two cases now, but I'd put this more >> > generally: if the link status has gone bad, it's an indicator to >> > userspace that the circumstances may have changed, and userspace should >> > query the kernel for the list of available modes again. It should no >> > longer trust information obtained prior to getting the bad link status, >> > including the current mode. >> > >> > But specifically, I think you're right, and AFAICT asking for the list >> > of modes again is the only way for the userspace to distinguish between >> > the two cases. I don't think there's a shortcut for deciding the current >> > mode is still valid. >> >> To avoid the busy-loop problem, I think I'd like this patch to re-query >> the kernel to ask about the current mode list, and only try to re-set >> the mode if our mode is still there. > > Yeah, I guess that sounds like a reasonable heuristics. More integrated > compositors (aka the wayland ones) might be able to make more useful > decisions, but for -modesetting that's probably the best we can do. > >> If the mode isn't there, then it's up to the DE to take action in >> response to the notification of new modes. If you don't have a DE to >> take appropriate action, you're kind of out of luck. >> >> As far as the ABI goes, this seems fine to me. The only concern I had >> about ABI was having to walk all the connectors on every uevent to see >> if any had gone bad -- couldn't we have a flag of some sort about what >> the uevent indicates? But uevents should be super rare, so I'd say the >> kernel could go ahead with the current plan. > > We've discussed finer-grained uevent singalling a few times, and I'd like > that to be an orthogonal abi extension. Right now we just don't have the > required tracking even within the kernel to know which connector changed > (and the tracking we do have missed a few things, too). My idea is to push > the tracking into the leaves of the callchains with a function that > increases some per-connector epoch counter. Then we'd just have to expose > that somehow cheaply to userspace (could probably just send it along in > the uevent). Plus expose it as a read-only property so that userspace can > re-synchronize. > > But again, that should be an orthogonal thing imo. Yeah, that was roughly my thought process, too.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
