dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers.
@ 2017-01-29  2:26 Andrey Grodzovsky
       [not found] ` <1485656811-4211-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
  2017-01-29  2:26 ` [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip Andrey Grodzovsky
  0 siblings, 2 replies; 11+ messages in thread
From: Andrey Grodzovsky @ 2017-01-29  2:26 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: dc_upstream-5C7GfCeVMHo, Andrey Grodzovsky,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w

This series is a folow-up on
https://patchwork.kernel.org/patch/9501787/

The first patch makes changes to atomic helpers to allow for 
drives with ASYNC flip support to use them.
Patch 2 is to use this in AMDGPU/DC.
Patch 3 is possible cleanup in nouveau/kms who seems to have to duplicate 
the helper as we did to support ASYNC flips. 

v2: 
Resend drm/atomic: Save flip flags in drm_plane_state since
the original patch was incomplete.
Squash 2 AMD changes into one to not break compilation.

v3:
Following Daniel's comments, save flip flags in crtc_state
instead of plane_state.

Andrey Grodzovsky (3):
  drm/atomic: Save flip flags in drm_crtct_state
  drm/nouveau/kms/nv50: Switch to using atomic helper for flip.
  drm/amd/display: Switch to using atomic_helper for flip.

 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |   1 -
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c    | 109 ++++-----------------
 drivers/gpu/drm/drm_atomic_helper.c                |  19 +---
 drivers/gpu/drm/nouveau/nv50_display.c             |  84 ++--------------
 include/drm/drm_crtc.h                             |   8 +-
 include/drm/drm_plane.h                            |   1 +
 6 files changed, 42 insertions(+), 180 deletions(-)

-- 
1.9.1

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state
       [not found] ` <1485656811-4211-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
@ 2017-01-29  2:26   ` Andrey Grodzovsky
       [not found]     ` <1485656811-4211-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
  2017-01-29  2:26   ` [v3 PATCH 2/3] drm/nouveau/kms/nv50: Switch to using atomic flip helper Andrey Grodzovsky
  2017-01-30  9:18   ` [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers Emil Velikov
  2 siblings, 1 reply; 11+ messages in thread
From: Andrey Grodzovsky @ 2017-01-29  2:26 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: dc_upstream-5C7GfCeVMHo, Andrey Grodzovsky,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w

Allows using atomic flip helpers for drivers
using ASYNC flip.
Remove ASYNC_FLIP restriction in helpers and
caches the page flip flags in drm_crtc_state
to be used in the low level drivers.

v2:
Resending the patch since the original was broken.

v3:
Save flag in crtc_state instead of plane_state

Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 19 +++++--------------
 include/drm/drm_crtc.h              |  8 +++++++-
 include/drm/drm_plane.h             |  1 +
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index a4e5477..28065ee 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2737,7 +2737,8 @@ 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_pending_vblank_event *event,
+				uint32_t flags)
 {
 	struct drm_plane *plane = crtc->primary;
 	struct drm_plane_state *plane_state;
@@ -2749,12 +2750,12 @@ static int page_flip_common(
 		return PTR_ERR(crtc_state);
 
 	crtc_state->event = event;
+	crtc_state->pflip_flags = flags;
 
 	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;
@@ -2781,10 +2782,6 @@ static int page_flip_common(
  * 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
- * no provisions for this yet.
- *
  * Returns:
  * Returns 0 on success, negative errno numbers on failure.
  *
@@ -2800,9 +2797,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 	struct drm_atomic_state *state;
 	int ret = 0;
 
-	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
-		return -EINVAL;
-
 	state = drm_atomic_state_alloc(plane->dev);
 	if (!state)
 		return -ENOMEM;
@@ -2810,7 +2804,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
 
 retry:
-	ret = page_flip_common(state, crtc, fb, event);
+	ret = page_flip_common(state, crtc, fb, event, flags);
 	if (ret != 0)
 		goto fail;
 
@@ -2865,9 +2859,6 @@ int drm_atomic_helper_page_flip_target(
 	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;
@@ -2875,7 +2866,7 @@ int drm_atomic_helper_page_flip_target(
 	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
 
 retry:
-	ret = page_flip_common(state, crtc, fb, event);
+	ret = page_flip_common(state, crtc, fb, event, flags);
 	if (ret != 0)
 		goto fail;
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5c77c3f..76457a4 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -162,10 +162,16 @@ struct drm_crtc_state {
 	 * Target vertical blank period when a page flip
 	 * should take effect.
 	 */
-
 	u32 target_vblank;
 
 	/**
+	 * @pflip_flags:
+	 *
+	 * Flip related config options
+	 */
+	u32 pflip_flags;
+
+	/**
 	 * @event:
 	 *
 	 * Optional pointer to a DRM event to signal upon completion of the
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index db3bbde..57414ae 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -122,6 +122,7 @@ struct drm_plane_state {
 	 */
 	bool visible;
 
 	struct drm_atomic_state *state;
 };
 
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [v3 PATCH 2/3] drm/nouveau/kms/nv50: Switch to using atomic flip helper.
       [not found] ` <1485656811-4211-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
  2017-01-29  2:26   ` [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state Andrey Grodzovsky
@ 2017-01-29  2:26   ` Andrey Grodzovsky
  2017-01-30  9:18   ` [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers Emil Velikov
  2 siblings, 0 replies; 11+ messages in thread
From: Andrey Grodzovsky @ 2017-01-29  2:26 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: dc_upstream-5C7GfCeVMHo, Andrey Grodzovsky,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w

v3:
Update code after flip_flags moved from plane_state
to crtc_state

Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
---

 drivers/gpu/drm/nouveau/nv50_display.c | 84 ++++------------------------------
 1 file changed, 10 insertions(+), 74 deletions(-)

 P.S This patch hasn't been tested since i don't have nVidia card.

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 2c2c645..0d194ae 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -831,7 +831,8 @@ struct nv50_wndw_func {
 static int
 nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw,
 			       struct nv50_wndw_atom *asyw,
-			       struct nv50_head_atom *asyh)
+			       struct nv50_head_atom *asyh,
+			       u32 pflip_flags)
 {
 	struct nouveau_framebuffer *fb = nouveau_framebuffer(asyw->state.fb);
 	struct nouveau_drm *drm = nouveau_drm(wndw->plane.dev);
@@ -846,6 +847,9 @@ struct nv50_wndw_func {
 	asyw->image.w = fb->base.width;
 	asyw->image.h = fb->base.height;
 	asyw->image.kind = (fb->nvbo->tile_flags & 0x0000ff00) >> 8;
+
+	asyw->interval = pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? 0 : 1;
+
 	if (asyw->image.kind) {
 		asyw->image.layout = 0;
 		if (drm->device.info.chipset >= 0xc0)
@@ -883,6 +887,7 @@ struct nv50_wndw_func {
 	struct nv50_head_atom *harm = NULL, *asyh = NULL;
 	bool varm = false, asyv = false, asym = false;
 	int ret;
+	u32 pflip_flags = 0;
 
 	NV_ATOMIC(drm, "%s atomic_check\n", plane->name);
 	if (asyw->state.crtc) {
@@ -891,6 +896,7 @@ struct nv50_wndw_func {
 			return PTR_ERR(asyh);
 		asym = drm_atomic_crtc_needs_modeset(&asyh->state);
 		asyv = asyh->state.active;
+		pflip_flags = asyh->state.pflip_flags;
 	}
 
 	if (armw->state.crtc) {
@@ -907,7 +913,8 @@ struct nv50_wndw_func {
 			asyw->set.point = true;
 
 		if (!varm || asym || armw->state.fb != asyw->state.fb) {
-			ret = nv50_wndw_atomic_check_acquire(wndw, asyw, asyh);
+			ret = nv50_wndw_atomic_check_acquire(
+					wndw, asyw, asyh, pflip_flags);
 			if (ret)
 				return ret;
 		}
@@ -2221,77 +2228,6 @@ struct nv50_base {
 	.atomic_check = nv50_head_atomic_check,
 };
 
-/* This is identical to the version in the atomic helpers, except that
- * it supports non-vblanked ("async") page flips.
- */
-static int
-nv50_head_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
-		    struct drm_pending_vblank_event *event, u32 flags)
-{
-	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;
-
-	state = drm_atomic_state_alloc(plane->dev);
-	if (!state)
-		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);
-		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;
-	}
-
-	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
-		nv50_wndw_atom(plane_state)->interval = 0;
-
-	ret = drm_atomic_nonblocking_commit(state);
-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;
-}
-
 static int
 nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		    uint32_t size)
@@ -2386,7 +2322,7 @@ struct nv50_base {
 	.gamma_set = nv50_head_gamma_set,
 	.destroy = nv50_head_destroy,
 	.set_config = drm_atomic_helper_set_config,
-	.page_flip = nv50_head_page_flip,
+	.page_flip = drm_atomic_helper_page_flip,
 	.set_property = drm_atomic_helper_crtc_set_property,
 	.atomic_duplicate_state = nv50_head_atomic_duplicate_state,
 	.atomic_destroy_state = nv50_head_atomic_destroy_state,
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip.
  2017-01-29  2:26 [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers Andrey Grodzovsky
       [not found] ` <1485656811-4211-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
@ 2017-01-29  2:26 ` Andrey Grodzovsky
       [not found]   ` <1485656811-4211-4-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Andrey Grodzovsky @ 2017-01-29  2:26 UTC (permalink / raw)
  To: dri-devel; +Cc: dc_upstream, nouveau, amd-gfx, laurent.pinchart, daniel.vetter

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers.
       [not found] ` <1485656811-4211-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
  2017-01-29  2:26   ` [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state Andrey Grodzovsky
  2017-01-29  2:26   ` [v3 PATCH 2/3] drm/nouveau/kms/nv50: Switch to using atomic flip helper Andrey Grodzovsky
@ 2017-01-30  9:18   ` Emil Velikov
  2 siblings, 0 replies; 11+ messages in thread
From: Emil Velikov @ 2017-01-30  9:18 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: dc_upstream-5C7GfCeVMHo, ML nouveau, ML dri-devel,
	amd-gfx mailing list, Laurent Pinchart, Vetter, Daniel

Hi Andrey,

Unrelated suggestion:

A handy trick - to save yourself a bit of time (and "get it right")
you can use `git format-patch -vX ...' [it also works with send-email]
to have the version in the subject prefix.
Feel free to share it with the team - it seems that many people
manually edit the patch(es).

-Emil
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state
       [not found]     ` <1485656811-4211-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
@ 2017-01-30 11:05       ` Laurent Pinchart
  2017-01-30 19:42         ` Grodzovsky, Andrey
  0 siblings, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2017-01-30 11:05 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: dc_upstream-5C7GfCeVMHo,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w

Hi Andrey,

Thank you for the patch.

On Saturday 28 Jan 2017 21:26:49 Andrey Grodzovsky wrote:
> Allows using atomic flip helpers for drivers
> using ASYNC flip.
> Remove ASYNC_FLIP restriction in helpers and
> caches the page flip flags in drm_crtc_state
> to be used in the low level drivers.
> 
> v2:
> Resending the patch since the original was broken.
> 
> v3:
> Save flag in crtc_state instead of plane_state
> 
> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 19 +++++--------------
>  include/drm/drm_crtc.h              |  8 +++++++-
>  include/drm/drm_plane.h             |  1 +
>  3 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..28065ee 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2737,7 +2737,8 @@ 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_pending_vblank_event *event,
> +				uint32_t flags)
>  {
>  	struct drm_plane *plane = crtc->primary;
>  	struct drm_plane_state *plane_state;
> @@ -2749,12 +2750,12 @@ static int page_flip_common(
>  		return PTR_ERR(crtc_state);
> 
>  	crtc_state->event = event;
> +	crtc_state->pflip_flags = flags;
> 
>  	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;
> @@ -2781,10 +2782,6 @@ static int page_flip_common(
>   * 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 - * no provisions for this yet.
> - *
>   * Returns:
>   * Returns 0 on success, negative errno numbers on failure.
>   *
> @@ -2800,9 +2797,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> struct drm_atomic_state *state;
>  	int ret = 0;
> 
> -	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> -		return -EINVAL;
> -
>  	state = drm_atomic_state_alloc(plane->dev);
>  	if (!state)
>  		return -ENOMEM;
> @@ -2810,7 +2804,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> 
>  retry:
> -	ret = page_flip_common(state, crtc, fb, event);
> +	ret = page_flip_common(state, crtc, fb, event, flags);
>  	if (ret != 0)
>  		goto fail;
> 
> @@ -2865,9 +2859,6 @@ int drm_atomic_helper_page_flip_target(
>  	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;
> @@ -2875,7 +2866,7 @@ int drm_atomic_helper_page_flip_target(
>  	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> 
>  retry:
> -	ret = page_flip_common(state, crtc, fb, event);
> +	ret = page_flip_common(state, crtc, fb, event, flags);
>  	if (ret != 0)
>  		goto fail;
> 
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5c77c3f..76457a4 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -162,10 +162,16 @@ struct drm_crtc_state {
>  	 * Target vertical blank period when a page flip
>  	 * should take effect.
>  	 */
> -
>  	u32 target_vblank;
> 
>  	/**
> +	 * @pflip_flags:
> +	 *
> +	 * Flip related config options

This isn't detailed enough. I propose something along the lines of

"DRM_MODE_PAGE_FLIP_* page flip flags, as passed to the page flip ioctl. 
Always zero for atomic commits that don't originate from a page flip ioctl."

You will then also need to reset the field to 0 at an appropriate point, as 
it's more an atomic commit transaction information than a state. Apart from 
that this patch looks good to me.

> +	 */
> +	u32 pflip_flags;
> +
> +	/**
>  	 * @event:
>  	 *
>  	 * Optional pointer to a DRM event to signal upon completion of the
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index db3bbde..57414ae 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -122,6 +122,7 @@ struct drm_plane_state {
>  	 */
>  	bool visible;
> 
>  	struct drm_atomic_state *state;
>  };

-- 
Regards,

Laurent Pinchart

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip.
       [not found]   ` <1485656811-4211-4-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
@ 2017-01-30 15:38     ` Harry Wentland
       [not found]       ` <3bc6646b-1b65-6e4d-f250-287ef6325896-5C7GfCeVMHo@public.gmane.org>
  2017-02-01 11:00     ` Andrzej Hajda
  1 sibling, 1 reply; 11+ messages in thread
From: Harry Wentland @ 2017-01-30 15:38 UTC (permalink / raw)
  To: Andrey Grodzovsky, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: dc_upstream-5C7GfCeVMHo,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w

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;
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state
  2017-01-30 11:05       ` Laurent Pinchart
@ 2017-01-30 19:42         ` Grodzovsky, Andrey
       [not found]           ` <DM5PR12MB1147E9F2CB563F4CC75416B7EA4B0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Grodzovsky, Andrey @ 2017-01-30 19:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dc_upstream, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w



> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Monday, January 30, 2017 6:05 AM
> To: Grodzovsky, Andrey
> Cc: dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org;
> nouveau@lists.freedesktop.org; daniel.vetter@intel.com; dc_upstream
> Subject: Re: [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state
> 
> Hi Andrey,
> 
> Thank you for the patch.
> 
> On Saturday 28 Jan 2017 21:26:49 Andrey Grodzovsky wrote:
> > Allows using atomic flip helpers for drivers using ASYNC flip.
> > Remove ASYNC_FLIP restriction in helpers and caches the page flip
> > flags in drm_crtc_state to be used in the low level drivers.
> >
> > v2:
> > Resending the patch since the original was broken.
> >
> > v3:
> > Save flag in crtc_state instead of plane_state
> >
> > Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 19 +++++--------------
> >  include/drm/drm_crtc.h              |  8 +++++++-
> >  include/drm/drm_plane.h             |  1 +
> >  3 files changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..28065ee 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -2737,7 +2737,8 @@ 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_pending_vblank_event *event,
> > +				uint32_t flags)
> >  {
> >  	struct drm_plane *plane = crtc->primary;
> >  	struct drm_plane_state *plane_state; @@ -2749,12 +2750,12 @@
> static
> > int page_flip_common(
> >  		return PTR_ERR(crtc_state);
> >
> >  	crtc_state->event = event;
> > +	crtc_state->pflip_flags = flags;
> >
> >  	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;
> > @@ -2781,10 +2782,6 @@ static int page_flip_common(
> >   * 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 - * no provisions for this yet.
> > - *
> >   * Returns:
> >   * Returns 0 on success, negative errno numbers on failure.
> >   *
> > @@ -2800,9 +2797,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> > *crtc, struct drm_atomic_state *state;
> >  	int ret = 0;
> >
> > -	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> > -		return -EINVAL;
> > -
> >  	state = drm_atomic_state_alloc(plane->dev);
> >  	if (!state)
> >  		return -ENOMEM;
> > @@ -2810,7 +2804,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc
> > *crtc,
> > state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> >
> >  retry:
> > -	ret = page_flip_common(state, crtc, fb, event);
> > +	ret = page_flip_common(state, crtc, fb, event, flags);
> >  	if (ret != 0)
> >  		goto fail;
> >
> > @@ -2865,9 +2859,6 @@ int drm_atomic_helper_page_flip_target(
> >  	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;
> > @@ -2875,7 +2866,7 @@ int drm_atomic_helper_page_flip_target(
> >  	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> >
> >  retry:
> > -	ret = page_flip_common(state, crtc, fb, event);
> > +	ret = page_flip_common(state, crtc, fb, event, flags);
> >  	if (ret != 0)
> >  		goto fail;
> >
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> > 5c77c3f..76457a4 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -162,10 +162,16 @@ struct drm_crtc_state {
> >  	 * Target vertical blank period when a page flip
> >  	 * should take effect.
> >  	 */
> > -
> >  	u32 target_vblank;
> >
> >  	/**
> > +	 * @pflip_flags:
> > +	 *
> > +	 * Flip related config options
> 
> This isn't detailed enough. I propose something along the lines of
> 
> "DRM_MODE_PAGE_FLIP_* page flip flags, as passed to the page flip ioctl.
> Always zero for atomic commits that don't originate from a page flip ioctl."
> 
> You will then also need to reset the field to 0 at an appropriate point, as it's
> more an atomic commit transaction information than a state. Apart from that
> this patch looks good to me.
Thanks for your comments, i am not sure I understand why the reset is needed, 
for any future commit on same crtc the new state will have the field empty and 
if it's a flip IOCTL the field will be filled as needed in page_flip_common, otherwise it 
will stay empty. If the last commit on that crtc was a flip then why not keep this field 
with the bits that the user mode set ?
Thanks,
[Grodzovsky, Andrey] 
> 
> > +	 */
> > +	u32 pflip_flags;
> > +
> > +	/**
> >  	 * @event:
> >  	 *
> >  	 * Optional pointer to a DRM event to signal upon completion of the
> > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index
> > db3bbde..57414ae 100644
> > --- a/include/drm/drm_plane.h
> > +++ b/include/drm/drm_plane.h
> > @@ -122,6 +122,7 @@ struct drm_plane_state {
> >  	 */
> >  	bool visible;
> >
> >  	struct drm_atomic_state *state;
> >  };
> 
> --
> Regards,
> 
> Laurent Pinchart

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state
       [not found]           ` <DM5PR12MB1147E9F2CB563F4CC75416B7EA4B0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-02-01 10:17             ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2017-02-01 10:17 UTC (permalink / raw)
  To: Grodzovsky, Andrey
  Cc: dc_upstream, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w

Hi Andrey,

(by the way there's a typo in the subject)

On Monday 30 Jan 2017 19:42:23 Grodzovsky, Andrey wrote:
> On Monday, January 30, 2017 6:05 AM Laurent Pinchart wrote:
> > On Saturday 28 Jan 2017 21:26:49 Andrey Grodzovsky wrote:
> >> Allows using atomic flip helpers for drivers using ASYNC flip.
> >> Remove ASYNC_FLIP restriction in helpers and caches the page flip
> >> flags in drm_crtc_state to be used in the low level drivers.
> >> 
> >> v2:
> >> Resending the patch since the original was broken.
> >> 
> >> v3:
> >> Save flag in crtc_state instead of plane_state
> >> 
> >> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
> >> ---
> >> 
> >>  drivers/gpu/drm/drm_atomic_helper.c | 19 +++++--------------
> >>  include/drm/drm_crtc.h              |  8 +++++++-
> >>  include/drm/drm_plane.h             |  1 +
> >>  3 files changed, 13 insertions(+), 15 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> >> b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..28065ee 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c

[snip]

> >> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index
> >> 5c77c3f..76457a4 100644
> >> --- a/include/drm/drm_crtc.h
> >> +++ b/include/drm/drm_crtc.h
> >> @@ -162,10 +162,16 @@ struct drm_crtc_state {
> >>  	 * Target vertical blank period when a page flip
> >>  	 * should take effect.
> >>  	 */
> >>  	u32 target_vblank;
> >>  	
> >>  	/**
> >> +	 * @pflip_flags:
> >> +	 *
> >> +	 * Flip related config options
> > 
> > This isn't detailed enough. I propose something along the lines of
> > 
> > "DRM_MODE_PAGE_FLIP_* page flip flags, as passed to the page flip ioctl.
> > Always zero for atomic commits that don't originate from a page flip
> > ioctl."
> > 
> > You will then also need to reset the field to 0 at an appropriate point,
> > as it's more an atomic commit transaction information than a state. Apart
> > from that this patch looks good to me.
> 
> Thanks for your comments, i am not sure I understand why the reset is
> needed, for any future commit on same crtc the new state will have the
> field empty

It won't, __drm_atomic_helper_crtc_duplicate_state() memcpy's the state, so 
the page flip flags will be copied to the new state until another legacy page 
flip overwrites them.

> and if it's a flip IOCTL the field will be filled as needed in
> page_flip_common, otherwise it will stay empty. If the last commit on that
> crtc was a flip then why not keep this field with the bits that the user
> mode set ?

The page flip flags are not state information. They describe an operation, not 
a state. They're needed when performing the operation, but if we don't reset 
them when it completes (at the latest when duplicating the state for the next 
atomic commit, but possibly earlier) there's a risk that a driver will 
mistakenly perform an async page flip during a later operation.

> >> +	 */
> >> +	u32 pflip_flags;

[snip]

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip.
       [not found]   ` <1485656811-4211-4-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
  2017-01-30 15:38     ` Harry Wentland
@ 2017-02-01 11:00     ` Andrzej Hajda
  1 sibling, 0 replies; 11+ messages in thread
From: Andrzej Hajda @ 2017-02-01 11:00 UTC (permalink / raw)
  To: Andrey Grodzovsky, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: daniel.vetter-ral2JQCrhuEAvxtiuMwx3w, dc_upstream-5C7GfCeVMHo,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip.
       [not found]       ` <3bc6646b-1b65-6e4d-f250-287ef6325896-5C7GfCeVMHo@public.gmane.org>
@ 2017-02-01 12:26         ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2017-02-01 12:26 UTC (permalink / raw)
  To: Harry Wentland
  Cc: Andrey Grodzovsky, dc_upstream-5C7GfCeVMHo,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w

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]

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-02-01 12:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-29  2:26 [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers Andrey Grodzovsky
     [not found] ` <1485656811-4211-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
2017-01-29  2:26   ` [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state Andrey Grodzovsky
     [not found]     ` <1485656811-4211-2-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
2017-01-30 11:05       ` Laurent Pinchart
2017-01-30 19:42         ` Grodzovsky, Andrey
     [not found]           ` <DM5PR12MB1147E9F2CB563F4CC75416B7EA4B0-2J9CzHegvk9IwjGPM8RDJwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-02-01 10:17             ` Laurent Pinchart
2017-01-29  2:26   ` [v3 PATCH 2/3] drm/nouveau/kms/nv50: Switch to using atomic flip helper Andrey Grodzovsky
2017-01-30  9:18   ` [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers Emil Velikov
2017-01-29  2:26 ` [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip Andrey Grodzovsky
     [not found]   ` <1485656811-4211-4-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
2017-01-30 15:38     ` Harry Wentland
     [not found]       ` <3bc6646b-1b65-6e4d-f250-287ef6325896-5C7GfCeVMHo@public.gmane.org>
2017-02-01 12:26         ` Laurent Pinchart
2017-02-01 11:00     ` Andrzej Hajda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).