v4:
  - Do the disable sequence as part of the sanitization step after
    hardware readout instead of initial modeset commit. (Jani)

Signed-off-by: Gustavo Sousa <gustavo.so...@intel.com>
---

Jani, I hope I have captured what you meant with doing handling the disabling
during sanitization here.

I sent this as a separate "squash" patch because I'm not sure if this is the
correct way of doing it.

One thing I don't like very much about this is that we are forcing pipe(s) to be
disabled for platforms that require a modeset for disabling the CMTG. The
previous solution caused a modeset during initial commit for this case, which
seems a bit better to me.

 drivers/gpu/drm/i915/display/intel_cmtg.c     | 165 +++++-------------
 drivers/gpu/drm/i915/display/intel_cmtg.h     |   7 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  11 --
 .../drm/i915/display/intel_modeset_setup.c    |  17 +-
 4 files changed, 62 insertions(+), 138 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c 
b/drivers/gpu/drm/i915/display/intel_cmtg.c
index 493bc5ad7c76..8a2e19a794c2 100644
--- a/drivers/gpu/drm/i915/display/intel_cmtg.c
+++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
@@ -161,45 +161,6 @@ void intel_cmtg_readout_hw_state(struct intel_display 
*display)
        intel_cmtg_dump_state(display, cmtg_state);
 }
 
-static struct intel_cmtg_state *
-intel_atomic_get_cmtg_state(struct intel_atomic_state *state)
-{
-       struct intel_display *display = to_intel_display(state);
-       struct intel_global_state *obj_state =
-               intel_atomic_get_global_obj_state(state, &display->cmtg.obj);
-
-       if (IS_ERR(obj_state))
-               return ERR_CAST(obj_state);
-
-       return to_intel_cmtg_state(obj_state);
-}
-
-static struct intel_cmtg_state *
-intel_atomic_get_old_cmtg_state(struct intel_atomic_state *state)
-{
-       struct intel_display *display = to_intel_display(state);
-       struct intel_global_state *obj_state =
-               intel_atomic_get_old_global_obj_state(state, 
&display->cmtg.obj);
-
-       if (!obj_state)
-               return NULL;
-
-       return to_intel_cmtg_state(obj_state);
-}
-
-static struct intel_cmtg_state *
-intel_atomic_get_new_cmtg_state(struct intel_atomic_state *state)
-{
-       struct intel_display *display = to_intel_display(state);
-       struct intel_global_state *obj_state =
-               intel_atomic_get_new_global_obj_state(state, 
&display->cmtg.obj);
-
-       if (!obj_state)
-               return NULL;
-
-       return to_intel_cmtg_state(obj_state);
-}
-
 static bool intel_cmtg_state_changed(struct intel_cmtg_state *old_cmtg_state,
                                     struct intel_cmtg_state *new_cmtg_state)
 {
@@ -212,89 +173,18 @@ static bool intel_cmtg_state_changed(struct 
intel_cmtg_state *old_cmtg_state,
                old_cmtg_state->trans_b_secondary != 
new_cmtg_state->trans_b_secondary;
 }
 
-static int intel_cmtg_check_modeset(struct intel_atomic_state *state,
-                                   struct intel_cmtg_state *old_cmtg_state,
-                                   struct intel_cmtg_state *new_cmtg_state)
-{
-       struct intel_display *display = to_intel_display(state);
-       u8 pipe_mask;
-
-       if (!intel_cmtg_requires_modeset(display))
-               return 0;
-
-       pipe_mask = 0;
-
-       if (old_cmtg_state->trans_a_secondary != 
new_cmtg_state->trans_a_secondary)
-               pipe_mask |= BIT(PIPE_A);
-
-       if (old_cmtg_state->trans_b_secondary != 
new_cmtg_state->trans_b_secondary)
-               pipe_mask |= BIT(PIPE_B);
-
-       if (!pipe_mask)
-               return 0;
-
-       return intel_modeset_pipes_in_mask_early(state, "updating CMTG config", 
pipe_mask);
-}
-
-int intel_cmtg_force_disabled(struct intel_atomic_state *state)
-{
-       struct intel_display *display = to_intel_display(state);
-       struct intel_cmtg_state *new_cmtg_state;
-
-       if (!HAS_CMTG(display))
-               return 0;
-
-       new_cmtg_state = intel_atomic_get_cmtg_state(state);
-       if (IS_ERR(new_cmtg_state))
-               return PTR_ERR(new_cmtg_state);
-
-       new_cmtg_state->cmtg_a_enable = false;
-       new_cmtg_state->cmtg_b_enable = false;
-       new_cmtg_state->trans_a_secondary = false;
-       new_cmtg_state->trans_b_secondary = false;
-
-       return 0;
-}
-
-int intel_cmtg_atomic_check(struct intel_atomic_state *state)
+static void intel_cmtg_state_set_disabled(struct intel_cmtg_state *cmtg_state)
 {
-       struct intel_display *display = to_intel_display(state);
-       struct intel_cmtg_state *old_cmtg_state;
-       struct intel_cmtg_state *new_cmtg_state;
-       int ret;
-
-       if (!HAS_CMTG(display))
-               return 0;
-
-       old_cmtg_state = intel_atomic_get_old_cmtg_state(state);
-       new_cmtg_state = intel_atomic_get_new_cmtg_state(state);
-       if (!intel_cmtg_state_changed(old_cmtg_state, new_cmtg_state))
-               return 0;
-
-       ret = intel_cmtg_check_modeset(state, old_cmtg_state, new_cmtg_state);
-       if (ret)
-               return ret;
-
-       return intel_atomic_serialize_global_state(&new_cmtg_state->base);
+       cmtg_state->cmtg_a_enable = false;
+       cmtg_state->cmtg_b_enable = false;
+       cmtg_state->trans_a_secondary = false;
+       cmtg_state->trans_b_secondary = false;
 }
 
-/*
- * Access to CMTG registers require the PHY PLL that provides its clock to be
- * running (which is configured via CMTG_CLK_SEL). As such, this function needs
- * to be called before intel_commit_modeset_disables() to ensure that the PHY
- * PLL is still enabled when doing this.
- */
-void intel_cmtg_disable(struct intel_atomic_state *state)
+static void intel_cmtg_disable(struct intel_display *display,
+                              struct intel_cmtg_state *old_cmtg_state,
+                              struct intel_cmtg_state *new_cmtg_state)
 {
-       struct intel_display *display = to_intel_display(state);
-       struct intel_cmtg_state *old_cmtg_state;
-       struct intel_cmtg_state *new_cmtg_state;
-
-       if (!HAS_CMTG(display))
-               return;
-
-       old_cmtg_state = intel_atomic_get_old_cmtg_state(state);
-       new_cmtg_state = intel_atomic_get_new_cmtg_state(state);
        if (!intel_cmtg_state_changed(old_cmtg_state, new_cmtg_state))
                return;
 
@@ -320,3 +210,42 @@ void intel_cmtg_disable(struct intel_atomic_state *state)
                intel_de_rmw(display, CMTG_CLK_SEL, clk_sel_clr, clk_sel_set);
        }
 }
