drm/i915: Add hardware csc readout for ilk+

Read out the pipe/output csc matrices on ilk+ and stash the results
(in the hardware specific format) into the appropriate place
in the crtc state.

Note that on skl/glk/icl the pipe csc unit suffers from an issue
where *reads* of the coefficient/offset registers also disarm
the double buffer update (if currently armed via CSC_MODE write).
So it's rather important that the readout only happens after the
csc registers have been latched. Fortunately the state checker
only runs after the start of vblank where the latching happens.

And on skl/glk the DMC + CSC register read has the potential to
corrupt the latched CSC register values, so let's add a comment
reminding us that the DC states should remain off until the
readout has been completed.

TODO: maybe we could somehow check to make sure PSR has in fact
latched the new register values already, and that DC states
have been off all along?

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230329135002.3096-9-ville.syrjala@linux.intel.com
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 2988c91..efa8a6d 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -70,6 +70,11 @@ struct intel_color_funcs {
                          const struct drm_property_blob *blob1,
                          const struct drm_property_blob *blob2,
                          bool is_pre_csc_lut);
+       /*
+        * Read out the CSCs (if any) from the hardware into the
+        * software state. Used by eg. the hardware state checker.
+        */
+       void (*read_csc)(struct intel_crtc_state *crtc_state);
 };
 
 #define CTM_COEFF_SIGN (1ULL << 63)
@@ -227,6 +232,72 @@ static void ilk_update_pipe_csc(struct intel_crtc *crtc,
        intel_de_write_fw(i915, PIPE_CSC_POSTOFF_LO(pipe), csc->postoff[2]);
 }
 
+static void ilk_read_pipe_csc(struct intel_crtc *crtc,
+                             struct intel_csc_matrix *csc)
+{
+       struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+       enum pipe pipe = crtc->pipe;
+       u32 tmp;
+
+       csc->preoff[0] = intel_de_read_fw(i915, PIPE_CSC_PREOFF_HI(pipe));
+       csc->preoff[1] = intel_de_read_fw(i915, PIPE_CSC_PREOFF_ME(pipe));
+       csc->preoff[2] = intel_de_read_fw(i915, PIPE_CSC_PREOFF_LO(pipe));
+
+       tmp = intel_de_read_fw(i915, PIPE_CSC_COEFF_RY_GY(pipe));
+       csc->coeff[0] = tmp >> 16;
+       csc->coeff[1] = tmp & 0xffff;
+       tmp = intel_de_read_fw(i915, PIPE_CSC_COEFF_BY(pipe));
+       csc->coeff[2] = tmp >> 16;
+
+       tmp = intel_de_read_fw(i915, PIPE_CSC_COEFF_RU_GU(pipe));
+       csc->coeff[3] = tmp >> 16;
+       csc->coeff[4] = tmp & 0xffff;
+       tmp = intel_de_read_fw(i915, PIPE_CSC_COEFF_BU(pipe));
+       csc->coeff[5] = tmp >> 16;
+
+       tmp = intel_de_read_fw(i915, PIPE_CSC_COEFF_RV_GV(pipe));
+       csc->coeff[6] = tmp >> 16;
+       csc->coeff[7] = tmp & 0xffff;
+       tmp = intel_de_read_fw(i915, PIPE_CSC_COEFF_BV(pipe));
+       csc->coeff[8] = tmp >> 16;
+
+       if (DISPLAY_VER(i915) < 7)
+               return;
+
+       csc->postoff[0] = intel_de_read_fw(i915, PIPE_CSC_POSTOFF_HI(pipe));
+       csc->postoff[1] = intel_de_read_fw(i915, PIPE_CSC_POSTOFF_ME(pipe));
+       csc->postoff[2] = intel_de_read_fw(i915, PIPE_CSC_POSTOFF_LO(pipe));
+}
+
+static void ilk_read_csc(struct intel_crtc_state *crtc_state)
+{
+       struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+       if (crtc_state->csc_enable)
+               ilk_read_pipe_csc(crtc, &crtc_state->csc);
+}
+
+static void skl_read_csc(struct intel_crtc_state *crtc_state)
+{
+       struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+       /*
+        * Display WA #1184: skl,glk
+        * Wa_1406463849: icl
+        *
+        * Danger! On SKL-ICL *reads* from the CSC coeff/offset registers
+        * will disarm an already armed CSC double buffer update.
+        * So this must not be called while armed. Fortunately the state checker
+        * readout happens only after the update has been already been latched.
+        *
+        * On earlier and later platforms only writes to said registers will
+        * disarm the update. This is considered normal behavior and also
+        * happens with various other hardware units.
+        */
+       if (crtc_state->csc_enable)
+               ilk_read_pipe_csc(crtc, &crtc_state->csc);
+}
+
 static void icl_update_output_csc(struct intel_crtc *crtc,
                                  const struct intel_csc_matrix *csc)
 {
@@ -257,6 +328,56 @@ static void icl_update_output_csc(struct intel_crtc *crtc,
        intel_de_write_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_LO(pipe), csc->postoff[2]);
 }
 
+static void icl_read_output_csc(struct intel_crtc *crtc,
+                               struct intel_csc_matrix *csc)
+{
+       struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+       enum pipe pipe = crtc->pipe;
+       u32 tmp;
+
+       csc->preoff[0] = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_PREOFF_HI(pipe));
+       csc->preoff[1] = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_PREOFF_ME(pipe));
+       csc->preoff[2] = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_PREOFF_LO(pipe));
+
+       tmp = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_COEFF_RY_GY(pipe));
+       csc->coeff[0] = tmp >> 16;
+       csc->coeff[1] = tmp & 0xffff;
+       tmp = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_COEFF_BY(pipe));
+       csc->coeff[2] = tmp >> 16;
+
+       tmp = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_COEFF_RU_GU(pipe));
+       csc->coeff[3] = tmp >> 16;
+       csc->coeff[4] = tmp & 0xffff;
+       tmp = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_COEFF_BU(pipe));
+       csc->coeff[5] = tmp >> 16;
+
+       tmp = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_COEFF_RV_GV(pipe));
+       csc->coeff[6] = tmp >> 16;
+       csc->coeff[7] = tmp & 0xffff;
+       tmp = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_COEFF_BV(pipe));
+       csc->coeff[8] = tmp >> 16;
+
+       csc->postoff[0] = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_HI(pipe));
+       csc->postoff[1] = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_ME(pipe));
+       csc->postoff[2] = intel_de_read_fw(i915, PIPE_CSC_OUTPUT_POSTOFF_LO(pipe));
+}
+
+static void icl_read_csc(struct intel_crtc_state *crtc_state)
+{
+       struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+       /*
+        * Wa_1406463849: icl
+        *
+        * See skl_read_csc()
+        */
+       if (crtc_state->csc_mode & ICL_CSC_ENABLE)
+               ilk_read_pipe_csc(crtc, &crtc_state->csc);
+
+       if (crtc_state->csc_mode & ICL_OUTPUT_CSC_ENABLE)
+               icl_read_output_csc(crtc, &crtc_state->output_csc);
+}
+
 static bool ilk_limited_range(const struct intel_crtc_state *crtc_state)
 {
        struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
@@ -1581,6 +1702,9 @@ void intel_color_get_config(struct intel_crtc_state *crtc_state)
        struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
 
        i915->display.funcs.color->read_luts(crtc_state);
+
+       if (i915->display.funcs.color->read_csc)
+               i915->display.funcs.color->read_csc(crtc_state);
 }
 
 bool intel_color_lut_equal(const struct intel_crtc_state *crtc_state,
@@ -3229,6 +3353,7 @@ static const struct intel_color_funcs tgl_color_funcs = {
        .load_luts = icl_load_luts,
        .read_luts = icl_read_luts,
        .lut_equal = icl_lut_equal,
+       .read_csc = icl_read_csc,
 };
 
 static const struct intel_color_funcs icl_color_funcs = {
@@ -3239,6 +3364,7 @@ static const struct intel_color_funcs icl_color_funcs = {
        .load_luts = icl_load_luts,
        .read_luts = icl_read_luts,
        .lut_equal = icl_lut_equal,
+       .read_csc = icl_read_csc,
 };
 
 static const struct intel_color_funcs glk_color_funcs = {
@@ -3248,6 +3374,7 @@ static const struct intel_color_funcs glk_color_funcs = {
        .load_luts = glk_load_luts,
        .read_luts = glk_read_luts,
        .lut_equal = glk_lut_equal,
+       .read_csc = skl_read_csc,
 };
 
 static const struct intel_color_funcs skl_color_funcs = {
@@ -3257,6 +3384,7 @@ static const struct intel_color_funcs skl_color_funcs = {
        .load_luts = bdw_load_luts,
        .read_luts = bdw_read_luts,
        .lut_equal = ivb_lut_equal,
+       .read_csc = skl_read_csc,
 };
 
 static const struct intel_color_funcs bdw_color_funcs = {
@@ -3266,6 +3394,7 @@ static const struct intel_color_funcs bdw_color_funcs = {
        .load_luts = bdw_load_luts,
        .read_luts = bdw_read_luts,
        .lut_equal = ivb_lut_equal,
+       .read_csc = ilk_read_csc,
 };
 
 static const struct intel_color_funcs hsw_color_funcs = {
@@ -3275,6 +3404,7 @@ static const struct intel_color_funcs hsw_color_funcs = {
        .load_luts = ivb_load_luts,
        .read_luts = ivb_read_luts,
        .lut_equal = ivb_lut_equal,
+       .read_csc = ilk_read_csc,
 };
 
 static const struct intel_color_funcs ivb_color_funcs = {
@@ -3284,6 +3414,7 @@ static const struct intel_color_funcs ivb_color_funcs = {
        .load_luts = ivb_load_luts,
        .read_luts = ivb_read_luts,
        .lut_equal = ivb_lut_equal,
+       .read_csc = ilk_read_csc,
 };
 
 static const struct intel_color_funcs ilk_color_funcs = {
@@ -3293,6 +3424,7 @@ static const struct intel_color_funcs ilk_color_funcs = {
        .load_luts = ilk_load_luts,
        .read_luts = ilk_read_luts,
        .lut_equal = ilk_lut_equal,
+       .read_csc = ilk_read_csc,
 };
 
 void intel_color_crtc_init(struct intel_crtc *crtc)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 010ee79..270658e 100644 (file)
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7313,6 +7313,12 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
         * 7. New _arm() registers are finally written
         * 8. Hardware finally latches a complete set of new
         *    register values, and subsequent frames will be OK again
+        *
+        * Also note that due to the pipe CSC hardware issues on
+        * SKL/GLK DC states must remain off until the pipe CSC
+        * state readout has happened. Otherwise we risk corrupting
+        * the CSC latched register values with the readout (see
+        * skl_read_csc() and skl_color_commit_noarm()).
         */
        wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_DC_OFF);