On Sat, Sep 15, 2012 at 9:53 AM, Ville Syrjälä <syrjala@xxxxxx> wrote: > On Fri, Sep 14, 2012 at 05:46:35PM -0400, Kristian Høgsberg wrote: >> I think (hope) the consensus coming out of this thread is something >> along these lines: >> >> - We use properties for specifying what to change to be future >> compatible with new crtc features, but also to allow exposing >> hw-specific properties and tie them into the atomicity of the >> pageflip. The KMS overlays are a lowest-common denominator for all >> the various overlay types out there and it should be possible to write >> a piece of chipset specific compositor code to use features that can't >> be expressed through KMS overlays. > > Properties are good. Check. > >> - We have two types of properties: dynamic and non-dynamic ones. >> Dynamic properties can always be changed in the next frame (fb bos, hw >> cursor position, overlay position, for example), non-dynamic >> properties typically involve changing the way bandwidth are allocated >> and changing them may fail. > > There's just no way to make such a general split. The simple fact is > that even moving an overlay can fail due to timing/bandwith related > constraints. fwiw, the driver should indicate this by setting a flag on the property, this way userspace knows what can be changed dynamically and what can not. >> - We need a test ioctl that can verify whether changing non-dynamic >> properties will work. Using the atomic modeset for that with a >> test-only flag seems like a good option since that already has the >> logic to analyze bandwidth allocation across all crtcs. On the other >> hand, it may make more sense to use the multiflip ioctl as well here. >> What we need to check is whether the change made by a multifflip is >> possible, so it seems natural to communicate that change to the kernel >> using the same ioctl and data structs as the multiflip itself. The >> bandwidth calculation is a global decision and involves all crtcs and >> the current state, so the kernel can decide just fine if a multiflip >> is possible or not, based on the current state and the requested >> multiflip. > > Ie. multiflip and atomic modeset need exactly the same thing here. > >> - Atomic multiflip for one crtc is essential for avoiding flicker and >> artifacts, but ill-defined for multiple crtcs simultaneously and even >> in the genlock case, the failure mode is hardly noticable (one crtc >> may drop a frame in case the compositor is racing with vsync, in which >> case multiflip just means both crtcs drop a frame). > > Sorry I don't follow. With two ioctls in the genlocked case, one crtc > could drop, and the other might not. That is going to be a problem if > both crtcs handle parts of the same physical display. Apart from the > possible IVI and phone/tablet/gadget uses, I can imagine this being > useful for large advertisement/presentation/simulation displays too. > > Also allowing multi crtc flips in the non-genlocked case makes cloned > displays trivial to implement. This is especially useful if the system > is push based like surfaceflinger. > >> For flipping >> multiple fbs and planes, on one crtc, however, atomicity means that we >> can combine gpu rendering and overlays in a reliable way, without >> having to worry about flicker when sprites turn on a frame later after >> we've already erased the surface contents from the main fb. We need >> to be able to render the scene graph split across various planes at >> certain positions and know for certain that when we flip, that's the >> configuration that ends up on the output. > > Sure, that's the main goal of this work. > >> - Pageflip events can be controlled by a flag (as for the current >> pageflip ioctl) or perhaps disabled by setting user_data to 0, but the >> user data is passed in with each nuclear pageflip ioctl and each ioctl >> generates one event (if requested) which returns the user data that >> was passed in at ioctl time. This is how it currently works, the >> event mechanism is already in place, I see no reason to change this >> behaviour. Surely, we're not concerned about 8 extra bytes in the >> ioctl struct? The atomic modeset event (in test mode or not) never >> generates an event, so there's no need for user data there. > > There's no reason why you couldn't send the event in the blocking > modeset case too. Also it would open the door for asynchronous modeset, > if someone has the cojones to implement it. > >> - Pageflip for multiple crtc may be useful in case of gen-locked >> crtc, but it is a corner case and not likely to be present or relevant >> in mainstream hw. > > I've already provided many ideas where it could be used, and I don't > even consider myself a very imaginative person. > > I don't see the point of forcing everyone with such a setup to add > hacks in order to work around artificial restrictions imposed by the > API. Do we want to make a system that people *want* to use, or one they > *have* to use. > >> With the properties being an extensible mechanism, >> we could probably expose gen-locked crtcs through the properties or >> such and in worst case make a new ioctl as Jesses suggests. > > Well, I just don't see the point of going about it in such a > roundabout way. > > My current prototype code basically handles this case already, except > that I added an artifical restriction to avoid the async apply code > path in the multi-crtc case since some people were suggesting that. > With just a few lines of code change I will lift that restriction > and it'll work just fine. > > I honestly do not see why some people want this restriction. From > where I'm standing it doesn't make the code any less complex. What > is the benefit you're trying to extract from the restriction? > > And even if someone thinks that they can't implement the multi crtc > case, then they're free to return an error from the check ioctl. So > there's no harm in allowing it. > > > So I propose that we have: > - One ioctl that takes an arbitrary number of obj/prop/value tuples well, to be fair, if we convert everything to properties, maybe drm/kms only needs one ioctl for everything :-P But different ioctls are cheap.. I don't think it hurts to have two instead of one. I really don't see modeset and pageflip as the same thing. Maybe from the front-end of the video pipe, they are. But from encoder and back they are different. Modeset can take many vblank cycles to complete. And is infrequent. Introducing a state-machine to try to make this asynchronous is just adding a lot of complexity and potential fail for not really much gain. Even flip on multiple crtcs introduces some new edge cases, like moving a plane from one crtc to another. If this is split into two ioctl calls, then the test on the 2nd crtc can -EBUSY because the plane is still pending disconnect from the first. But the test on the 1st crtc can succeed. I can see the usefulness of flip on multi-crtc... but since it isn't nearly as useful/important as flipping multiple planes on a single crtc, I don't see the harm in starting simple and adding this later. BR, -R > - A flag to specify "test only" mode > - A flag to demand asynchronous operation > - Flags to request completion events for each crtc. A bitmask of crtc > index will do. Each event will contain the relevant crtc ID. > - user_data is passed to the ioctl, and included in the events > originating from that operation. > > From my POV the only significant API issues left are: > - Truly useful error reporting. Perhaps there is no nice way to do it. > - Returning a list of retired FBs in the events > > -- > Ville Syrjälä > syrjala@xxxxxx > http://www.sci.fi/~syrjala/ > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
