All of lore.kernel.org
 help / color / mirror / Atom feed
* [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers.
@ 2017-01-29  2:26 Andrey Grodzovsky
  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-1-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
  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
@ 2017-01-29  2:26 ` Andrey Grodzovsky
       [not found]   ` <1485656811-4211-4-git-send-email-Andrey.Grodzovsky-5C7GfCeVMHo@public.gmane.org>
       [not found] ` <1485656811-4211-1-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@public.gmane.org,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org



> -----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@public.gmane.org,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	daniel.vetter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.