[PATCH] drm/atomic: Add target_vblank support in atomic helpers (v2)

[Daniel Vetter]

On Thu, Jan 12, 2017 at 06:18:20PM +0000, Grodzovsky, Andrey wrote:
> > -----Original Message-----
> > From: daniel.vetter at ffwll.ch [mailto:daniel.vetter at ffwll.ch] On Behalf Of
> > Daniel Vetter
> > Sent: Thursday, January 12, 2017 3:51 AM
> > To: Michel Dänzer
> > Cc: Grodzovsky, Andrey; Deucher, Alexander; dri-
> > devel at lists.freedesktop.org
> > Subject: Re: [PATCH] drm/atomic: Add target_vblank support in atomic
> > helpers (v2)
> > 
> > On Thu, Jan 12, 2017 at 9:28 AM, Michel Dänzer <michel at daenzer.net>
> > wrote:
> > > On 12/01/17 12:48 AM, Grodzovsky, Andrey wrote:
> > >>> From: Michel Dänzer [mailto:michel at daenzer.net] On 09/01/17 06:59
> > >>> PM, Daniel Vetter wrote:
> > >>>> On Fri, Jan 06, 2017 at 03:39:40PM -0500, Andrey Grodzovsky wrote:
> > >>>>> Allows usage of the new page_flip_target hook for drivers
> > >>>>> implementing the atomic path.
> > >>>>> Provides default atomic helper for the new hook.
> > >>>>>
> > >>>>> v2:
> > >>>>> Update code sharing logic between exsiting and the new flip hooks.
> > >>>>> Improve kerneldoc.
> > >>>>>
> > >>>>> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky at amd.com>
> > >>>>
> > >>>> Looks all reasonable, I think an ack from Alex that the amd side is
> > >>>> in shape too, and I'll pull this into drm-misc.
> > >>>
> > >>> Andrey, is there an updated patch 2 adapted to current patch 1?
> > >>> Other than that and some questionable indentation of parameters in
> > >>> function signatures, looks good to me FWIW.
> > >>
> > >> We are unable to use the atomic helpers both for page_flip and
> > >> page_flip_target At their current form mostly due to
> > DRM_MODE_PAGE_FLIP_ASYNC flag rejection they do.
> > >> I discussed this with Daniel Vetter on IRC and suggested to remove
> > >> the rejection but he said the precise semantics of atomic async flip
> > >> is not clear yet and it's better to leave that out for now until
> > >> there is a  userspace asking for it.
> > >> So I tested it by just hacking  the helper to remove the rejection.
> > >> Until that settled the original change [PATCH 2/2] drm/amd/dal:
> > >> Switch to page_flip_target hook in DAL Is what we plan to use in DAL
> > >
> > > IIRC Daniel suggested (on IRC?) to use the helper for non-async flips
> > > and the current DC code for async flips. Is that feasible?
> > 
> > We do have some async flip flag reserved for atomic, so we could route it
> > through. But since atm there's no one asking for async flips on the atomic
> > ioctl I'm a bit wary for fear of ending up with ill-defined semantics. But I guess
> > if we do this for legacy pageflips only, and make sure we do still reject async
> > flips submitted through the atomic ioctl that should be all fine. And it should
> > allow amdgpu to entirely get rid of the legacy flip path, which would be nice.
> > 
> > Once we have that we could even use it for cursor plane updates (through
> > the legacy ioctl, for drivers with universal planes), for those drivers that
> > support it.
> > -Daniel
> 
> So are we ok with a follow-up patch removing the ASYNC_FLIP restriction in the legacy IOCTL
> + adding the drm_mode_crtc_page_flip_target.flags to drm_crtc_state ? In that case as I said before,
> at least for DAL we could drop our own page_flip hook and use the avaialbe helpers.

Yeah, reconsidering all I think that'd be rather reasonable approach.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch