[Nouveau] [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state

[Daniel Vetter]

On Thu, Jan 26, 2017 at 05:01:01AM +0000, Grodzovsky, Andrey wrote:
> 
> 
> > -----Original Message-----
> > From: Cheng, Tony
> > Sent: Monday, January 23, 2017 2:49 PM
> > To: Daniel Vetter; Grodzovsky, Andrey
> > Cc: Deucher, Alexander; nouveau at lists.freedesktop.org; amd-
> > gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
> > daniel.vetter at intel.com; dc_upstream
> > Subject: RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On
> > > Behalf Of Daniel Vetter
> > > Sent: Monday, January 23, 2017 3:55 AM
> > > To: Grodzovsky, Andrey <Andrey.Grodzovsky at amd.com>
> > > Cc: Deucher, Alexander <Alexander.Deucher at amd.com>;
> > > nouveau at lists.freedesktop.org; amd-gfx at lists.freedesktop.org; dri-
> > > devel at lists.freedesktop.org; daniel.vetter at intel.com
> > > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in
> > > drm_plane_state
> > >
> > > On Mon, Jan 16, 2017 at 10:44:55AM -0500, Andrey Grodzovsky wrote:
> > > > Allows using atomic flip helpers for drivers using ASYNC flip.
> > > > Remove ASYNC_FLIP restriction in helpers and caches the page flip
> > > > flags in drm_plane_state to be used in the low level drivers.
> > > >
> > > > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky at amd.com>
> > >
> > > It's mostly guesswork, but I think we should have the flip flags in
> > > the crtc, not in each plane. Similar to how we move the event from planes
> > to crtc.
> > > -Daniel
> > 
> Do you mean here crtc or crtc_state ?

Yes.

> > What does ASYNC flip mean?  HW flip as soon as possible and result in tearing
> > on screen?  If so I could imaging some use case where you have some UI
> > control/menu overlay on top, and some game running on a underlay plane,
> > and the game want to be able to flip as soon as possible.  Or Daniel do you
> > think ASYNC property will apply to all planes in CRTC?
> > 
> Also the client code both in nouveau and AMD/DAL iterates over new planes and their new states
> so it seems to me easier to retrieve the parameter from the plane_state and not adding loop
> to find matching crtc or its  state, especially if it's before drm_atomic_helper_swap_state is called,
>  when crtc->state still points to the old state ...

It's pretty easy to go from plane_state to crtc_state. No need to loop at
all. And the implementation shouldn't justify where we put it, the
important part is the semantics.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch