[PATCH 0/7] prepare for atomic.. the great propertyification

[Daniel Vetter]

On Thu, Jul 24, 2014 at 10:56:07AM -0400, Rob Clark wrote:
[snip]

> ok, it's entirely possibly that I'm missunderstanding a bit your proposal..

Yeah I get that feeling a bit too. I'll cut out the technical details for
now and will try to concentrate on one example. Maybe that clarifies.


[snip]

> >> > So here's my proposal for how to get this in without breaking the world
> >> >
> >> > 1. We add a complete new set of ->atomic_foo driver entry points to all
> >> > relevant objects. So instead of changing all the set_prop functions in a
> >> > flag-day like operation we just add a new interface. I haven't double
> >> > checked, but I guess that means per-obj ->atomic_set_prop functions plus a
> >> > global ->atomic_check and ->atomic_commit.
> >>
> >> that sounds much worse
> >
> > Why that? Afaics your patch only changes the interfaces but leaves the
> > semantics the same (i915 does set_config_internal all over the place in
> > set_prop). So essentially you have both interface, but merged into one.
> > And especially for the set_prop the semantics our different.
> 
> well, it was the doubling up of code paths in core for handling legacy
> and atomic side-by-side that I was trying to avoid
> 
> I do remember seeing i915 set_config_internal (looks like it has been
> refactored into intel_set_mode())..  iirc, it was all on
> connector->set_prop(), so it would essentially be it's own
> atomic/transaction.

So currently our set_prop functions work like that:
1. We parse the property, sanity check it and store it in the relevant
object structure.
2. We check whether (after all the checking and parsing) there has been an
actual change in configuration. E.g. for the audio stuff if the use sets
back to "auto" we check whether the old config matches the new autoconfig
state or not. So it's not just "has the prop value changed", we actually
diff the resulting hw config.
3. If there is a change, we update the hw state with a call to mode_set
iff the pipe is on.

There's a few reasons why this works and why it looks like that.
- Our internal mode set code currently doesn't notice changes in property
  values. We plan to fix this eventually by shuffling the compute_config
  code around, but that's a lot of work.
- Our internal mode set function _always_ forces a full modeset on the
  crtc you pass it (besides doing modeset on other crtcs where tracked
  state has changed). We need to have this hack to make the above set_prop
  sequence work.

Now with atomic we want completely different semantics:
- Set_prop only updates state. We need to drop the state computation and
  diffing and the forced mode_set.
- The eventual commit will force a mode_set. Note that calling
  ->set_config will not be enough since that doesn't have the "force full
  mode-set" hack which the internal version has. And we can't do this
  since userspace uses set_crtc/->set_config to update frontbuffers which
  absolutely must not result in a full modeset.

So looking just at the ->set_prop function we have 2 completely different
semantics. Now I agree that with your patch i915 keeps on working. But the
problem I have is converting i915 over to the new world. Since you've
removed the old entry point I am left with no other choice than to convert
everything at once. And there's a lot more than just the hdmi audio
property - page_flip has slightly different semantics with atomic,
update_plane dito, set-crtc the same.

Which means I either have to convert i915 in one go (impossible given the
usual churn I face) or I just end up implementing the infrastructure I've
asked for (which means I get to reinstante all the old legacy ioctl
paths). Since with the doubled-up entry points I can e.g. just convert
hdmi set_prop to atomic, which means I have a minimal use-case to validate
the core infrastructure in i915 (i.e. the state diffing we need to improve
in the mode_set code) and can ignore all the nonsense in the tv connectors
and sdvo encoders and plane props and crtcs props and hacked-up other crap
we have all around. It's still going to be a major pain, but I expect the
transition to be much more smoothly.

My experience with the universal plane stuff really is that even slight
semantic differences in the interfaces bite you hard and there's no way to
work around that. The only sane way really is to have parallel entry
points with helpers so that transitioned drivers can remap legacy
interfaces to the more powerful new ones.

I hope this explains a bit better where I see the big risk with your
approach and what exactly I'm proposing.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch