new userspace API approaches atomic *

new userspace API approaches atomic *

[Dave Airlie]

So this is a how to get new features pronouncement,

>From what I can see people would like to have atomic interfaces for
pageflip and modesetting merged,

Now how I think developing and merging these will work (i.e. do it
this way or don't bother)

a) get an API you are happy with working, it doesn't need to be perfect

b) rework the internal drm core/driver APIs for all drivers to allow
this new interface to be used. Remove
the old internal apis and create an interface layer between the old
userspace interface and the new API.

c) throw away (a)

d) reimplement a userspace API on top of the new internal driver API
fixing all the things you have learned,
were crazy, insane, nuts etc.

e) have a lot of tests.

f) get b merged standalone, transition phase is fine, but every driver
needs to be ported before the API
goes in.

g) throw away d

h) write final API and get it merged.

Yes this is probably more work than you or your manager is willing to
buy in, but then maybe the feature isn't that important.

Dave.

Re: new userspace API approaches atomic *

[Rob Clark]

On Thu, Dec 13, 2012 at 3:13 PM, Dave Airlie <airlied@gmail.com> wrote:
> So this is a how to get new features pronouncement,
>
> From what I can see people would like to have atomic interfaces for
> pageflip and modesetting merged,
>
> Now how I think developing and merging these will work (i.e. do it
> this way or don't bother)
>
> a) get an API you are happy with working, it doesn't need to be perfect
>
> b) rework the internal drm core/driver APIs for all drivers to allow
> this new interface to be used. Remove
> the old internal apis and create an interface layer between the old
> userspace interface and the new API.

fwiw, I had started to do this w/ my atomic series.. what I did was to
keep (for example) old .page_flip(), as a legacy path so we didn't
have to merge changes for all drivers in one shot, but the idea was
to, after all drivers are updated, to remove the legacy paths.

The userspace API for atomic pageflip is really a small part of the
whole thing.  The bigger thing is atomic fxns and property conversion.
 So we could even leave off the new userspace API for now.  Probably
the addition of the new ioctl(s) should be the last part.

I had started merging my changes w/ Ville's changes.  I got a bit
distracted on writing a new kms driver (should have RFC to send for
that, hopefully this afternoon).. but hopefully should have some more
time to spend on this RSN..

BR,
-R

> c) throw away (a)
>
> d) reimplement a userspace API on top of the new internal driver API
> fixing all the things you have learned,
> were crazy, insane, nuts etc.
>
> e) have a lot of tests.
>
> f) get b merged standalone, transition phase is fine, but every driver
> needs to be ported before the API
> goes in.
>
> g) throw away d
>
> h) write final API and get it merged.
>
> Yes this is probably more work than you or your manager is willing to
> buy in, but then maybe the feature isn't that important.
>
> Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: new userspace API approaches atomic *

[Ville Syrjälä]

On Fri, Dec 14, 2012 at 07:13:33AM +1000, Dave Airlie wrote:
> So this is a how to get new features pronouncement,
> 
> >From what I can see people would like to have atomic interfaces for
> pageflip and modesetting merged,
> 
> Now how I think developing and merging these will work (i.e. do it
> this way or don't bother)
> 
> a) get an API you are happy with working, it doesn't need to be perfect
> 
> b) rework the internal drm core/driver APIs for all drivers to allow
> this new interface to be used. Remove
> the old internal apis and create an interface layer between the old
> userspace interface and the new API.

There are several problems with this:

- I can't test other drivers

- I don't have the knowledge or inclination to implement atomic semantics
  for everyone's favorite hardware, and without that there's little
  point in doing the work. Some of my initial code was layered on top of
  drm_crtc_helper though, so it might be possible to use that as a basis
  for an atomic helper, but there would actually be no benefit from
  using it apart from allowing those drivers to respond to the atomic
  ioctl. But we wouldn't use any of that w/ i915, so it would be better is
  someone else does that part.

- Replacing all the legacy codepaths with new code in one go increases the
  chance that we get a regression, and then we have no choice but to
  back out the whole thing. Also it seems that no-one apart from Rob has
  even looked at the code, so it seems likely that there would be heavy
  opposition to replacing the current code with something new.

- These are the reasons I would like to merge the thing without touching
  the legcay codepaths too much. Then each driver author could move their
  code over the new APIs. I'm willing to help of course, but the driver
  authors are in a much better position to make something that actually
  works for their hardware.

> c) throw away (a)
> 
> d) reimplement a userspace API on top of the new internal driver API
> fixing all the things you have learned,
> were crazy, insane, nuts etc.
> 
> e) have a lot of tests.

Sure more tests would be nice. I'll try to cook up something to stresses
the modeset side soon.

> f) get b merged standalone, transition phase is fine, but every driver
> needs to be ported before the API
> goes in.

Why? The current drivers are not using the same APIs internally anyway.
i915 doesn't use drm_crtc_helper for example. You didn't demand that
Daniel rewrite drm_crtc_helper to suit i915 and fix up all the other
drivers, did you?

> g) throw away d
> 
> h) write final API and get it merged.
> 
> Yes this is probably more work than you or your manager is willing to
> buy in, but then maybe the feature isn't that important.

Right, so either I rewrite the modeset and pageflip code for all drivers,
or I wait until all the driver authors decide to help me. The first one
will take approximately five years given that I don't know the hardware
and I have other tasks on my plate, and based on the past interest the
second one doesn't seem likely to happen anytime soon 

All this make me think I should just try to push it as an i915 private
feature. Damn the other drivers. That should make the management happy too
since everyone that needs atomic display updates on Linux will need to buy
Intel hardware.

Oh well, the world is supposedly ending in a few days anyway, so perhaps
I can just relax and stop caring :)

-- 
Ville Syrjälä
Intel OTC

Re: new userspace API approaches atomic *

[Dave Airlie]

> There are several problems with this:
>
> - I can't test other drivers
>
> - I don't have the knowledge or inclination to implement atomic semantics
>   for everyone's favorite hardware, and without that there's little
>   point in doing the work. Some of my initial code was layered on top of
>   drm_crtc_helper though, so it might be possible to use that as a basis
>   for an atomic helper, but there would actually be no benefit from
>   using it apart from allowing those drivers to respond to the atomic
>   ioctl. But we wouldn't use any of that w/ i915, so it would be better is
>   someone else does that part.

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.

> - Replacing all the legacy codepaths with new code in one go increases the
>   chance that we get a regression, and then we have no choice but to
>   back out the whole thing. Also it seems that no-one apart from Rob has
>   even looked at the code, so it seems likely that there would be heavy
>   opposition to replacing the current code with something new.

I never said one go, I said before pushing the new API. This means you
get everything internally converted in the drm/driver interface, drop
the old drm/driver interface, and the new userspace API. At least
regressions should be a lot more obvious and we can port a driver at a
time if needed. We can also block new drivers from using the old
interfaces from being merged.

> - These are the reasons I would like to merge the thing without touching
>   the legcay codepaths too much. Then each driver author could move their
>   code over the new APIs. I'm willing to help of course, but the driver
>   authors are in a much better position to make something that actually
>   works for their hardware.

Yup thats what you want, but you don't want to bring in the new API
before this happens, since only once you've ported a bunch of drivers
will you know the API actually works.

>> f) get b merged standalone, transition phase is fine, but every driver
>> needs to be ported before the API
>> goes in.
>
> Why? The current drivers are not using the same APIs internally anyway.
> i915 doesn't use drm_crtc_helper for example. You didn't demand that
> Daniel rewrite drm_crtc_helper to suit i915 and fix up all the other
> drivers, did you?
>

helpers are not APIs. The API from drm_crtc.c to driver is the main
modesetting API, for instance Daniel is trying to stop taking locks in
the drm_crtc side of things, to do this he is going and reimplementing
lots of bits in lots of drivers at the same time.

>
> Right, so either I rewrite the modeset and pageflip code for all drivers,
> or I wait until all the driver authors decide to help me. The first one
> will take approximately five years given that I don't know the hardware
> and I have other tasks on my plate, and based on the past interest the
> second one doesn't seem likely to happen anytime soon
>
> All this make me think I should just try to push it as an i915 private
> feature. Damn the other drivers. That should make the management happy too
> since everyone that needs atomic display updates on Linux will need to buy
> Intel hardware.
>
> Oh well, the world is supposedly ending in a few days anyway, so perhaps
> I can just relax and stop caring :)

Well start by converting Intel to a new internal API without the
external, kick a few driver authors to start porting other drivers to
the new internal API. The API seems like it should be pretty much hw
independent. But I expect getting other maintainers interested might
involve actually going out and actually writing crappy patches and
sending them to them to pick up. I can also help kick heads in the
right direction.

I just don't want to implement any userspace APIs on only one driver,
and this and atomic modeset are things that are definitely heading in
that direction.

Dave.

Re: new userspace API approaches atomic *

[Daniel Vetter]

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).

- 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.

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 ...

>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.

Yours, Daniel

PS: My totally broken exynos patch, which Inki Dae fixed up for me,
and the 3 radeon patches are already merged. It's really not rocket
science afaict ... just requires pinging relevant people early so that
they can check your patches quickly and know what you're up to.
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: new userspace API approaches atomic *

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

Re: new userspace API approaches atomic *

[Ville Syrjälä]

On Sun, Dec 16, 2012 at 04:04:01PM +1000, Dave Airlie wrote:
> > There are several problems with this:
> >
> > - I can't test other drivers
> >
> > - I don't have the knowledge or inclination to implement atomic semantics
> >   for everyone's favorite hardware, and without that there's little
> >   point in doing the work. Some of my initial code was layered on top of
> >   drm_crtc_helper though, so it might be possible to use that as a basis
> >   for an atomic helper, but there would actually be no benefit from
> >   using it apart from allowing those drivers to respond to the atomic
> >   ioctl. But we wouldn't use any of that w/ i915, so it would be better is
> >   someone else does that part.
> 
> 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.

How/when the hardware state needs to be updated to guarantee atomic
behaviour _is_ hardware specific. I suppose eventually you could distill
it down to a handful of models that could cover most hardware. But
note that on some hardware even different scanout engines on the same
chip behave differently, so to update them atomically with each other is
going be a delicate dance.

> > - Replacing all the legacy codepaths with new code in one go increases the
> >   chance that we get a regression, and then we have no choice but to
> >   back out the whole thing. Also it seems that no-one apart from Rob has
> >   even looked at the code, so it seems likely that there would be heavy
> >   opposition to replacing the current code with something new.
> 
> I never said one go, I said before pushing the new API. This means you
> get everything internally converted in the drm/driver interface, drop
> the old drm/driver interface, and the new userspace API. At least
> regressions should be a lot more obvious and we can port a driver at a
> time if needed. We can also block new drivers from using the old
> interfaces from being merged.

Now you're confusing me. How can we port one driver at a time if we
can't merge the feature before all drivers are ported? Or do you mean
porting one driver at a time in some staging tree, and then once all are
ported merge the whole thing? Doable, but painful due to code flux.

> > - These are the reasons I would like to merge the thing without touching
> >   the legcay codepaths too much. Then each driver author could move their
> >   code over the new APIs. I'm willing to help of course, but the driver
> >   authors are in a much better position to make something that actually
> >   works for their hardware.
> 
> Yup thats what you want, but you don't want to bring in the new API
> before this happens, since only once you've ported a bunch of drivers
> will you know the API actually works.

You mean the external API? I know it works. It makes no assumptions about
the hardware behaviour. It even resembles OpenWF Display quite decently,
which should reassure some people. It's entirely property based, so if it
wouldn't work then the current property API would be totally broken as well.

If you mean the internal API, then it will also work because it makes no
assumptions either. It's a transaction type of thing, which just builds
the state, and finally commits it. How you do that commit part is
hardware specific, but the input for the commit will be just the new
device state, so each driver has full freedom to handle it as best they
see fit.

> >> f) get b merged standalone, transition phase is fine, but every driver
> >> needs to be ported before the API
> >> goes in.
> >
> > Why? The current drivers are not using the same APIs internally anyway.
> > i915 doesn't use drm_crtc_helper for example. You didn't demand that
> > Daniel rewrite drm_crtc_helper to suit i915 and fix up all the other
> > drivers, did you?
> >
> 
> helpers are not APIs. The API from drm_crtc.c to driver is the main
> modesetting API, for instance Daniel is trying to stop taking locks in
> the drm_crtc side of things, to do this he is going and reimplementing
> lots of bits in lots of drivers at the same time.

I know. But none of that requires writing a lot of hardware specific
logic.

> > Right, so either I rewrite the modeset and pageflip code for all drivers,
> > or I wait until all the driver authors decide to help me. The first one
> > will take approximately five years given that I don't know the hardware
> > and I have other tasks on my plate, and based on the past interest the
> > second one doesn't seem likely to happen anytime soon
> >
> > All this make me think I should just try to push it as an i915 private
> > feature. Damn the other drivers. That should make the management happy too
> > since everyone that needs atomic display updates on Linux will need to buy
> > Intel hardware.
> >
> > Oh well, the world is supposedly ending in a few days anyway, so perhaps
> > I can just relax and stop caring :)
> 
> Well start by converting Intel to a new internal API without the
> external, kick a few driver authors to start porting other drivers to
> the new internal API. The API seems like it should be pretty much hw
> independent. But I expect getting other maintainers interested might
> involve actually going out and actually writing crappy patches and
> sending them to them to pick up. I can also help kick heads in the
> right direction.
> 
> I just don't want to implement any userspace APIs on only one driver,
> and this and atomic modeset are things that are definitely heading in
> that direction.

Convincing others to work on it would be easier if it would be in
the kernel, rather than some private tree. But if you're commited
to merging it eventually, then that's at least some kind of
carrot/stick.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/