[RFC 0/9] nuclear pageflip
Rob Clark
rob.clark at linaro.org
Sat Sep 15 13:10:11 PDT 2012
On Sat, Sep 15, 2012 at 1:00 PM, Ville Syrjälä <syrjala at sci.fi> wrote:
> On Sat, Sep 15, 2012 at 11:05:29AM -0500, Rob Clark wrote:
>> On Sat, Sep 15, 2012 at 9:53 AM, Ville Syrjälä <syrjala at sci.fi> 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.
>
> OK maybe user space could notice that all of the properties it's
> going to manipulate have that flag in the correct position. User space
> could then skip the check ioctl, and proceed straight to the commit
> phase with the nice feeling that it should not fail. But that's just
> an optimization.
>
> Or are you actually suggesting that changing any property with the
> flag in the wrong position would require a full modeset, ie. force
> you to take the blocking code path? That just won't fly.
no, no.. the flag would just be a hint to userspace that it was a
property that could be safely changed without a test step. Otherwise,
userspace should do the test call first to confirm that the hw could
handle the new property values.
>> > 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
>
> Sure, why not ;) Well, we still have all the enumerations stuff and
> whatnot. I see no point in changing those when they work adequately.
> But actually setting the state of the hardware can be handled through
> a single ioctl.
>
>> 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.
>
> I don't entirely agree with the infrequent part. Fullscreen video on
> external displays is one case where you really may want to change the
> mode quite often. Or you may not want to change the actual timings on
> the display, but you still want to change the resolution of the CRTC,
> and let a panel fitter handle the difference in input and output
> resolutions. But thanks to the way kms is designed those two things
> are both linked to the display mode of the CRTC, so you still need a
> modeset to handle it.
well, possibly some properties could be added to circumvent that.. I
was more thinking of actual timing changes
>> 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.
>
> Forcing you to rewrite user space multiple times. And keeping all the
> old codepaths around in both kernel and user space side to maintain
> ABI compatibility both ways. Also the speed at which these things
> trickle through to the actual users is very slow, so adding new ioctls
> every six months to handle overlapping tasks is not sensible IMHO.
well, I'm not quite advocating a new ioctl every 6 months.. but what I
meant is that the cost isn't high to have two ioctls, one for async
pageflip, and one for modeset.. and the sync modeset ioctl is required
if you are changing timings or lighting up a new display.
I think for pageflip, we could have just one ioctl, with a single
user-data. And later add flags to indicate when the event should be
sent back (once, once per crtc, etc). Sorting out exactly what we
want for multi-crtc flips shouldn't block getting the basics in place
for single crtc flip.
> My _point_ is that there is no need to hardcode these restrictions
> into the API. We already know what kind of API will handle all these
> cases, so why can't we just go with that API from the very beginning?
well, I don't think we necessarily need to change the ioctl to support
flip on multiple crtc's.. it should be more a matter of adding a cap
to indicate to userspace that you can change multiple crtc's on a
single pageflip, and maybe defining some new flags.
BR,
-R
> --
> Ville Syrjälä
> syrjala at sci.fi
> http://www.sci.fi/~syrjala/
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list