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

["Grodzovsky, Andrey" <Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>]

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Monday, January 16, 2017 5:18 PM
> To: dri-devel@lists.freedesktop.org
> Cc: Grodzovsky, Andrey; nouveau@lists.freedesktop.org; amd-
> gfx@lists.freedesktop.org; Deucher, Alexander; daniel.vetter@intel.com
> Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state
> 
> Hi Andrey,
> 
> Thank you for the patch.
> 
> On Monday 16 Jan 2017 10:44:55 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@amd.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 10 +++-------
> >  include/drm/drm_plane.h             |  8 ++++++++
> >  2 files changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..f83dc43 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -2737,7 +2737,8 @@ static int page_flip_common(
> >  				struct drm_atomic_state *state,
> >  				struct drm_crtc *crtc,
> >  				struct drm_framebuffer *fb,
> > -				struct drm_pending_vblank_event *event)
> > +				struct drm_pending_vblank_event *event,
> > +				uint32_t flags)
> >  {
> >  	struct drm_plane *plane = crtc->primary;
> >  	struct drm_plane_state *plane_state; @@ -2754,6 +2755,7 @@ static
> > int page_flip_common(
> >  	if (IS_ERR(plane_state))
> >  		return PTR_ERR(plane_state);
> >
> > +	plane_state->pflip_flags = flags;
> >
> >  	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> >  	if (ret != 0)
> > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> > *crtc, struct drm_atomic_state *state;
> >  	int ret = 0;
> >
> > -	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> > -		return -EINVAL;
> > -
> 
> With this change all drivers using the helper will not reject that async flag,
> even if they don't implement support for async page flip. You need to either
> patch them all to reject the flag, or implement async page flip support for all
> of them (preferable in the helpers, and then remove the

Please check drm_mode_page_flip_ioctl, one of the checks in the beginning is

if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && !dev->mode_config.async_page_flip)
		return -EINVAL;

We in DC explicitly set dev->mode_config.async_page_flip = true, any driver which is 
Not supporting ASYNC flip will fail the IOCTL at this point.
Same in drm_mode_atomic_ioctl
> 
>  * Note that for now so called async page flips (i.e. updates which are not
>  * synchronized to vblank) are not supported, since the atomic interfaces
> have
>  * no provisions for this yet.
> 
> comment).

Thanks, that a good catch, will remove.

Andrey
> 
> >  	state = drm_atomic_state_alloc(plane->dev);
> >  	if (!state)
> >  		return -ENOMEM;
> > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target(
> >  	struct drm_crtc_state *crtc_state;
> >  	int ret = 0;
> >
> > -	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> > -		return -EINVAL;
> > -
> >  	state = drm_atomic_state_alloc(plane->dev);
> >  	if (!state)
> >  		return -ENOMEM;
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
> > db3bbde..86d8ffc 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -122,6 +122,14 @@ struct drm_plane_state {
> >  	 */
> >  	bool visible;
> >
> > +
> > +	/**
> > +	 * @pflip_flags:
> > +	 *
> > +	 * Flip related config options
> > +	 */
> > +	u32 pflip_flags;
> > +
> >  	struct drm_atomic_state *state;
> >  };
> 
> --
> Regards,
> 
> Laurent Pinchart

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx