[Nouveau] [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip.

Harry Wentland harry.wentland at amd.com
Mon Jan 30 15:38:47 UTC 2017


On 2017-01-28 09:26 PM, Andrey Grodzovsky wrote:
> Swicth to use atomic helper.
> Start using actual user's given target_vblank value for flip 
> instead of current value.
> 
> v3:
> Update for movig pflip flags to crtc_state
> 
> Change-Id: I25dae6d8c29de5d022a42aa99a18a32674b56cda
> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |   1 -
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c    | 109 ++++-----------------
>  2 files changed, 19 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 4c0a86e..3ff3c14 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -443,7 +443,6 @@ struct amdgpu_crtc {
>  	enum amdgpu_interrupt_state vsync_timer_enabled;
>  
>  	int otg_inst;
> -	uint32_t flip_flags;
>  	/* After Set Mode target will be non-NULL */
>  	struct dc_target *target;
>  };
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> index a443b70..148780d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property(
>  	return 0;
>  }
>  
> -
> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc,
> -				struct drm_framebuffer *fb,
> -				struct drm_pending_vblank_event *event,
> -				uint32_t flags)
> -{
> -	struct drm_plane *plane = crtc->primary;
> -	struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
> -	struct drm_atomic_state *state;
> -	struct drm_plane_state *plane_state;
> -	struct drm_crtc_state *crtc_state;
> -	int ret = 0;
> -
> -	state = drm_atomic_state_alloc(plane->dev);
> -	if (!state)
> -		return -ENOMEM;
> -
> -	ret = drm_crtc_vblank_get(crtc);
> -	if (ret)
> -		return ret;
> -
> -	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);
> -		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_set_crtc_for_plane(plane_state, crtc);
> -	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);
> -		ret = -EINVAL;
> -		goto fail;
> -	}
> -	acrtc->flip_flags = flags;
> -
> -	ret = drm_atomic_nonblocking_commit(state);
> -
> -fail:
> -	if (ret == -EDEADLK)
> -		goto backoff;
> -
> -	if (ret)
> -		drm_crtc_vblank_put(crtc);
> -
> -	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;
> -}
> -
>  /* Implemented only the options currently availible for the driver */
>  static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
>  	.reset = drm_atomic_helper_crtc_reset,
> @@ -1145,7 +1068,7 @@ static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc,
>  	.destroy = amdgpu_dm_crtc_destroy,
>  	.gamma_set = amdgpu_dm_atomic_crtc_gamma_set,
>  	.set_config = drm_atomic_helper_set_config,
> -	.page_flip = amdgpu_atomic_helper_page_flip,
> +	.page_flip_target = drm_atomic_helper_page_flip_target,
>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  	.atomic_set_property = dm_crtc_funcs_atomic_set_property
> @@ -1626,7 +1549,8 @@ static void clear_unrelated_fields(struct drm_plane_state *state)
>  static bool page_flip_needed(
>  	const struct drm_plane_state *new_state,
>  	const struct drm_plane_state *old_state,
> -	bool commit_surface_required)
> +	bool commit_surface_required,
> +	uint32_t pflip_flags)
>  {
>  	struct drm_plane_state old_state_tmp;
>  	struct drm_plane_state new_state_tmp;
> @@ -1679,7 +1603,7 @@ static bool page_flip_needed(
>  				    sizeof(old_state_tmp)) == 0 ? true:false;
>  	if (new_state->crtc && page_flip_required == false) {
>  		acrtc_new = to_amdgpu_crtc(new_state->crtc);
> -		if (acrtc_new->flip_flags & DRM_MODE_PAGE_FLIP_ASYNC)
> +		if (pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC)
>  			page_flip_required = true;
>  	}
>  	return page_flip_required;
> @@ -2696,7 +2620,9 @@ int amdgpu_dm_atomic_commit(
>  		 * 1. This commit is not a page flip.
>  		 * 2. This commit is a page flip, and targets are created.
>  		 */
> -		if (!page_flip_needed(plane_state, old_plane_state, true) ||
> +		if (!page_flip_needed(
> +				plane_state, old_plane_state, true, crtc->state->pflip_flags)
> +				||
>  				action == DM_COMMIT_ACTION_DPMS_ON ||
>  				action == DM_COMMIT_ACTION_SET) {

Might be good to clean up indentation to conform a bit more to kernel
style. Something like the following, I think (I hope Thunderbird doesn't
mangle it):

		if (!page_flip_needed(plane_state,
				      old_plane_state,
				      true,
				      crtc->state->pflip_flags) ||
				action == DM_COMMIT_ACTION_DPMS_ON ||
				action == DM_COMMIT_ACTION_SET) {

>  			list_for_each_entry(connector,
> @@ -2760,21 +2686,22 @@ int amdgpu_dm_atomic_commit(
>  	for_each_plane_in_state(state, plane, old_plane_state, i) {
>  		struct drm_plane_state *plane_state = plane->state;
>  		struct drm_crtc *crtc = plane_state->crtc;
> -		struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
>  		struct drm_framebuffer *fb = plane_state->fb;
> +		uint32_t pflip_flags;
>  
>  		if (!fb || !crtc || !crtc->state->planes_changed ||
>  			!crtc->state->active)
>  			continue;
> -
> -		if (page_flip_needed(plane_state, old_plane_state, false)) {
> +		
> +		pflip_flags = crtc->state->pflip_flags;
> +		if (page_flip_needed(
> +				plane_state, old_plane_state, false, pflip_flags)) {

Indentation again.


>  			ret = amdgpu_crtc_page_flip_target(crtc,
>  							   fb,
>  							   crtc->state->event,
> -							   acrtc->flip_flags,
> -							   drm_crtc_vblank_count(crtc));
> -			/*clean up the flags for next usage*/
> -			acrtc->flip_flags = 0;
> +							   crtc->state->pflip_flags,
> +							   crtc->state->target_vblank);
> +
>  			if (ret)
>  				break;
>  		}
> @@ -3138,13 +3065,15 @@ int amdgpu_dm_atomic_check(struct drm_device *dev,
>  				continue;
>  
>  			action = get_dm_commit_action(crtc->state);
> +			crtc_state = drm_atomic_get_existing_crtc_state(state, crtc);
>  
>  			/* Surfaces are created under two scenarios:
>  			 * 1. This commit is not a page flip.
>  			 * 2. This commit is a page flip, and targets are created.
>  			 */
> -			if (!page_flip_needed(plane_state, old_plane_state,
> -					      true) ||
> +			if (!page_flip_needed(
> +					plane_state, old_plane_state, true, crtc_state->pflip_flags)
> +					||

And here as well.

Otherwise patch looks good.

Harry

>  					action == DM_COMMIT_ACTION_DPMS_ON ||
>  					action == DM_COMMIT_ACTION_SET) {
>  				struct dc_surface *surface;
> 


More information about the Nouveau mailing list