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