public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers
@ 2021-10-20 22:33 Ville Syrjala
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 1/4] drm/i915: Move function prototypes to the correct header Ville Syrjala
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Ville Syrjala @ 2021-10-20 22:33 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude Paul

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Finally got around to refreshing my vblank worker gamma LUT series.
Since I started this (ahem, some years ago) Lyude took over the
actual vblank worker implementation, mostly rewrote it I think,
and landed it for use in nouveau. So now I'm just left with the
easy task of using it for i915 gamma LUT updates. And so here
we are.

CC: Lyude Paul <lyude@redhat.com>

Ville Syrjälä (4):
  drm/i915: Move function prototypes to the correct header
  drm/i915: Do vrr push before sampling the freame counter
  drm/i915: Use vblank workers for gamma updates
  drm/i915: Use unlocked register accesses for LUT loads

 drivers/gpu/drm/i915/display/intel_color.c    | 128 +++++++++---------
 drivers/gpu/drm/i915/display/intel_crtc.c     |  82 ++++++++++-
 drivers/gpu/drm/i915/display/intel_crtc.h     |   7 +
 drivers/gpu/drm/i915/display/intel_display.c  |   9 +-
 .../drm/i915/display/intel_display_types.h    |   8 ++
 drivers/gpu/drm/i915/display/intel_dsb.c      |   4 +-
 drivers/gpu/drm/i915/display/intel_psr.c      |   2 +-
 drivers/gpu/drm/i915/display/intel_sprite.h   |   4 -
 drivers/gpu/drm/i915/i915_trace.h             |  42 ++++++
 9 files changed, 203 insertions(+), 83 deletions(-)

-- 
2.32.0


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

* [Intel-gfx] [PATCH 1/4] drm/i915: Move function prototypes to the correct header
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
@ 2021-10-20 22:33 ` Ville Syrjala
  2021-10-21 10:26   ` Jani Nikula
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 2/4] drm/i915: Do vrr push before sampling the freame counter Ville Syrjala
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2021-10-20 22:33 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

A bunch of function prototypes were left behind when the
plane/crtc code got reshuffled to new files. Move the
prototypes as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.h   | 5 +++++
 drivers/gpu/drm/i915/display/intel_psr.c    | 2 +-
 drivers/gpu/drm/i915/display/intel_sprite.h | 4 ----
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
index a5ae997581aa..22363fbbc925 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.h
+++ b/drivers/gpu/drm/i915/display/intel_crtc.h
@@ -9,10 +9,13 @@
 #include <linux/types.h>
 
 enum pipe;
+struct drm_display_mode;
 struct drm_i915_private;
 struct intel_crtc;
 struct intel_crtc_state;
 
+int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
+			     int usecs);
 u32 intel_crtc_max_vblank_count(const struct intel_crtc_state *crtc_state);
 int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe);
 struct intel_crtc_state *intel_crtc_state_alloc(struct intel_crtc *crtc);
@@ -21,5 +24,7 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
 u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
 void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
 void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
+void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
+void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
 
 #endif
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 49c2dfbd4055..ccffe05784d3 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -28,13 +28,13 @@
 
 #include "i915_drv.h"
 #include "intel_atomic.h"
+#include "intel_crtc.h"
 #include "intel_de.h"
 #include "intel_display_types.h"
 #include "intel_dp_aux.h"
 #include "intel_hdmi.h"
 #include "intel_psr.h"
 #include "intel_snps_phy.h"
-#include "intel_sprite.h"
 #include "skl_universal_plane.h"
 
 /**
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.h b/drivers/gpu/drm/i915/display/intel_sprite.h
index c085eb87705c..4f63e4967731 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.h
+++ b/drivers/gpu/drm/i915/display/intel_sprite.h
@@ -27,14 +27,10 @@ struct intel_plane_state;
 #define VBLANK_EVASION_TIME_US 100
 #endif
 
-int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
-			     int usecs);
 struct intel_plane *intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 					      enum pipe pipe, int plane);
 int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data,
 				    struct drm_file *file_priv);
-void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
-void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
 int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state);
 int chv_plane_check_rotation(const struct intel_plane_state *plane_state);
 
-- 
2.32.0


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

* [Intel-gfx] [PATCH 2/4] drm/i915: Do vrr push before sampling the freame counter
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 1/4] drm/i915: Move function prototypes to the correct header Ville Syrjala
@ 2021-10-20 22:33 ` Ville Syrjala
  2021-11-08 19:36   ` Shankar, Uma
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates Ville Syrjala
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2021-10-20 22:33 UTC (permalink / raw)
  To: intel-gfx; +Cc: Manasi Navare

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Do the vrr push before we sample the frame counter to
know when the commit has been latched. Doing these in the
wrong order could lead us to complete the flip before it
has actually happened.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 254e67141a77..0f8b48b6911c 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -542,6 +542,9 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 
 	trace_intel_pipe_update_end(crtc, end_vbl_count, scanline_end);
 
+	/* Send VRR Push to terminate Vblank */
+	intel_vrr_send_push(new_crtc_state);
+
 	/*
 	 * Incase of mipi dsi command mode, we need to set frame update
 	 * request for every commit.
@@ -568,9 +571,6 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 
 	local_irq_enable();
 
-	/* Send VRR Push to terminate Vblank */
-	intel_vrr_send_push(new_crtc_state);
-
 	if (intel_vgpu_active(dev_priv))
 		return;
 
-- 
2.32.0


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

* [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 1/4] drm/i915: Move function prototypes to the correct header Ville Syrjala
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 2/4] drm/i915: Do vrr push before sampling the freame counter Ville Syrjala
@ 2021-10-20 22:33 ` Ville Syrjala
  2021-10-21 10:35   ` Jani Nikula
  2021-11-08 20:41   ` Shankar, Uma
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT loads Ville Syrjala
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 18+ messages in thread
From: Ville Syrjala @ 2021-10-20 22:33 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude Paul

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The pipe gamma registers are single buffered so they should only
be updated during the vblank to avoid screen tearing. In fact they
really should only be updated between start of vblank and frame
start because that is the only time the pipe is guaranteed to be
empty. Already at frame start the pipe begins to fill up with
data for the next frame.

Unfortunately frame start happens ~1 scanline after the start
of vblank which in practice doesn't always leave us enough time to
finish the gamma update in time (gamma LUTs can be several KiB of
data we have to bash into the registers). However we must try our
best and so we'll add a vblank work for each pipe from where we
can do the gamma update. Additionally we could consider pushing
frame start forward to the max of ~4 scanlines after start of
vblank. But not sure that's exactly a validated configuration.
As it stands the ~100 first pixels tend to make it through with
the old gamma values.

Even though the vblank worker is running on a high prority thread
we still have to contend with C-states. If the CPU happens be in
a deep C-state when the vblank interrupt arrives even the irq
handler gets delayed massively (I've observed dozens of scanlines
worth of latency). To avoid that problem we'll use the qos mechanism
to keep the CPU awake while the vblank work is scheduled.

With all this hooked up we can finally enjoy near atomic gamma
updates. It even works across several pipes from the same atomic
commit which previously was a total fail because we did the
gamma updates for each pipe serially after waiting for all
pipes to have latched the double buffered registers.

In the future the DSB should take over this responsibility
which will hopefully avoid some of these issues.

Kudos to Lyude for finishing the actual vblank workers.
Works like the proverbial train toilet.

v2: Add missing intel_atomic_state fwd declaration
v3: Clean up properly when not scheduling the worker
v4: Clean up the rest and add tracepoints

CC: Lyude Paul <lyude@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c     | 76 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_crtc.h     |  4 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  9 +--
 .../drm/i915/display/intel_display_types.h    |  8 ++
 drivers/gpu/drm/i915/i915_trace.h             | 42 ++++++++++
 5 files changed, 129 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 0f8b48b6911c..4758c61adae8 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -3,12 +3,14 @@
  * Copyright © 2020 Intel Corporation
  */
 #include <linux/kernel.h>
+#include <linux/pm_qos.h>
 #include <linux/slab.h>
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_plane.h>
 #include <drm/drm_plane_helper.h>
+#include <drm/drm_vblank_work.h>
 
 #include "i915_trace.h"
 #include "i915_vgpu.h"
@@ -167,6 +169,8 @@ static void intel_crtc_destroy(struct drm_crtc *_crtc)
 {
 	struct intel_crtc *crtc = to_intel_crtc(_crtc);
 
+	cpu_latency_qos_remove_request(&crtc->vblank_pm_qos);
+
 	drm_crtc_cleanup(&crtc->base);
 	kfree(crtc);
 }
@@ -344,6 +348,8 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 	intel_crtc_crc_init(crtc);
 
+	cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE);
+
 	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) != crtc->pipe);
 
 	return 0;
@@ -354,6 +360,65 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	return ret;
 }
 
+static bool intel_crtc_needs_vblank_work(const struct intel_crtc_state *crtc_state)
+{
+	return crtc_state->hw.active &&
+		!intel_crtc_needs_modeset(crtc_state) &&
+		!crtc_state->preload_luts &&
+		(crtc_state->uapi.color_mgmt_changed ||
+		 crtc_state->update_pipe);
+}
+
+static void intel_crtc_vblank_work(struct kthread_work *base)
+{
+	struct drm_vblank_work *work = to_drm_vblank_work(base);
+	struct intel_crtc_state *crtc_state =
+		container_of(work, typeof(*crtc_state), vblank_work);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	trace_intel_crtc_vblank_work_start(crtc);
+
+	intel_color_load_luts(crtc_state);
+
+	if (crtc_state->uapi.event) {
+		spin_lock_irq(&crtc->base.dev->event_lock);
+		drm_crtc_send_vblank_event(&crtc->base, crtc_state->uapi.event);
+		crtc_state->uapi.event = NULL;
+		spin_unlock_irq(&crtc->base.dev->event_lock);
+	}
+
+	trace_intel_crtc_vblank_work_end(crtc);
+}
+
+static void intel_crtc_vblank_work_init(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	drm_vblank_work_init(&crtc_state->vblank_work, &crtc->base,
+			     intel_crtc_vblank_work);
+	/*
+	 * Interrupt latency is critical for getting the vblank
+	 * work executed as early as possible during the vblank.
+	 */
+	cpu_latency_qos_update_request(&crtc->vblank_pm_qos, 0);
+}
+
+void intel_wait_for_vblank_works(struct intel_atomic_state *state)
+{
+	struct intel_crtc_state *crtc_state;
+	struct intel_crtc *crtc;
+	int i;
+
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
+		if (!intel_crtc_needs_vblank_work(crtc_state))
+			continue;
+
+		drm_vblank_work_flush(&crtc_state->vblank_work);
+		cpu_latency_qos_update_request(&crtc->vblank_pm_qos,
+					       PM_QOS_DEFAULT_VALUE);
+	}
+}
+
 int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
 			     int usecs)
 {
@@ -387,7 +452,7 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
  * until a subsequent call to intel_pipe_update_end(). That is done to
  * avoid random delays.
  */
-void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
+void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -402,6 +467,9 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
 	if (new_crtc_state->uapi.async_flip)
 		return;
 
+	if (intel_crtc_needs_vblank_work(new_crtc_state))
+		intel_crtc_vblank_work_init(new_crtc_state);
+
 	if (new_crtc_state->vrr.enable)
 		vblank_start = intel_vrr_vmax_vblank_start(new_crtc_state);
 	else
@@ -557,7 +625,11 @@ 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 (intel_crtc_needs_vblank_work(new_crtc_state)) {
+		drm_vblank_work_schedule(&new_crtc_state->vblank_work,
+					 drm_crtc_accurate_vblank_count(&crtc->base) + 1,
+					 false);
+	} else if (new_crtc_state->uapi.event) {
 		drm_WARN_ON(&dev_priv->drm,
 			    drm_crtc_vblank_get(&crtc->base) != 0);
 
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
index 22363fbbc925..25eb58bce0dd 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.h
+++ b/drivers/gpu/drm/i915/display/intel_crtc.h
@@ -11,6 +11,7 @@
 enum pipe;
 struct drm_display_mode;
 struct drm_i915_private;
+struct intel_atomic_state;
 struct intel_crtc;
 struct intel_crtc_state;
 
@@ -24,7 +25,8 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
 u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
 void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
 void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
-void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
+void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state);
 void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
+void intel_wait_for_vblank_works(struct intel_atomic_state *state);
 
 #endif
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 79a7552af7b5..1375d963c0a8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8818,6 +8818,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 		intel_set_cdclk_post_plane_update(state);
 	}
 
+	intel_wait_for_vblank_works(state);
+
 	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
 	 * already, but still need the state for the delayed optimization. To
 	 * fix this:
@@ -8832,13 +8834,6 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
 		if (new_crtc_state->uapi.async_flip)
 			intel_crtc_disable_flip_done(state, crtc);
-
-		if (new_crtc_state->hw.active &&
-		    !intel_crtc_needs_modeset(new_crtc_state) &&
-		    !new_crtc_state->preload_luts &&
-		    (new_crtc_state->uapi.color_mgmt_changed ||
-		     new_crtc_state->update_pipe))
-			intel_color_load_luts(new_crtc_state);
 	}
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 1e42bf901263..8bb9264022f5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -28,6 +28,7 @@
 
 #include <linux/async.h>
 #include <linux/i2c.h>
+#include <linux/pm_qos.h>
 #include <linux/pwm.h>
 #include <linux/sched/clock.h>
 
@@ -41,6 +42,7 @@
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_rect.h>
 #include <drm/drm_vblank.h>
+#include <drm/drm_vblank_work.h>
 #include <drm/i915_mei_hdcp_interface.h>
 #include <media/cec-notifier.h>
 
@@ -1241,6 +1243,9 @@ struct intel_crtc_state {
 		u8 link_count;
 		u8 pixel_overlap;
 	} splitter;
+
+	/* for loading single buffered registers during vblank */
+	struct drm_vblank_work vblank_work;
 };
 
 enum intel_pipe_crc_source {
@@ -1325,6 +1330,9 @@ struct intel_crtc {
 	/* scalers available on this crtc */
 	int num_scalers;
 
+	/* for loading single buffered registers during vblank */
+	struct pm_qos_request vblank_pm_qos;
+
 #ifdef CONFIG_DEBUG_FS
 	struct intel_pipe_crc pipe_crc;
 #endif
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 9795f456cccf..fc48a16d7a4f 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -404,6 +404,48 @@ TRACE_EVENT(intel_fbc_nuke,
 
 /* pipe updates */
 
+TRACE_EVENT(intel_crtc_vblank_work_start,
+	    TP_PROTO(struct intel_crtc *crtc),
+	    TP_ARGS(crtc),
+
+	    TP_STRUCT__entry(
+			     __field(enum pipe, pipe)
+			     __field(u32, frame)
+			     __field(u32, scanline)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->pipe = crtc->pipe;
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   ),
+
+	    TP_printk("pipe %c, frame=%u, scanline=%u",
+		      pipe_name(__entry->pipe), __entry->frame,
+		       __entry->scanline)
+);
+
+TRACE_EVENT(intel_crtc_vblank_work_end,
+	    TP_PROTO(struct intel_crtc *crtc),
+	    TP_ARGS(crtc),
+
+	    TP_STRUCT__entry(
+			     __field(enum pipe, pipe)
+			     __field(u32, frame)
+			     __field(u32, scanline)
+			     ),
+
+	    TP_fast_assign(
+			   __entry->pipe = crtc->pipe;
+			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
+			   __entry->scanline = intel_get_crtc_scanline(crtc);
+			   ),
+
+	    TP_printk("pipe %c, frame=%u, scanline=%u",
+		      pipe_name(__entry->pipe), __entry->frame,
+		       __entry->scanline)
+);
+
 TRACE_EVENT(intel_pipe_update_start,
 	    TP_PROTO(struct intel_crtc *crtc),
 	    TP_ARGS(crtc),
-- 
2.32.0


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

* [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT loads
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
                   ` (2 preceding siblings ...)
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates Ville Syrjala
@ 2021-10-20 22:33 ` Ville Syrjala
  2021-11-08 20:59   ` Shankar, Uma
  2021-10-21  0:59 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: (near)atomic gamma LUT updates via vblank workers Patchwork
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2021-10-20 22:33 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We have to bash in a lot of registers to load the higher
precision LUT modes. The locking overhead is significant, especially
as we have to get this done as quickly as possible during vblank.
So let's switch to unlocked accesses for these. Fortunately the LUT
registers are mostly spread around such that two pipes do not have
any registers on the same cacheline. So as long as commits on the
same pipe are serialized (which they are) we should get away with
this without angering the hardware.

The only exceptions are the PREC_PIPEGCMAX registers on ilk/snb which
we don't use atm as they are only used in the 12bit gamma mode. If/when
we add support for that we may need to remember to still serialize
those registers, though I'm not sure ilk/snb are actually affected
by the same cacheline issue. I think ivb/hsw at least were, but they
use a different set of registers for the precision LUT.

I have a test case which is updating the LUTs on two pipes from a
single atomic commit. Running that in a loop for a minute I get the
following worst case with the locks in place:
 intel_crtc_vblank_work_start: pipe B, frame=10037, scanline=1081
 intel_crtc_vblank_work_start: pipe A, frame=12274, scanline=769
 intel_crtc_vblank_work_end: pipe A, frame=12274, scanline=58
 intel_crtc_vblank_work_end: pipe B, frame=10037, scanline=74

And here's the worst case with the locks removed:
 intel_crtc_vblank_work_start: pipe B, frame=5869, scanline=1081
 intel_crtc_vblank_work_start: pipe A, frame=7616, scanline=769
 intel_crtc_vblank_work_end: pipe B, frame=5869, scanline=1096
 intel_crtc_vblank_work_end: pipe A, frame=7616, scanline=777

The test was done on a snb using the 10bit 1024 entry LUT mode.
The vtotals for the two displays are 793 and 1125. So we can
see that with the locks ripped out the LUT updates are pretty
nicely confined within the vblank, whereas with the locks in
place we're routinely blasting past the vblank end which causes
visual artifacts near the top of the screen.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 128 ++++++++++-----------
 drivers/gpu/drm/i915/display/intel_dsb.c   |   4 +-
 2 files changed, 66 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 5359b7305a78..c870a0e50cb1 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -552,8 +552,8 @@ static void i9xx_load_lut_8(struct intel_crtc *crtc,
 	lut = blob->data;
 
 	for (i = 0; i < 256; i++)
-		intel_de_write(dev_priv, PALETTE(pipe, i),
-			       i9xx_lut_8(&lut[i]));
+		intel_de_write_fw(dev_priv, PALETTE(pipe, i),
+				  i9xx_lut_8(&lut[i]));
 }
 
 static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
@@ -576,15 +576,15 @@ static void i965_load_lut_10p6(struct intel_crtc *crtc,
 	enum pipe pipe = crtc->pipe;
 
 	for (i = 0; i < lut_size - 1; i++) {
-		intel_de_write(dev_priv, PALETTE(pipe, 2 * i + 0),
-			       i965_lut_10p6_ldw(&lut[i]));
-		intel_de_write(dev_priv, PALETTE(pipe, 2 * i + 1),
-			       i965_lut_10p6_udw(&lut[i]));
+		intel_de_write_fw(dev_priv, PALETTE(pipe, 2 * i + 0),
+				  i965_lut_10p6_ldw(&lut[i]));
+		intel_de_write_fw(dev_priv, PALETTE(pipe, 2 * i + 1),
+				  i965_lut_10p6_udw(&lut[i]));
 	}
 
-	intel_de_write(dev_priv, PIPEGCMAX(pipe, 0), lut[i].red);
-	intel_de_write(dev_priv, PIPEGCMAX(pipe, 1), lut[i].green);
-	intel_de_write(dev_priv, PIPEGCMAX(pipe, 2), lut[i].blue);
+	intel_de_write_fw(dev_priv, PIPEGCMAX(pipe, 0), lut[i].red);
+	intel_de_write_fw(dev_priv, PIPEGCMAX(pipe, 1), lut[i].green);
+	intel_de_write_fw(dev_priv, PIPEGCMAX(pipe, 2), lut[i].blue);
 }
 
 static void i965_load_luts(const struct intel_crtc_state *crtc_state)
@@ -618,8 +618,8 @@ static void ilk_load_lut_8(struct intel_crtc *crtc,
 	lut = blob->data;
 
 	for (i = 0; i < 256; i++)
-		intel_de_write(dev_priv, LGC_PALETTE(pipe, i),
-			       i9xx_lut_8(&lut[i]));
+		intel_de_write_fw(dev_priv, LGC_PALETTE(pipe, i),
+				  i9xx_lut_8(&lut[i]));
 }
 
 static void ilk_load_lut_10(struct intel_crtc *crtc,
@@ -631,8 +631,8 @@ static void ilk_load_lut_10(struct intel_crtc *crtc,
 	enum pipe pipe = crtc->pipe;
 
 	for (i = 0; i < lut_size; i++)
-		intel_de_write(dev_priv, PREC_PALETTE(pipe, i),
-			       ilk_lut_10(&lut[i]));
+		intel_de_write_fw(dev_priv, PREC_PALETTE(pipe, i),
+				  ilk_lut_10(&lut[i]));
 }
 
 static void ilk_load_luts(const struct intel_crtc_state *crtc_state)
@@ -681,16 +681,16 @@ static void ivb_load_lut_10(struct intel_crtc *crtc,
 		const struct drm_color_lut *entry =
 			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
 
-		intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), prec_index++);
-		intel_de_write(dev_priv, PREC_PAL_DATA(pipe),
-			       ilk_lut_10(entry));
+		intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), prec_index++);
+		intel_de_write_fw(dev_priv, PREC_PAL_DATA(pipe),
+				  ilk_lut_10(entry));
 	}
 
 	/*
 	 * Reset the index, otherwise it prevents the legacy palette to be
 	 * written properly.
 	 */
-	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), 0);
+	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), 0);
 }
 
 /* On BDW+ the index auto increment mode actually works */
@@ -704,23 +704,23 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
 	int i, lut_size = drm_color_lut_size(blob);
 	enum pipe pipe = crtc->pipe;
 
-	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe),
-		       prec_index | PAL_PREC_AUTO_INCREMENT);
+	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe),
+			  prec_index | PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < hw_lut_size; i++) {
 		/* We discard half the user entries in split gamma mode */
 		const struct drm_color_lut *entry =
 			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
 
-		intel_de_write(dev_priv, PREC_PAL_DATA(pipe),
-			       ilk_lut_10(entry));
+		intel_de_write_fw(dev_priv, PREC_PAL_DATA(pipe),
+				  ilk_lut_10(entry));
 	}
 
 	/*
 	 * Reset the index, otherwise it prevents the legacy palette to be
 	 * written properly.
 	 */
-	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), 0);
+	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), 0);
 }
 
 static void ivb_load_lut_ext_max(const struct intel_crtc_state *crtc_state)
@@ -821,9 +821,9 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
 	 * ignore the index bits, so we need to reset it to index 0
 	 * separately.
 	 */
-	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
-	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
-		       PRE_CSC_GAMC_AUTO_INCREMENT);
+	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
+	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
+			  PRE_CSC_GAMC_AUTO_INCREMENT);
 
 	for (i = 0; i < lut_size; i++) {
 		/*
@@ -839,15 +839,15 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
 		 * ToDo: Extend to max 7.0. Enable 32 bit input value
 		 * as compared to just 16 to achieve this.
 		 */
-		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe),
-			       lut[i].green);
+		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe),
+				  lut[i].green);
 	}
 
 	/* Clamp values > 1.0. */
 	while (i++ < 35)
-		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
+		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
 
-	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
+	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
 }
 
 static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_state)
@@ -862,21 +862,21 @@ static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_stat
 	 * ignore the index bits, so we need to reset it to index 0
 	 * separately.
 	 */
-	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
-	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
-		       PRE_CSC_GAMC_AUTO_INCREMENT);
+	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
+	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
+			  PRE_CSC_GAMC_AUTO_INCREMENT);
 
 	for (i = 0; i < lut_size; i++) {
 		u32 v = (i << 16) / (lut_size - 1);
 
-		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe), v);
+		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), v);
 	}
 
 	/* Clamp values > 1.0. */
 	while (i++ < 35)
-		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
+		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
 
-	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
+	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
 }
 
 static void glk_load_luts(const struct intel_crtc_state *crtc_state)
@@ -1071,10 +1071,10 @@ static void chv_load_cgm_degamma(struct intel_crtc *crtc,
 	enum pipe pipe = crtc->pipe;
 
 	for (i = 0; i < lut_size; i++) {
-		intel_de_write(dev_priv, CGM_PIPE_DEGAMMA(pipe, i, 0),
-			       chv_cgm_degamma_ldw(&lut[i]));
-		intel_de_write(dev_priv, CGM_PIPE_DEGAMMA(pipe, i, 1),
-			       chv_cgm_degamma_udw(&lut[i]));
+		intel_de_write_fw(dev_priv, CGM_PIPE_DEGAMMA(pipe, i, 0),
+				  chv_cgm_degamma_ldw(&lut[i]));
+		intel_de_write_fw(dev_priv, CGM_PIPE_DEGAMMA(pipe, i, 1),
+				  chv_cgm_degamma_udw(&lut[i]));
 	}
 }
 
@@ -1105,10 +1105,10 @@ static void chv_load_cgm_gamma(struct intel_crtc *crtc,
 	enum pipe pipe = crtc->pipe;
 
 	for (i = 0; i < lut_size; i++) {
-		intel_de_write(dev_priv, CGM_PIPE_GAMMA(pipe, i, 0),
-			       chv_cgm_gamma_ldw(&lut[i]));
-		intel_de_write(dev_priv, CGM_PIPE_GAMMA(pipe, i, 1),
-			       chv_cgm_gamma_udw(&lut[i]));
+		intel_de_write_fw(dev_priv, CGM_PIPE_GAMMA(pipe, i, 0),
+				  chv_cgm_gamma_ldw(&lut[i]));
+		intel_de_write_fw(dev_priv, CGM_PIPE_GAMMA(pipe, i, 1),
+				  chv_cgm_gamma_udw(&lut[i]));
 	}
 }
 
@@ -1131,8 +1131,8 @@ static void chv_load_luts(const struct intel_crtc_state *crtc_state)
 	else
 		i965_load_luts(crtc_state);
 
-	intel_de_write(dev_priv, CGM_PIPE_MODE(crtc->pipe),
-		       crtc_state->cgm_mode);
+	intel_de_write_fw(dev_priv, CGM_PIPE_MODE(crtc->pipe),
+			  crtc_state->cgm_mode);
 }
 
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
@@ -1808,7 +1808,7 @@ static struct drm_property_blob *i9xx_read_lut_8(struct intel_crtc *crtc)
 	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
-		u32 val = intel_de_read(dev_priv, PALETTE(pipe, i));
+		u32 val = intel_de_read_fw(dev_priv, PALETTE(pipe, i));
 
 		i9xx_lut_8_pack(&lut[i], val);
 	}
@@ -1843,15 +1843,15 @@ static struct drm_property_blob *i965_read_lut_10p6(struct intel_crtc *crtc)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size - 1; i++) {
-		u32 ldw = intel_de_read(dev_priv, PALETTE(pipe, 2 * i + 0));
-		u32 udw = intel_de_read(dev_priv, PALETTE(pipe, 2 * i + 1));
+		u32 ldw = intel_de_read_fw(dev_priv, PALETTE(pipe, 2 * i + 0));
+		u32 udw = intel_de_read_fw(dev_priv, PALETTE(pipe, 2 * i + 1));
 
 		i965_lut_10p6_pack(&lut[i], ldw, udw);
 	}
 
-	lut[i].red = i965_lut_11p6_max_pack(intel_de_read(dev_priv, PIPEGCMAX(pipe, 0)));
-	lut[i].green = i965_lut_11p6_max_pack(intel_de_read(dev_priv, PIPEGCMAX(pipe, 1)));
-	lut[i].blue = i965_lut_11p6_max_pack(intel_de_read(dev_priv, PIPEGCMAX(pipe, 2)));
+	lut[i].red = i965_lut_11p6_max_pack(intel_de_read_fw(dev_priv, PIPEGCMAX(pipe, 0)));
+	lut[i].green = i965_lut_11p6_max_pack(intel_de_read_fw(dev_priv, PIPEGCMAX(pipe, 1)));
+	lut[i].blue = i965_lut_11p6_max_pack(intel_de_read_fw(dev_priv, PIPEGCMAX(pipe, 2)));
 
 	return blob;
 }
@@ -1886,8 +1886,8 @@ static struct drm_property_blob *chv_read_cgm_gamma(struct intel_crtc *crtc)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
-		u32 ldw = intel_de_read(dev_priv, CGM_PIPE_GAMMA(pipe, i, 0));
-		u32 udw = intel_de_read(dev_priv, CGM_PIPE_GAMMA(pipe, i, 1));
+		u32 ldw = intel_de_read_fw(dev_priv, CGM_PIPE_GAMMA(pipe, i, 0));
+		u32 udw = intel_de_read_fw(dev_priv, CGM_PIPE_GAMMA(pipe, i, 1));
 
 		chv_cgm_gamma_pack(&lut[i], ldw, udw);
 	}
@@ -1922,7 +1922,7 @@ static struct drm_property_blob *ilk_read_lut_8(struct intel_crtc *crtc)
 	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
-		u32 val = intel_de_read(dev_priv, LGC_PALETTE(pipe, i));
+		u32 val = intel_de_read_fw(dev_priv, LGC_PALETTE(pipe, i));
 
 		i9xx_lut_8_pack(&lut[i], val);
 	}
@@ -1947,7 +1947,7 @@ static struct drm_property_blob *ilk_read_lut_10(struct intel_crtc *crtc)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
-		u32 val = intel_de_read(dev_priv, PREC_PALETTE(pipe, i));
+		u32 val = intel_de_read_fw(dev_priv, PREC_PALETTE(pipe, i));
 
 		ilk_lut_10_pack(&lut[i], val);
 	}
@@ -1999,16 +1999,16 @@ static struct drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
 
 	lut = blob->data;
 
-	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe),
-		       prec_index | PAL_PREC_AUTO_INCREMENT);
+	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe),
+			  prec_index | PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < lut_size; i++) {
-		u32 val = intel_de_read(dev_priv, PREC_PAL_DATA(pipe));
+		u32 val = intel_de_read_fw(dev_priv, PREC_PAL_DATA(pipe));
 
 		ilk_lut_10_pack(&lut[i], val);
 	}
 
-	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), 0);
+	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), 0);
 
 	return blob;
 }
@@ -2050,17 +2050,17 @@ icl_read_lut_multi_segment(struct intel_crtc *crtc)
 
 	lut = blob->data;
 
-	intel_de_write(dev_priv, PREC_PAL_MULTI_SEG_INDEX(pipe),
-		       PAL_PREC_AUTO_INCREMENT);
+	intel_de_write_fw(dev_priv, PREC_PAL_MULTI_SEG_INDEX(pipe),
+			  PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < 9; i++) {
-		u32 ldw = intel_de_read(dev_priv, PREC_PAL_MULTI_SEG_DATA(pipe));
-		u32 udw = intel_de_read(dev_priv, PREC_PAL_MULTI_SEG_DATA(pipe));
+		u32 ldw = intel_de_read_fw(dev_priv, PREC_PAL_MULTI_SEG_DATA(pipe));
+		u32 udw = intel_de_read_fw(dev_priv, PREC_PAL_MULTI_SEG_DATA(pipe));
 
 		icl_lut_multi_seg_pack(&lut[i], ldw, udw);
 	}
 
-	intel_de_write(dev_priv, PREC_PAL_MULTI_SEG_INDEX(pipe), 0);
+	intel_de_write_fw(dev_priv, PREC_PAL_MULTI_SEG_INDEX(pipe), 0);
 
 	/*
 	 * FIXME readouts from PAL_PREC_DATA register aren't giving
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 62a8a69f9f5d..83a69a4a4fea 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -100,7 +100,7 @@ void intel_dsb_indexed_reg_write(const struct intel_crtc_state *crtc_state,
 	u32 reg_val;
 
 	if (!dsb) {
-		intel_de_write(dev_priv, reg, val);
+		intel_de_write_fw(dev_priv, reg, val);
 		return;
 	}
 	buf = dsb->cmd_buf;
@@ -177,7 +177,7 @@ void intel_dsb_reg_write(const struct intel_crtc_state *crtc_state,
 
 	dsb = crtc_state->dsb;
 	if (!dsb) {
-		intel_de_write(dev_priv, reg, val);
+		intel_de_write_fw(dev_priv, reg, val);
 		return;
 	}
 
-- 
2.32.0


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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: (near)atomic gamma LUT updates via vblank workers
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
                   ` (3 preceding siblings ...)
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT loads Ville Syrjala
@ 2021-10-21  0:59 ` Patchwork
  2021-10-21  1:00 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2021-10-21  0:59 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: (near)atomic gamma LUT updates via vblank workers
URL   : https://patchwork.freedesktop.org/series/96089/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
889072d8ac0e drm/i915: Move function prototypes to the correct header
0b7c1b5f5243 drm/i915: Do vrr push before sampling the freame counter
b9a5bb22d814 drm/i915: Use vblank workers for gamma updates
-:289: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#289: FILE: drivers/gpu/drm/i915/i915_trace.h:411:
+	    TP_STRUCT__entry(

-:295: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#295: FILE: drivers/gpu/drm/i915/i915_trace.h:417:
+	    TP_fast_assign(

-:310: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#310: FILE: drivers/gpu/drm/i915/i915_trace.h:432:
+	    TP_STRUCT__entry(

-:316: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#316: FILE: drivers/gpu/drm/i915/i915_trace.h:438:
+	    TP_fast_assign(

total: 0 errors, 0 warnings, 4 checks, 241 lines checked
6c8274b2061c drm/i915: Use unlocked register accesses for LUT loads



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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: (near)atomic gamma LUT updates via vblank workers
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
                   ` (4 preceding siblings ...)
  2021-10-21  0:59 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: (near)atomic gamma LUT updates via vblank workers Patchwork
@ 2021-10-21  1:00 ` Patchwork
  2021-10-21  1:29 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2021-10-21  1:00 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: (near)atomic gamma LUT updates via vblank workers
URL   : https://patchwork.freedesktop.org/series/96089/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_reset.c:1392:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/i915_perf.c:1442:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1496:15: warning: memset with byte count of 16777216
+./include/asm-generic/bitops/find.h:112:45: warning: shift count is negative (-262080)
+./include/asm-generic/bitops/find.h:32:31: warning: shift count is negative (-262080)
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block



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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: (near)atomic gamma LUT updates via vblank workers
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
                   ` (5 preceding siblings ...)
  2021-10-21  1:00 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2021-10-21  1:29 ` Patchwork
  2021-10-21  7:11 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2021-10-26 22:02 ` [Intel-gfx] [PATCH 0/4] " Lyude Paul
  8 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2021-10-21  1:29 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 4063 bytes --]

== Series Details ==

Series: drm/i915: (near)atomic gamma LUT updates via vblank workers
URL   : https://patchwork.freedesktop.org/series/96089/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10767 -> Patchwork_21399
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@sync-fork-compute0:
    - fi-snb-2600:        NOTRUN -> [SKIP][1] ([fdo#109271]) +17 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/fi-snb-2600/igt@amdgpu/amd_cs_nop@sync-fork-compute0.html

  * igt@debugfs_test@read_all_entries:
    - fi-kbl-soraka:      [PASS][2] -> [DMESG-WARN][3] ([i915#1982] / [i915#262])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/fi-kbl-soraka/igt@debugfs_test@read_all_entries.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/fi-kbl-soraka/igt@debugfs_test@read_all_entries.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cml-u2:          [PASS][4] -> [DMESG-WARN][5] ([i915#4269])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@hangcheck:
    - fi-snb-2600:        [INCOMPLETE][6] ([i915#3921]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@kms_flip@basic-plain-flip@c-dp1:
    - fi-cfl-8109u:       [FAIL][8] ([i915#4165]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp1.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp1.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b:
    - fi-cfl-8109u:       [DMESG-WARN][10] ([i915#295]) -> [PASS][11] +14 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/fi-cfl-8109u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/fi-cfl-8109u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262
  [i915#295]: https://gitlab.freedesktop.org/drm/intel/issues/295
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4165]: https://gitlab.freedesktop.org/drm/intel/issues/4165
  [i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269


Participating hosts (41 -> 37)
------------------------------

  Missing    (4): fi-ctg-p8600 fi-bsw-cyan bat-dg1-6 fi-hsw-4200u 


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

  * Linux: CI_DRM_10767 -> Patchwork_21399

  CI-20190529: 20190529
  CI_DRM_10767: 4d947bb057406e5c30081736db70da3f5726e0cd @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6258: 4c80c71d7dec29b6376846ae96bd04dc0b6e34d9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_21399: 6c8274b2061ce9afe497ca680479ab7fb295e9f0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6c8274b2061c drm/i915: Use unlocked register accesses for LUT loads
b9a5bb22d814 drm/i915: Use vblank workers for gamma updates
0b7c1b5f5243 drm/i915: Do vrr push before sampling the freame counter
889072d8ac0e drm/i915: Move function prototypes to the correct header

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/index.html

[-- Attachment #2: Type: text/html, Size: 4826 bytes --]

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: (near)atomic gamma LUT updates via vblank workers
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
                   ` (6 preceding siblings ...)
  2021-10-21  1:29 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2021-10-21  7:11 ` Patchwork
  2021-10-26 22:02 ` [Intel-gfx] [PATCH 0/4] " Lyude Paul
  8 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2021-10-21  7:11 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 30282 bytes --]

== Series Details ==

Series: drm/i915: (near)atomic gamma LUT updates via vblank workers
URL   : https://patchwork.freedesktop.org/series/96089/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10767_full -> Patchwork_21399_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_create@create-massive:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][1] ([i915#3002])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl7/igt@gem_create@create-massive.html

  * igt@gem_ctx_persistence@legacy-engines-persistence:
    - shard-snb:          NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#1099]) +4 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-snb7/igt@gem_ctx_persistence@legacy-engines-persistence.html

  * igt@gem_exec_fair@basic-none@rcs0:
    - shard-kbl:          [PASS][3] -> [FAIL][4] ([i915#2842]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl7/igt@gem_exec_fair@basic-none@rcs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl3/igt@gem_exec_fair@basic-none@rcs0.html

  * igt@gem_exec_fair@basic-pace@bcs0:
    - shard-tglb:         [PASS][5] -> [FAIL][6] ([i915#2842]) +5 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-tglb6/igt@gem_exec_fair@basic-pace@bcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb1/igt@gem_exec_fair@basic-pace@bcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-tglb:         NOTRUN -> [FAIL][7] ([i915#2842])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb6/igt@gem_exec_fair@basic-throttle@rcs0.html
    - shard-iclb:         [PASS][8] -> [FAIL][9] ([i915#2849])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-iclb8/igt@gem_exec_fair@basic-throttle@rcs0.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb7/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_params@no-vebox:
    - shard-skl:          NOTRUN -> [SKIP][10] ([fdo#109271]) +76 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl4/igt@gem_exec_params@no-vebox.html

  * igt@gem_exec_reloc@basic-wc-gtt-noreloc:
    - shard-skl:          [PASS][11] -> [DMESG-WARN][12] ([i915#1982])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl9/igt@gem_exec_reloc@basic-wc-gtt-noreloc.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl10/igt@gem_exec_reloc@basic-wc-gtt-noreloc.html

  * igt@gem_userptr_blits@readonly-unsync:
    - shard-tglb:         NOTRUN -> [SKIP][13] ([i915#3297])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@gem_userptr_blits@readonly-unsync.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [PASS][14] -> [DMESG-WARN][15] ([i915#1436] / [i915#716])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl2/igt@gen9_exec_parse@allowed-single.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl8/igt@gen9_exec_parse@allowed-single.html

  * igt@gen9_exec_parse@basic-rejected:
    - shard-iclb:         NOTRUN -> [SKIP][16] ([i915#2856])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@gen9_exec_parse@basic-rejected.html
    - shard-tglb:         NOTRUN -> [SKIP][17] ([i915#2856])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@gen9_exec_parse@basic-rejected.html

  * igt@i915_pm_rpm@dpms-non-lpsp:
    - shard-tglb:         NOTRUN -> [SKIP][18] ([fdo#111644] / [i915#1397] / [i915#2411])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@i915_pm_rpm@dpms-non-lpsp.html

  * igt@kms_big_fb@linear-32bpp-rotate-0:
    - shard-glk:          [PASS][19] -> [DMESG-WARN][20] ([i915#118]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-glk5/igt@kms_big_fb@linear-32bpp-rotate-0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-glk1/igt@kms_big_fb@linear-32bpp-rotate-0.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-apl:          NOTRUN -> [SKIP][21] ([fdo#109271] / [i915#3777])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl1/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0-hflip:
    - shard-kbl:          NOTRUN -> [SKIP][22] ([fdo#109271] / [i915#3777])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl6/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0-hflip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0-hflip:
    - shard-tglb:         NOTRUN -> [SKIP][23] ([fdo#111615])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0-hflip.html
    - shard-iclb:         NOTRUN -> [SKIP][24] ([fdo#110723])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0-hflip.html

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_ccs:
    - shard-snb:          NOTRUN -> [SKIP][25] ([fdo#109271]) +381 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-snb7/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_ccs.html

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
    - shard-iclb:         NOTRUN -> [SKIP][26] ([fdo#109278] / [i915#3886])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-b-ccs-on-another-bo-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][27] ([fdo#109271] / [i915#3886]) +8 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl1/igt@kms_ccs@pipe-b-ccs-on-another-bo-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-crc-primary-basic-y_tiled_gen12_mc_ccs:
    - shard-kbl:          NOTRUN -> [SKIP][28] ([fdo#109271] / [i915#3886]) +4 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl6/igt@kms_ccs@pipe-c-crc-primary-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-bad-rotation-90-y_tiled_gen12_mc_ccs:
    - shard-iclb:         NOTRUN -> [SKIP][29] ([fdo#109278]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@kms_ccs@pipe-d-bad-rotation-90-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-crc-primary-rotation-180-yf_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][30] ([i915#3689])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb6/igt@kms_ccs@pipe-d-crc-primary-rotation-180-yf_tiled_ccs.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - shard-snb:          NOTRUN -> [SKIP][31] ([fdo#109271] / [fdo#111827]) +22 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-snb7/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_chamelium@hdmi-mode-timings:
    - shard-iclb:         NOTRUN -> [SKIP][32] ([fdo#109284] / [fdo#111827])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@kms_chamelium@hdmi-mode-timings.html

  * igt@kms_chamelium@vga-edid-read:
    - shard-apl:          NOTRUN -> [SKIP][33] ([fdo#109271] / [fdo#111827]) +16 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl7/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_color_chamelium@pipe-b-ctm-0-75:
    - shard-tglb:         NOTRUN -> [SKIP][34] ([fdo#109284] / [fdo#111827])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb6/igt@kms_color_chamelium@pipe-b-ctm-0-75.html

  * igt@kms_color_chamelium@pipe-b-ctm-limited-range:
    - shard-skl:          NOTRUN -> [SKIP][35] ([fdo#109271] / [fdo#111827]) +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl4/igt@kms_color_chamelium@pipe-b-ctm-limited-range.html

  * igt@kms_color_chamelium@pipe-c-ctm-0-25:
    - shard-kbl:          NOTRUN -> [SKIP][36] ([fdo#109271] / [fdo#111827]) +12 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl6/igt@kms_color_chamelium@pipe-c-ctm-0-25.html

  * igt@kms_cursor_crc@pipe-b-cursor-512x170-random:
    - shard-tglb:         NOTRUN -> [SKIP][37] ([fdo#109279] / [i915#3359])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@kms_cursor_crc@pipe-b-cursor-512x170-random.html
    - shard-iclb:         NOTRUN -> [SKIP][38] ([fdo#109278] / [fdo#109279])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@kms_cursor_crc@pipe-b-cursor-512x170-random.html

  * igt@kms_cursor_crc@pipe-c-cursor-max-size-random:
    - shard-tglb:         NOTRUN -> [SKIP][39] ([i915#3359])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@kms_cursor_crc@pipe-c-cursor-max-size-random.html

  * igt@kms_cursor_edge_walk@pipe-b-128x128-right-edge:
    - shard-snb:          [PASS][40] -> [SKIP][41] ([fdo#109271])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-snb7/igt@kms_cursor_edge_walk@pipe-b-128x128-right-edge.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-snb2/igt@kms_cursor_edge_walk@pipe-b-128x128-right-edge.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-tglb:         NOTRUN -> [SKIP][42] ([fdo#111825]) +2 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
    - shard-iclb:         NOTRUN -> [SKIP][43] ([fdo#109274] / [fdo#109278])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          [PASS][44] -> [FAIL][45] ([i915#2346] / [i915#533])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@b-hdmi-a1:
    - shard-glk:          [PASS][46] -> [FAIL][47] ([i915#2122])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-glk4/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-hdmi-a1.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-glk5/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [PASS][48] -> [DMESG-WARN][49] ([i915#180]) +1 similar issue
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][50] ([i915#180])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_flip@flip-vs-suspend@a-dp1:
    - shard-apl:          NOTRUN -> [DMESG-WARN][51] ([i915#180]) +1 similar issue
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl2/igt@kms_flip@flip-vs-suspend@a-dp1.html

  * igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1:
    - shard-skl:          [PASS][52] -> [FAIL][53] ([i915#2122])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl4/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl9/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs:
    - shard-apl:          NOTRUN -> [SKIP][54] ([fdo#109271] / [i915#2672])
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl1/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-blt:
    - shard-kbl:          NOTRUN -> [SKIP][55] ([fdo#109271]) +186 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-blt:
    - shard-iclb:         NOTRUN -> [SKIP][56] ([fdo#109280])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-blt.html

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-d:
    - shard-kbl:          NOTRUN -> [SKIP][57] ([fdo#109271] / [i915#533]) +2 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl2/igt@kms_pipe_crc_basic@hang-read-crc-pipe-d.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-d-frame-sequence:
    - shard-skl:          NOTRUN -> [SKIP][58] ([fdo#109271] / [i915#533])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl4/igt@kms_pipe_crc_basic@read-crc-pipe-d-frame-sequence.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-apl:          NOTRUN -> [SKIP][59] ([fdo#109271] / [i915#533]) +2 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl1/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-apl:          NOTRUN -> [FAIL][60] ([fdo#108145] / [i915#265]) +1 similar issue
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl1/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][61] -> [FAIL][62] ([fdo#108145] / [i915#265]) +2 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-c-scaler-with-clipping-clamping:
    - shard-kbl:          NOTRUN -> [SKIP][63] ([fdo#109271] / [i915#2733])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl2/igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-c-scaler-with-clipping-clamping.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2:
    - shard-apl:          NOTRUN -> [SKIP][64] ([fdo#109271] / [i915#658]) +5 similar issues
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl3/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2.html

  * igt@kms_psr2_sf@plane-move-sf-dmg-area-3:
    - shard-kbl:          NOTRUN -> [SKIP][65] ([fdo#109271] / [i915#658]) +4 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl2/igt@kms_psr2_sf@plane-move-sf-dmg-area-3.html
    - shard-skl:          NOTRUN -> [SKIP][66] ([fdo#109271] / [i915#658])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl4/igt@kms_psr2_sf@plane-move-sf-dmg-area-3.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-5:
    - shard-tglb:         NOTRUN -> [SKIP][67] ([i915#2920])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-5.html

  * igt@kms_vblank@pipe-d-ts-continuation-idle:
    - shard-apl:          NOTRUN -> [SKIP][68] ([fdo#109271]) +220 similar issues
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl2/igt@kms_vblank@pipe-d-ts-continuation-idle.html

  * igt@kms_writeback@writeback-check-output:
    - shard-apl:          NOTRUN -> [SKIP][69] ([fdo#109271] / [i915#2437])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl1/igt@kms_writeback@writeback-check-output.html

  * igt@kms_writeback@writeback-pixel-formats:
    - shard-kbl:          NOTRUN -> [SKIP][70] ([fdo#109271] / [i915#2437])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl6/igt@kms_writeback@writeback-pixel-formats.html

  * igt@nouveau_crc@pipe-d-ctx-flip-skip-current-frame:
    - shard-tglb:         NOTRUN -> [SKIP][71] ([i915#2530])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@nouveau_crc@pipe-d-ctx-flip-skip-current-frame.html

  * igt@prime_vgem@fence-write-hang:
    - shard-tglb:         NOTRUN -> [SKIP][72] ([fdo#109295])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-tglb2/igt@prime_vgem@fence-write-hang.html

  * igt@sysfs_clients@fair-0:
    - shard-kbl:          NOTRUN -> [SKIP][73] ([fdo#109271] / [i915#2994])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl2/igt@sysfs_clients@fair-0.html

  * igt@sysfs_clients@recycle-many:
    - shard-apl:          NOTRUN -> [SKIP][74] ([fdo#109271] / [i915#2994]) +4 similar issues
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl3/igt@sysfs_clients@recycle-many.html

  
#### Possible fixes ####

  * igt@drm_mm@all@evict:
    - shard-skl:          [INCOMPLETE][75] ([i915#198]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl7/igt@drm_mm@all@evict.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl5/igt@drm_mm@all@evict.html

  * igt@gem_exec_fair@basic-none-rrul@rcs0:
    - shard-kbl:          [FAIL][77] ([i915#2842]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl3/igt@gem_exec_fair@basic-none-rrul@rcs0.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl4/igt@gem_exec_fair@basic-none-rrul@rcs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-glk:          [FAIL][79] ([i915#2842]) -> [PASS][80] +1 similar issue
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-glk3/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-glk1/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-kbl:          [SKIP][81] ([fdo#109271]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl4/igt@gem_exec_fair@basic-pace@vecs0.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl6/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@gem_exec_whisper@basic-fds-all:
    - shard-glk:          [DMESG-WARN][83] ([i915#118]) -> [PASS][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-glk2/igt@gem_exec_whisper@basic-fds-all.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-glk8/igt@gem_exec_whisper@basic-fds-all.html

  * igt@i915_pm_dc@dc9-dpms:
    - shard-iclb:         [FAIL][85] ([i915#4275]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-iclb1/igt@i915_pm_dc@dc9-dpms.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb5/igt@i915_pm_dc@dc9-dpms.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [FAIL][87] ([i915#2346]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl9/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-kbl:          [INCOMPLETE][89] ([i915#180] / [i915#636]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl6/igt@kms_fbcon_fbt@fbc-suspend.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl6/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1:
    - shard-skl:          [FAIL][91] ([i915#2122]) -> [PASS][92]
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1:
    - shard-skl:          [FAIL][93] ([i915#79]) -> [PASS][94] +2 similar issues
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@b-dp1:
    - shard-apl:          [DMESG-WARN][95] ([i915#180]) -> [PASS][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible@b-dp1.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-apl7/igt@kms_flip@flip-vs-suspend-interruptible@b-dp1.html

  * igt@kms_flip@modeset-vs-vblank-race-interruptible@c-hdmi-a1:
    - shard-glk:          [FAIL][97] ([i915#407]) -> [PASS][98]
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-glk6/igt@kms_flip@modeset-vs-vblank-race-interruptible@c-hdmi-a1.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-glk3/igt@kms_flip@modeset-vs-vblank-race-interruptible@c-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [DMESG-WARN][99] ([i915#180]) -> [PASS][100] +5 similar issues
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [FAIL][101] ([i915#1188]) -> [PASS][102]
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl10/igt@kms_hdr@bpc-switch-suspend.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl9/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][103] ([fdo#108145] / [i915#265]) -> [PASS][104]
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][105] ([i915#180] / [i915#295]) -> [PASS][106]
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Warnings ####

  * igt@gem_exec_fair@basic-none-rrul@rcs0:
    - shard-iclb:         [FAIL][107] ([i915#2852]) -> [FAIL][108] ([i915#2842])
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-iclb7/igt@gem_exec_fair@basic-none-rrul@rcs0.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb4/igt@gem_exec_fair@basic-none-rrul@rcs0.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         [WARN][109] ([i915#1804] / [i915#2684]) -> [WARN][110] ([i915#2684])
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-iclb4/igt@i915_pm_rc6_residency@rc6-idle.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-iclb1/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_flip@flip-vs-suspend-interruptible@b-dp1:
    - shard-kbl:          [INCOMPLETE][111] -> [DMESG-WARN][112] ([i915#180])
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@b-dp1.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible@b-dp1.html

  * igt@runner@aborted:
    - shard-kbl:          ([FAIL][113], [FAIL][114], [FAIL][115], [FAIL][116], [FAIL][117], [FAIL][118], [FAIL][119], [FAIL][120], [FAIL][121]) ([fdo#109271] / [i915#180] / [i915#1814] / [i915#3363] / [i915#4312] / [i915#602] / [i915#92]) -> ([FAIL][122], [FAIL][123], [FAIL][124], [FAIL][125]) ([i915#1436] / [i915#180] / [i915#1814] / [i915#3002] / [i915#3363] / [i915#4312])
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl6/igt@runner@aborted.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl3/igt@runner@aborted.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl6/igt@runner@aborted.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl3/igt@runner@aborted.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl6/igt@runner@aborted.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl3/igt@runner@aborted.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl3/igt@runner@aborted.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl6/igt@runner@aborted.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-kbl6/igt@runner@aborted.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl3/igt@runner@aborted.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl3/igt@runner@aborted.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl6/igt@runner@aborted.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-kbl7/igt@runner@aborted.html
    - shard-skl:          ([FAIL][126], [FAIL][127]) ([i915#3002] / [i915#3363] / [i915#4312]) -> ([FAIL][128], [FAIL][129], [FAIL][130]) ([i915#1436] / [i915#3002] / [i915#3363] / [i915#4312])
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl8/igt@runner@aborted.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10767/shard-skl10/igt@runner@aborted.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl8/igt@runner@aborted.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl4/igt@runner@aborted.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/shard-skl2/igt@runner@aborted.html

  
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1099]: https://gitlab.freedesktop.org/drm/intel/issues/1099
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1804]: https://gitlab.freedesktop.org/drm/intel/issues/1804
  [i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2530]: https://gitlab.freedesktop.org/drm/intel/issues/2530
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2684]: https://gitlab.freedesktop.org/drm/intel/issues/2684
  [i915#2733]: https://gitlab.freedesktop.org/drm/intel/issues/2733
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2849]: https://gitlab.freedesktop.org/drm/intel/issues/2849
  [i915#2852]: https://gitlab.freedesktop.org/drm/intel/issues/2852
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#295]: https://gitlab.freedesktop.org/drm/intel/issues/295
  [i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
  [i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3363]: https://gitlab.freedesktop.org/drm/intel/issues/3363
  [i915#36

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21399/index.html

[-- Attachment #2: Type: text/html, Size: 38202 bytes --]

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

* Re: [Intel-gfx] [PATCH 1/4] drm/i915: Move function prototypes to the correct header
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 1/4] drm/i915: Move function prototypes to the correct header Ville Syrjala
@ 2021-10-21 10:26   ` Jani Nikula
  0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2021-10-21 10:26 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 21 Oct 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> A bunch of function prototypes were left behind when the
> plane/crtc code got reshuffled to new files. Move the
> prototypes as well.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crtc.h   | 5 +++++
>  drivers/gpu/drm/i915/display/intel_psr.c    | 2 +-
>  drivers/gpu/drm/i915/display/intel_sprite.h | 4 ----
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
> index a5ae997581aa..22363fbbc925 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.h
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
> @@ -9,10 +9,13 @@
>  #include <linux/types.h>
>  
>  enum pipe;
> +struct drm_display_mode;
>  struct drm_i915_private;
>  struct intel_crtc;
>  struct intel_crtc_state;
>  
> +int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> +			     int usecs);
>  u32 intel_crtc_max_vblank_count(const struct intel_crtc_state *crtc_state);
>  int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe);
>  struct intel_crtc_state *intel_crtc_state_alloc(struct intel_crtc *crtc);
> @@ -21,5 +24,7 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
>  u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
>  void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
>  void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
> +void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
> +void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
>  
>  #endif
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 49c2dfbd4055..ccffe05784d3 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -28,13 +28,13 @@
>  
>  #include "i915_drv.h"
>  #include "intel_atomic.h"
> +#include "intel_crtc.h"
>  #include "intel_de.h"
>  #include "intel_display_types.h"
>  #include "intel_dp_aux.h"
>  #include "intel_hdmi.h"
>  #include "intel_psr.h"
>  #include "intel_snps_phy.h"
> -#include "intel_sprite.h"
>  #include "skl_universal_plane.h"
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.h b/drivers/gpu/drm/i915/display/intel_sprite.h
> index c085eb87705c..4f63e4967731 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.h
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.h
> @@ -27,14 +27,10 @@ struct intel_plane_state;
>  #define VBLANK_EVASION_TIME_US 100
>  #endif
>  
> -int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> -			     int usecs);
>  struct intel_plane *intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  					      enum pipe pipe, int plane);
>  int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data,
>  				    struct drm_file *file_priv);
> -void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
> -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
>  int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state);
>  int chv_plane_check_rotation(const struct intel_plane_state *plane_state);

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates Ville Syrjala
@ 2021-10-21 10:35   ` Jani Nikula
  2021-10-21 10:42     ` Ville Syrjälä
  2021-11-08 20:41   ` Shankar, Uma
  1 sibling, 1 reply; 18+ messages in thread
From: Jani Nikula @ 2021-10-21 10:35 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Lyude Paul

On Thu, 21 Oct 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The pipe gamma registers are single buffered so they should only
> be updated during the vblank to avoid screen tearing. In fact they
> really should only be updated between start of vblank and frame
> start because that is the only time the pipe is guaranteed to be
> empty. Already at frame start the pipe begins to fill up with
> data for the next frame.
>
> Unfortunately frame start happens ~1 scanline after the start
> of vblank which in practice doesn't always leave us enough time to
> finish the gamma update in time (gamma LUTs can be several KiB of
> data we have to bash into the registers). However we must try our
> best and so we'll add a vblank work for each pipe from where we
> can do the gamma update. Additionally we could consider pushing
> frame start forward to the max of ~4 scanlines after start of
> vblank. But not sure that's exactly a validated configuration.
> As it stands the ~100 first pixels tend to make it through with
> the old gamma values.
>
> Even though the vblank worker is running on a high prority thread
> we still have to contend with C-states. If the CPU happens be in
> a deep C-state when the vblank interrupt arrives even the irq
> handler gets delayed massively (I've observed dozens of scanlines
> worth of latency). To avoid that problem we'll use the qos mechanism
> to keep the CPU awake while the vblank work is scheduled.
>
> With all this hooked up we can finally enjoy near atomic gamma
> updates. It even works across several pipes from the same atomic
> commit which previously was a total fail because we did the
> gamma updates for each pipe serially after waiting for all
> pipes to have latched the double buffered registers.
>
> In the future the DSB should take over this responsibility
> which will hopefully avoid some of these issues.
>
> Kudos to Lyude for finishing the actual vblank workers.
> Works like the proverbial train toilet.
>
> v2: Add missing intel_atomic_state fwd declaration
> v3: Clean up properly when not scheduling the worker
> v4: Clean up the rest and add tracepoints
>
> CC: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crtc.c     | 76 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_crtc.h     |  4 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |  9 +--
>  .../drm/i915/display/intel_display_types.h    |  8 ++
>  drivers/gpu/drm/i915/i915_trace.h             | 42 ++++++++++
>  5 files changed, 129 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 0f8b48b6911c..4758c61adae8 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -3,12 +3,14 @@
>   * Copyright © 2020 Intel Corporation
>   */
>  #include <linux/kernel.h>
> +#include <linux/pm_qos.h>
>  #include <linux/slab.h>
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_plane.h>
>  #include <drm/drm_plane_helper.h>
> +#include <drm/drm_vblank_work.h>
>  
>  #include "i915_trace.h"
>  #include "i915_vgpu.h"
> @@ -167,6 +169,8 @@ static void intel_crtc_destroy(struct drm_crtc *_crtc)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(_crtc);
>  
> +	cpu_latency_qos_remove_request(&crtc->vblank_pm_qos);
> +
>  	drm_crtc_cleanup(&crtc->base);
>  	kfree(crtc);
>  }
> @@ -344,6 +348,8 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  
>  	intel_crtc_crc_init(crtc);
>  
> +	cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE);
> +
>  	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) != crtc->pipe);
>  
>  	return 0;
> @@ -354,6 +360,65 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  	return ret;
>  }
>  
> +static bool intel_crtc_needs_vblank_work(const struct intel_crtc_state *crtc_state)
> +{
> +	return crtc_state->hw.active &&
> +		!intel_crtc_needs_modeset(crtc_state) &&
> +		!crtc_state->preload_luts &&
> +		(crtc_state->uapi.color_mgmt_changed ||
> +		 crtc_state->update_pipe);
> +}
> +
> +static void intel_crtc_vblank_work(struct kthread_work *base)
> +{
> +	struct drm_vblank_work *work = to_drm_vblank_work(base);
> +	struct intel_crtc_state *crtc_state =
> +		container_of(work, typeof(*crtc_state), vblank_work);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +
> +	trace_intel_crtc_vblank_work_start(crtc);
> +
> +	intel_color_load_luts(crtc_state);
> +
> +	if (crtc_state->uapi.event) {
> +		spin_lock_irq(&crtc->base.dev->event_lock);
> +		drm_crtc_send_vblank_event(&crtc->base, crtc_state->uapi.event);
> +		crtc_state->uapi.event = NULL;
> +		spin_unlock_irq(&crtc->base.dev->event_lock);
> +	}
> +
> +	trace_intel_crtc_vblank_work_end(crtc);
> +}
> +
> +static void intel_crtc_vblank_work_init(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +
> +	drm_vblank_work_init(&crtc_state->vblank_work, &crtc->base,
> +			     intel_crtc_vblank_work);
> +	/*
> +	 * Interrupt latency is critical for getting the vblank
> +	 * work executed as early as possible during the vblank.
> +	 */
> +	cpu_latency_qos_update_request(&crtc->vblank_pm_qos, 0);
> +}
> +
> +void intel_wait_for_vblank_works(struct intel_atomic_state *state)
> +{
> +	struct intel_crtc_state *crtc_state;
> +	struct intel_crtc *crtc;
> +	int i;
> +
> +	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		if (!intel_crtc_needs_vblank_work(crtc_state))
> +			continue;
> +
> +		drm_vblank_work_flush(&crtc_state->vblank_work);
> +		cpu_latency_qos_update_request(&crtc->vblank_pm_qos,
> +					       PM_QOS_DEFAULT_VALUE);
> +	}
> +}
> +
>  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>  			     int usecs)
>  {
> @@ -387,7 +452,7 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
>   * until a subsequent call to intel_pipe_update_end(). That is done to
>   * avoid random delays.
>   */
> -void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
> +void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -402,6 +467,9 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
>  	if (new_crtc_state->uapi.async_flip)
>  		return;
>  
> +	if (intel_crtc_needs_vblank_work(new_crtc_state))
> +		intel_crtc_vblank_work_init(new_crtc_state);
> +
>  	if (new_crtc_state->vrr.enable)
>  		vblank_start = intel_vrr_vmax_vblank_start(new_crtc_state);
>  	else
> @@ -557,7 +625,11 @@ 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 (intel_crtc_needs_vblank_work(new_crtc_state)) {
> +		drm_vblank_work_schedule(&new_crtc_state->vblank_work,
> +					 drm_crtc_accurate_vblank_count(&crtc->base) + 1,
> +					 false);
> +	} else if (new_crtc_state->uapi.event) {
>  		drm_WARN_ON(&dev_priv->drm,
>  			    drm_crtc_vblank_get(&crtc->base) != 0);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
> index 22363fbbc925..25eb58bce0dd 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.h
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
> @@ -11,6 +11,7 @@
>  enum pipe;
>  struct drm_display_mode;
>  struct drm_i915_private;
> +struct intel_atomic_state;
>  struct intel_crtc;
>  struct intel_crtc_state;
>  
> @@ -24,7 +25,8 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
>  u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
>  void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
>  void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
> -void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
> +void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state);
>  void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
> +void intel_wait_for_vblank_works(struct intel_atomic_state *state);
>  
>  #endif
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 79a7552af7b5..1375d963c0a8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8818,6 +8818,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>  		intel_set_cdclk_post_plane_update(state);
>  	}
>  
> +	intel_wait_for_vblank_works(state);

Nitpick, I think the function name can be confusing due to the plural
vs. verb here. intel_wait_for_vblank_work_end(), _finish(), _done()?

BR,
Jani.


> +
>  	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
>  	 * already, but still need the state for the delayed optimization. To
>  	 * fix this:
> @@ -8832,13 +8834,6 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>  	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		if (new_crtc_state->uapi.async_flip)
>  			intel_crtc_disable_flip_done(state, crtc);
> -
> -		if (new_crtc_state->hw.active &&
> -		    !intel_crtc_needs_modeset(new_crtc_state) &&
> -		    !new_crtc_state->preload_luts &&
> -		    (new_crtc_state->uapi.color_mgmt_changed ||
> -		     new_crtc_state->update_pipe))
> -			intel_color_load_luts(new_crtc_state);
>  	}
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 1e42bf901263..8bb9264022f5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -28,6 +28,7 @@
>  
>  #include <linux/async.h>
>  #include <linux/i2c.h>
> +#include <linux/pm_qos.h>
>  #include <linux/pwm.h>
>  #include <linux/sched/clock.h>
>  
> @@ -41,6 +42,7 @@
>  #include <drm/drm_probe_helper.h>
>  #include <drm/drm_rect.h>
>  #include <drm/drm_vblank.h>
> +#include <drm/drm_vblank_work.h>
>  #include <drm/i915_mei_hdcp_interface.h>
>  #include <media/cec-notifier.h>
>  
> @@ -1241,6 +1243,9 @@ struct intel_crtc_state {
>  		u8 link_count;
>  		u8 pixel_overlap;
>  	} splitter;
> +
> +	/* for loading single buffered registers during vblank */
> +	struct drm_vblank_work vblank_work;
>  };
>  
>  enum intel_pipe_crc_source {
> @@ -1325,6 +1330,9 @@ struct intel_crtc {
>  	/* scalers available on this crtc */
>  	int num_scalers;
>  
> +	/* for loading single buffered registers during vblank */
> +	struct pm_qos_request vblank_pm_qos;
> +
>  #ifdef CONFIG_DEBUG_FS
>  	struct intel_pipe_crc pipe_crc;
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 9795f456cccf..fc48a16d7a4f 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -404,6 +404,48 @@ TRACE_EVENT(intel_fbc_nuke,
>  
>  /* pipe updates */
>  
> +TRACE_EVENT(intel_crtc_vblank_work_start,
> +	    TP_PROTO(struct intel_crtc *crtc),
> +	    TP_ARGS(crtc),
> +
> +	    TP_STRUCT__entry(
> +			     __field(enum pipe, pipe)
> +			     __field(u32, frame)
> +			     __field(u32, scanline)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->pipe = crtc->pipe;
> +			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
> +			   __entry->scanline = intel_get_crtc_scanline(crtc);
> +			   ),
> +
> +	    TP_printk("pipe %c, frame=%u, scanline=%u",
> +		      pipe_name(__entry->pipe), __entry->frame,
> +		       __entry->scanline)
> +);
> +
> +TRACE_EVENT(intel_crtc_vblank_work_end,
> +	    TP_PROTO(struct intel_crtc *crtc),
> +	    TP_ARGS(crtc),
> +
> +	    TP_STRUCT__entry(
> +			     __field(enum pipe, pipe)
> +			     __field(u32, frame)
> +			     __field(u32, scanline)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->pipe = crtc->pipe;
> +			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
> +			   __entry->scanline = intel_get_crtc_scanline(crtc);
> +			   ),
> +
> +	    TP_printk("pipe %c, frame=%u, scanline=%u",
> +		      pipe_name(__entry->pipe), __entry->frame,
> +		       __entry->scanline)
> +);
> +
>  TRACE_EVENT(intel_pipe_update_start,
>  	    TP_PROTO(struct intel_crtc *crtc),
>  	    TP_ARGS(crtc),

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates
  2021-10-21 10:35   ` Jani Nikula
@ 2021-10-21 10:42     ` Ville Syrjälä
  2021-10-21 12:51       ` Jani Nikula
  0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2021-10-21 10:42 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Lyude Paul

On Thu, Oct 21, 2021 at 01:35:12PM +0300, Jani Nikula wrote:
> On Thu, 21 Oct 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The pipe gamma registers are single buffered so they should only
> > be updated during the vblank to avoid screen tearing. In fact they
> > really should only be updated between start of vblank and frame
> > start because that is the only time the pipe is guaranteed to be
> > empty. Already at frame start the pipe begins to fill up with
> > data for the next frame.
> >
> > Unfortunately frame start happens ~1 scanline after the start
> > of vblank which in practice doesn't always leave us enough time to
> > finish the gamma update in time (gamma LUTs can be several KiB of
> > data we have to bash into the registers). However we must try our
> > best and so we'll add a vblank work for each pipe from where we
> > can do the gamma update. Additionally we could consider pushing
> > frame start forward to the max of ~4 scanlines after start of
> > vblank. But not sure that's exactly a validated configuration.
> > As it stands the ~100 first pixels tend to make it through with
> > the old gamma values.
> >
> > Even though the vblank worker is running on a high prority thread
> > we still have to contend with C-states. If the CPU happens be in
> > a deep C-state when the vblank interrupt arrives even the irq
> > handler gets delayed massively (I've observed dozens of scanlines
> > worth of latency). To avoid that problem we'll use the qos mechanism
> > to keep the CPU awake while the vblank work is scheduled.
> >
> > With all this hooked up we can finally enjoy near atomic gamma
> > updates. It even works across several pipes from the same atomic
> > commit which previously was a total fail because we did the
> > gamma updates for each pipe serially after waiting for all
> > pipes to have latched the double buffered registers.
> >
> > In the future the DSB should take over this responsibility
> > which will hopefully avoid some of these issues.
> >
> > Kudos to Lyude for finishing the actual vblank workers.
> > Works like the proverbial train toilet.
> >
> > v2: Add missing intel_atomic_state fwd declaration
> > v3: Clean up properly when not scheduling the worker
> > v4: Clean up the rest and add tracepoints
> >
> > CC: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_crtc.c     | 76 ++++++++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_crtc.h     |  4 +-
> >  drivers/gpu/drm/i915/display/intel_display.c  |  9 +--
> >  .../drm/i915/display/intel_display_types.h    |  8 ++
> >  drivers/gpu/drm/i915/i915_trace.h             | 42 ++++++++++
> >  5 files changed, 129 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> > index 0f8b48b6911c..4758c61adae8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> > @@ -3,12 +3,14 @@
> >   * Copyright © 2020 Intel Corporation
> >   */
> >  #include <linux/kernel.h>
> > +#include <linux/pm_qos.h>
> >  #include <linux/slab.h>
> >  
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_fourcc.h>
> >  #include <drm/drm_plane.h>
> >  #include <drm/drm_plane_helper.h>
> > +#include <drm/drm_vblank_work.h>
> >  
> >  #include "i915_trace.h"
> >  #include "i915_vgpu.h"
> > @@ -167,6 +169,8 @@ static void intel_crtc_destroy(struct drm_crtc *_crtc)
> >  {
> >  	struct intel_crtc *crtc = to_intel_crtc(_crtc);
> >  
> > +	cpu_latency_qos_remove_request(&crtc->vblank_pm_qos);
> > +
> >  	drm_crtc_cleanup(&crtc->base);
> >  	kfree(crtc);
> >  }
> > @@ -344,6 +348,8 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  
> >  	intel_crtc_crc_init(crtc);
> >  
> > +	cpu_latency_qos_add_request(&crtc->vblank_pm_qos, PM_QOS_DEFAULT_VALUE);
> > +
> >  	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) != crtc->pipe);
> >  
> >  	return 0;
> > @@ -354,6 +360,65 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  	return ret;
> >  }
> >  
> > +static bool intel_crtc_needs_vblank_work(const struct intel_crtc_state *crtc_state)
> > +{
> > +	return crtc_state->hw.active &&
> > +		!intel_crtc_needs_modeset(crtc_state) &&
> > +		!crtc_state->preload_luts &&
> > +		(crtc_state->uapi.color_mgmt_changed ||
> > +		 crtc_state->update_pipe);
> > +}
> > +
> > +static void intel_crtc_vblank_work(struct kthread_work *base)
> > +{
> > +	struct drm_vblank_work *work = to_drm_vblank_work(base);
> > +	struct intel_crtc_state *crtc_state =
> > +		container_of(work, typeof(*crtc_state), vblank_work);
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +
> > +	trace_intel_crtc_vblank_work_start(crtc);
> > +
> > +	intel_color_load_luts(crtc_state);
> > +
> > +	if (crtc_state->uapi.event) {
> > +		spin_lock_irq(&crtc->base.dev->event_lock);
> > +		drm_crtc_send_vblank_event(&crtc->base, crtc_state->uapi.event);
> > +		crtc_state->uapi.event = NULL;
> > +		spin_unlock_irq(&crtc->base.dev->event_lock);
> > +	}
> > +
> > +	trace_intel_crtc_vblank_work_end(crtc);
> > +}
> > +
> > +static void intel_crtc_vblank_work_init(struct intel_crtc_state *crtc_state)
> > +{
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +
> > +	drm_vblank_work_init(&crtc_state->vblank_work, &crtc->base,
> > +			     intel_crtc_vblank_work);
> > +	/*
> > +	 * Interrupt latency is critical for getting the vblank
> > +	 * work executed as early as possible during the vblank.
> > +	 */
> > +	cpu_latency_qos_update_request(&crtc->vblank_pm_qos, 0);
> > +}
> > +
> > +void intel_wait_for_vblank_works(struct intel_atomic_state *state)
> > +{
> > +	struct intel_crtc_state *crtc_state;
> > +	struct intel_crtc *crtc;
> > +	int i;
> > +
> > +	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> > +		if (!intel_crtc_needs_vblank_work(crtc_state))
> > +			continue;
> > +
> > +		drm_vblank_work_flush(&crtc_state->vblank_work);
> > +		cpu_latency_qos_update_request(&crtc->vblank_pm_qos,
> > +					       PM_QOS_DEFAULT_VALUE);
> > +	}
> > +}
> > +
> >  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
> >  			     int usecs)
> >  {
> > @@ -387,7 +452,7 @@ static int intel_mode_vblank_start(const struct drm_display_mode *mode)
> >   * until a subsequent call to intel_pipe_update_end(). That is done to
> >   * avoid random delays.
> >   */
> > -void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
> > +void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
> >  {
> >  	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > @@ -402,6 +467,9 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
> >  	if (new_crtc_state->uapi.async_flip)
> >  		return;
> >  
> > +	if (intel_crtc_needs_vblank_work(new_crtc_state))
> > +		intel_crtc_vblank_work_init(new_crtc_state);
> > +
> >  	if (new_crtc_state->vrr.enable)
> >  		vblank_start = intel_vrr_vmax_vblank_start(new_crtc_state);
> >  	else
> > @@ -557,7 +625,11 @@ 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 (intel_crtc_needs_vblank_work(new_crtc_state)) {
> > +		drm_vblank_work_schedule(&new_crtc_state->vblank_work,
> > +					 drm_crtc_accurate_vblank_count(&crtc->base) + 1,
> > +					 false);
> > +	} else if (new_crtc_state->uapi.event) {
> >  		drm_WARN_ON(&dev_priv->drm,
> >  			    drm_crtc_vblank_get(&crtc->base) != 0);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h b/drivers/gpu/drm/i915/display/intel_crtc.h
> > index 22363fbbc925..25eb58bce0dd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
> > @@ -11,6 +11,7 @@
> >  enum pipe;
> >  struct drm_display_mode;
> >  struct drm_i915_private;
> > +struct intel_atomic_state;
> >  struct intel_crtc;
> >  struct intel_crtc_state;
> >  
> > @@ -24,7 +25,8 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
> >  u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
> >  void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);
> >  void intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state);
> > -void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
> > +void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state);
> >  void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
> > +void intel_wait_for_vblank_works(struct intel_atomic_state *state);
> >  
> >  #endif
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 79a7552af7b5..1375d963c0a8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -8818,6 +8818,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
> >  		intel_set_cdclk_post_plane_update(state);
> >  	}
> >  
> > +	intel_wait_for_vblank_works(state);
> 
> Nitpick, I think the function name can be confusing due to the plural
> vs. verb here. intel_wait_for_vblank_work_end(), _finish(), _done()?

I guess _end() would match what I called the tracepoint. Another
idea could be s/works/workers/

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates
  2021-10-21 10:42     ` Ville Syrjälä
@ 2021-10-21 12:51       ` Jani Nikula
  0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2021-10-21 12:51 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Lyude Paul

On Thu, 21 Oct 2021, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Oct 21, 2021 at 01:35:12PM +0300, Jani Nikula wrote:
>> On Thu, 21 Oct 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> > index 79a7552af7b5..1375d963c0a8 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> > @@ -8818,6 +8818,8 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
>> >  		intel_set_cdclk_post_plane_update(state);
>> >  	}
>> >  
>> > +	intel_wait_for_vblank_works(state);
>> 
>> Nitpick, I think the function name can be confusing due to the plural
>> vs. verb here. intel_wait_for_vblank_work_end(), _finish(), _done()?
>
> I guess _end() would match what I called the tracepoint. Another
> idea could be s/works/workers/

Either works for me.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers
  2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
                   ` (7 preceding siblings ...)
  2021-10-21  7:11 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
@ 2021-10-26 22:02 ` Lyude Paul
  8 siblings, 0 replies; 18+ messages in thread
From: Lyude Paul @ 2021-10-26 22:02 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Hey! I'll try to review this tomorrow or the day after if you're still
interested in me doing so :)

On Thu, 2021-10-21 at 01:33 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Finally got around to refreshing my vblank worker gamma LUT series.
> Since I started this (ahem, some years ago) Lyude took over the
> actual vblank worker implementation, mostly rewrote it I think,
> and landed it for use in nouveau. So now I'm just left with the
> easy task of using it for i915 gamma LUT updates. And so here
> we are.
> 
> CC: Lyude Paul <lyude@redhat.com>
> 
> Ville Syrjälä (4):
>   drm/i915: Move function prototypes to the correct header
>   drm/i915: Do vrr push before sampling the freame counter
>   drm/i915: Use vblank workers for gamma updates
>   drm/i915: Use unlocked register accesses for LUT loads
> 
>  drivers/gpu/drm/i915/display/intel_color.c    | 128 +++++++++---------
>  drivers/gpu/drm/i915/display/intel_crtc.c     |  82 ++++++++++-
>  drivers/gpu/drm/i915/display/intel_crtc.h     |   7 +
>  drivers/gpu/drm/i915/display/intel_display.c  |   9 +-
>  .../drm/i915/display/intel_display_types.h    |   8 ++
>  drivers/gpu/drm/i915/display/intel_dsb.c      |   4 +-
>  drivers/gpu/drm/i915/display/intel_psr.c      |   2 +-
>  drivers/gpu/drm/i915/display/intel_sprite.h   |   4 -
>  drivers/gpu/drm/i915/i915_trace.h             |  42 ++++++
>  9 files changed, 203 insertions(+), 83 deletions(-)
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat


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

* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Do vrr push before sampling the freame counter
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 2/4] drm/i915: Do vrr push before sampling the freame counter Ville Syrjala
@ 2021-11-08 19:36   ` Shankar, Uma
  0 siblings, 0 replies; 18+ messages in thread
From: Shankar, Uma @ 2021-11-08 19:36 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> Sent: Thursday, October 21, 2021 4:04 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Navare, Manasi D <manasi.d.navare@intel.com>
> Subject: [Intel-gfx] [PATCH 2/4] drm/i915: Do vrr push before sampling the freame
> counter

Nit: typo in frame

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Do the vrr push before we sample the frame counter to know when the commit has
> been latched. Doing these in the wrong order could lead us to complete the flip
> before it has actually happened.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crtc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
> b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 254e67141a77..0f8b48b6911c 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -542,6 +542,9 @@ void intel_pipe_update_end(struct intel_crtc_state
> *new_crtc_state)
> 
>  	trace_intel_pipe_update_end(crtc, end_vbl_count, scanline_end);
> 
> +	/* Send VRR Push to terminate Vblank */
> +	intel_vrr_send_push(new_crtc_state);
> +

This looks right to me.  We are at risk of arming vblank before the push,
i.e. actual vblank termination.

With the typo fixed:
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>  	/*
>  	 * Incase of mipi dsi command mode, we need to set frame update
>  	 * request for every commit.
> @@ -568,9 +571,6 @@ void intel_pipe_update_end(struct intel_crtc_state
> *new_crtc_state)
> 
>  	local_irq_enable();
> 
> -	/* Send VRR Push to terminate Vblank */
> -	intel_vrr_send_push(new_crtc_state);
> -
>  	if (intel_vgpu_active(dev_priv))
>  		return;
> 
> --
> 2.32.0


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

* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates Ville Syrjala
  2021-10-21 10:35   ` Jani Nikula
@ 2021-11-08 20:41   ` Shankar, Uma
  1 sibling, 0 replies; 18+ messages in thread
From: Shankar, Uma @ 2021-11-08 20:41 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> Sent: Thursday, October 21, 2021 4:04 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Lyude Paul <lyude@redhat.com>
> Subject: [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The pipe gamma registers are single buffered so they should only be updated during
> the vblank to avoid screen tearing. In fact they really should only be updated
> between start of vblank and frame start because that is the only time the pipe is
> guaranteed to be empty. Already at frame start the pipe begins to fill up with data
> for the next frame.
> 
> Unfortunately frame start happens ~1 scanline after the start of vblank which in
> practice doesn't always leave us enough time to finish the gamma update in time
> (gamma LUTs can be several KiB of data we have to bash into the registers).
> However we must try our best and so we'll add a vblank work for each pipe from
> where we can do the gamma update. Additionally we could consider pushing frame
> start forward to the max of ~4 scanlines after start of vblank. But not sure that's
> exactly a validated configuration.
> As it stands the ~100 first pixels tend to make it through with the old gamma values.
> 
> Even though the vblank worker is running on a high prority thread we still have to
> contend with C-states. If the CPU happens be in a deep C-state when the vblank
> interrupt arrives even the irq handler gets delayed massively (I've observed dozens of
> scanlines worth of latency). To avoid that problem we'll use the qos mechanism to
> keep the CPU awake while the vblank work is scheduled.
> 
> With all this hooked up we can finally enjoy near atomic gamma updates. It even
> works across several pipes from the same atomic commit which previously was a
> total fail because we did the gamma updates for each pipe serially after waiting for
> all pipes to have latched the double buffered registers.
> 
> In the future the DSB should take over this responsibility which will hopefully avoid
> some of these issues.
> 
> Kudos to Lyude for finishing the actual vblank workers.
> Works like the proverbial train toilet.
> 
> v2: Add missing intel_atomic_state fwd declaration
> v3: Clean up properly when not scheduling the worker
> v4: Clean up the rest and add tracepoints
> 
> CC: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crtc.c     | 76 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_crtc.h     |  4 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |  9 +--
>  .../drm/i915/display/intel_display_types.h    |  8 ++
>  drivers/gpu/drm/i915/i915_trace.h             | 42 ++++++++++
>  5 files changed, 129 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
> b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 0f8b48b6911c..4758c61adae8 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -3,12 +3,14 @@
>   * Copyright © 2020 Intel Corporation
>   */
>  #include <linux/kernel.h>
> +#include <linux/pm_qos.h>
>  #include <linux/slab.h>
> 
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_plane.h>
>  #include <drm/drm_plane_helper.h>
> +#include <drm/drm_vblank_work.h>
> 
>  #include "i915_trace.h"
>  #include "i915_vgpu.h"
> @@ -167,6 +169,8 @@ static void intel_crtc_destroy(struct drm_crtc *_crtc)  {
>  	struct intel_crtc *crtc = to_intel_crtc(_crtc);
> 
> +	cpu_latency_qos_remove_request(&crtc->vblank_pm_qos);
> +
>  	drm_crtc_cleanup(&crtc->base);
>  	kfree(crtc);
>  }
> @@ -344,6 +348,8 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum
> pipe pipe)
> 
>  	intel_crtc_crc_init(crtc);
> 
> +	cpu_latency_qos_add_request(&crtc->vblank_pm_qos,
> +PM_QOS_DEFAULT_VALUE);
> +
>  	drm_WARN_ON(&dev_priv->drm, drm_crtc_index(&crtc->base) != crtc-
> >pipe);
> 
>  	return 0;
> @@ -354,6 +360,65 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum
> pipe pipe)
>  	return ret;
>  }
> 
> +static bool intel_crtc_needs_vblank_work(const struct intel_crtc_state
> +*crtc_state) {
> +	return crtc_state->hw.active &&
> +		!intel_crtc_needs_modeset(crtc_state) &&
> +		!crtc_state->preload_luts &&
> +		(crtc_state->uapi.color_mgmt_changed ||
> +		 crtc_state->update_pipe);
> +}
> +
> +static void intel_crtc_vblank_work(struct kthread_work *base) {
> +	struct drm_vblank_work *work = to_drm_vblank_work(base);
> +	struct intel_crtc_state *crtc_state =
> +		container_of(work, typeof(*crtc_state), vblank_work);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +
> +	trace_intel_crtc_vblank_work_start(crtc);
> +
> +	intel_color_load_luts(crtc_state);
> +
> +	if (crtc_state->uapi.event) {
> +		spin_lock_irq(&crtc->base.dev->event_lock);
> +		drm_crtc_send_vblank_event(&crtc->base, crtc_state->uapi.event);
> +		crtc_state->uapi.event = NULL;
> +		spin_unlock_irq(&crtc->base.dev->event_lock);
> +	}
> +
> +	trace_intel_crtc_vblank_work_end(crtc);
> +}
> +
> +static void intel_crtc_vblank_work_init(struct intel_crtc_state
> +*crtc_state) {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +
> +	drm_vblank_work_init(&crtc_state->vblank_work, &crtc->base,
> +			     intel_crtc_vblank_work);
> +	/*
> +	 * Interrupt latency is critical for getting the vblank
> +	 * work executed as early as possible during the vblank.
> +	 */
> +	cpu_latency_qos_update_request(&crtc->vblank_pm_qos, 0); }
> +
> +void intel_wait_for_vblank_works(struct intel_atomic_state *state) {
> +	struct intel_crtc_state *crtc_state;
> +	struct intel_crtc *crtc;
> +	int i;
> +
> +	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> +		if (!intel_crtc_needs_vblank_work(crtc_state))
> +			continue;
> +
> +		drm_vblank_work_flush(&crtc_state->vblank_work);
> +		cpu_latency_qos_update_request(&crtc->vblank_pm_qos,
> +					       PM_QOS_DEFAULT_VALUE);
> +	}
> +}
> +
>  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>  			     int usecs)
>  {
> @@ -387,7 +452,7 @@ static int intel_mode_vblank_start(const struct
> drm_display_mode *mode)
>   * until a subsequent call to intel_pipe_update_end(). That is done to
>   * avoid random delays.
>   */
> -void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
> +void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -402,6
> +467,9 @@ void intel_pipe_update_start(const struct intel_crtc_state
> *new_crtc_state)
>  	if (new_crtc_state->uapi.async_flip)
>  		return;
> 
> +	if (intel_crtc_needs_vblank_work(new_crtc_state))
> +		intel_crtc_vblank_work_init(new_crtc_state);
> +
>  	if (new_crtc_state->vrr.enable)
>  		vblank_start = intel_vrr_vmax_vblank_start(new_crtc_state);
>  	else
> @@ -557,7 +625,11 @@ 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 (intel_crtc_needs_vblank_work(new_crtc_state)) {
> +		drm_vblank_work_schedule(&new_crtc_state->vblank_work,
> +					 drm_crtc_accurate_vblank_count(&crtc-
> >base) + 1,
> +					 false);
> +	} else if (new_crtc_state->uapi.event) {
>  		drm_WARN_ON(&dev_priv->drm,
>  			    drm_crtc_vblank_get(&crtc->base) != 0);
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.h
> b/drivers/gpu/drm/i915/display/intel_crtc.h
> index 22363fbbc925..25eb58bce0dd 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.h
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.h
> @@ -11,6 +11,7 @@
>  enum pipe;
>  struct drm_display_mode;
>  struct drm_i915_private;
> +struct intel_atomic_state;
>  struct intel_crtc;
>  struct intel_crtc_state;
> 
> @@ -24,7 +25,8 @@ void intel_crtc_state_reset(struct intel_crtc_state *crtc_state,
>  u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);  void
> intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state);  void
> intel_crtc_vblank_off(const struct intel_crtc_state *crtc_state); -void
> intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
> +void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state);
>  void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
> +void intel_wait_for_vblank_works(struct intel_atomic_state *state);
> 
>  #endif
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 79a7552af7b5..1375d963c0a8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8818,6 +8818,8 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
>  		intel_set_cdclk_post_plane_update(state);
>  	}
> 
> +	intel_wait_for_vblank_works(state);

Change looks perfect to me. Maybe we can just change this s/works/workers.
Overall a nice idea.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> +
>  	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
>  	 * already, but still need the state for the delayed optimization. To
>  	 * fix this:
> @@ -8832,13 +8834,6 @@ static void intel_atomic_commit_tail(struct
> intel_atomic_state *state)
>  	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		if (new_crtc_state->uapi.async_flip)
>  			intel_crtc_disable_flip_done(state, crtc);
> -
> -		if (new_crtc_state->hw.active &&
> -		    !intel_crtc_needs_modeset(new_crtc_state) &&
> -		    !new_crtc_state->preload_luts &&
> -		    (new_crtc_state->uapi.color_mgmt_changed ||
> -		     new_crtc_state->update_pipe))
> -			intel_color_load_luts(new_crtc_state);
>  	}
> 
>  	/*
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 1e42bf901263..8bb9264022f5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -28,6 +28,7 @@
> 
>  #include <linux/async.h>
>  #include <linux/i2c.h>
> +#include <linux/pm_qos.h>
>  #include <linux/pwm.h>
>  #include <linux/sched/clock.h>
> 
> @@ -41,6 +42,7 @@
>  #include <drm/drm_probe_helper.h>
>  #include <drm/drm_rect.h>
>  #include <drm/drm_vblank.h>
> +#include <drm/drm_vblank_work.h>
>  #include <drm/i915_mei_hdcp_interface.h>  #include <media/cec-notifier.h>
> 
> @@ -1241,6 +1243,9 @@ struct intel_crtc_state {
>  		u8 link_count;
>  		u8 pixel_overlap;
>  	} splitter;
> +
> +	/* for loading single buffered registers during vblank */
> +	struct drm_vblank_work vblank_work;
>  };
> 
>  enum intel_pipe_crc_source {
> @@ -1325,6 +1330,9 @@ struct intel_crtc {
>  	/* scalers available on this crtc */
>  	int num_scalers;
> 
> +	/* for loading single buffered registers during vblank */
> +	struct pm_qos_request vblank_pm_qos;
> +
>  #ifdef CONFIG_DEBUG_FS
>  	struct intel_pipe_crc pipe_crc;
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 9795f456cccf..fc48a16d7a4f 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -404,6 +404,48 @@ TRACE_EVENT(intel_fbc_nuke,
> 
>  /* pipe updates */
> 
> +TRACE_EVENT(intel_crtc_vblank_work_start,
> +	    TP_PROTO(struct intel_crtc *crtc),
> +	    TP_ARGS(crtc),
> +
> +	    TP_STRUCT__entry(
> +			     __field(enum pipe, pipe)
> +			     __field(u32, frame)
> +			     __field(u32, scanline)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->pipe = crtc->pipe;
> +			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
> +			   __entry->scanline = intel_get_crtc_scanline(crtc);
> +			   ),
> +
> +	    TP_printk("pipe %c, frame=%u, scanline=%u",
> +		      pipe_name(__entry->pipe), __entry->frame,
> +		       __entry->scanline)
> +);
> +
> +TRACE_EVENT(intel_crtc_vblank_work_end,
> +	    TP_PROTO(struct intel_crtc *crtc),
> +	    TP_ARGS(crtc),
> +
> +	    TP_STRUCT__entry(
> +			     __field(enum pipe, pipe)
> +			     __field(u32, frame)
> +			     __field(u32, scanline)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->pipe = crtc->pipe;
> +			   __entry->frame = intel_crtc_get_vblank_counter(crtc);
> +			   __entry->scanline = intel_get_crtc_scanline(crtc);
> +			   ),
> +
> +	    TP_printk("pipe %c, frame=%u, scanline=%u",
> +		      pipe_name(__entry->pipe), __entry->frame,
> +		       __entry->scanline)
> +);
> +
>  TRACE_EVENT(intel_pipe_update_start,
>  	    TP_PROTO(struct intel_crtc *crtc),
>  	    TP_ARGS(crtc),
> --
> 2.32.0


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

* Re: [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT loads
  2021-10-20 22:33 ` [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT loads Ville Syrjala
@ 2021-11-08 20:59   ` Shankar, Uma
  2021-11-09 22:56     ` Ville Syrjälä
  0 siblings, 1 reply; 18+ messages in thread
From: Shankar, Uma @ 2021-11-08 20:59 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> Sent: Thursday, October 21, 2021 4:04 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT
> loads
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We have to bash in a lot of registers to load the higher precision LUT modes. The
> locking overhead is significant, especially as we have to get this done as quickly as
> possible during vblank.
> So let's switch to unlocked accesses for these. Fortunately the LUT registers are
> mostly spread around such that two pipes do not have any registers on the same
> cacheline. So as long as commits on the same pipe are serialized (which they are) we
> should get away with this without angering the hardware.
> 
> The only exceptions are the PREC_PIPEGCMAX registers on ilk/snb which we don't
> use atm as they are only used in the 12bit gamma mode. If/when we add support for
> that we may need to remember to still serialize those registers, though I'm not sure
> ilk/snb are actually affected by the same cacheline issue. I think ivb/hsw at least
> were, but they use a different set of registers for the precision LUT.
> 
> I have a test case which is updating the LUTs on two pipes from a single atomic
> commit. Running that in a loop for a minute I get the following worst case with the
> locks in place:
>  intel_crtc_vblank_work_start: pipe B, frame=10037, scanline=1081
>  intel_crtc_vblank_work_start: pipe A, frame=12274, scanline=769
>  intel_crtc_vblank_work_end: pipe A, frame=12274, scanline=58
>  intel_crtc_vblank_work_end: pipe B, frame=10037, scanline=74
> 
> And here's the worst case with the locks removed:
>  intel_crtc_vblank_work_start: pipe B, frame=5869, scanline=1081
>  intel_crtc_vblank_work_start: pipe A, frame=7616, scanline=769
>  intel_crtc_vblank_work_end: pipe B, frame=5869, scanline=1096
>  intel_crtc_vblank_work_end: pipe A, frame=7616, scanline=777
> 
> The test was done on a snb using the 10bit 1024 entry LUT mode.
> The vtotals for the two displays are 793 and 1125. So we can see that with the locks
> ripped out the LUT updates are pretty nicely confined within the vblank, whereas
> with the locks in place we're routinely blasting past the vblank end which causes
> visual artifacts near the top of the screen.

Unprotected writes should be ok to use in lut updates. Looks good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

One side query though, what happens when we go for higher refresh rates like 300+,
Some cases where vblank period is shrunk to as low as 8us (480Hz RR panels).

Regards,
Uma Shankar

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 128 ++++++++++-----------
>  drivers/gpu/drm/i915/display/intel_dsb.c   |   4 +-
>  2 files changed, 66 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 5359b7305a78..c870a0e50cb1 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -552,8 +552,8 @@ static void i9xx_load_lut_8(struct intel_crtc *crtc,
>  	lut = blob->data;
> 
>  	for (i = 0; i < 256; i++)
> -		intel_de_write(dev_priv, PALETTE(pipe, i),
> -			       i9xx_lut_8(&lut[i]));
> +		intel_de_write_fw(dev_priv, PALETTE(pipe, i),
> +				  i9xx_lut_8(&lut[i]));
>  }
> 
>  static void i9xx_load_luts(const struct intel_crtc_state *crtc_state) @@ -576,15
> +576,15 @@ static void i965_load_lut_10p6(struct intel_crtc *crtc,
>  	enum pipe pipe = crtc->pipe;
> 
>  	for (i = 0; i < lut_size - 1; i++) {
> -		intel_de_write(dev_priv, PALETTE(pipe, 2 * i + 0),
> -			       i965_lut_10p6_ldw(&lut[i]));
> -		intel_de_write(dev_priv, PALETTE(pipe, 2 * i + 1),
> -			       i965_lut_10p6_udw(&lut[i]));
> +		intel_de_write_fw(dev_priv, PALETTE(pipe, 2 * i + 0),
> +				  i965_lut_10p6_ldw(&lut[i]));
> +		intel_de_write_fw(dev_priv, PALETTE(pipe, 2 * i + 1),
> +				  i965_lut_10p6_udw(&lut[i]));
>  	}
> 
> -	intel_de_write(dev_priv, PIPEGCMAX(pipe, 0), lut[i].red);
> -	intel_de_write(dev_priv, PIPEGCMAX(pipe, 1), lut[i].green);
> -	intel_de_write(dev_priv, PIPEGCMAX(pipe, 2), lut[i].blue);
> +	intel_de_write_fw(dev_priv, PIPEGCMAX(pipe, 0), lut[i].red);
> +	intel_de_write_fw(dev_priv, PIPEGCMAX(pipe, 1), lut[i].green);
> +	intel_de_write_fw(dev_priv, PIPEGCMAX(pipe, 2), lut[i].blue);
>  }
> 
>  static void i965_load_luts(const struct intel_crtc_state *crtc_state) @@ -618,8
> +618,8 @@ static void ilk_load_lut_8(struct intel_crtc *crtc,
>  	lut = blob->data;
> 
>  	for (i = 0; i < 256; i++)
> -		intel_de_write(dev_priv, LGC_PALETTE(pipe, i),
> -			       i9xx_lut_8(&lut[i]));
> +		intel_de_write_fw(dev_priv, LGC_PALETTE(pipe, i),
> +				  i9xx_lut_8(&lut[i]));
>  }
> 
>  static void ilk_load_lut_10(struct intel_crtc *crtc, @@ -631,8 +631,8 @@ static void
> ilk_load_lut_10(struct intel_crtc *crtc,
>  	enum pipe pipe = crtc->pipe;
> 
>  	for (i = 0; i < lut_size; i++)
> -		intel_de_write(dev_priv, PREC_PALETTE(pipe, i),
> -			       ilk_lut_10(&lut[i]));
> +		intel_de_write_fw(dev_priv, PREC_PALETTE(pipe, i),
> +				  ilk_lut_10(&lut[i]));
>  }
> 
>  static void ilk_load_luts(const struct intel_crtc_state *crtc_state) @@ -681,16
> +681,16 @@ static void ivb_load_lut_10(struct intel_crtc *crtc,
>  		const struct drm_color_lut *entry =
>  			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
> 
> -		intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), prec_index++);
> -		intel_de_write(dev_priv, PREC_PAL_DATA(pipe),
> -			       ilk_lut_10(entry));
> +		intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), prec_index++);
> +		intel_de_write_fw(dev_priv, PREC_PAL_DATA(pipe),
> +				  ilk_lut_10(entry));
>  	}
> 
>  	/*
>  	 * Reset the index, otherwise it prevents the legacy palette to be
>  	 * written properly.
>  	 */
> -	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), 0);
> +	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), 0);
>  }
> 
>  /* On BDW+ the index auto increment mode actually works */ @@ -704,23 +704,23
> @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
>  	int i, lut_size = drm_color_lut_size(blob);
>  	enum pipe pipe = crtc->pipe;
> 
> -	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe),
> -		       prec_index | PAL_PREC_AUTO_INCREMENT);
> +	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe),
> +			  prec_index | PAL_PREC_AUTO_INCREMENT);
> 
>  	for (i = 0; i < hw_lut_size; i++) {
>  		/* We discard half the user entries in split gamma mode */
>  		const struct drm_color_lut *entry =
>  			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
> 
> -		intel_de_write(dev_priv, PREC_PAL_DATA(pipe),
> -			       ilk_lut_10(entry));
> +		intel_de_write_fw(dev_priv, PREC_PAL_DATA(pipe),
> +				  ilk_lut_10(entry));
>  	}
> 
>  	/*
>  	 * Reset the index, otherwise it prevents the legacy palette to be
>  	 * written properly.
>  	 */
> -	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), 0);
> +	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), 0);
>  }
> 
>  static void ivb_load_lut_ext_max(const struct intel_crtc_state *crtc_state) @@ -
> 821,9 +821,9 @@ static void glk_load_degamma_lut(const struct intel_crtc_state
> *crtc_state)
>  	 * ignore the index bits, so we need to reset it to index 0
>  	 * separately.
>  	 */
> -	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> -	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> -		       PRE_CSC_GAMC_AUTO_INCREMENT);
> +	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> +	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> +			  PRE_CSC_GAMC_AUTO_INCREMENT);
> 
>  	for (i = 0; i < lut_size; i++) {
>  		/*
> @@ -839,15 +839,15 @@ static void glk_load_degamma_lut(const struct
> intel_crtc_state *crtc_state)
>  		 * ToDo: Extend to max 7.0. Enable 32 bit input value
>  		 * as compared to just 16 to achieve this.
>  		 */
> -		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe),
> -			       lut[i].green);
> +		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe),
> +				  lut[i].green);
>  	}
> 
>  	/* Clamp values > 1.0. */
>  	while (i++ < 35)
> -		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> +		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> 
> -	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> +	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
>  }
> 
>  static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_state)
> @@ -862,21 +862,21 @@ static void glk_load_degamma_lut_linear(const struct
> intel_crtc_state *crtc_stat
>  	 * ignore the index bits, so we need to reset it to index 0
>  	 * separately.
>  	 */
> -	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> -	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> -		       PRE_CSC_GAMC_AUTO_INCREMENT);
> +	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> +	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> +			  PRE_CSC_GAMC_AUTO_INCREMENT);
> 
>  	for (i = 0; i < lut_size; i++) {
>  		u32 v = (i << 16) / (lut_size - 1);
> 
> -		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe), v);
> +		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), v);
>  	}
> 
>  	/* Clamp values > 1.0. */
>  	while (i++ < 35)
> -		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> +		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> 
> -	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> +	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
>  }
> 
>  static void glk_load_luts(const struct intel_crtc_state *crtc_state) @@ -1071,10
> +1071,10 @@ static void chv_load_cgm_degamma(struct intel_crtc *crtc,
>  	enum pipe pipe = crtc->pipe;
> 
>  	for (i = 0; i < lut_size; i++) {
> -		intel_de_write(dev_priv, CGM_PIPE_DEGAMMA(pipe, i, 0),
> -			       chv_cgm_degamma_ldw(&lut[i]));
> -		intel_de_write(dev_priv, CGM_PIPE_DEGAMMA(pipe, i, 1),
> -			       chv_cgm_degamma_udw(&lut[i]));
> +		intel_de_write_fw(dev_priv, CGM_PIPE_DEGAMMA(pipe, i, 0),
> +				  chv_cgm_degamma_ldw(&lut[i]));
> +		intel_de_write_fw(dev_priv, CGM_PIPE_DEGAMMA(pipe, i, 1),
> +				  chv_cgm_degamma_udw(&lut[i]));
>  	}
>  }
> 
> @@ -1105,10 +1105,10 @@ static void chv_load_cgm_gamma(struct intel_crtc
> *crtc,
>  	enum pipe pipe = crtc->pipe;
> 
>  	for (i = 0; i < lut_size; i++) {
> -		intel_de_write(dev_priv, CGM_PIPE_GAMMA(pipe, i, 0),
> -			       chv_cgm_gamma_ldw(&lut[i]));
> -		intel_de_write(dev_priv, CGM_PIPE_GAMMA(pipe, i, 1),
> -			       chv_cgm_gamma_udw(&lut[i]));
> +		intel_de_write_fw(dev_priv, CGM_PIPE_GAMMA(pipe, i, 0),
> +				  chv_cgm_gamma_ldw(&lut[i]));
> +		intel_de_write_fw(dev_priv, CGM_PIPE_GAMMA(pipe, i, 1),
> +				  chv_cgm_gamma_udw(&lut[i]));
>  	}
>  }
> 
> @@ -1131,8 +1131,8 @@ static void chv_load_luts(const struct intel_crtc_state
> *crtc_state)
>  	else
>  		i965_load_luts(crtc_state);
> 
> -	intel_de_write(dev_priv, CGM_PIPE_MODE(crtc->pipe),
> -		       crtc_state->cgm_mode);
> +	intel_de_write_fw(dev_priv, CGM_PIPE_MODE(crtc->pipe),
> +			  crtc_state->cgm_mode);
>  }
> 
>  void intel_color_load_luts(const struct intel_crtc_state *crtc_state) @@ -1808,7
> +1808,7 @@ static struct drm_property_blob *i9xx_read_lut_8(struct intel_crtc
> *crtc)
>  	lut = blob->data;
> 
>  	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
> -		u32 val = intel_de_read(dev_priv, PALETTE(pipe, i));
> +		u32 val = intel_de_read_fw(dev_priv, PALETTE(pipe, i));
> 
>  		i9xx_lut_8_pack(&lut[i], val);
>  	}
> @@ -1843,15 +1843,15 @@ static struct drm_property_blob
> *i965_read_lut_10p6(struct intel_crtc *crtc)
>  	lut = blob->data;
> 
>  	for (i = 0; i < lut_size - 1; i++) {
> -		u32 ldw = intel_de_read(dev_priv, PALETTE(pipe, 2 * i + 0));
> -		u32 udw = intel_de_read(dev_priv, PALETTE(pipe, 2 * i + 1));
> +		u32 ldw = intel_de_read_fw(dev_priv, PALETTE(pipe, 2 * i + 0));
> +		u32 udw = intel_de_read_fw(dev_priv, PALETTE(pipe, 2 * i + 1));
> 
>  		i965_lut_10p6_pack(&lut[i], ldw, udw);
>  	}
> 
> -	lut[i].red = i965_lut_11p6_max_pack(intel_de_read(dev_priv,
> PIPEGCMAX(pipe, 0)));
> -	lut[i].green = i965_lut_11p6_max_pack(intel_de_read(dev_priv,
> PIPEGCMAX(pipe, 1)));
> -	lut[i].blue = i965_lut_11p6_max_pack(intel_de_read(dev_priv,
> PIPEGCMAX(pipe, 2)));
> +	lut[i].red = i965_lut_11p6_max_pack(intel_de_read_fw(dev_priv,
> PIPEGCMAX(pipe, 0)));
> +	lut[i].green = i965_lut_11p6_max_pack(intel_de_read_fw(dev_priv,
> PIPEGCMAX(pipe, 1)));
> +	lut[i].blue = i965_lut_11p6_max_pack(intel_de_read_fw(dev_priv,
> +PIPEGCMAX(pipe, 2)));
> 
>  	return blob;
>  }
> @@ -1886,8 +1886,8 @@ static struct drm_property_blob
> *chv_read_cgm_gamma(struct intel_crtc *crtc)
>  	lut = blob->data;
> 
>  	for (i = 0; i < lut_size; i++) {
> -		u32 ldw = intel_de_read(dev_priv, CGM_PIPE_GAMMA(pipe, i, 0));
> -		u32 udw = intel_de_read(dev_priv, CGM_PIPE_GAMMA(pipe, i, 1));
> +		u32 ldw = intel_de_read_fw(dev_priv, CGM_PIPE_GAMMA(pipe, i,
> 0));
> +		u32 udw = intel_de_read_fw(dev_priv, CGM_PIPE_GAMMA(pipe, i,
> 1));
> 
>  		chv_cgm_gamma_pack(&lut[i], ldw, udw);
>  	}
> @@ -1922,7 +1922,7 @@ static struct drm_property_blob *ilk_read_lut_8(struct
> intel_crtc *crtc)
>  	lut = blob->data;
> 
>  	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
> -		u32 val = intel_de_read(dev_priv, LGC_PALETTE(pipe, i));
> +		u32 val = intel_de_read_fw(dev_priv, LGC_PALETTE(pipe, i));
> 
>  		i9xx_lut_8_pack(&lut[i], val);
>  	}
> @@ -1947,7 +1947,7 @@ static struct drm_property_blob *ilk_read_lut_10(struct
> intel_crtc *crtc)
>  	lut = blob->data;
> 
>  	for (i = 0; i < lut_size; i++) {
> -		u32 val = intel_de_read(dev_priv, PREC_PALETTE(pipe, i));
> +		u32 val = intel_de_read_fw(dev_priv, PREC_PALETTE(pipe, i));
> 
>  		ilk_lut_10_pack(&lut[i], val);
>  	}
> @@ -1999,16 +1999,16 @@ static struct drm_property_blob
> *bdw_read_lut_10(struct intel_crtc *crtc,
> 
>  	lut = blob->data;
> 
> -	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe),
> -		       prec_index | PAL_PREC_AUTO_INCREMENT);
> +	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe),
> +			  prec_index | PAL_PREC_AUTO_INCREMENT);
> 
>  	for (i = 0; i < lut_size; i++) {
> -		u32 val = intel_de_read(dev_priv, PREC_PAL_DATA(pipe));
> +		u32 val = intel_de_read_fw(dev_priv, PREC_PAL_DATA(pipe));
> 
>  		ilk_lut_10_pack(&lut[i], val);
>  	}
> 
> -	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), 0);
> +	intel_de_write_fw(dev_priv, PREC_PAL_INDEX(pipe), 0);
> 
>  	return blob;
>  }
> @@ -2050,17 +2050,17 @@ icl_read_lut_multi_segment(struct intel_crtc *crtc)
> 
>  	lut = blob->data;
> 
> -	intel_de_write(dev_priv, PREC_PAL_MULTI_SEG_INDEX(pipe),
> -		       PAL_PREC_AUTO_INCREMENT);
> +	intel_de_write_fw(dev_priv, PREC_PAL_MULTI_SEG_INDEX(pipe),
> +			  PAL_PREC_AUTO_INCREMENT);
> 
>  	for (i = 0; i < 9; i++) {
> -		u32 ldw = intel_de_read(dev_priv,
> PREC_PAL_MULTI_SEG_DATA(pipe));
> -		u32 udw = intel_de_read(dev_priv,
> PREC_PAL_MULTI_SEG_DATA(pipe));
> +		u32 ldw = intel_de_read_fw(dev_priv,
> PREC_PAL_MULTI_SEG_DATA(pipe));
> +		u32 udw = intel_de_read_fw(dev_priv,
> PREC_PAL_MULTI_SEG_DATA(pipe));
> 
>  		icl_lut_multi_seg_pack(&lut[i], ldw, udw);
>  	}
> 
> -	intel_de_write(dev_priv, PREC_PAL_MULTI_SEG_INDEX(pipe), 0);
> +	intel_de_write_fw(dev_priv, PREC_PAL_MULTI_SEG_INDEX(pipe), 0);
> 
>  	/*
>  	 * FIXME readouts from PAL_PREC_DATA register aren't giving diff --git
> a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 62a8a69f9f5d..83a69a4a4fea 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -100,7 +100,7 @@ void intel_dsb_indexed_reg_write(const struct
> intel_crtc_state *crtc_state,
>  	u32 reg_val;
> 
>  	if (!dsb) {
> -		intel_de_write(dev_priv, reg, val);
> +		intel_de_write_fw(dev_priv, reg, val);
>  		return;
>  	}
>  	buf = dsb->cmd_buf;
> @@ -177,7 +177,7 @@ void intel_dsb_reg_write(const struct intel_crtc_state
> *crtc_state,
> 
>  	dsb = crtc_state->dsb;
>  	if (!dsb) {
> -		intel_de_write(dev_priv, reg, val);
> +		intel_de_write_fw(dev_priv, reg, val);
>  		return;
>  	}
> 
> --
> 2.32.0


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

* Re: [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT loads
  2021-11-08 20:59   ` Shankar, Uma
@ 2021-11-09 22:56     ` Ville Syrjälä
  0 siblings, 0 replies; 18+ messages in thread
From: Ville Syrjälä @ 2021-11-09 22:56 UTC (permalink / raw)
  To: Shankar, Uma; +Cc: intel-gfx@lists.freedesktop.org

On Mon, Nov 08, 2021 at 08:59:31PM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> > Sent: Thursday, October 21, 2021 4:04 AM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT
> > loads
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We have to bash in a lot of registers to load the higher precision LUT modes. The
> > locking overhead is significant, especially as we have to get this done as quickly as
> > possible during vblank.
> > So let's switch to unlocked accesses for these. Fortunately the LUT registers are
> > mostly spread around such that two pipes do not have any registers on the same
> > cacheline. So as long as commits on the same pipe are serialized (which they are) we
> > should get away with this without angering the hardware.
> > 
> > The only exceptions are the PREC_PIPEGCMAX registers on ilk/snb which we don't
> > use atm as they are only used in the 12bit gamma mode. If/when we add support for
> > that we may need to remember to still serialize those registers, though I'm not sure
> > ilk/snb are actually affected by the same cacheline issue. I think ivb/hsw at least
> > were, but they use a different set of registers for the precision LUT.
> > 
> > I have a test case which is updating the LUTs on two pipes from a single atomic
> > commit. Running that in a loop for a minute I get the following worst case with the
> > locks in place:
> >  intel_crtc_vblank_work_start: pipe B, frame=10037, scanline=1081
> >  intel_crtc_vblank_work_start: pipe A, frame=12274, scanline=769
> >  intel_crtc_vblank_work_end: pipe A, frame=12274, scanline=58
> >  intel_crtc_vblank_work_end: pipe B, frame=10037, scanline=74
> > 
> > And here's the worst case with the locks removed:
> >  intel_crtc_vblank_work_start: pipe B, frame=5869, scanline=1081
> >  intel_crtc_vblank_work_start: pipe A, frame=7616, scanline=769
> >  intel_crtc_vblank_work_end: pipe B, frame=5869, scanline=1096
> >  intel_crtc_vblank_work_end: pipe A, frame=7616, scanline=777
> > 
> > The test was done on a snb using the 10bit 1024 entry LUT mode.
> > The vtotals for the two displays are 793 and 1125. So we can see that with the locks
> > ripped out the LUT updates are pretty nicely confined within the vblank, whereas
> > with the locks in place we're routinely blasting past the vblank end which causes
> > visual artifacts near the top of the screen.
> 
> Unprotected writes should be ok to use in lut updates. Looks good to me.
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> 
> One side query though, what happens when we go for higher refresh rates like 300+,
> Some cases where vblank period is shrunk to as low as 8us (480Hz RR panels).

If the vblank is short enough then we're pretty much screwed and will
have to accept tearing. Maybe there's a little of bit of micro 
optimization left we could do to the .load_lut(). But I wouldn't expect
miracles from that since we still have to do the actual mmio, and that
we can't speed up.

Long ago I did have some numbers on how long each mmio access will
take on specific platforms but I don't recall the details anymore.
I guess it might be interesting to measure it again just to have
some idea on the lower bound, and then compare that to what we
currently achieve in .load_lut(). Would at least tell us if there's
any juice left in the tank.

And of course DSB might save us on new platforms since it should
be faster than mmio, but I've not done any measurements on it.
Should be interesting to find out how it performs.

-- 
Ville Syrjälä
Intel

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

end of thread, other threads:[~2021-11-09 22:56 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-20 22:33 [Intel-gfx] [PATCH 0/4] drm/i915: (near)atomic gamma LUT updates via vblank workers Ville Syrjala
2021-10-20 22:33 ` [Intel-gfx] [PATCH 1/4] drm/i915: Move function prototypes to the correct header Ville Syrjala
2021-10-21 10:26   ` Jani Nikula
2021-10-20 22:33 ` [Intel-gfx] [PATCH 2/4] drm/i915: Do vrr push before sampling the freame counter Ville Syrjala
2021-11-08 19:36   ` Shankar, Uma
2021-10-20 22:33 ` [Intel-gfx] [PATCH 3/4] drm/i915: Use vblank workers for gamma updates Ville Syrjala
2021-10-21 10:35   ` Jani Nikula
2021-10-21 10:42     ` Ville Syrjälä
2021-10-21 12:51       ` Jani Nikula
2021-11-08 20:41   ` Shankar, Uma
2021-10-20 22:33 ` [Intel-gfx] [PATCH 4/4] drm/i915: Use unlocked register accesses for LUT loads Ville Syrjala
2021-11-08 20:59   ` Shankar, Uma
2021-11-09 22:56     ` Ville Syrjälä
2021-10-21  0:59 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: (near)atomic gamma LUT updates via vblank workers Patchwork
2021-10-21  1:00 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-10-21  1:29 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-10-21  7:11 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-10-26 22:02 ` [Intel-gfx] [PATCH 0/4] " Lyude Paul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox