[PATCH] drm/atomic-helper: Calculate normalized zpos values

[Ville Syrjälä]

On Mon, Nov 13, 2017 at 09:56:19PM +0100, Thierry Reding wrote:
> On Mon, Nov 13, 2017 at 04:14:05PM +0200, Ville Syrjälä wrote:
> > On Mon, Nov 13, 2017 at 02:48:20PM +0100, Thierry Reding wrote:
> > > From: Thierry Reding <treding at nvidia.com>
> > > 
> > > kerneldoc for drm_plane_create_zpos_property() says that the DRM core
> > > will automatically calculate the normalized zpos values, but it won't
> > > currently do so. Modify the atomic helpers to behave as documented.
> > 
> > NAK
> > 
> > See commit  38d868e41c4b ("drm: Don't force all planes to be added to
> > the state due to zpos")
> 
> Thanks, that's interesting background. It's a little unfortunate that
> I'll have to go and add a custom ->atomic_check() just because of this.

IIRC I did propose having two variants in the helper, but Daniel didn't
like that. Of course that might not scale too well if more variants
would become necessary, so I do understand the desire to avoid many
helper variants.

> This is something in general that's been bugging me. How are we supposed
> to keep all of the custom ->atomic_check() implementations in sync with
> the atomic helpers? It looks to me like most drivers will at some point
> copy the atomic helper to their driver and then make driver-specific
> changes to them. But then when one of the helpers gets updated, the same
> changes are usually not propagated to the drivers that originally copied
> from the helpers.
> 
> One way I've seen used (and have used myself) occasionally is for the
> driver-specific implementations to "subclass" the helpers by calling the
> helper first and then calling the driver-specific bits. That helps a lot
> but will obviously not work if ordering is important.
> 
> Any ideas on how we can improve that? Other than periodically checking
> the git log for the helpers and updating drivers? I suppose if one were
> to closely follow the mailing list one might notice early on, and maybe
> speak up and have the changes applied to the drivers in the same patch
> as the helpers. But I don't think that's going to work for every driver.

Yeah, I guess keeping a close eye on or periodically checking for
changes is the best one can do.

Of course people shouldn't be breaking other drivers with their changes
to the helper. So if something gets added there that's really needed in
the open coded implementations as well, the patch author should at least
attempt to make those changes and make sure all relevant driver
maintainers are aware part of the process. That won't magically land
you new features though, unless the people implementing those are
feeling extra generous.

And I guess nothing beats regular and thorough testing if you're
concerned about other people breaking your stuff.

-- 
Ville Syrjälä
Intel OTC