drm/i915/dsb: Load LUTs using the DSB during vblank

Loading LUTs with the DSB outside of vblank doesn't really
work due to the palette anti-collision logic. Apparently the
DSB register writes don't get stalled like CPU mmio writes
do and instead we end up corrupting the LUT entries. Disabling
the anti-collision logic would allow us to successfully load
the LUT outside of vblank, but presumably that risks the LUT
reads from the scanout (temporarily) getting corrupted data
from the LUT instead.

The anti-collision logic isn't active during vblank so that
is when we can successfully load the LUT with the DSB. That is
what we want to do anyway to avoid tearing.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230606191504.18099-13-ville.syrjala@linux.intel.com
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index bb6eda00561374d4c5227a50111b0ec655172f92..7da79ffeb77bff5cceb3161470b0a1deff0765b4 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1741,12 +1741,6 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
                MISSING_CASE(crtc_state->gamma_mode);
                break;
        }
-
-       if (crtc_state->dsb) {
-               intel_dsb_finish(crtc_state->dsb);
-               intel_dsb_commit(crtc_state->dsb, false);
-               intel_dsb_wait(crtc_state->dsb);
-       }
 }
 
 static void vlv_load_luts(const struct intel_crtc_state *crtc_state)
@@ -1853,6 +1847,9 @@ void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
 {
        struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
 
+       if (crtc_state->dsb)
+               return;
+
        i915->display.funcs.color->load_luts(crtc_state);
 }
 
@@ -1869,6 +1866,9 @@ void intel_color_commit_arm(const struct intel_crtc_state *crtc_state)
        struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
 
        i915->display.funcs.color->color_commit_arm(crtc_state);
+
+       if (crtc_state->dsb)
+               intel_dsb_commit(crtc_state->dsb, true);
 }
 
 void intel_color_post_update(const struct intel_crtc_state *crtc_state)
@@ -1882,6 +1882,7 @@ void intel_color_post_update(const struct intel_crtc_state *crtc_state)
 void intel_color_prepare_commit(struct intel_crtc_state *crtc_state)
 {
        struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+       struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 
        /* FIXME DSB has issues loading LUTs, disable it for now */
        return;
@@ -1894,6 +1895,12 @@ void intel_color_prepare_commit(struct intel_crtc_state *crtc_state)
                return;
 
        crtc_state->dsb = intel_dsb_prepare(crtc, 1024);
+       if (!crtc_state->dsb)
+               return;
+
+       i915->display.funcs.color->load_luts(crtc_state);
+
+       intel_dsb_finish(crtc_state->dsb);
 }
 
 void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state)
@@ -1905,6 +1912,17 @@ void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state)
        crtc_state->dsb = NULL;
 }
 
+void intel_color_wait_commit(const struct intel_crtc_state *crtc_state)
+{
+       if (crtc_state->dsb)
+               intel_dsb_wait(crtc_state->dsb);
+}
+
+bool intel_color_uses_dsb(const struct intel_crtc_state *crtc_state)
+{
+       return crtc_state->dsb;
+}
+
 static bool intel_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
 {
        struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);

diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
index 8002492be709096aec9169c4e89483d8ccac112f..8ecd36149def8995ebf66a766dfbdff958b0f931 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -19,6 +19,8 @@ void intel_color_crtc_init(struct intel_crtc *crtc);
 int intel_color_check(struct intel_crtc_state *crtc_state);
 void intel_color_prepare_commit(struct intel_crtc_state *crtc_state);
 void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state);
+bool intel_color_uses_dsb(const struct intel_crtc_state *crtc_state);
+void intel_color_wait_commit(const struct intel_crtc_state *crtc_state);
 void intel_color_commit_noarm(const struct intel_crtc_state *crtc_state);
 void intel_color_commit_arm(const struct intel_crtc_state *crtc_state);
 void intel_color_post_update(const struct intel_crtc_state *crtc_state);

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 22e85fe7e8aa46f333f8a623501d023bf6304ac4..492347bd0e9dab6daea9df598ac8390ae2c065b5 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -24,6 +24,7 @@
 #include "intel_display_trace.h"
 #include "intel_display_types.h"
 #include "intel_drrs.h"
+#include "intel_dsb.h"
 #include "intel_dsi.h"
 #include "intel_fifo_underrun.h"
 #include "intel_pipe_crc.h"
@@ -394,7 +395,8 @@ static bool intel_crtc_needs_vblank_work(const struct intel_crtc_state *crtc_sta
        return crtc_state->hw.active &&
                !intel_crtc_needs_modeset(crtc_state) &&
                !crtc_state->preload_luts &&
-               intel_crtc_needs_color_update(crtc_state);
+               intel_crtc_needs_color_update(crtc_state) &&
+               !intel_color_uses_dsb(crtc_state);
 }
 
 static void intel_crtc_vblank_work(struct kthread_work *base)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index edbcf5968804dea30ad4b6ac81c2de5ab9f634e5..4269b4f89d79d752c32f0a6b4f63bf3877c1fe85 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -77,6 +77,7 @@
 #include "intel_dpll_mgr.h"
 #include "intel_dpt.h"
 #include "intel_drrs.h"
+#include "intel_dsb.h"
 #include "intel_dsi.h"
 #include "intel_dvo.h"
 #include "intel_fb.h"
@@ -7144,6 +7145,8 @@ 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->do_async_flip)
                        intel_crtc_disable_flip_done(state, crtc);
+
+               intel_color_wait_commit(new_crtc_state);
        }
 
        /*