On Sun, Dec 16, 2012 at 12:11:13PM +0100, Daniel Vetter wrote:
On Sun, Dec 16, 2012 at 7:04 AM, Dave Airlie airlied@gmail.com wrote:
The "I can't test it" argument doesn't fly with me, you are creating a new API that adds a useful feature, it should be possible to get a few other driver maintainers interested, but also I'd hope that most of the ideas for converting to a new API would be mechanical in a lot of ways. Atomic semantics aren't hw specific from what i can see, if they are then the API is obviously wrong.
For actual semantics I think we need to split the discussion into the modeset and the pageflip part:
- For modeset it shouldn't be hard to whip up some helpers which use
the current crtc helpers to do modeset changes accross multiple crtcs. Like currently, drivers might refuse a modeset in one of their callbacks, and that might only happen once the hw touching has started already. Which means we can't efficiently implement a check flag mode for those drivers. But otoh if they have global constraints they might want to implement their own magic like i915. Or we could add a new global ->check_modeset callback which checks for these (after crtc/encoders have done the respective checking).
Like I said, my initial version was based on drm_crtc_helper, so it would work as a starting point.
- For pageflip things are a bit messier, since we really should aim
for all changes to be applied on the same vblank. But luckily the set of drivers which support more than one of cursors/pageflips/plane is manageable, and most are just cursosrs+pageflip. Since I've just read through them all I think it shouldn't be too hard to whip up a (totally broken) patch for each of them to guide driver maintainers.
OK so finally a volunteer to help. Great ;)
So imo to really exploit atomic modeset/pageflip we need special support from each driver to check a given configuration before committing it (this also seems to be the only sane way for userspace to figure out what works and what doesn't without causing tons of flickering). And doing that for each driver is obviously out of the question.
But reworking internal interfaces and converting drivers in a simplistic fashion which assume that no global state checking is required (assuming e.g. for modesets the current crtc/encoder checks are good enough) is a requirement I agree with. Which is why I think we should aim first for something much more restricted than the current "every possible feature for kms" approach, for example just pageflips on one crtc ...
Such a restricted feature is useless for any real world usage. At the very least you need to enable/disable planes as well as do pageflips. And you probably want to move/resize active planes too.
Since we already know that there are a lot more properties we want to manipulate atomically, adding a "pageflip only" restrictions to the API itself is simply counter productive. We will have to replace the API immediately anyway, and then we need another year or so of bikeshedding to get the replacement API in. Let's just get it in from the start.
In many cases once you've implemented the basic atomic semantics, adding other properties to be included in the atomic update is trivial. Often it's just a matter of setting some extra bits, or including some extra registers in the atomic commit.
I have in mind several plane properties I want to add to i915, which I could do trivially. But doing that before the basic feature is merged would bloat the patchset further.
If you want to restrict which properties you allow to be changed in one atomic operation, you can do that trivially in the driver itself.
From an i915 perspective I'm leaning towards pageflips because things are much simpler there (similar to all other drivers). It's much more immediately useful for us and for atomic modeset our driver-internal code is simple not yet ready to do global state-checking up-front before we start touching hw - we need to move around quite a bit of code until that's doable. So atomic modeset won't really help us yet to for 3-pipe pll sharing and similar shared global resources issues and how to correctly expose them to userspace.
It does make life for userspace more complex because userspace can't setup planes as a part of a modeset. Also all scanout duties should be moved over to drm_plane at some point, so planes will have to be part of the modeset eventually.