[PATCH v4 0/3] Allow ASYNC flip with atomic helpers.

[PATCH v4 0/3] Allow ASYNC flip with atomic helpers.

[Andrey Grodzovsky]

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.

v4:
Lauren's comment, reset flp flags before using again.
Harry's comment, fix identation in amd/display.

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    | 113 +++++----------------
 drivers/gpu/drm/drm_atomic_helper.c                |  20 ++--
 drivers/gpu/drm/nouveau/nv50_display.c             |  84 ++-------------
 include/drm/drm_crtc.h                             |   9 +-
 include/drm/drm_plane.h                            |   1 +
 6 files changed, 49 insertions(+), 179 deletions(-)

-- 
1.9.1

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

[PATCH v4 1/3] drm/atomic: Save flip flags in drm_crtct_state

[Andrey Grodzovsky]

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

v4:
Reset the flag before using again.

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

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index a4e5477..f84b5aa 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;
 
@@ -3072,6 +3063,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 	state->color_mgmt_changed = false;
 	state->zpos_changed = false;
 	state->event = NULL;
+	state->pflip_flags = 0;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 5c77c3f..b74dbe1 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -162,10 +162,17 @@ struct drm_crtc_state {
 	 * Target vertical blank period when a page flip
 	 * should take effect.
 	 */
-
 	u32 target_vblank;
 
 	/**
+	 * @pflip_flags:
+	 *
+	 * DRM_MODE_PAGE_FLIP_* flags, as passed to the page flip ioctl.
+	 * Zero in any other case.
+	 */
+	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

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

[PATCH v4 2/3] drm/nouveau/kms/nv50: Switch to using atomic helper for flip.

[Andrey Grodzovsky]

v2:
Update code after flip_flags moved from plane_state
to crtc_state

Change-Id: I5a3189c03e389af2ff6c13d870a7d28282b7b0ee
Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
---
 drivers/gpu/drm/nouveau/nv50_display.c | 84 ++++------------------------------
 1 file changed, 10 insertions(+), 74 deletions(-)

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

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

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

[Andrey Grodzovsky]

v2:
Modify for movig pflip flags to crtc_state

v4:
Fix identation.

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    | 113 +++++----------------
 2 files changed, 24 insertions(+), 90 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..78f65ba 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,11 @@ 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 +2688,24 @@ 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 +3069,17 @@ 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

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

Re: [PATCH v4 1/3] drm/atomic: Save flip flags in drm_crtct_state

[Ville Syrjälä]

On Wed, Feb 01, 2017 at 12:03:34PM -0500, 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
> 
> v4:
> Reset the flag before using again.
> 
> Change-Id: I0219c3ec3ecceb82143ee176d30cb50d9aa76bf0
> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 20 ++++++--------------
>  include/drm/drm_crtc.h              |  9 ++++++++-
>  include/drm/drm_plane.h             |  1 +
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index a4e5477..f84b5aa 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;

Still wondering why we can't afford to spell out "page_flip" completely?

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v4 1/3] drm/atomic: Save flip flags in drm_crtct_state

[Alex Deucher]

On Wed, Feb 1, 2017 at 12:03 PM, Andrey Grodzovsky
<Andrey.Grodzovsky@amd.com> 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
>
> v4:
> Reset the flag before using again.
>
> Change-Id: I0219c3ec3ecceb82143ee176d30cb50d9aa76bf0
> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>

typo in the patch title:
drm_crtct_state



> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 20 ++++++--------------
>  include/drm/drm_crtc.h              |  9 ++++++++-
>  include/drm/drm_plane.h             |  1 +
>  3 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index a4e5477..f84b5aa 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;
>
> @@ -3072,6 +3063,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>         state->color_mgmt_changed = false;
>         state->zpos_changed = false;
>         state->event = NULL;
> +       state->pflip_flags = 0;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5c77c3f..b74dbe1 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -162,10 +162,17 @@ struct drm_crtc_state {
>          * Target vertical blank period when a page flip
>          * should take effect.
>          */
> -
>         u32 target_vblank;
>
>         /**
> +        * @pflip_flags:
> +        *
> +        * DRM_MODE_PAGE_FLIP_* flags, as passed to the page flip ioctl.
> +        * Zero in any other case.
> +        */
> +       u32 pflip_flags;

I can go either way on whether we call this pflip or page_flip.  I
guess it's better to spell it out.

> +
> +       /**
>          * @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;
>  };

Unrelated hunk.  Please drop this.

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

Re: [PATCH v4 0/3] Allow ASYNC flip with atomic helpers.

[Alex Deucher]

On Wed, Feb 1, 2017 at 12:03 PM, Andrey Grodzovsky
<Andrey.Grodzovsky@amd.com> wrote:
> 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.
>
> v4:
> Lauren's comment, reset flp flags before using again.
> Harry's comment, fix identation in amd/display.
>
> 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.


A few comments on patch 1.  WIth those addressed the series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h           |   1 -
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c    | 113 +++++----------------
>  drivers/gpu/drm/drm_atomic_helper.c                |  20 ++--
>  drivers/gpu/drm/nouveau/nv50_display.c             |  84 ++-------------
>  include/drm/drm_crtc.h                             |   9 +-
>  include/drm/drm_plane.h                            |   1 +
>  6 files changed, 49 insertions(+), 179 deletions(-)
>
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx