| Message ID | 1485656811-4211-4-git-send-email-Andrey.Grodzovsky@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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; >
Hi Andrey, On 29.01.2017 03:26, 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 > Gerrit relic :) Regards Andrzej
Hi Harry, On Monday 30 Jan 2017 10:38:47 Harry Wentland wrote: > 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@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(-) [snip] > > 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 [snip] > > @@ -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) { In which case you should go for (tabs replaced by spaces to avoid e-mail mangling) 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) { [snip]
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) { 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)) { 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) + || action == DM_COMMIT_ACTION_DPMS_ON || action == DM_COMMIT_ACTION_SET) { struct dc_surface *surface;
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@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(-)