diff mbox

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

Message ID 1483735180-4173-1-git-send-email-Andrey.Grodzovsky@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrey Grodzovsky Jan. 6, 2017, 8:39 p.m. UTC
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@amd.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 133 ++++++++++++++++++++++++++++++------
 include/drm/drm_atomic_helper.h     |   6 ++
 include/drm/drm_crtc.h              |   9 +++
 3 files changed, 127 insertions(+), 21 deletions(-)

Comments

Daniel Vetter Jan. 9, 2017, 9:59 a.m. UTC | #1
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@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.

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 133 ++++++++++++++++++++++++++++++------
>  include/drm/drm_atomic_helper.h     |   6 ++
>  include/drm/drm_crtc.h              |   9 +++
>  3 files changed, 127 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 583f47f..a4e5477 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2733,6 +2733,44 @@ int drm_atomic_helper_resume(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
>  
> +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_plane *plane = crtc->primary;
> +	struct drm_plane_state *plane_state;
> +	struct drm_crtc_state *crtc_state;
> +	int ret = 0;
> +
> +	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +	if (IS_ERR(crtc_state))
> +		return PTR_ERR(crtc_state);
> +
> +	crtc_state->event = event;
> +
> +	plane_state = drm_atomic_get_plane_state(state, plane);
> +	if (IS_ERR(plane_state))
> +		return PTR_ERR(plane_state);
> +
> +
> +	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> +	if (ret != 0)
> +		return ret;
> +	drm_atomic_set_fb_for_plane(plane_state, fb);
> +
> +	/* Make sure we don't accidentally do a full modeset. */
> +	state->allow_modeset = false;
> +	if (!crtc_state->active) {
> +		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
> +				 crtc->base.id);
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
>  /**
>   * drm_atomic_helper_page_flip - execute a legacy page flip
>   * @crtc: DRM crtc
> @@ -2740,7 +2778,8 @@ int drm_atomic_helper_resume(struct drm_device *dev,
>   * @event: optional DRM event to signal upon completion
>   * @flags: flip flags for non-vblank sync'ed updates
>   *
> - * Provides a default page flip implementation using the atomic driver interface.
> + * Provides a default &drm_crtc_funcs.page_flip implementation
> + * using the atomic driver interface.
>   *
>   * 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
> @@ -2748,6 +2787,9 @@ int drm_atomic_helper_resume(struct drm_device *dev,
>   *
>   * Returns:
>   * Returns 0 on success, negative errno numbers on failure.
> + *
> + * See also:
> + * drm_atomic_helper_page_flip_target()
>   */
>  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  				struct drm_framebuffer *fb,
> @@ -2756,8 +2798,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  {
>  	struct drm_plane *plane = crtc->primary;
>  	struct drm_atomic_state *state;
> -	struct drm_plane_state *plane_state;
> -	struct drm_crtc_state *crtc_state;
>  	int ret = 0;
>  
>  	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> @@ -2768,35 +2808,86 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  		return -ENOMEM;
>  
>  	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +
>  retry:
> -	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> -	if (IS_ERR(crtc_state)) {
> -		ret = PTR_ERR(crtc_state);
> +	ret = page_flip_common(state, crtc, fb, event);
> +	if (ret != 0)
>  		goto fail;
> -	}
> -	crtc_state->event = event;
>  
> -	plane_state = drm_atomic_get_plane_state(state, plane);
> -	if (IS_ERR(plane_state)) {
> -		ret = PTR_ERR(plane_state);
> -		goto fail;
> -	}
> +	ret = drm_atomic_nonblocking_commit(state);
>  
> -	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> +fail:
> +	if (ret == -EDEADLK)
> +		goto backoff;
> +
> +	drm_atomic_state_put(state);
> +	return ret;
> +
> +backoff:
> +	drm_atomic_state_clear(state);
> +	drm_atomic_legacy_backoff(state);
> +
> +	/*
> +	 * Someone might have exchanged the framebuffer while we dropped locks
> +	 * in the backoff code. We need to fix up the fb refcount tracking the
> +	 * core does for us.
> +	 */
> +	plane->old_fb = plane->fb;
> +
> +	goto retry;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> +
> +/**
> + * drm_atomic_helper_page_flip_target - do page flip on target vblank period.
> + * @crtc: DRM crtc
> + * @fb: DRM framebuffer
> + * @event: optional DRM event to signal upon completion
> + * @flags: flip flags for non-vblank sync'ed updates
> + * @target: specifying the target vblank period when the flip to take effect
> + *
> + * Provides a default &drm_crtc_funcs.page_flip_target implementation.
> + * Similar to drm_atomic_helper_page_flip() with extra parameter to specify
> + * target vblank period to flip.
> + *
> + * Returns:
> + * Returns 0 on success, negative errno numbers on failure.
> + */
> +int drm_atomic_helper_page_flip_target(
> +				struct drm_crtc *crtc,
> +				struct drm_framebuffer *fb,
> +				struct drm_pending_vblank_event *event,
> +				uint32_t flags,
> +				uint32_t target)
> +{
> +	struct drm_plane *plane = crtc->primary;
> +	struct drm_atomic_state *state;
> +	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;
> +
> +	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +
> +retry:
> +	ret = page_flip_common(state, crtc, fb, event);
>  	if (ret != 0)
>  		goto fail;
> -	drm_atomic_set_fb_for_plane(plane_state, fb);
>  
> -	/* Make sure we don't accidentally do a full modeset. */
> -	state->allow_modeset = false;
> -	if (!crtc_state->active) {
> -		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
> -				 crtc->base.id);
> +	crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
> +	if (WARN_ON(!crtc_state)) {
>  		ret = -EINVAL;
>  		goto fail;
>  	}
> +	crtc_state->target_vblank = target;
>  
>  	ret = drm_atomic_nonblocking_commit(state);
> +
>  fail:
>  	if (ret == -EDEADLK)
>  		goto backoff;
> @@ -2817,7 +2908,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  
>  	goto retry;
>  }
> -EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> +EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
>  
>  /**
>   * drm_atomic_helper_connector_dpms() - connector dpms helper implementation
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 7ff92b0..b3b3abe 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -124,6 +124,12 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  				struct drm_framebuffer *fb,
>  				struct drm_pending_vblank_event *event,
>  				uint32_t flags);
> +int drm_atomic_helper_page_flip_target(
> +				struct drm_crtc *crtc,
> +				struct drm_framebuffer *fb,
> +				struct drm_pending_vblank_event *event,
> +				uint32_t flags,
> +				uint32_t target);
>  int drm_atomic_helper_connector_dpms(struct drm_connector *connector,
>  				     int mode);
>  struct drm_encoder *
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 946672f..5c77c3f 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -157,6 +157,15 @@ struct drm_crtc_state {
>  	struct drm_property_blob *gamma_lut;
>  
>  	/**
> +	 * @target_vblank:
> +	 *
> +	 * Target vertical blank period when a page flip
> +	 * should take effect.
> +	 */
> +
> +	u32 target_vblank;
> +
> +	/**
>  	 * @event:
>  	 *
>  	 * Optional pointer to a DRM event to signal upon completion of the
> -- 
> 1.9.1
>
Michel Dänzer Jan. 11, 2017, 8:33 a.m. UTC | #2
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@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.
Andrey Grodzovsky Jan. 11, 2017, 3:48 p.m. UTC | #3
> -----Original Message-----

> From: Michel Dänzer [mailto:michel@daenzer.net]

> Sent: Wednesday, January 11, 2017 3:33 AM

> To: Daniel Vetter; Grodzovsky, Andrey

> Cc: Deucher, Alexander; dri-devel@lists.freedesktop.org

> Subject: Re: [PATCH] drm/atomic: Add target_vblank support in atomic

> helpers (v2)

> 

> 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@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

Andrey
> 

> 

> --

> Earthling Michel Dänzer               |               http://www.amd.com

> Libre software enthusiast             |             Mesa and X developer
Alex Deucher Jan. 11, 2017, 7:57 p.m. UTC | #4
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, January 09, 2017 4:59 AM
> To: Grodzovsky, Andrey
> Cc: dri-devel@lists.freedesktop.org; daniel.vetter@ffwll.ch; Deucher,
> Alexander; Daenzer, Michel; Wentland, Harry
> Subject: Re: [PATCH] drm/atomic: Add target_vblank support in atomic
> helpers (v2)
> 
> 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@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.

I had a discussion about this with Andrey and I think we are in good shape.

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> 
> Thanks, Daniel
> 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 133
> ++++++++++++++++++++++++++++++------
> >  include/drm/drm_atomic_helper.h     |   6 ++
> >  include/drm/drm_crtc.h              |   9 +++
> >  3 files changed, 127 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> > index 583f47f..a4e5477 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -2733,6 +2733,44 @@ int drm_atomic_helper_resume(struct
> drm_device *dev,
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
> >
> > +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_plane *plane = crtc->primary;
> > +	struct drm_plane_state *plane_state;
> > +	struct drm_crtc_state *crtc_state;
> > +	int ret = 0;
> > +
> > +	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > +	if (IS_ERR(crtc_state))
> > +		return PTR_ERR(crtc_state);
> > +
> > +	crtc_state->event = event;
> > +
> > +	plane_state = drm_atomic_get_plane_state(state, plane);
> > +	if (IS_ERR(plane_state))
> > +		return PTR_ERR(plane_state);
> > +
> > +
> > +	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> > +	if (ret != 0)
> > +		return ret;
> > +	drm_atomic_set_fb_for_plane(plane_state, fb);
> > +
> > +	/* Make sure we don't accidentally do a full modeset. */
> > +	state->allow_modeset = false;
> > +	if (!crtc_state->active) {
> > +		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy
> flip\n",
> > +				 crtc->base.id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  /**
> >   * drm_atomic_helper_page_flip - execute a legacy page flip
> >   * @crtc: DRM crtc
> > @@ -2740,7 +2778,8 @@ int drm_atomic_helper_resume(struct
> drm_device *dev,
> >   * @event: optional DRM event to signal upon completion
> >   * @flags: flip flags for non-vblank sync'ed updates
> >   *
> > - * Provides a default page flip implementation using the atomic driver
> interface.
> > + * Provides a default &drm_crtc_funcs.page_flip implementation
> > + * using the atomic driver interface.
> >   *
> >   * 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
> > @@ -2748,6 +2787,9 @@ int drm_atomic_helper_resume(struct
> drm_device *dev,
> >   *
> >   * Returns:
> >   * Returns 0 on success, negative errno numbers on failure.
> > + *
> > + * See also:
> > + * drm_atomic_helper_page_flip_target()
> >   */
> >  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> >  				struct drm_framebuffer *fb,
> > @@ -2756,8 +2798,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> *crtc,
> >  {
> >  	struct drm_plane *plane = crtc->primary;
> >  	struct drm_atomic_state *state;
> > -	struct drm_plane_state *plane_state;
> > -	struct drm_crtc_state *crtc_state;
> >  	int ret = 0;
> >
> >  	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> > @@ -2768,35 +2808,86 @@ int drm_atomic_helper_page_flip(struct
> drm_crtc *crtc,
> >  		return -ENOMEM;
> >
> >  	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> > +
> >  retry:
> > -	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > -	if (IS_ERR(crtc_state)) {
> > -		ret = PTR_ERR(crtc_state);
> > +	ret = page_flip_common(state, crtc, fb, event);
> > +	if (ret != 0)
> >  		goto fail;
> > -	}
> > -	crtc_state->event = event;
> >
> > -	plane_state = drm_atomic_get_plane_state(state, plane);
> > -	if (IS_ERR(plane_state)) {
> > -		ret = PTR_ERR(plane_state);
> > -		goto fail;
> > -	}
> > +	ret = drm_atomic_nonblocking_commit(state);
> >
> > -	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> > +fail:
> > +	if (ret == -EDEADLK)
> > +		goto backoff;
> > +
> > +	drm_atomic_state_put(state);
> > +	return ret;
> > +
> > +backoff:
> > +	drm_atomic_state_clear(state);
> > +	drm_atomic_legacy_backoff(state);
> > +
> > +	/*
> > +	 * Someone might have exchanged the framebuffer while we
> dropped locks
> > +	 * in the backoff code. We need to fix up the fb refcount tracking the
> > +	 * core does for us.
> > +	 */
> > +	plane->old_fb = plane->fb;
> > +
> > +	goto retry;
> > +}
> > +EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> > +
> > +/**
> > + * drm_atomic_helper_page_flip_target - do page flip on target vblank
> period.
> > + * @crtc: DRM crtc
> > + * @fb: DRM framebuffer
> > + * @event: optional DRM event to signal upon completion
> > + * @flags: flip flags for non-vblank sync'ed updates
> > + * @target: specifying the target vblank period when the flip to take
> effect
> > + *
> > + * Provides a default &drm_crtc_funcs.page_flip_target implementation.
> > + * Similar to drm_atomic_helper_page_flip() with extra parameter to
> specify
> > + * target vblank period to flip.
> > + *
> > + * Returns:
> > + * Returns 0 on success, negative errno numbers on failure.
> > + */
> > +int drm_atomic_helper_page_flip_target(
> > +				struct drm_crtc *crtc,
> > +				struct drm_framebuffer *fb,
> > +				struct drm_pending_vblank_event *event,
> > +				uint32_t flags,
> > +				uint32_t target)
> > +{
> > +	struct drm_plane *plane = crtc->primary;
> > +	struct drm_atomic_state *state;
> > +	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;
> > +
> > +	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> > +
> > +retry:
> > +	ret = page_flip_common(state, crtc, fb, event);
> >  	if (ret != 0)
> >  		goto fail;
> > -	drm_atomic_set_fb_for_plane(plane_state, fb);
> >
> > -	/* Make sure we don't accidentally do a full modeset. */
> > -	state->allow_modeset = false;
> > -	if (!crtc_state->active) {
> > -		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy
> flip\n",
> > -				 crtc->base.id);
> > +	crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
> > +	if (WARN_ON(!crtc_state)) {
> >  		ret = -EINVAL;
> >  		goto fail;
> >  	}
> > +	crtc_state->target_vblank = target;
> >
> >  	ret = drm_atomic_nonblocking_commit(state);
> > +
> >  fail:
> >  	if (ret == -EDEADLK)
> >  		goto backoff;
> > @@ -2817,7 +2908,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> *crtc,
> >
> >  	goto retry;
> >  }
> > -EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> > +EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
> >
> >  /**
> >   * drm_atomic_helper_connector_dpms() - connector dpms helper
> implementation
> > diff --git a/include/drm/drm_atomic_helper.h
> b/include/drm/drm_atomic_helper.h
> > index 7ff92b0..b3b3abe 100644
> > --- a/include/drm/drm_atomic_helper.h
> > +++ b/include/drm/drm_atomic_helper.h
> > @@ -124,6 +124,12 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> *crtc,
> >  				struct drm_framebuffer *fb,
> >  				struct drm_pending_vblank_event *event,
> >  				uint32_t flags);
> > +int drm_atomic_helper_page_flip_target(
> > +				struct drm_crtc *crtc,
> > +				struct drm_framebuffer *fb,
> > +				struct drm_pending_vblank_event *event,
> > +				uint32_t flags,
> > +				uint32_t target);
> >  int drm_atomic_helper_connector_dpms(struct drm_connector
> *connector,
> >  				     int mode);
> >  struct drm_encoder *
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 946672f..5c77c3f 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -157,6 +157,15 @@ struct drm_crtc_state {
> >  	struct drm_property_blob *gamma_lut;
> >
> >  	/**
> > +	 * @target_vblank:
> > +	 *
> > +	 * Target vertical blank period when a page flip
> > +	 * should take effect.
> > +	 */
> > +
> > +	u32 target_vblank;
> > +
> > +	/**
> >  	 * @event:
> >  	 *
> >  	 * Optional pointer to a DRM event to signal upon completion of the
> > --
> > 1.9.1
> >
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter Jan. 12, 2017, 7:58 a.m. UTC | #5
On Wed, Jan 11, 2017 at 07:57:11PM +0000, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Monday, January 09, 2017 4:59 AM
> > To: Grodzovsky, Andrey
> > Cc: dri-devel@lists.freedesktop.org; daniel.vetter@ffwll.ch; Deucher,
> > Alexander; Daenzer, Michel; Wentland, Harry
> > Subject: Re: [PATCH] drm/atomic: Add target_vblank support in atomic
> > helpers (v2)
> > 
> > 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@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.
> 
> I had a discussion about this with Andrey and I think we are in good shape.
> 
> Acked-by: Alex Deucher <alexander.deucher@amd.com>

Applied to drm-misc, thanks.
-Daniel

> 
> > 
> > Thanks, Daniel
> > 
> > > ---
> > >  drivers/gpu/drm/drm_atomic_helper.c | 133
> > ++++++++++++++++++++++++++++++------
> > >  include/drm/drm_atomic_helper.h     |   6 ++
> > >  include/drm/drm_crtc.h              |   9 +++
> > >  3 files changed, 127 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 583f47f..a4e5477 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -2733,6 +2733,44 @@ int drm_atomic_helper_resume(struct
> > drm_device *dev,
> > >  }
> > >  EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
> > >
> > > +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_plane *plane = crtc->primary;
> > > +	struct drm_plane_state *plane_state;
> > > +	struct drm_crtc_state *crtc_state;
> > > +	int ret = 0;
> > > +
> > > +	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > > +	if (IS_ERR(crtc_state))
> > > +		return PTR_ERR(crtc_state);
> > > +
> > > +	crtc_state->event = event;
> > > +
> > > +	plane_state = drm_atomic_get_plane_state(state, plane);
> > > +	if (IS_ERR(plane_state))
> > > +		return PTR_ERR(plane_state);
> > > +
> > > +
> > > +	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> > > +	if (ret != 0)
> > > +		return ret;
> > > +	drm_atomic_set_fb_for_plane(plane_state, fb);
> > > +
> > > +	/* Make sure we don't accidentally do a full modeset. */
> > > +	state->allow_modeset = false;
> > > +	if (!crtc_state->active) {
> > > +		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy
> > flip\n",
> > > +				 crtc->base.id);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  /**
> > >   * drm_atomic_helper_page_flip - execute a legacy page flip
> > >   * @crtc: DRM crtc
> > > @@ -2740,7 +2778,8 @@ int drm_atomic_helper_resume(struct
> > drm_device *dev,
> > >   * @event: optional DRM event to signal upon completion
> > >   * @flags: flip flags for non-vblank sync'ed updates
> > >   *
> > > - * Provides a default page flip implementation using the atomic driver
> > interface.
> > > + * Provides a default &drm_crtc_funcs.page_flip implementation
> > > + * using the atomic driver interface.
> > >   *
> > >   * 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
> > > @@ -2748,6 +2787,9 @@ int drm_atomic_helper_resume(struct
> > drm_device *dev,
> > >   *
> > >   * Returns:
> > >   * Returns 0 on success, negative errno numbers on failure.
> > > + *
> > > + * See also:
> > > + * drm_atomic_helper_page_flip_target()
> > >   */
> > >  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> > >  				struct drm_framebuffer *fb,
> > > @@ -2756,8 +2798,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> > *crtc,
> > >  {
> > >  	struct drm_plane *plane = crtc->primary;
> > >  	struct drm_atomic_state *state;
> > > -	struct drm_plane_state *plane_state;
> > > -	struct drm_crtc_state *crtc_state;
> > >  	int ret = 0;
> > >
> > >  	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> > > @@ -2768,35 +2808,86 @@ int drm_atomic_helper_page_flip(struct
> > drm_crtc *crtc,
> > >  		return -ENOMEM;
> > >
> > >  	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> > > +
> > >  retry:
> > > -	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > > -	if (IS_ERR(crtc_state)) {
> > > -		ret = PTR_ERR(crtc_state);
> > > +	ret = page_flip_common(state, crtc, fb, event);
> > > +	if (ret != 0)
> > >  		goto fail;
> > > -	}
> > > -	crtc_state->event = event;
> > >
> > > -	plane_state = drm_atomic_get_plane_state(state, plane);
> > > -	if (IS_ERR(plane_state)) {
> > > -		ret = PTR_ERR(plane_state);
> > > -		goto fail;
> > > -	}
> > > +	ret = drm_atomic_nonblocking_commit(state);
> > >
> > > -	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> > > +fail:
> > > +	if (ret == -EDEADLK)
> > > +		goto backoff;
> > > +
> > > +	drm_atomic_state_put(state);
> > > +	return ret;
> > > +
> > > +backoff:
> > > +	drm_atomic_state_clear(state);
> > > +	drm_atomic_legacy_backoff(state);
> > > +
> > > +	/*
> > > +	 * Someone might have exchanged the framebuffer while we
> > dropped locks
> > > +	 * in the backoff code. We need to fix up the fb refcount tracking the
> > > +	 * core does for us.
> > > +	 */
> > > +	plane->old_fb = plane->fb;
> > > +
> > > +	goto retry;
> > > +}
> > > +EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> > > +
> > > +/**
> > > + * drm_atomic_helper_page_flip_target - do page flip on target vblank
> > period.
> > > + * @crtc: DRM crtc
> > > + * @fb: DRM framebuffer
> > > + * @event: optional DRM event to signal upon completion
> > > + * @flags: flip flags for non-vblank sync'ed updates
> > > + * @target: specifying the target vblank period when the flip to take
> > effect
> > > + *
> > > + * Provides a default &drm_crtc_funcs.page_flip_target implementation.
> > > + * Similar to drm_atomic_helper_page_flip() with extra parameter to
> > specify
> > > + * target vblank period to flip.
> > > + *
> > > + * Returns:
> > > + * Returns 0 on success, negative errno numbers on failure.
> > > + */
> > > +int drm_atomic_helper_page_flip_target(
> > > +				struct drm_crtc *crtc,
> > > +				struct drm_framebuffer *fb,
> > > +				struct drm_pending_vblank_event *event,
> > > +				uint32_t flags,
> > > +				uint32_t target)
> > > +{
> > > +	struct drm_plane *plane = crtc->primary;
> > > +	struct drm_atomic_state *state;
> > > +	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;
> > > +
> > > +	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> > > +
> > > +retry:
> > > +	ret = page_flip_common(state, crtc, fb, event);
> > >  	if (ret != 0)
> > >  		goto fail;
> > > -	drm_atomic_set_fb_for_plane(plane_state, fb);
> > >
> > > -	/* Make sure we don't accidentally do a full modeset. */
> > > -	state->allow_modeset = false;
> > > -	if (!crtc_state->active) {
> > > -		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy
> > flip\n",
> > > -				 crtc->base.id);
> > > +	crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
> > > +	if (WARN_ON(!crtc_state)) {
> > >  		ret = -EINVAL;
> > >  		goto fail;
> > >  	}
> > > +	crtc_state->target_vblank = target;
> > >
> > >  	ret = drm_atomic_nonblocking_commit(state);
> > > +
> > >  fail:
> > >  	if (ret == -EDEADLK)
> > >  		goto backoff;
> > > @@ -2817,7 +2908,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> > *crtc,
> > >
> > >  	goto retry;
> > >  }
> > > -EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> > > +EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
> > >
> > >  /**
> > >   * drm_atomic_helper_connector_dpms() - connector dpms helper
> > implementation
> > > diff --git a/include/drm/drm_atomic_helper.h
> > b/include/drm/drm_atomic_helper.h
> > > index 7ff92b0..b3b3abe 100644
> > > --- a/include/drm/drm_atomic_helper.h
> > > +++ b/include/drm/drm_atomic_helper.h
> > > @@ -124,6 +124,12 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> > *crtc,
> > >  				struct drm_framebuffer *fb,
> > >  				struct drm_pending_vblank_event *event,
> > >  				uint32_t flags);
> > > +int drm_atomic_helper_page_flip_target(
> > > +				struct drm_crtc *crtc,
> > > +				struct drm_framebuffer *fb,
> > > +				struct drm_pending_vblank_event *event,
> > > +				uint32_t flags,
> > > +				uint32_t target);
> > >  int drm_atomic_helper_connector_dpms(struct drm_connector
> > *connector,
> > >  				     int mode);
> > >  struct drm_encoder *
> > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > index 946672f..5c77c3f 100644
> > > --- a/include/drm/drm_crtc.h
> > > +++ b/include/drm/drm_crtc.h
> > > @@ -157,6 +157,15 @@ struct drm_crtc_state {
> > >  	struct drm_property_blob *gamma_lut;
> > >
> > >  	/**
> > > +	 * @target_vblank:
> > > +	 *
> > > +	 * Target vertical blank period when a page flip
> > > +	 * should take effect.
> > > +	 */
> > > +
> > > +	u32 target_vblank;
> > > +
> > > +	/**
> > >  	 * @event:
> > >  	 *
> > >  	 * Optional pointer to a DRM event to signal upon completion of the
> > > --
> > > 1.9.1
> > >
> > 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
Michel Dänzer Jan. 12, 2017, 8:28 a.m. UTC | #6
On 12/01/17 12:48 AM, Grodzovsky, Andrey wrote:
>> From: Michel Dänzer [mailto:michel@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@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?
Daniel Vetter Jan. 12, 2017, 8:51 a.m. UTC | #7
On Thu, Jan 12, 2017 at 9:28 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On 12/01/17 12:48 AM, Grodzovsky, Andrey wrote:
>>> From: Michel Dänzer [mailto:michel@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@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
Andrey Grodzovsky Jan. 12, 2017, 6:18 p.m. UTC | #8
> -----Original Message-----

> From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@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@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@daenzer.net>

> wrote:

> > On 12/01/17 12:48 AM, Grodzovsky, Andrey wrote:

> >>> From: Michel Dänzer [mailto:michel@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@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.

Andrey
> --

> Daniel Vetter

> Software Engineer, Intel Corporation

> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Daniel Vetter Jan. 12, 2017, 7:22 p.m. UTC | #9
On Thu, Jan 12, 2017 at 06:18:20PM +0000, Grodzovsky, Andrey wrote:
> > -----Original Message-----
> > From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@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@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@daenzer.net>
> > wrote:
> > > On 12/01/17 12:48 AM, Grodzovsky, Andrey wrote:
> > >>> From: Michel Dänzer [mailto:michel@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@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
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 583f47f..a4e5477 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2733,6 +2733,44 @@  int drm_atomic_helper_resume(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
 
+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_plane *plane = crtc->primary;
+	struct drm_plane_state *plane_state;
+	struct drm_crtc_state *crtc_state;
+	int ret = 0;
+
+	crtc_state = drm_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	crtc_state->event = event;
+
+	plane_state = drm_atomic_get_plane_state(state, plane);
+	if (IS_ERR(plane_state))
+		return PTR_ERR(plane_state);
+
+
+	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
+	if (ret != 0)
+		return ret;
+	drm_atomic_set_fb_for_plane(plane_state, fb);
+
+	/* Make sure we don't accidentally do a full modeset. */
+	state->allow_modeset = false;
+	if (!crtc_state->active) {
+		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
+				 crtc->base.id);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
 /**
  * drm_atomic_helper_page_flip - execute a legacy page flip
  * @crtc: DRM crtc
@@ -2740,7 +2778,8 @@  int drm_atomic_helper_resume(struct drm_device *dev,
  * @event: optional DRM event to signal upon completion
  * @flags: flip flags for non-vblank sync'ed updates
  *
- * Provides a default page flip implementation using the atomic driver interface.
+ * Provides a default &drm_crtc_funcs.page_flip implementation
+ * using the atomic driver interface.
  *
  * 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
@@ -2748,6 +2787,9 @@  int drm_atomic_helper_resume(struct drm_device *dev,
  *
  * Returns:
  * Returns 0 on success, negative errno numbers on failure.
+ *
+ * See also:
+ * drm_atomic_helper_page_flip_target()
  */
 int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 				struct drm_framebuffer *fb,
@@ -2756,8 +2798,6 @@  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 {
 	struct drm_plane *plane = crtc->primary;
 	struct drm_atomic_state *state;
-	struct drm_plane_state *plane_state;
-	struct drm_crtc_state *crtc_state;
 	int ret = 0;
 
 	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
@@ -2768,35 +2808,86 @@  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 		return -ENOMEM;
 
 	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+
 retry:
-	crtc_state = drm_atomic_get_crtc_state(state, crtc);
-	if (IS_ERR(crtc_state)) {
-		ret = PTR_ERR(crtc_state);
+	ret = page_flip_common(state, crtc, fb, event);
+	if (ret != 0)
 		goto fail;
-	}
-	crtc_state->event = event;
 
-	plane_state = drm_atomic_get_plane_state(state, plane);
-	if (IS_ERR(plane_state)) {
-		ret = PTR_ERR(plane_state);
-		goto fail;
-	}
+	ret = drm_atomic_nonblocking_commit(state);
 
-	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
+fail:
+	if (ret == -EDEADLK)
+		goto backoff;
+
+	drm_atomic_state_put(state);
+	return ret;
+
+backoff:
+	drm_atomic_state_clear(state);
+	drm_atomic_legacy_backoff(state);
+
+	/*
+	 * Someone might have exchanged the framebuffer while we dropped locks
+	 * in the backoff code. We need to fix up the fb refcount tracking the
+	 * core does for us.
+	 */
+	plane->old_fb = plane->fb;
+
+	goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_page_flip);
+
+/**
+ * drm_atomic_helper_page_flip_target - do page flip on target vblank period.
+ * @crtc: DRM crtc
+ * @fb: DRM framebuffer
+ * @event: optional DRM event to signal upon completion
+ * @flags: flip flags for non-vblank sync'ed updates
+ * @target: specifying the target vblank period when the flip to take effect
+ *
+ * Provides a default &drm_crtc_funcs.page_flip_target implementation.
+ * Similar to drm_atomic_helper_page_flip() with extra parameter to specify
+ * target vblank period to flip.
+ *
+ * Returns:
+ * Returns 0 on success, negative errno numbers on failure.
+ */
+int drm_atomic_helper_page_flip_target(
+				struct drm_crtc *crtc,
+				struct drm_framebuffer *fb,
+				struct drm_pending_vblank_event *event,
+				uint32_t flags,
+				uint32_t target)
+{
+	struct drm_plane *plane = crtc->primary;
+	struct drm_atomic_state *state;
+	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;
+
+	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+
+retry:
+	ret = page_flip_common(state, crtc, fb, event);
 	if (ret != 0)
 		goto fail;
-	drm_atomic_set_fb_for_plane(plane_state, fb);
 
-	/* Make sure we don't accidentally do a full modeset. */
-	state->allow_modeset = false;
-	if (!crtc_state->active) {
-		DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
-				 crtc->base.id);
+	crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
+	if (WARN_ON(!crtc_state)) {
 		ret = -EINVAL;
 		goto fail;
 	}
+	crtc_state->target_vblank = target;
 
 	ret = drm_atomic_nonblocking_commit(state);
+
 fail:
 	if (ret == -EDEADLK)
 		goto backoff;
@@ -2817,7 +2908,7 @@  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 
 	goto retry;
 }
-EXPORT_SYMBOL(drm_atomic_helper_page_flip);
+EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
 
 /**
  * drm_atomic_helper_connector_dpms() - connector dpms helper implementation
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 7ff92b0..b3b3abe 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -124,6 +124,12 @@  int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 				struct drm_framebuffer *fb,
 				struct drm_pending_vblank_event *event,
 				uint32_t flags);
+int drm_atomic_helper_page_flip_target(
+				struct drm_crtc *crtc,
+				struct drm_framebuffer *fb,
+				struct drm_pending_vblank_event *event,
+				uint32_t flags,
+				uint32_t target);
 int drm_atomic_helper_connector_dpms(struct drm_connector *connector,
 				     int mode);
 struct drm_encoder *
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 946672f..5c77c3f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -157,6 +157,15 @@  struct drm_crtc_state {
 	struct drm_property_blob *gamma_lut;
 
 	/**
+	 * @target_vblank:
+	 *
+	 * Target vertical blank period when a page flip
+	 * should take effect.
+	 */
+
+	u32 target_vblank;
+
+	/**
 	 * @event:
 	 *
 	 * Optional pointer to a DRM event to signal upon completion of the