+
+static u32 intel_cmtg_modeset_crtc_mask(struct intel_display *display,
+                                       struct intel_cmtg_state *old_cmtg_state,
+                                       struct intel_cmtg_state *new_cmtg_state)
+{
+       u32 crtc_mask;
+
+       if (intel_cmtg_requires_modeset(display))
+               return 0;
+
+       crtc_mask = 0;
+
+       if (old_cmtg_state->trans_a_secondary != 
new_cmtg_state->trans_a_secondary)
+               crtc_mask |= drm_crtc_mask(&intel_crtc_for_pipe(display, 
PIPE_A)->base);
+
+       if (old_cmtg_state->trans_b_secondary != 
new_cmtg_state->trans_b_secondary)
+               crtc_mask |= drm_crtc_mask(&intel_crtc_for_pipe(display, 
PIPE_B)->base);
+
+       return crtc_mask;
+}
+
+/*
+ * Disable CMTG if enabled and return a mask of pipes that need to be disabled
+ * (for platforms where disabling the CMTG requires a modeset).
+ */
+u32 intel_cmtg_sanitize_state(struct intel_display *display)
+{
+       struct intel_cmtg_state *cmtg_state = 
to_intel_cmtg_state(display->cmtg.obj.state);
+       struct intel_cmtg_state old_cmtg_state;
+
+       if (!HAS_CMTG(display))
+               return 0;
+
+       old_cmtg_state = *cmtg_state;
+       intel_cmtg_state_set_disabled(cmtg_state);
+       intel_cmtg_disable(display, &old_cmtg_state, cmtg_state);
+
+       return intel_cmtg_modeset_crtc_mask(display, &old_cmtg_state, 
cmtg_state);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.h 
b/drivers/gpu/drm/i915/display/intel_cmtg.h
index 64c42e345665..3c51e144aa3f 100644
--- a/drivers/gpu/drm/i915/display/intel_cmtg.h
+++ b/drivers/gpu/drm/i915/display/intel_cmtg.h
@@ -6,14 +6,13 @@
 #ifndef __INTEL_CMTG_H__
 #define __INTEL_CMTG_H__
 
-struct intel_atomic_state;
+#include <linux/types.h>
+
 struct intel_display;
 struct intel_global_state;
 
 int intel_cmtg_init(struct intel_display *display);
 void intel_cmtg_readout_hw_state(struct intel_display *display);
-int intel_cmtg_force_disabled(struct intel_atomic_state *state);
-int intel_cmtg_atomic_check(struct intel_atomic_state *state);
-void intel_cmtg_disable(struct intel_atomic_state *state);
+u32 intel_cmtg_sanitize_state(struct intel_display *display);
 
 #endif /* __INTEL_CMTG_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 098985ad7ad4..4271da219b41 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -62,7 +62,6 @@
 #include "intel_bw.h"
 #include "intel_cdclk.h"
 #include "intel_clock_gating.h"
-#include "intel_cmtg.h"
 #include "intel_color.h"
 #include "intel_crt.h"
 #include "intel_crtc.h"
@@ -6829,10 +6828,6 @@ int intel_atomic_check(struct drm_device *dev,
        if (ret)
                goto fail;
 
-       ret = intel_cmtg_atomic_check(state);
-       if (ret)
-               goto fail;
-
        for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
                if (!intel_crtc_needs_modeset(new_crtc_state))
                        continue;
@@ -7762,8 +7757,6 @@ static void intel_atomic_commit_tail(struct 
intel_atomic_state *state)
                        intel_modeset_get_crtc_power_domains(new_crtc_state, 
&put_domains[crtc->pipe]);
        }
 
-       intel_cmtg_disable(state);
-
        intel_commit_modeset_disables(state);
 
        intel_dp_tunnel_atomic_alloc_bw(state);
@@ -8589,10 +8582,6 @@ int intel_initial_commit(struct drm_device *dev)
                }
        }
 
-       ret = intel_cmtg_force_disabled(to_intel_atomic_state(state));
-       if (ret)
-               goto out;
-
        ret = drm_atomic_commit(state);
 
 out:
diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c 
b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index e0845ae21d82..c6eeb8a00a7b 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -474,10 +474,12 @@ static void intel_sanitize_fifo_underrun_reporting(const 
struct intel_crtc_state
 }
 
 static bool intel_sanitize_crtc(struct intel_crtc *crtc,
-                               struct drm_modeset_acquire_ctx *ctx)
+                               struct drm_modeset_acquire_ctx *ctx,
+                               u32 force_off_crtc_mask)
 {
        struct drm_i915_private *i915 = to_i915(crtc->base.dev);
        struct intel_crtc_state *crtc_state = 
to_intel_crtc_state(crtc->base.state);
+       u32 crtc_mask = drm_crtc_mask(&crtc->base);
        bool needs_link_reset;
 
        if (crtc_state->hw.active) {
@@ -508,7 +510,8 @@ static bool intel_sanitize_crtc(struct intel_crtc *crtc,
         * Adjust the state of the output pipe according to whether we have
         * active connectors/encoders.
         */
-       if (!needs_link_reset && intel_crtc_has_encoders(crtc))
+       if (!(crtc_mask & force_off_crtc_mask) &&
+           !needs_link_reset && intel_crtc_has_encoders(crtc))
                return false;
 
        intel_crtc_disable_noatomic(crtc, ctx);
@@ -526,7 +529,8 @@ static bool intel_sanitize_crtc(struct intel_crtc *crtc,
 }
 
 static void intel_sanitize_all_crtcs(struct drm_i915_private *i915,
-                                    struct drm_modeset_acquire_ctx *ctx)
+                                    struct drm_modeset_acquire_ctx *ctx,
+                                    u32 force_off_crtc_mask)
 {
        struct intel_crtc *crtc;
        u32 crtcs_forced_off = 0;
@@ -546,7 +550,7 @@ static void intel_sanitize_all_crtcs(struct 
drm_i915_private *i915,
                        if (crtcs_forced_off & crtc_mask)
                                continue;
 
-                       if (intel_sanitize_crtc(crtc, ctx))
+                       if (intel_sanitize_crtc(crtc, ctx, force_off_crtc_mask))
                                crtcs_forced_off |= crtc_mask;
                }
                if (crtcs_forced_off == old_mask)
@@ -967,6 +971,7 @@ void intel_modeset_setup_hw_state(struct drm_i915_private 
*i915,
        struct intel_encoder *encoder;
        struct intel_crtc *crtc;
        intel_wakeref_t wakeref;
+       u32 force_off_crtc_mask;
 
        wakeref = intel_display_power_get(i915, POWER_DOMAIN_INIT);
 
@@ -1009,7 +1014,9 @@ void intel_modeset_setup_hw_state(struct drm_i915_private 
*i915,
         */
        intel_modeset_update_connector_atomic_state(i915);
 
-       intel_sanitize_all_crtcs(i915, ctx);
+       force_off_crtc_mask = intel_cmtg_sanitize_state(display);
+
+       intel_sanitize_all_crtcs(i915, ctx, force_off_crtc_mask);
 
        intel_dpll_sanitize_state(i915);
 
-- 
2.47.1

Reply via email to