Other features like PSR2 also needs to be disabled while getting CRC
so lets rename ips_force_disable to crc_enabled, drop all this checks
for pipe A and HSW and BDW and make it generic and
hsw_compute_ips_config() will take care of all the checks removed
from here.

v2: Renaming and parameter changes to the functions that prepares the
commit (Ville)

Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.v...@intel.com>
Signed-off-by: José Roberto de Souza <jose.so...@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  | 10 ++++--
 drivers/gpu/drm/i915/intel_drv.h      |  3 +-
 drivers/gpu/drm/i915/intel_pipe_crc.c | 47 +++++++++++----------------
 3 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9312b3f35eb0..b3a5d8462251 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6751,7 +6751,13 @@ static bool hsw_compute_ips_config(struct 
intel_crtc_state *crtc_state)
        if (!hsw_crtc_state_ips_capable(crtc_state))
                return false;
 
-       if (crtc_state->ips_force_disable)
+       /*
+        * When IPS gets enabled, the pipe CRC changes. Since IPS gets
+        * enabled and disabled dynamically based on package C states,
+        * user space can't make reliable use of the CRCs, so let's just
+        * completely disable it.
+        */
+       if (crtc_state->crc_enabled)
                return false;
 
        /* IPS should be fine as long as at least one plane is enabled. */
@@ -11687,7 +11693,7 @@ clear_intel_crtc_state(struct intel_crtc_state 
*crtc_state)
        saved_state->shared_dpll = crtc_state->shared_dpll;
        saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
        saved_state->pch_pfit.force_thru = crtc_state->pch_pfit.force_thru;
-       saved_state->ips_force_disable = crtc_state->ips_force_disable;
+       saved_state->crc_enabled = crtc_state->crc_enabled;
        if (IS_G4X(dev_priv) ||
            IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
                saved_state->wm = crtc_state->wm;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5412373e2f98..2be64529e4a2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -999,7 +999,8 @@ struct intel_crtc_state {
        struct intel_link_m_n fdi_m_n;
 
        bool ips_enabled;
-       bool ips_force_disable;
+
+       bool crc_enabled;
 
        bool enable_fbc;
 
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c 
b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 53d4ec68d3c4..af64597c5c6e 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -280,15 +280,15 @@ static int ilk_pipe_crc_ctl_reg(enum 
intel_pipe_crc_source *source,
        return 0;
 }
 
-static void hsw_pipe_A_crc_wa(struct drm_i915_private *dev_priv,
-                             bool enable)
+static void
+intel_crtc_crc_setup_workarounds(struct intel_crtc *crtc, bool enable)
 {
+       struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
        struct drm_device *dev = &dev_priv->drm;
-       struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
        struct intel_crtc_state *pipe_config;
        struct drm_atomic_state *state;
        struct drm_modeset_acquire_ctx ctx;
-       int ret = 0;
+       int ret;
 
        drm_modeset_acquire_init(&ctx, 0);
 
@@ -307,17 +307,9 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private 
*dev_priv,
                goto put_state;
        }
 
-       if (HAS_IPS(dev_priv)) {
-               /*
-                * When IPS gets enabled, the pipe CRC changes. Since IPS gets
-                * enabled and disabled dynamically based on package C states,
-                * user space can't make reliable use of the CRCs, so let's just
-                * completely disable it.
-                */
-               pipe_config->ips_force_disable = enable;
-       }
+       pipe_config->crc_enabled = enable;
 
-       if (IS_HASWELL(dev_priv)) {
+       if (IS_HASWELL(dev_priv) && crtc->pipe == PIPE_A) {
                pipe_config->pch_pfit.force_thru = enable;
                if (pipe_config->cpu_transcoder == TRANSCODER_EDP &&
                    pipe_config->pch_pfit.enabled != enable)
@@ -343,8 +335,7 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private 
*dev_priv,
 static int ivb_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
                                enum pipe pipe,
                                enum intel_pipe_crc_source *source,
-                               u32 *val,
-                               bool set_wa)
+                               u32 *val)
 {
        if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
                *source = INTEL_PIPE_CRC_SOURCE_PIPE;
@@ -357,10 +348,6 @@ static int ivb_pipe_crc_ctl_reg(struct drm_i915_private 
*dev_priv,
                *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_SPRITE_IVB;
                break;
        case INTEL_PIPE_CRC_SOURCE_PIPE:
-               if (set_wa && (IS_HASWELL(dev_priv) ||
-                    IS_BROADWELL(dev_priv)) && pipe == PIPE_A)
-                       hsw_pipe_A_crc_wa(dev_priv, true);
-
                *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PF_IVB;
                break;
        case INTEL_PIPE_CRC_SOURCE_NONE:
@@ -418,8 +405,7 @@ static int skl_pipe_crc_ctl_reg(struct drm_i915_private 
*dev_priv,
 
 static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
                               enum pipe pipe,
-                              enum intel_pipe_crc_source *source, u32 *val,
-                              bool set_wa)
+                              enum intel_pipe_crc_source *source, u32 *val)
 {
        if (IS_GEN(dev_priv, 2))
                return i8xx_pipe_crc_ctl_reg(source, val);
@@ -430,7 +416,7 @@ static int get_new_crc_ctl_reg(struct drm_i915_private 
*dev_priv,
        else if (IS_GEN_RANGE(dev_priv, 5, 6))
                return ilk_pipe_crc_ctl_reg(source, val);
        else if (INTEL_GEN(dev_priv) < 9)
-               return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val, 
set_wa);
+               return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
        else
                return skl_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
 }
@@ -605,6 +591,7 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const 
char *source_name)
        intel_wakeref_t wakeref;
        u32 val = 0; /* shut up gcc */
        int ret = 0;
+       bool enable;
 
        if (display_crc_ctl_parse_source(source_name, &source) < 0) {
                DRM_DEBUG_DRIVER("unknown source %s\n", source_name);
@@ -618,7 +605,11 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const 
char *source_name)
                return -EIO;
        }
 
-       ret = get_new_crc_ctl_reg(dev_priv, crtc->index, &source, &val, true);
+       enable = source != INTEL_PIPE_CRC_SOURCE_NONE;
+       if (enable)
+               intel_crtc_crc_setup_workarounds(to_intel_crtc(crtc), enable);
+
+       ret = get_new_crc_ctl_reg(dev_priv, crtc->index, &source, &val);
        if (ret != 0)
                goto out;
 
@@ -629,14 +620,14 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, 
const char *source_name)
        if (!source) {
                if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
                        vlv_undo_pipe_scramble_reset(dev_priv, crtc->index);
-               else if ((IS_HASWELL(dev_priv) ||
-                         IS_BROADWELL(dev_priv)) && crtc->index == PIPE_A)
-                       hsw_pipe_A_crc_wa(dev_priv, false);
        }
 
        pipe_crc->skipped = 0;
 
 out:
+       if (!enable)
+               intel_crtc_crc_setup_workarounds(to_intel_crtc(crtc), enable);
+
        intel_display_power_put(dev_priv, power_domain, wakeref);
 
        return ret;
@@ -652,7 +643,7 @@ void intel_crtc_enable_pipe_crc(struct intel_crtc 
*intel_crtc)
        if (!crtc->crc.opened)
                return;
 
-       if (get_new_crc_ctl_reg(dev_priv, crtc->index, &pipe_crc->source, &val, 
false) < 0)
+       if (get_new_crc_ctl_reg(dev_priv, crtc->index, &pipe_crc->source, &val) 
< 0)
                return;
 
        /* Don't need pipe_crc->lock here, IRQs are not generated. */
-- 
2.21.0

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

Reply via email to