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

Daniel Vetter daniel at ffwll.ch
Thu Jan 12 07:58:21 UTC 2017


On Wed, Jan 11, 2017 at 07:57:11PM +0000, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Monday, January 09, 2017 4:59 AM
> > To: Grodzovsky, Andrey
> > Cc: dri-devel at lists.freedesktop.org; daniel.vetter at 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 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.
> 
> I had a discussion about this with Andrey and I think we are in good shape.
> 
> Acked-by: Alex Deucher <alexander.deucher at 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list