On Thu, May 16, 2019 at 02:07:34PM +0200, Daniel Vetter wrote: > On Thu, May 16, 2019 at 2:02 PM Laurent Pinchart > <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > > > > Hi Daniel, > > > > On Mon, May 13, 2019 at 04:47:47PM +0200, Daniel Vetter wrote: > > > On Sat, May 11, 2019 at 10:12:02PM +0300, Laurent Pinchart wrote: > > > > On Thu, May 02, 2019 at 03:49:46PM -0400, Sean Paul wrote: > > > >> From: Sean Paul <seanpaul@xxxxxxxxxxxx> > > > >> > > > >> Everyone who implements connector_helper_funcs->atomic_check reaches > > > >> into the connector state to get the atomic state. Instead of continuing > > > >> this pattern, change the callback signature to just give atomic state > > > >> and let the driver determine what it does and does not need from it. > > > >> > > > >> Eventually all atomic functions should do this, but that's just too much > > > >> busy work for me. > > > > > > > > Given that drivers also access the connector state, isn't this slightly > > > > more inefficient ? > > > > > > It's atomic code, we're trying to optimize for clean code at the expense > > > of a bit of runtime overhead due to more pointer chasing. And I agree with > > > the general push, the pile of old/new_state pointers of various objects > > > we're passing around is confusing. Passing the overall drm_atomic_state > > > seems much more reasonable, and with that we can get everything else. Plus > > > it's much more obvious whether you have the old/new state (since that's > > > explicit when you look it up from the drm_atomic_state). > > > > Yes, I agree it's cleaner. I just hope the atomic state tracking cost > > can be kept under control :-) > > > > By the way, this is likely not going to happen as it would be way too > > intrusive, but it would be nice to rename drm_atomic_state to > > drm_atomic_transaction (or something similar). It doesn't model a state, > > but a change between an old state to a new state. This confused me in > > the past, and I'm sure it can still be confusing to newcomers. > > Why are you the first to suggest this, this is awesome! Can't quite tell if that's irony or not. Anyways, this has been suggested before but no volunteers stepped forward. > drm_atomic_state is indeed not a state, but a transaction representing > how we go from the old to the new state. On a semi-related topic, I've occasionally pondered about moving mode_changed & co. from the obj states to the top level state/transaction (maybe stored as a bitmask). But that would definitely not be a trivial sed job. -- Ville Syrjälä Intel
