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

[Andrzej Hajda]

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
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Laurent Pinchart]

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

-- 
Regards,

Laurent Pinchart

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Harry Wentland]

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

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

[Andrey Grodzovsky]

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 
---
 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) {