[Intel-gfx] [PATCH v2 0/6] Asynchronous flip implementation for i915

[Intel-gfx] [PATCH v2 0/6] Asynchronous flip implementation for i915

[Karthik B S]

Without async flip support in the kernel, fullscreen apps where game
resolution is equal to the screen resolution, must perform an extra blit
per frame prior to flipping.

Asynchronous page flips will also boost the FPS of Mesa benchmarks.

v2: Few patches have been squashed and patches have been shuffled as
    per the reviews on the previous version.

Karthik B S (6):
  drm/i915: Add enable/disable flip done and flip done handler
  drm/i915: Add support for async flips in I915
  drm/i915: Enable async flips in i915
  drm/i915: Make commit call blocking in case of async flips
  drm/i915: Add checks specific to async flips
  drm/i915: Do not call drm_crtc_arm_vblank_event in async flips

 drivers/gpu/drm/i915/display/intel_display.c | 87 ++++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_sprite.c  |  2 +-
 drivers/gpu/drm/i915/i915_irq.c              | 51 ++++++++++++
 drivers/gpu/drm/i915/i915_irq.h              |  2 +
 drivers/gpu/drm/i915/i915_reg.h              |  1 +
 5 files changed, 137 insertions(+), 6 deletions(-)

-- 
2.22.0

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

[Intel-gfx] [PATCH v2 1/6] drm/i915: Add enable/disable flip done and flip done handler

[Karthik B S]

Add enable/disable flip done functions and the flip done handler
function which handles the flip done interrupt.

Enable the flip done interrupt in IER.

Enable flip done function is called before writing the
surface address register as the write to this register triggers
the flip done interrupt

Flip done handler is used to send the page flip event as soon as the
surface address is written as per the requirement of async flips.
The interrupt is disabled after the event is sent.

v2: -Change function name from icl_* to skl_* (Paulo)
    -Move flip handler to this patch (Paulo)
    -Remove vblank_put() (Paulo)
    -Enable flip done interrupt for gen9+ only (Paulo)
    -Enable flip done interrupt in power_well_post_enable hook (Paulo)
    -Removed the event check in flip done handler to handle async
     flips without pageflip events.

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  7 +++
 drivers/gpu/drm/i915/i915_irq.c              | 51 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_irq.h              |  2 +
 3 files changed, 60 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index bae1d89875d6..3ce80634d047 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15391,6 +15391,13 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	if (state->modeset)
 		icl_dbuf_slice_pre_update(state);
 
+	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (new_crtc_state->uapi.async_flip) {
+			skl_enable_flip_done(&crtc->base);
+			break;
+		}
+	}
+
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	dev_priv->display.commit_modeset_enables(state);
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1502ab44f1a5..9b64ed78523e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1253,6 +1253,22 @@ display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
 			     u32 crc4) {}
 #endif
 
+static void flip_done_handler(struct drm_i915_private *dev_priv,
+			      unsigned int pipe)
+{
+	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+	struct drm_crtc_state *crtc_state = crtc->base.state;
+	struct drm_device *dev = &dev_priv->drm;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev->event_lock, irqflags);
+
+	drm_crtc_send_vblank_event(&crtc->base, crtc_state->event);
+	crtc_state->event = NULL;
+
+	spin_unlock_irqrestore(&dev->event_lock, irqflags);
+	skl_disable_flip_done(&crtc->base);
+}
 
 static void hsw_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
 				     enum pipe pipe)
@@ -2355,6 +2371,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 		if (iir & GEN8_PIPE_VBLANK)
 			intel_handle_vblank(dev_priv, pipe);
 
+		if (iir & GEN9_PIPE_PLANE1_FLIP_DONE)
+			flip_done_handler(dev_priv, pipe);
+
 		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
 			hsw_pipe_crc_irq_handler(dev_priv, pipe);
 
@@ -2636,6 +2655,19 @@ int bdw_enable_vblank(struct drm_crtc *crtc)
 	return 0;
 }
 
+void skl_enable_flip_done(struct drm_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+
+	bdw_enable_pipe_irq(dev_priv, pipe, GEN9_PIPE_PLANE1_FLIP_DONE);
+
+	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
 /* Called from drm generic code, passed 'crtc' which
  * we use as a pipe index
  */
@@ -2696,6 +2728,19 @@ void bdw_disable_vblank(struct drm_crtc *crtc)
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 
+void skl_disable_flip_done(struct drm_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+
+	bdw_disable_pipe_irq(dev_priv, pipe, GEN9_PIPE_PLANE1_FLIP_DONE);
+
+	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
 static void ibx_irq_reset(struct drm_i915_private *dev_priv)
 {
 	struct intel_uncore *uncore = &dev_priv->uncore;
@@ -2893,6 +2938,9 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
 	u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
 	enum pipe pipe;
 
+	if (INTEL_GEN(dev_priv) >= 9)
+		extra_ier |= GEN9_PIPE_PLANE1_FLIP_DONE;
+
 	spin_lock_irq(&dev_priv->irq_lock);
 
 	if (!intel_irqs_enabled(dev_priv)) {
@@ -3387,6 +3435,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
 					   GEN8_PIPE_FIFO_UNDERRUN;
 
+	if (INTEL_GEN(dev_priv) >= 9)
+		de_pipe_enables |= GEN9_PIPE_PLANE1_FLIP_DONE;
+
 	de_port_enables = de_port_masked;
 	if (IS_GEN9_LP(dev_priv))
 		de_port_enables |= BXT_DE_PORT_HOTPLUG_MASK;
diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
index 25f25cd95818..2f10c8135116 100644
--- a/drivers/gpu/drm/i915/i915_irq.h
+++ b/drivers/gpu/drm/i915/i915_irq.h
@@ -112,11 +112,13 @@ int i915gm_enable_vblank(struct drm_crtc *crtc);
 int i965_enable_vblank(struct drm_crtc *crtc);
 int ilk_enable_vblank(struct drm_crtc *crtc);
 int bdw_enable_vblank(struct drm_crtc *crtc);
+void skl_enable_flip_done(struct drm_crtc *crtc);
 void i8xx_disable_vblank(struct drm_crtc *crtc);
 void i915gm_disable_vblank(struct drm_crtc *crtc);
 void i965_disable_vblank(struct drm_crtc *crtc);
 void ilk_disable_vblank(struct drm_crtc *crtc);
 void bdw_disable_vblank(struct drm_crtc *crtc);
+void skl_disable_flip_done(struct drm_crtc *crtc);
 
 void gen2_irq_reset(struct intel_uncore *uncore);
 void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
-- 
2.22.0

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

[Intel-gfx] [PATCH v2 2/6] drm/i915: Add support for async flips in I915

[Karthik B S]

Set the Async Address Update Enable bit in plane ctl
when async flip is requested.

v2: -Move the Async flip enablement to individual patch (Paulo)

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 3 +++
 drivers/gpu/drm/i915/i915_reg.h              | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 3ce80634d047..cf8f5779dee4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4766,6 +4766,9 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
 			plane_ctl |= PLANE_CTL_YUV_RANGE_CORRECTION_DISABLE;
 	}
 
+	if (crtc_state->uapi.async_flip)
+		plane_ctl |= PLANE_CTL_ASYNC_FLIP;
+
 	plane_ctl |= skl_plane_ctl_format(fb->format->format);
 	plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
 	plane_ctl |= skl_plane_ctl_rotate(rotation & DRM_MODE_ROTATE_MASK);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9e192629d5e5..5a9a94a300ac 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6887,6 +6887,7 @@ enum {
 #define   PLANE_CTL_TILED_X			(1 << 10)
 #define   PLANE_CTL_TILED_Y			(4 << 10)
 #define   PLANE_CTL_TILED_YF			(5 << 10)
+#define   PLANE_CTL_ASYNC_FLIP			(1 << 9)
 #define   PLANE_CTL_FLIP_HORIZONTAL		(1 << 8)
 #define   PLANE_CTL_MEDIA_DECOMPRESSION_ENABLE	(1 << 4) /* TGL+ */
 #define   PLANE_CTL_ALPHA_MASK			(0x3 << 4) /* Pre-GLK */
-- 
2.22.0

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

[Intel-gfx] [PATCH v2 3/6] drm/i915: Enable async flips in i915

[Karthik B S]

Enable asynchronous flips in i915 for gen9+ platforms.

v2: -Async flip enablement should be a stand alone patch (Paulo)

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index cf8f5779dee4..8601b159f425 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -17574,6 +17574,9 @@ static void intel_mode_config_init(struct drm_i915_private *i915)
 
 	mode_config->funcs = &intel_mode_funcs;
 
+	if (INTEL_GEN(i915) >= 9)
+		mode_config->async_page_flip = true;
+
 	/*
 	 * Maximum framebuffer dimensions, chosen to match
 	 * the maximum render engine surface size on gen4+.
-- 
2.22.0

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

[Intel-gfx] [PATCH v2 4/6] drm/i915: Make commit call blocking in case of async flips

[Karthik B S]

Make the commit call blocking in case of async flips
so that there is no delay in completing the flip.

v2: -Rebased

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8601b159f425..a5203de24045 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15563,7 +15563,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 {
 	struct intel_atomic_state *state = to_intel_atomic_state(_state);
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret = 0;
+	struct intel_crtc_state *new_crtc_state;
+	struct intel_crtc *crtc;
+	int ret = 0, i;
 
 	state->wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
 
@@ -15589,10 +15591,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 	 * (assuming we had any) would solve these problems.
 	 */
 	if (INTEL_GEN(dev_priv) < 9 && state->base.legacy_cursor_update) {
-		struct intel_crtc_state *new_crtc_state;
-		struct intel_crtc *crtc;
-		int i;
-
 		for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
 			if (new_crtc_state->wm.need_postvbl_update ||
 			    new_crtc_state->update_wm_post)
@@ -15634,6 +15632,13 @@ static int intel_atomic_commit(struct drm_device *dev,
 	drm_atomic_state_get(&state->base);
 	INIT_WORK(&state->base.commit_work, intel_atomic_commit_work);
 
+	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (new_crtc_state->uapi.async_flip) {
+			nonblock = false;
+			break;
+		}
+	}
+
 	i915_sw_fence_commit(&state->commit_ready);
 	if (nonblock && state->modeset) {
 		queue_work(dev_priv->modeset_wq, &state->base.commit_work);
-- 
2.22.0

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

[Intel-gfx] [PATCH v2 5/6] drm/i915: Add checks specific to async flips

[Karthik B S]

Support added only for async flips on primary plane.
If flip is requested on any other plane, reject it.

Make sure there is no change in fbc, offset and framebuffer modifiers
when async flip is requested.

If any of these are modified, reject async flip.

v2: -Replace DRM_ERROR (Paulo)
    -Add check for changes in OFFSET, FBC, RC(Paulo)

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 59 ++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a5203de24045..ac7f26a9ac4a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14669,6 +14669,57 @@ static bool intel_cpu_transcoders_need_modeset(struct intel_atomic_state *state,
 	return false;
 }
 
+static int intel_atomic_check_async(struct intel_atomic_state *state)
+{
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
+	struct intel_plane_state *new_plane_state, *old_plane_state;
+	struct intel_crtc *crtc;
+	struct intel_plane *intel_plane;
+	int i, j;
+
+	/*FIXME: Async flip is only supported for primary plane currently
+	 * Support for overlays to be added.
+	 */
+
+	/*TODO: Check if the user space can handle the EINVAL return
+	 * or if it needs to be handled differently
+	 */
+	for_each_new_plane_in_state(&state->base, plane, plane_state, j) {
+		if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
+			DRM_DEBUG_KMS("Async flips is NOT supported for non-primary plane\n");
+			return -EINVAL;
+		}
+	}
+
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		if (old_crtc_state->enable_fbc != new_crtc_state->enable_fbc) {
+			DRM_DEBUG_KMS("FBC status cannot be changed in async flip\n");
+			return -EINVAL;
+		}
+	}
+
+	for_each_oldnew_intel_plane_in_state(state, intel_plane, old_plane_state,
+					     new_plane_state, i) {
+		if ((old_plane_state->color_plane[0].x !=
+		     new_plane_state->color_plane[0].x) ||
+		    (old_plane_state->color_plane[0].y !=
+		     new_plane_state->color_plane[0].y)) {
+			DRM_DEBUG_KMS("Offsets cannot be changed in async\n");
+			return -EINVAL;
+		}
+
+		if (old_plane_state->uapi.fb->modifier !=
+		    new_plane_state->uapi.fb->modifier) {
+			DRM_DEBUG_KMS("Framebuffer modifiers cannot be changed in async flip\n");
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
 /**
  * intel_atomic_check - validate state object
  * @dev: drm device
@@ -14697,6 +14748,14 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
+		if (new_crtc_state->uapi.async_flip) {
+			ret = intel_atomic_check_async(state);
+			if  (ret)
+				goto fail;
+		}
+	}
+
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		if (!needs_modeset(new_crtc_state)) {
-- 
2.22.0

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

[Intel-gfx] [PATCH v2 6/6] drm/i915: Do not call drm_crtc_arm_vblank_event in async flips

[Karthik B S]

Since the flip done event will be sent in the flip_done_handler,
no need to add the event to the list and delay it for later.

v2: -Moved the async check above vblank_get as it
     was causing issues for PSR.

Signed-off-by: Karthik B S <karthik.b.s@intel.com>
---
 drivers/gpu/drm/i915/display/intel_sprite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 0000ec7055f7..9b9f29768336 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -205,7 +205,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 	 * Would be slightly nice to just grab the vblank count and arm the
 	 * event outside of the critical section - the spinlock might spin for a
 	 * while ... */
-	if (new_crtc_state->uapi.event) {
+	if (new_crtc_state->uapi.event && !new_crtc_state->uapi.async_flip) {
 		drm_WARN_ON(&dev_priv->drm,
 			    drm_crtc_vblank_get(&crtc->base) != 0);
 
-- 
2.22.0

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Asynchronous flip implementation for i915 (rev2)

[Patchwork]

== Series Details ==

Series: Asynchronous flip implementation for i915 (rev2)
URL   : https://patchwork.freedesktop.org/series/74386/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ae7aa012537f drm/i915: Add enable/disable flip done and flip done handler
3c944ffc2524 drm/i915: Add support for async flips in I915
5c51d5526531 drm/i915: Enable async flips in i915
65949de8f695 drm/i915: Make commit call blocking in case of async flips
bf1d60193151 drm/i915: Add checks specific to async flips
-:61: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'old_plane_state->color_plane[0].x !=
 		     new_plane_state->color_plane[0].x'
#61: FILE: drivers/gpu/drm/i915/display/intel_display.c:14706:
+		if ((old_plane_state->color_plane[0].x !=
+		     new_plane_state->color_plane[0].x) ||
+		    (old_plane_state->color_plane[0].y !=
+		     new_plane_state->color_plane[0].y)) {

-:61: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'old_plane_state->color_plane[0].y !=
 		     new_plane_state->color_plane[0].y'
#61: FILE: drivers/gpu/drm/i915/display/intel_display.c:14706:
+		if ((old_plane_state->color_plane[0].x !=
+		     new_plane_state->color_plane[0].x) ||
+		    (old_plane_state->color_plane[0].y !=
+		     new_plane_state->color_plane[0].y)) {

total: 0 errors, 0 warnings, 2 checks, 71 lines checked
c2228f5dc3a5 drm/i915: Do not call drm_crtc_arm_vblank_event in async flips

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

[Intel-gfx] ✓ Fi.CI.BAT: success for Asynchronous flip implementation for i915 (rev2)

[Patchwork]

== Series Details ==

Series: Asynchronous flip implementation for i915 (rev2)
URL   : https://patchwork.freedesktop.org/series/74386/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8327 -> Patchwork_17378
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/index.html

Known issues
------------

  Here are the changes found in Patchwork_17378 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - fi-skl-6770hq:      [PASS][1] -> [SKIP][2] ([fdo#109271]) +20 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/fi-skl-6770hq/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/fi-skl-6770hq/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-icl-dsi:         [FAIL][3] ([i915#1569]) -> [FAIL][4] ([i915#1569] / [i915#1750])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/fi-icl-dsi/igt@runner@aborted.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/fi-icl-dsi/igt@runner@aborted.html
    - fi-icl-u2:          [FAIL][5] ([i915#1569] / [k.org#202973]) -> [FAIL][6] ([i915#1569] / [i915#1750] / [k.org#202973])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/fi-icl-u2/igt@runner@aborted.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/fi-icl-u2/igt@runner@aborted.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1569]: https://gitlab.freedesktop.org/drm/intel/issues/1569
  [i915#1750]: https://gitlab.freedesktop.org/drm/intel/issues/1750
  [k.org#202973]: https://bugzilla.kernel.org/show_bug.cgi?id=202973


Participating hosts (49 -> 44)
------------------------------

  Additional (1): fi-kbl-7560u 
  Missing    (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bdw-samus fi-byt-clapper fi-skl-6700k2 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8327 -> Patchwork_17378

  CI-20190529: 20190529
  CI_DRM_8327: 17e0a63ab93b19ea2bfccd9a0425c93e52a65246 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5602: a8fcccd15dcc2dd409edd23785a2d6f6e85fb682 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17378: c2228f5dc3a5f5f256a9d3a429297ad69602b056 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c2228f5dc3a5 drm/i915: Do not call drm_crtc_arm_vblank_event in async flips
bf1d60193151 drm/i915: Add checks specific to async flips
65949de8f695 drm/i915: Make commit call blocking in case of async flips
5c51d5526531 drm/i915: Enable async flips in i915
3c944ffc2524 drm/i915: Add support for async flips in I915
ae7aa012537f drm/i915: Add enable/disable flip done and flip done handler

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.IGT: failure for Asynchronous flip implementation for i915 (rev2)

[Patchwork]

== Series Details ==

Series: Asynchronous flip implementation for i915 (rev2)
URL   : https://patchwork.freedesktop.org/series/74386/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8327_full -> Patchwork_17378_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_17378_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_17378_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_17378_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_whisper@basic-normal-all:
    - shard-tglb:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-tglb7/igt@gem_exec_whisper@basic-normal-all.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-tglb3/igt@gem_exec_whisper@basic-normal-all.html

  * igt@runner@aborted:
    - shard-tglb:         NOTRUN -> [FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-tglb3/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_17378_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_workarounds@suspend-resume-context:
    - shard-kbl:          [PASS][4] -> [DMESG-WARN][5] ([i915#180])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-kbl2/igt@gem_workarounds@suspend-resume-context.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-kbl2/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_big_fb@linear-32bpp-rotate-0:
    - shard-kbl:          [PASS][6] -> [FAIL][7] ([i915#1119] / [i915#93] / [i915#95])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-kbl1/igt@kms_big_fb@linear-32bpp-rotate-0.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-kbl1/igt@kms_big_fb@linear-32bpp-rotate-0.html
    - shard-apl:          [PASS][8] -> [FAIL][9] ([i915#1119] / [i915#95])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-apl3/igt@kms_big_fb@linear-32bpp-rotate-0.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-apl3/igt@kms_big_fb@linear-32bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [PASS][10] -> [DMESG-WARN][11] ([i915#180]) +1 similar issue
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][12] -> [SKIP][13] ([fdo#109441]) +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-iclb6/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [PASS][14] -> [FAIL][15] ([i915#31])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-kbl2/igt@kms_setmode@basic.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-kbl2/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [DMESG-WARN][16] ([i915#180]) -> [PASS][17] +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-apl2/igt@gem_softpin@noreloc-s3.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-apl2/igt@gem_softpin@noreloc-s3.html

  * igt@i915_suspend@fence-restore-untiled:
    - shard-skl:          [INCOMPLETE][18] ([i915#69]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-skl1/igt@i915_suspend@fence-restore-untiled.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-skl3/igt@i915_suspend@fence-restore-untiled.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x128-random:
    - shard-skl:          [FAIL][20] ([i915#54]) -> [PASS][21] +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-skl1/igt@kms_cursor_crc@pipe-b-cursor-128x128-random.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-skl3/igt@kms_cursor_crc@pipe-b-cursor-128x128-random.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][22] ([i915#180]) -> [PASS][23] +5 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-kbl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [SKIP][24] ([fdo#109349]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-iclb8/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_flip_tiling@flip-to-x-tiled:
    - shard-skl:          [FAIL][26] ([i915#167]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-skl4/igt@kms_flip_tiling@flip-to-x-tiled.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-skl8/igt@kms_flip_tiling@flip-to-x-tiled.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][28] ([i915#1188]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-skl3/igt@kms_hdr@bpc-switch-dpms.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-skl6/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-iclb:         [SKIP][30] ([fdo#109441]) -> [PASS][31] +1 similar issue
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-iclb4/igt@kms_psr@psr2_primary_mmap_gtt.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-iclb2/igt@kms_psr@psr2_primary_mmap_gtt.html

  * {igt@perf@blocking-parameterized}:
    - shard-iclb:         [FAIL][32] ([i915#1542]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-iclb1/igt@perf@blocking-parameterized.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-iclb8/igt@perf@blocking-parameterized.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][34] ([i915#588]) -> [SKIP][35] ([i915#658])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-iclb7/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-tglb:         [FAIL][36] ([i915#454]) -> [SKIP][37] ([i915#468])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-tglb7/igt@i915_pm_dc@dc6-psr.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-tglb2/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@gem-mmap-type:
    - shard-snb:          [SKIP][38] ([fdo#109271]) -> [INCOMPLETE][39] ([i915#82])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8327/shard-snb6/igt@i915_pm_rpm@gem-mmap-type.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/shard-snb2/igt@i915_pm_rpm@gem-mmap-type.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#167]: https://gitlab.freedesktop.org/drm/intel/issues/167
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8327 -> Patchwork_17378

  CI-20190529: 20190529
  CI_DRM_8327: 17e0a63ab93b19ea2bfccd9a0425c93e52a65246 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5602: a8fcccd15dcc2dd409edd23785a2d6f6e85fb682 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17378: c2228f5dc3a5f5f256a9d3a429297ad69602b056 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17378/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Add checks specific to async flips

[Paulo Zanoni]

Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
> Support added only for async flips on primary plane.
> If flip is requested on any other plane, reject it.
> 
> Make sure there is no change in fbc, offset and framebuffer modifiers
> when async flip is requested.
> 
> If any of these are modified, reject async flip.
> 
> v2: -Replace DRM_ERROR (Paulo)
>     -Add check for changes in OFFSET, FBC, RC(Paulo)
> 
> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 59 ++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a5203de24045..ac7f26a9ac4a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14669,6 +14669,57 @@ static bool intel_cpu_transcoders_need_modeset(struct intel_atomic_state *state,
>  	return false;
>  }
>  
> +static int intel_atomic_check_async(struct intel_atomic_state *state)
> +{
> +	struct drm_plane *plane;
> +	struct drm_plane_state *plane_state;
> +	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> +	struct intel_plane_state *new_plane_state, *old_plane_state;
> +	struct intel_crtc *crtc;
> +	struct intel_plane *intel_plane;
> +	int i, j;
> +
> +	/*FIXME: Async flip is only supported for primary plane currently
> +	 * Support for overlays to be added.
> +	 */
> +
> +	/*TODO: Check if the user space can handle the EINVAL return
> +	 * or if it needs to be handled differently
> +	 */

Does this mean we still didn't test the series against real user space?
I mean, X server with xf86-video-modesetting and some real workload
instead of just igt. I can't feel confident to give r-b tags if I know
the patches were not tested yet. The series should probably have been
marked as an RFC.


> +	for_each_new_plane_in_state(&state->base, plane, plane_state, j) {
> +		if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
> +			DRM_DEBUG_KMS("Async flips is NOT supported for non-primary plane\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> +					    new_crtc_state, i) {
> +		if (old_crtc_state->enable_fbc != new_crtc_state->enable_fbc) {
> +			DRM_DEBUG_KMS("FBC status cannot be changed in async flip\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	for_each_oldnew_intel_plane_in_state(state, intel_plane, old_plane_state,
> +					     new_plane_state, i) {
> +		if ((old_plane_state->color_plane[0].x !=
> +		     new_plane_state->color_plane[0].x) ||
> +		    (old_plane_state->color_plane[0].y !=
> +		     new_plane_state->color_plane[0].y)) {
> +			DRM_DEBUG_KMS("Offsets cannot be changed in async\n");
> +			return -EINVAL;
> +		}
> +
> +		if (old_plane_state->uapi.fb->modifier !=
> +		    new_plane_state->uapi.fb->modifier) {
> +			DRM_DEBUG_KMS("Framebuffer modifiers cannot be changed in async flip\n");
> +			return -EINVAL;
> +		}
> +	}
> +	return 0;
> +}
> +
>  /**
>   * intel_atomic_check - validate state object
>   * @dev: drm device
> @@ -14697,6 +14748,14 @@ static int intel_atomic_check(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (new_crtc_state->uapi.async_flip) {
> +			ret = intel_atomic_check_async(state);
> +			if  (ret)
> +				goto fail;
> +		}
> +	}
> +
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
>  		if (!needs_modeset(new_crtc_state)) {

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

Re: [Intel-gfx] [PATCH v2 3/6] drm/i915: Enable async flips in i915

[Paulo Zanoni]

Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
> Enable asynchronous flips in i915 for gen9+ platforms.
> 
> v2: -Async flip enablement should be a stand alone patch (Paulo)

... and at the very end of the series.

If someone is bisecting the Kernel for some problem unrelated to async
flips, and they end up exactly at this commit, and their user space
happens to try to do async flips, will their system be broken? A quick
check at patches 4, 5 and 6 suggests they are necessary for the feature
to work, so here we're enabling a feature that we know won't work
because its support is not fully merged yet.

A patch series is not allowed to break the Kernel in the middle and
then fix it later.

> 
> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index cf8f5779dee4..8601b159f425 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -17574,6 +17574,9 @@ static void intel_mode_config_init(struct drm_i915_private *i915)
>  
>  	mode_config->funcs = &intel_mode_funcs;
>  
> +	if (INTEL_GEN(i915) >= 9)
> +		mode_config->async_page_flip = true;
> +
>  	/*
>  	 * Maximum framebuffer dimensions, chosen to match
>  	 * the maximum render engine surface size on gen4+.

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

Re: [Intel-gfx] [PATCH v2 1/6] drm/i915: Add enable/disable flip done and flip done handler

[Paulo Zanoni]

Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
> Add enable/disable flip done functions and the flip done handler
> function which handles the flip done interrupt.
> 
> Enable the flip done interrupt in IER.
> 
> Enable flip done function is called before writing the
> surface address register as the write to this register triggers
> the flip done interrupt
> 
> Flip done handler is used to send the page flip event as soon as the
> surface address is written as per the requirement of async flips.
> The interrupt is disabled after the event is sent.
> 
> v2: -Change function name from icl_* to skl_* (Paulo)
>     -Move flip handler to this patch (Paulo)
>     -Remove vblank_put() (Paulo)
>     -Enable flip done interrupt for gen9+ only (Paulo)
>     -Enable flip done interrupt in power_well_post_enable hook (Paulo)
>     -Removed the event check in flip done handler to handle async
>      flips without pageflip events.
> 
> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  7 +++
>  drivers/gpu/drm/i915/i915_irq.c              | 51 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_irq.h              |  2 +
>  3 files changed, 60 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index bae1d89875d6..3ce80634d047 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15391,6 +15391,13 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>  	if (state->modeset)
>  		icl_dbuf_slice_pre_update(state);
>  
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (new_crtc_state->uapi.async_flip) {
> +			skl_enable_flip_done(&crtc->base);
> +			break;
> +		}
> +	}
> +
>  	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>  	dev_priv->display.commit_modeset_enables(state);
>  
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1502ab44f1a5..9b64ed78523e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1253,6 +1253,22 @@ display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>  			     u32 crc4) {}
>  #endif
>  
> +static void flip_done_handler(struct drm_i915_private *dev_priv,
> +			      unsigned int pipe)
> +{
> +	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> +	struct drm_crtc_state *crtc_state = crtc->base.state;
> +	struct drm_device *dev = &dev_priv->drm;
> +	unsigned long irqflags;
> +
> +	spin_lock_irqsave(&dev->event_lock, irqflags);
> +
> +	drm_crtc_send_vblank_event(&crtc->base, crtc_state->event);
> +	crtc_state->event = NULL;
> +
> +	spin_unlock_irqrestore(&dev->event_lock, irqflags);
> +	skl_disable_flip_done(&crtc->base);

I am trying to understand the code here but I'm not 100% confident, so
my comments may be wrong. Please correct me if needed.

Can you please elaborate on why we have to disable the interrupt from
the interrupt handler? This looks racy to me, but I may be wrong, so an
explanation would help.

In my head this would be the ideal:

- If the whole ioctl is blocked until we get the interrupt (which is
what patch 04 suggests), then whatever is blocking waiting on the
interrupt should enable+disable the interrupt (so no disable_flip_done
here).

- If the ioctl is not blocked, then isn't there a race risk in case
user space finds a way to submit 2 ioctls before we get an interrupt?
If no, why would this be impossible? Some sort of refcounting could
help in this case. I'm also thinking in cases like alternating between
flips requiring events and flips not requiring events.

> +}
>  
>  static void hsw_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>  				     enum pipe pipe)
> @@ -2355,6 +2371,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  		if (iir & GEN8_PIPE_VBLANK)
>  			intel_handle_vblank(dev_priv, pipe);
>  
> +		if (iir & GEN9_PIPE_PLANE1_FLIP_DONE)
> +			flip_done_handler(dev_priv, pipe);
> +
>  		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>  			hsw_pipe_crc_irq_handler(dev_priv, pipe);
>  
> @@ -2636,6 +2655,19 @@ int bdw_enable_vblank(struct drm_crtc *crtc)
>  	return 0;
>  }
>  
> +void skl_enable_flip_done(struct drm_crtc *crtc)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	unsigned long irqflags;
> +
> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> +
> +	bdw_enable_pipe_irq(dev_priv, pipe, GEN9_PIPE_PLANE1_FLIP_DONE);
> +
> +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> +}
> +
>  /* Called from drm generic code, passed 'crtc' which
>   * we use as a pipe index
>   */
> @@ -2696,6 +2728,19 @@ void bdw_disable_vblank(struct drm_crtc *crtc)
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  }
>  
> +void skl_disable_flip_done(struct drm_crtc *crtc)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	unsigned long irqflags;
> +
> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> +
> +	bdw_disable_pipe_irq(dev_priv, pipe, GEN9_PIPE_PLANE1_FLIP_DONE);
> +
> +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> +}
> +
>  static void ibx_irq_reset(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_uncore *uncore = &dev_priv->uncore;
> @@ -2893,6 +2938,9 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
>  	u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
>  	enum pipe pipe;
>  
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		extra_ier |= GEN9_PIPE_PLANE1_FLIP_DONE;
> +
>  	spin_lock_irq(&dev_priv->irq_lock);
>  
>  	if (!intel_irqs_enabled(dev_priv)) {
> @@ -3387,6 +3435,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
>  					   GEN8_PIPE_FIFO_UNDERRUN;
>  
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		de_pipe_enables |= GEN9_PIPE_PLANE1_FLIP_DONE;
> +
>  	de_port_enables = de_port_masked;
>  	if (IS_GEN9_LP(dev_priv))
>  		de_port_enables |= BXT_DE_PORT_HOTPLUG_MASK;
> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
> index 25f25cd95818..2f10c8135116 100644
> --- a/drivers/gpu/drm/i915/i915_irq.h
> +++ b/drivers/gpu/drm/i915/i915_irq.h
> @@ -112,11 +112,13 @@ int i915gm_enable_vblank(struct drm_crtc *crtc);
>  int i965_enable_vblank(struct drm_crtc *crtc);
>  int ilk_enable_vblank(struct drm_crtc *crtc);
>  int bdw_enable_vblank(struct drm_crtc *crtc);
> +void skl_enable_flip_done(struct drm_crtc *crtc);
>  void i8xx_disable_vblank(struct drm_crtc *crtc);
>  void i915gm_disable_vblank(struct drm_crtc *crtc);
>  void i965_disable_vblank(struct drm_crtc *crtc);
>  void ilk_disable_vblank(struct drm_crtc *crtc);
>  void bdw_disable_vblank(struct drm_crtc *crtc);
> +void skl_disable_flip_done(struct drm_crtc *crtc);
>  
>  void gen2_irq_reset(struct intel_uncore *uncore);
>  void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,

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

Re: [Intel-gfx] [PATCH v2 4/6] drm/i915: Make commit call blocking in case of async flips

[Paulo Zanoni]

Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
> Make the commit call blocking in case of async flips
> so that there is no delay in completing the flip.
> 

I'm trying to understand the code. Can you please elaborate more here
in this commit message? Why exactly does the call need to block? What
would be the problem of not having this patch? And how does blocking
ensures no delay?

> v2: -Rebased
> 
> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8601b159f425..a5203de24045 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15563,7 +15563,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>  {
>  	struct intel_atomic_state *state = to_intel_atomic_state(_state);
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int ret = 0;
> +	struct intel_crtc_state *new_crtc_state;
> +	struct intel_crtc *crtc;
> +	int ret = 0, i;
>  
>  	state->wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>  
> @@ -15589,10 +15591,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	 * (assuming we had any) would solve these problems.
>  	 */
>  	if (INTEL_GEN(dev_priv) < 9 && state->base.legacy_cursor_update) {
> -		struct intel_crtc_state *new_crtc_state;
> -		struct intel_crtc *crtc;
> -		int i;
> -
>  		for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
>  			if (new_crtc_state->wm.need_postvbl_update ||
>  			    new_crtc_state->update_wm_post)
> @@ -15634,6 +15632,13 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	drm_atomic_state_get(&state->base);
>  	INIT_WORK(&state->base.commit_work, intel_atomic_commit_work);
>  
> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> +		if (new_crtc_state->uapi.async_flip) {
> +			nonblock = false;
> +			break;
> +		}
> +	}
> +
>  	i915_sw_fence_commit(&state->commit_ready);
>  	if (nonblock && state->modeset) {
>  		queue_work(dev_priv->modeset_wq, &state->base.commit_work);

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

Re: [Intel-gfx] [PATCH v2 1/6] drm/i915: Add enable/disable flip done and flip done handler

[Karthik B S]

On 4/24/2020 11:14 PM, Paulo Zanoni wrote:
> Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
>> Add enable/disable flip done functions and the flip done handler
>> function which handles the flip done interrupt.
>>
>> Enable the flip done interrupt in IER.
>>
>> Enable flip done function is called before writing the
>> surface address register as the write to this register triggers
>> the flip done interrupt
>>
>> Flip done handler is used to send the page flip event as soon as the
>> surface address is written as per the requirement of async flips.
>> The interrupt is disabled after the event is sent.
>>
>> v2: -Change function name from icl_* to skl_* (Paulo)
>>      -Move flip handler to this patch (Paulo)
>>      -Remove vblank_put() (Paulo)
>>      -Enable flip done interrupt for gen9+ only (Paulo)
>>      -Enable flip done interrupt in power_well_post_enable hook (Paulo)
>>      -Removed the event check in flip done handler to handle async
>>       flips without pageflip events.
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c |  7 +++
>>   drivers/gpu/drm/i915/i915_irq.c              | 51 ++++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_irq.h              |  2 +
>>   3 files changed, 60 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index bae1d89875d6..3ce80634d047 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -15391,6 +15391,13 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>>   	if (state->modeset)
>>   		icl_dbuf_slice_pre_update(state);
>>   
>> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>> +		if (new_crtc_state->uapi.async_flip) {
>> +			skl_enable_flip_done(&crtc->base);
>> +			break;
>> +		}
>> +	}
>> +
>>   	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>>   	dev_priv->display.commit_modeset_enables(state);
>>   
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 1502ab44f1a5..9b64ed78523e 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1253,6 +1253,22 @@ display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>>   			     u32 crc4) {}
>>   #endif
>>   
>> +static void flip_done_handler(struct drm_i915_private *dev_priv,
>> +			      unsigned int pipe)
>> +{
>> +	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
>> +	struct drm_crtc_state *crtc_state = crtc->base.state;
>> +	struct drm_device *dev = &dev_priv->drm;
>> +	unsigned long irqflags;
>> +
>> +	spin_lock_irqsave(&dev->event_lock, irqflags);
>> +
>> +	drm_crtc_send_vblank_event(&crtc->base, crtc_state->event);
>> +	crtc_state->event = NULL;
>> +
>> +	spin_unlock_irqrestore(&dev->event_lock, irqflags);
>> +	skl_disable_flip_done(&crtc->base);
> 
> I am trying to understand the code here but I'm not 100% confident, so
> my comments may be wrong. Please correct me if needed.
> 
> Can you please elaborate on why we have to disable the interrupt from
> the interrupt handler? This looks racy to me, but I may be wrong, so an
> explanation would help.
> 
> In my head this would be the ideal:
> 
> - If the whole ioctl is blocked until we get the interrupt (which is
> what patch 04 suggests), then whatever is blocking waiting on the
> interrupt should enable+disable the interrupt (so no disable_flip_done
> here).
> 
> - If the ioctl is not blocked, then isn't there a race risk in case
> user space finds a way to submit 2 ioctls before we get an interrupt?
> If no, why would this be impossible? Some sort of refcounting could
> help in this case. I'm also thinking in cases like alternating between
> flips requiring events and flips not requiring events.
> 

Agreed on this fully. My thinking was to just disable the interrupt soon 
after we've got it. But doing this in commit tail makes more sense, and 
especially now as I've made the call non blocking, it will definitely 
have a risk of being racy as you've mentioned.

Moved this out of interrupt handler to intel_atomic_commit_tail 
function, where I'm disabling it after the wait for flip done.

Thanks,
Karthik.B.S
>> +}
>>   
>>   static void hsw_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>>   				     enum pipe pipe)
>> @@ -2355,6 +2371,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>>   		if (iir & GEN8_PIPE_VBLANK)
>>   			intel_handle_vblank(dev_priv, pipe);
>>   
>> +		if (iir & GEN9_PIPE_PLANE1_FLIP_DONE)
>> +			flip_done_handler(dev_priv, pipe);
>> +
>>   		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>>   			hsw_pipe_crc_irq_handler(dev_priv, pipe);
>>   
>> @@ -2636,6 +2655,19 @@ int bdw_enable_vblank(struct drm_crtc *crtc)
>>   	return 0;
>>   }
>>   
>> +void skl_enable_flip_done(struct drm_crtc *crtc)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>> +	unsigned long irqflags;
>> +
>> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>> +
>> +	bdw_enable_pipe_irq(dev_priv, pipe, GEN9_PIPE_PLANE1_FLIP_DONE);
>> +
>> +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>> +}
>> +
>>   /* Called from drm generic code, passed 'crtc' which
>>    * we use as a pipe index
>>    */
>> @@ -2696,6 +2728,19 @@ void bdw_disable_vblank(struct drm_crtc *crtc)
>>   	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>>   }
>>   
>> +void skl_disable_flip_done(struct drm_crtc *crtc)
>> +{
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>> +	unsigned long irqflags;
>> +
>> +	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>> +
>> +	bdw_disable_pipe_irq(dev_priv, pipe, GEN9_PIPE_PLANE1_FLIP_DONE);
>> +
>> +	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>> +}
>> +
>>   static void ibx_irq_reset(struct drm_i915_private *dev_priv)
>>   {
>>   	struct intel_uncore *uncore = &dev_priv->uncore;
>> @@ -2893,6 +2938,9 @@ void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
>>   	u32 extra_ier = GEN8_PIPE_VBLANK | GEN8_PIPE_FIFO_UNDERRUN;
>>   	enum pipe pipe;
>>   
>> +	if (INTEL_GEN(dev_priv) >= 9)
>> +		extra_ier |= GEN9_PIPE_PLANE1_FLIP_DONE;
>> +
>>   	spin_lock_irq(&dev_priv->irq_lock);
>>   
>>   	if (!intel_irqs_enabled(dev_priv)) {
>> @@ -3387,6 +3435,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>>   	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
>>   					   GEN8_PIPE_FIFO_UNDERRUN;
>>   
>> +	if (INTEL_GEN(dev_priv) >= 9)
>> +		de_pipe_enables |= GEN9_PIPE_PLANE1_FLIP_DONE;
>> +
>>   	de_port_enables = de_port_masked;
>>   	if (IS_GEN9_LP(dev_priv))
>>   		de_port_enables |= BXT_DE_PORT_HOTPLUG_MASK;
>> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
>> index 25f25cd95818..2f10c8135116 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.h
>> +++ b/drivers/gpu/drm/i915/i915_irq.h
>> @@ -112,11 +112,13 @@ int i915gm_enable_vblank(struct drm_crtc *crtc);
>>   int i965_enable_vblank(struct drm_crtc *crtc);
>>   int ilk_enable_vblank(struct drm_crtc *crtc);
>>   int bdw_enable_vblank(struct drm_crtc *crtc);
>> +void skl_enable_flip_done(struct drm_crtc *crtc);
>>   void i8xx_disable_vblank(struct drm_crtc *crtc);
>>   void i915gm_disable_vblank(struct drm_crtc *crtc);
>>   void i965_disable_vblank(struct drm_crtc *crtc);
>>   void ilk_disable_vblank(struct drm_crtc *crtc);
>>   void bdw_disable_vblank(struct drm_crtc *crtc);
>> +void skl_disable_flip_done(struct drm_crtc *crtc);
>>   
>>   void gen2_irq_reset(struct intel_uncore *uncore);
>>   void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 3/6] drm/i915: Enable async flips in i915

[Karthik B S]

On 4/20/2020 11:34 PM, Paulo Zanoni wrote:
> Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
>> Enable asynchronous flips in i915 for gen9+ platforms.
>>
>> v2: -Async flip enablement should be a stand alone patch (Paulo)
> 
> ... and at the very end of the series.
> 
> If someone is bisecting the Kernel for some problem unrelated to async
> flips, and they end up exactly at this commit, and their user space
> happens to try to do async flips, will their system be broken? A quick
> check at patches 4, 5 and 6 suggests they are necessary for the feature
> to work, so here we're enabling a feature that we know won't work
> because its support is not fully merged yet.
> 
> A patch series is not allowed to break the Kernel in the middle and
> then fix it later.
> 

Understood.
Moved this patch to the end of the series.

Thanks,
Karthik.B.S
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index cf8f5779dee4..8601b159f425 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -17574,6 +17574,9 @@ static void intel_mode_config_init(struct drm_i915_private *i915)
>>   
>>   	mode_config->funcs = &intel_mode_funcs;
>>   
>> +	if (INTEL_GEN(i915) >= 9)
>> +		mode_config->async_page_flip = true;
>> +
>>   	/*
>>   	 * Maximum framebuffer dimensions, chosen to match
>>   	 * the maximum render engine surface size on gen4+.
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/6] drm/i915: Make commit call blocking in case of async flips

[Karthik B S]

On 4/24/2020 11:16 PM, Paulo Zanoni wrote:
> Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
>> Make the commit call blocking in case of async flips
>> so that there is no delay in completing the flip.
>>
> 
> I'm trying to understand the code. Can you please elaborate more here
> in this commit message? Why exactly does the call need to block? What
> would be the problem of not having this patch? And how does blocking
> ensures no delay?

My initial assumption was that blocking call would ensure lesser delay 
as commit tail is immediately called, but after running some 
benchmarking tests, its clearly not the case.

So no point in having this, removed this patch in V3.

Thanks,
Karthik.B.S
> 
>> v2: -Rebased
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 8601b159f425..a5203de24045 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -15563,7 +15563,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>>   {
>>   	struct intel_atomic_state *state = to_intel_atomic_state(_state);
>>   	struct drm_i915_private *dev_priv = to_i915(dev);
>> -	int ret = 0;
>> +	struct intel_crtc_state *new_crtc_state;
>> +	struct intel_crtc *crtc;
>> +	int ret = 0, i;
>>   
>>   	state->wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>>   
>> @@ -15589,10 +15591,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>>   	 * (assuming we had any) would solve these problems.
>>   	 */
>>   	if (INTEL_GEN(dev_priv) < 9 && state->base.legacy_cursor_update) {
>> -		struct intel_crtc_state *new_crtc_state;
>> -		struct intel_crtc *crtc;
>> -		int i;
>> -
>>   		for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
>>   			if (new_crtc_state->wm.need_postvbl_update ||
>>   			    new_crtc_state->update_wm_post)
>> @@ -15634,6 +15632,13 @@ static int intel_atomic_commit(struct drm_device *dev,
>>   	drm_atomic_state_get(&state->base);
>>   	INIT_WORK(&state->base.commit_work, intel_atomic_commit_work);
>>   
>> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>> +		if (new_crtc_state->uapi.async_flip) {
>> +			nonblock = false;
>> +			break;
>> +		}
>> +	}
>> +
>>   	i915_sw_fence_commit(&state->commit_ready);
>>   	if (nonblock && state->modeset) {
>>   		queue_work(dev_priv->modeset_wq, &state->base.commit_work);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Add checks specific to async flips

[Karthik B S]

On 4/20/2020 11:28 PM, Paulo Zanoni wrote:
> Em seg, 2020-04-20 às 15:17 +0530, Karthik B S escreveu:
>> Support added only for async flips on primary plane.
>> If flip is requested on any other plane, reject it.
>>
>> Make sure there is no change in fbc, offset and framebuffer modifiers
>> when async flip is requested.
>>
>> If any of these are modified, reject async flip.
>>
>> v2: -Replace DRM_ERROR (Paulo)
>>      -Add check for changes in OFFSET, FBC, RC(Paulo)
>>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c | 59 ++++++++++++++++++++
>>   1 file changed, 59 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index a5203de24045..ac7f26a9ac4a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -14669,6 +14669,57 @@ static bool intel_cpu_transcoders_need_modeset(struct intel_atomic_state *state,
>>   	return false;
>>   }
>>   
>> +static int intel_atomic_check_async(struct intel_atomic_state *state)
>> +{
>> +	struct drm_plane *plane;
>> +	struct drm_plane_state *plane_state;
>> +	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
>> +	struct intel_plane_state *new_plane_state, *old_plane_state;
>> +	struct intel_crtc *crtc;
>> +	struct intel_plane *intel_plane;
>> +	int i, j;
>> +
>> +	/*FIXME: Async flip is only supported for primary plane currently
>> +	 * Support for overlays to be added.
>> +	 */
>> +
>> +	/*TODO: Check if the user space can handle the EINVAL return
>> +	 * or if it needs to be handled differently
>> +	 */
> 
> Does this mean we still didn't test the series against real user space?
> I mean, X server with xf86-video-modesetting and some real workload
> instead of just igt. I can't feel confident to give r-b tags if I know
> the patches were not tested yet. The series should probably have been
> marked as an RFC.
> 

I've now run benchmarking tests for this series on Gen9 platform and the 
initial results are positive.

Thanks,
Karthik.B.S
> 
>> +	for_each_new_plane_in_state(&state->base, plane, plane_state, j) {
>> +		if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
>> +			DRM_DEBUG_KMS("Async flips is NOT supported for non-primary plane\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>> +					    new_crtc_state, i) {
>> +		if (old_crtc_state->enable_fbc != new_crtc_state->enable_fbc) {
>> +			DRM_DEBUG_KMS("FBC status cannot be changed in async flip\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	for_each_oldnew_intel_plane_in_state(state, intel_plane, old_plane_state,
>> +					     new_plane_state, i) {
>> +		if ((old_plane_state->color_plane[0].x !=
>> +		     new_plane_state->color_plane[0].x) ||
>> +		    (old_plane_state->color_plane[0].y !=
>> +		     new_plane_state->color_plane[0].y)) {
>> +			DRM_DEBUG_KMS("Offsets cannot be changed in async\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		if (old_plane_state->uapi.fb->modifier !=
>> +		    new_plane_state->uapi.fb->modifier) {
>> +			DRM_DEBUG_KMS("Framebuffer modifiers cannot be changed in async flip\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +	return 0;
>> +}
>> +
>>   /**
>>    * intel_atomic_check - validate state object
>>    * @dev: drm device
>> @@ -14697,6 +14748,14 @@ static int intel_atomic_check(struct drm_device *dev,
>>   	if (ret)
>>   		goto fail;
>>   
>> +	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>> +		if (new_crtc_state->uapi.async_flip) {
>> +			ret = intel_atomic_check_async(state);
>> +			if  (ret)
>> +				goto fail;
>> +		}
>> +	}
>> +
>>   	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>>   					    new_crtc_state, i) {
>>   		if (!needs_modeset(new_crtc_state)) {
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx