Re: [PATCH v3 1/6] drm: Add drm_mode_config->normalize_zpos boolean

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jan 25, 2018 at 04:40:48PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 25, 2018 at 04:26:25PM +0200, Peter Ujfalusi wrote:
> > Instead of drivers duplicating the drm_atomic_helper_check() code to be
> > able to normalize the zpos they can use the normalize_zpos flag to let the
> > drm core to do it.
> > 
> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 11 +++++++++++
> >  include/drm/drm_mode_config.h       |  8 ++++++++
> >  include/drm/drm_plane.h             |  4 ++--
> >  3 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index ab4032167094..0f6a4949e6dc 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -873,6 +873,11 @@ EXPORT_SYMBOL(drm_atomic_helper_check_planes);
> >   * functions depend upon an updated adjusted_mode.clock to e.g. properly compute
> >   * watermarks.
> >   *
> > + * Note that zpos normalization will add all enable planes to the state which
> > + * might not desired for some drivers.
> > + * For example enable/disable of a cursor plane which have fixed zpos value
> > + * would trigger all other enabled planes to be forced to the state change.
> > + *
> >   * RETURNS:
> >   * Zero for success or -errno
> >   */
> > @@ -885,6 +890,12 @@ int drm_atomic_helper_check(struct drm_device *dev,
> >  	if (ret)
> >  		return ret;
> >  
> > +	if (dev->mode_config.normalize_zpos) {
> > +		ret = drm_atomic_normalize_zpos(dev, state);
> > +		if (ret)
> > +			return ret;
> > +	}
> 
> I think we originally had this in drm_atomic_helper_check_planes().
> Looking through some of the drivers it looks like we could maybe
> kill a few more LOC by putting it there.

Actually, I guess it's fine as is. I though the "async" flip thing I
saw in some of the drivers wasn't in the atomic helper. But it is
there.

That actually makes me slightly worried whether it's safe to just
blindly replace the hand rolled stuff w/o "async" with
drm_atomic_helper_check(). The commit messages should perhaps
justify that somehow.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel