[Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes

All of lore.kernel.org
 help / color / mirror / Atom feed

* [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes
@ 2022-06-17 16:04 Ville Syrjala
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 01/16] drm/i915: Relocate intel_crtc_dotclock() Ville Syrjala
                   ` (17 more replies)
  0 siblings, 18 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:04 UTC (permalink / raw)
  To: intel-gfx

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

And with fastset made to not suck we can consider allowing
seameless M/N changes on eDP panels that support such things.
I've given that a quick test here on a TGL and it seemed to work
OK.

The rough parts:
- The DPLL stuff is kinda messy still, a lot of which is due to
  the dpll_mgr vs. not depending on platform/output type. Maybe
  it's finally time to start cleaning that mess up...
- the port_dpll[] stuff probably needs to be reworked at some
  point to make a bit more sense
- fastboot I *think* should mostly keep working now that we
  try to match the GOP/VBIOS M/N behaviour. FDI M/N vs. DPLL is
  a bit of a challenge for the platforms where the encoder live
  in the PCH, but I'm going to declare that as not so important
- DSI clock handling is snafu
- DP link computation policy might need a bit more work since we
  may now consume more bandwidth than before on machines where
  seamless M/N changes are possible

I also did a quick smoke test through the series on tgl and 
snb in the hopes of keeping this at least mostly bisectable.

Changes in v2:
- bunch of stuff already merged
- a bit more refactoring to make things nicer
- Tweak the M/N rounding for fastboot
- don't mess with the DP link rate on platforms (pre-BDW)
  where we haven't implemented seamsless M/N chages

Ville Syrjälä (16):
  drm/i915: Relocate intel_crtc_dotclock()
  drm/i915: Shuffle some PLL code around
  drm/i915: Extract has_double_buffered_m_n()
  drm/i915: Do .crtc_compute_clock() earlier
  drm/i915: Reassign DPLLs only for crtcs going throug .compute_config()
  drm/i915: Feed the DPLL output freq back into crtc_state
  drm/i915: Compute clocks earlier
  drm/i915: Make M/N checks non-fuzzy
  drm/i915: Make all clock checks non-fuzzy
  drm/i915: Set active dpll early for icl+
  drm/i915: Nuke fastet state copy hacks
  drm/i915: Skip intel_modeset_pipe_config_late() if the pipe is not
    enabled
  drm/i915: Add intel_panel_highest_mode()
  drm/i915: Allow M/N change during fastset on bdw+
  drm/i915: Use a fixed N value always
  drm/i915: Round TMDS clock to nearest

 drivers/gpu/drm/i915/display/intel_crt.c      |   3 +
 drivers/gpu/drm/i915/display/intel_ddi.c      |  22 --
 drivers/gpu/drm/i915/display/intel_display.c  | 204 +++++++-----------
 drivers/gpu/drm/i915/display/intel_display.h  |   3 +-
 .../drm/i915/display/intel_display_types.h    |   1 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  50 +++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |   3 +-
 drivers/gpu/drm/i915/display/intel_dpll.c     |  69 +++++-
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 204 ++++++++++--------
 drivers/gpu/drm/i915/display/intel_fdi.c      |   2 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c     |   2 +-
 .../drm/i915/display/intel_modeset_verify.c   |   6 +-
 drivers/gpu/drm/i915/display/intel_panel.c    |  15 ++
 drivers/gpu/drm/i915/display/intel_panel.h    |   3 +
 .../gpu/drm/i915/display/intel_pch_refclk.c   |  10 +
 .../gpu/drm/i915/display/intel_pch_refclk.h   |   1 +
 16 files changed, 333 insertions(+), 265 deletions(-)

-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 01/16] drm/i915: Relocate intel_crtc_dotclock()
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
@ 2022-06-17 16:04 ` Ville Syrjala
  2022-06-20  9:01   ` Jani Nikula
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 02/16] drm/i915: Shuffle some PLL code around Ville Syrjala
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:04 UTC (permalink / raw)
  To: intel-gfx

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

intel_crtc_dotclock() is a bit misplaced. In lieu of a better
place let's just move it next to its friends in intel_display.c.

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

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 272e1bf6006b..51bf26dcb209 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -323,28 +323,6 @@ static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv,
 	}
 }
 
-int intel_crtc_dotclock(const struct intel_crtc_state *pipe_config)
-{
-	int dotclock;
-
-	if (intel_crtc_has_dp_encoder(pipe_config))
-		dotclock = intel_dotclock_calculate(pipe_config->port_clock,
-						    &pipe_config->dp_m_n);
-	else if (pipe_config->has_hdmi_sink && pipe_config->pipe_bpp > 24)
-		dotclock = pipe_config->port_clock * 24 / pipe_config->pipe_bpp;
-	else
-		dotclock = pipe_config->port_clock;
-
-	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
-	    !intel_crtc_has_dp_encoder(pipe_config))
-		dotclock *= 2;
-
-	if (pipe_config->pixel_multiplier)
-		dotclock /= pipe_config->pixel_multiplier;
-
-	return dotclock;
-}
-
 static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
 {
 	/* CRT dotclock is determined via other means */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 90bd26431e31..b24784c4522d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4569,6 +4569,28 @@ int intel_dotclock_calculate(int link_freq,
 	return div_u64(mul_u32_u32(m_n->link_m, link_freq), m_n->link_n);
 }
 
+int intel_crtc_dotclock(const struct intel_crtc_state *pipe_config)
+{
+	int dotclock;
+
+	if (intel_crtc_has_dp_encoder(pipe_config))
+		dotclock = intel_dotclock_calculate(pipe_config->port_clock,
+						    &pipe_config->dp_m_n);
+	else if (pipe_config->has_hdmi_sink && pipe_config->pipe_bpp > 24)
+		dotclock = pipe_config->port_clock * 24 / pipe_config->pipe_bpp;
+	else
+		dotclock = pipe_config->port_clock;
+
+	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
+	    !intel_crtc_has_dp_encoder(pipe_config))
+		dotclock *= 2;
+
+	if (pipe_config->pixel_multiplier)
+		dotclock /= pipe_config->pixel_multiplier;
+
+	return dotclock;
+}
+
 /* Returns the currently programmed mode of the given encoder. */
 struct drm_display_mode *
 intel_encoder_current_mode(struct intel_encoder *encoder)
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 02/16] drm/i915: Shuffle some PLL code around
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 01/16] drm/i915: Relocate intel_crtc_dotclock() Ville Syrjala
@ 2022-06-17 16:04 ` Ville Syrjala
  2022-06-20  9:01   ` Jani Nikula
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 03/16] drm/i915: Extract has_double_buffered_m_n() Ville Syrjala
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:04 UTC (permalink / raw)
  To: intel-gfx

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

Shuffle some PLL functions around a bit to avoid ugle
forward declarations later on. No functional changes.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 176 +++++++++---------
 1 file changed, 88 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index ddae7e42ac46..bfccc96f16fe 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -905,37 +905,6 @@ hsw_ddi_calculate_wrpll(int clock /* in Hz */,
 	*r2_out = best.r2;
 }
 
-static int
-hsw_ddi_wrpll_compute_dpll(struct intel_atomic_state *state,
-			   struct intel_crtc *crtc)
-{
-	struct intel_crtc_state *crtc_state =
-		intel_atomic_get_new_crtc_state(state, crtc);
-	unsigned int p, n2, r2;
-
-	hsw_ddi_calculate_wrpll(crtc_state->port_clock * 1000, &r2, &n2, &p);
-
-	crtc_state->dpll_hw_state.wrpll =
-		WRPLL_PLL_ENABLE | WRPLL_REF_LCPLL |
-		WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
-		WRPLL_DIVIDER_POST(p);
-
-	return 0;
-}
-
-static struct intel_shared_dpll *
-hsw_ddi_wrpll_get_dpll(struct intel_atomic_state *state,
-		       struct intel_crtc *crtc)
-{
-	struct intel_crtc_state *crtc_state =
-		intel_atomic_get_new_crtc_state(state, crtc);
-
-	return intel_find_shared_dpll(state, crtc,
-				      &crtc_state->dpll_hw_state,
-				      BIT(DPLL_ID_WRPLL2) |
-				      BIT(DPLL_ID_WRPLL1));
-}
-
 static int hsw_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
 				  const struct intel_shared_dpll *pll,
 				  const struct intel_dpll_hw_state *pll_state)
@@ -976,6 +945,37 @@ static int hsw_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
 	return (refclk * n / 10) / (p * r) * 2;
 }
 
+static int
+hsw_ddi_wrpll_compute_dpll(struct intel_atomic_state *state,
+			   struct intel_crtc *crtc)
+{
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+	unsigned int p, n2, r2;
+
+	hsw_ddi_calculate_wrpll(crtc_state->port_clock * 1000, &r2, &n2, &p);
+
+	crtc_state->dpll_hw_state.wrpll =
+		WRPLL_PLL_ENABLE | WRPLL_REF_LCPLL |
+		WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
+		WRPLL_DIVIDER_POST(p);
+
+	return 0;
+}
+
+static struct intel_shared_dpll *
+hsw_ddi_wrpll_get_dpll(struct intel_atomic_state *state,
+		       struct intel_crtc *crtc)
+{
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+
+	return intel_find_shared_dpll(state, crtc,
+				      &crtc_state->dpll_hw_state,
+				      BIT(DPLL_ID_WRPLL2) |
+				      BIT(DPLL_ID_WRPLL1));
+}
+
 static int
 hsw_ddi_lcpll_compute_dpll(struct intel_crtc_state *crtc_state)
 {
@@ -1618,43 +1618,6 @@ skl_ddi_calculate_wrpll(int clock /* in Hz */,
 	return 0;
 }
 
-static int skl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
-{
-	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
-	struct skl_wrpll_params wrpll_params = {};
-	u32 ctrl1, cfgcr1, cfgcr2;
-	int ret;
-
-	/*
-	 * See comment in intel_dpll_hw_state to understand why we always use 0
-	 * as the DPLL id in this function.
-	 */
-	ctrl1 = DPLL_CTRL1_OVERRIDE(0);
-
-	ctrl1 |= DPLL_CTRL1_HDMI_MODE(0);
-
-	ret = skl_ddi_calculate_wrpll(crtc_state->port_clock * 1000,
-				      i915->dpll.ref_clks.nssc, &wrpll_params);
-	if (ret)
-		return ret;
-
-	cfgcr1 = DPLL_CFGCR1_FREQ_ENABLE |
-		DPLL_CFGCR1_DCO_FRACTION(wrpll_params.dco_fraction) |
-		wrpll_params.dco_integer;
-
-	cfgcr2 = DPLL_CFGCR2_QDIV_RATIO(wrpll_params.qdiv_ratio) |
-		DPLL_CFGCR2_QDIV_MODE(wrpll_params.qdiv_mode) |
-		DPLL_CFGCR2_KDIV(wrpll_params.kdiv) |
-		DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
-		wrpll_params.central_freq;
-
-	crtc_state->dpll_hw_state.ctrl1 = ctrl1;
-	crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
-	crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
-
-	return 0;
-}
-
 static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
 				  const struct intel_shared_dpll *pll,
 				  const struct intel_dpll_hw_state *pll_state)
@@ -1726,6 +1689,43 @@ static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
 	return dco_freq / (p0 * p1 * p2 * 5);
 }
 
+static int skl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+	struct skl_wrpll_params wrpll_params = {};
+	u32 ctrl1, cfgcr1, cfgcr2;
+	int ret;
+
+	/*
+	 * See comment in intel_dpll_hw_state to understand why we always use 0
+	 * as the DPLL id in this function.
+	 */
+	ctrl1 = DPLL_CTRL1_OVERRIDE(0);
+
+	ctrl1 |= DPLL_CTRL1_HDMI_MODE(0);
+
+	ret = skl_ddi_calculate_wrpll(crtc_state->port_clock * 1000,
+				      i915->dpll.ref_clks.nssc, &wrpll_params);
+	if (ret)
+		return ret;
+
+	cfgcr1 = DPLL_CFGCR1_FREQ_ENABLE |
+		DPLL_CFGCR1_DCO_FRACTION(wrpll_params.dco_fraction) |
+		wrpll_params.dco_integer;
+
+	cfgcr2 = DPLL_CFGCR2_QDIV_RATIO(wrpll_params.qdiv_ratio) |
+		DPLL_CFGCR2_QDIV_MODE(wrpll_params.qdiv_mode) |
+		DPLL_CFGCR2_KDIV(wrpll_params.kdiv) |
+		DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
+		wrpll_params.central_freq;
+
+	crtc_state->dpll_hw_state.ctrl1 = ctrl1;
+	crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
+	crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
+
+	return 0;
+}
+
 static int
 skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 {
@@ -2245,26 +2245,6 @@ static int bxt_ddi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
 	return 0;
 }
 
-static int
-bxt_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
-{
-	struct dpll clk_div = {};
-
-	bxt_ddi_dp_pll_dividers(crtc_state, &clk_div);
-
-	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
-}
-
-static int
-bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
-{
-	struct dpll clk_div = {};
-
-	bxt_ddi_hdmi_pll_dividers(crtc_state, &clk_div);
-
-	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
-}
-
 static int bxt_ddi_pll_get_freq(struct drm_i915_private *i915,
 				const struct intel_shared_dpll *pll,
 				const struct intel_dpll_hw_state *pll_state)
@@ -2282,6 +2262,26 @@ static int bxt_ddi_pll_get_freq(struct drm_i915_private *i915,
 	return chv_calc_dpll_params(i915->dpll.ref_clks.nssc, &clock);
 }
 
+static int
+bxt_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
+{
+	struct dpll clk_div = {};
+
+	bxt_ddi_dp_pll_dividers(crtc_state, &clk_div);
+
+	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
+}
+
+static int
+bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
+{
+	struct dpll clk_div = {};
+
+	bxt_ddi_hdmi_pll_dividers(crtc_state, &clk_div);
+
+	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
+}
+
 static int bxt_compute_dpll(struct intel_atomic_state *state,
 			    struct intel_crtc *crtc,
 			    struct intel_encoder *encoder)
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 03/16] drm/i915: Extract has_double_buffered_m_n()
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 01/16] drm/i915: Relocate intel_crtc_dotclock() Ville Syrjala
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 02/16] drm/i915: Shuffle some PLL code around Ville Syrjala
@ 2022-06-17 16:04 ` Ville Syrjala
  2022-06-20  9:05   ` Jani Nikula
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 04/16] drm/i915: Do .crtc_compute_clock() earlier Ville Syrjala
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:04 UTC (permalink / raw)
  To: intel-gfx

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

We have a couple of places that want to make distinction between
double buffered M/N registers vs. the split M1/N1+M2/N2 registers.
Add a helper for that.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++-
 drivers/gpu/drm/i915/display/intel_display.h | 1 +
 drivers/gpu/drm/i915/display/intel_dp.c      | 3 +--
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b24784c4522d..5559688047b3 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2798,6 +2798,11 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
 	return 0;
 }
 
+bool has_double_buffered_m_n(struct drm_i915_private *i915)
+{
+	return DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915);
+}
+
 static void
 intel_reduce_m_n_ratio(u32 *num, u32 *den)
 {
@@ -5900,7 +5905,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	PIPE_CONF_CHECK_I(lane_count);
 	PIPE_CONF_CHECK_X(lane_lat_optim_mask);
 
-	if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) {
+	if (has_double_buffered_m_n(dev_priv)) {
 		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
 	} else {
 		PIPE_CONF_CHECK_M_N(dp_m_n);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 2feb8ae5d5d4..44c88aadfc30 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -543,6 +543,7 @@ int intel_atomic_add_affected_planes(struct intel_atomic_state *state,
 				     struct intel_crtc *crtc);
 u8 intel_calc_active_pipes(struct intel_atomic_state *state,
 			   u8 active_pipes);
+bool has_double_buffered_m_n(struct drm_i915_private *i915);
 void intel_link_compute_m_n(u16 bpp, int nlanes,
 			    int pixel_clock, int link_clock,
 			    struct intel_link_m_n *m_n,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 2fac76bcf06d..75645508080a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1842,8 +1842,7 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
 static bool cpu_transcoder_has_drrs(struct drm_i915_private *i915,
 				    enum transcoder cpu_transcoder)
 {
-	/* M1/N1 is double buffered */
-	if (DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915))
+	if (has_double_buffered_m_n(i915))
 		return true;
 
 	return intel_cpu_transcoder_has_m2_n2(i915, cpu_transcoder);
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 04/16] drm/i915: Do .crtc_compute_clock() earlier
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (2 preceding siblings ...)
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 03/16] drm/i915: Extract has_double_buffered_m_n() Ville Syrjala
@ 2022-06-17 16:04 ` Ville Syrjala
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 05/16] drm/i915: Reassign DPLLs only for crtcs going throug .compute_config() Ville Syrjala
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:04 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Currently we calculate a lot of things (pixel rate, watermarks,
cdclk) trusting that the DPLL can generate the exact frequency
we ask it. In practice that is not true and there can be
certain amount of rounding involved.

To allow us to eventually get accurate numbers for all our
DPLL clock derived state we need to move the DPLL calculation
to hapen much earlier. To that end we hoist it up to the just
after the fastset checks. For now we just do the easy code
motion, and the actual back feeding of the final DPLL clock
into the state will come later.

A slight change here is that now .crtc_compute_clock()
can get called while the shared_dpll is still assigned.
But since .crtc_compute_clock() no longer assignes new
shared_dplls this is perfectly fine.

TODO: I'd actually like to do this before the fastset check
so that if the DPLL state should change we actually do the
modeset. Which I think is what the video aficionados want,
but it might not be what the fans of fastboot want. Not yet
sure how to reconcile those conflicting requirements...

v2: s/return/goto/ in error handling

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_display.c | 9 +++++----
 drivers/gpu/drm/i915/display/intel_dpll.c    | 3 ---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5559688047b3..b8c0ede1f7fd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4900,10 +4900,6 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
 		crtc_state->update_wm_post = true;
 
 	if (mode_changed) {
-		ret = intel_dpll_crtc_compute_clock(state, crtc);
-		if (ret)
-			return ret;
-
 		ret = intel_dpll_crtc_get_shared_dpll(state, crtc);
 		if (ret)
 			return ret;
@@ -7047,6 +7043,11 @@ static int intel_atomic_check(struct drm_device *dev,
 					    new_crtc_state, i) {
 		if (intel_crtc_needs_modeset(new_crtc_state)) {
 			any_ms = true;
+
+			ret = intel_dpll_crtc_compute_clock(state, crtc);
+			if (ret)
+				goto fail;
+
 			continue;
 		}
 
diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c
index 5262f16b45ac..8d095f28fa20 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll.c
@@ -1411,9 +1411,6 @@ int intel_dpll_crtc_compute_clock(struct intel_atomic_state *state,
 
 	drm_WARN_ON(&i915->drm, !intel_crtc_needs_modeset(crtc_state));
 
-	if (drm_WARN_ON(&i915->drm, crtc_state->shared_dpll))
-		return 0;
-
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 05/16] drm/i915: Reassign DPLLs only for crtcs going throug .compute_config()
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (3 preceding siblings ...)
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 04/16] drm/i915: Do .crtc_compute_clock() earlier Ville Syrjala
@ 2022-06-17 16:04 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 06/16] drm/i915: Feed the DPLL output freq back into crtc_state Ville Syrjala
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:04 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Only reassign the pipe's DPLL if it's going through a full
.compute_config() cycle. If OTOH it's just getting modeset
eg. in order to change cdclk there doesn't seem much point in
picking a new DPLL for it.

This should also prevent .get_dplls() from seeing a funky port_clock
for DP even in cases where the readout produces a non-standard
clock and we (for some reason) have decided to not fully recompute
the state to remedy the situation.

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_display.c | 17 +----------------
 drivers/gpu/drm/i915/display/intel_dpll.c    |  6 ++----
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b8c0ede1f7fd..59dd66642c5f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6201,20 +6201,6 @@ intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state)
 	}
 }
 
-static void intel_modeset_clear_plls(struct intel_atomic_state *state)
-{
-	struct intel_crtc_state *new_crtc_state;
-	struct intel_crtc *crtc;
-	int i;
-
-	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
-		if (!intel_crtc_needs_modeset(new_crtc_state))
-			continue;
-
-		intel_release_shared_dplls(state, crtc);
-	}
-}
-
 /*
  * This implements the workaround described in the "notes" section of the mode
  * set sequence documentation. When going from no pipes or single pipe to
@@ -7048,6 +7034,7 @@ static int intel_atomic_check(struct drm_device *dev,
 			if (ret)
 				goto fail;
 
+			intel_release_shared_dplls(state, crtc);
 			continue;
 		}
 
@@ -7095,8 +7082,6 @@ static int intel_atomic_check(struct drm_device *dev,
 		ret = intel_modeset_calc_cdclk(state);
 		if (ret)
 			return ret;
-
-		intel_modeset_clear_plls(state);
 	}
 
 	ret = intel_atomic_check_crtcs(state);
diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c
index 8d095f28fa20..69dc018385db 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll.c
@@ -1436,11 +1436,9 @@ int intel_dpll_crtc_get_shared_dpll(struct intel_atomic_state *state,
 	int ret;
 
 	drm_WARN_ON(&i915->drm, !intel_crtc_needs_modeset(crtc_state));
+	drm_WARN_ON(&i915->drm, !crtc_state->hw.enable && crtc_state->shared_dpll);
 
-	if (drm_WARN_ON(&i915->drm, crtc_state->shared_dpll))
-		return 0;
-
-	if (!crtc_state->hw.enable)
+	if (!crtc_state->hw.enable || crtc_state->shared_dpll)
 		return 0;
 
 	if (!i915->dpll_funcs->crtc_get_shared_dpll)
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 06/16] drm/i915: Feed the DPLL output freq back into crtc_state
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (4 preceding siblings ...)
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 05/16] drm/i915: Reassign DPLLs only for crtcs going throug .compute_config() Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 07/16] drm/i915: Compute clocks earlier Ville Syrjala
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Fill port_clock and hw.adjusted_mode.crtc_clock with the actual
frequency we're going to be getting from the hardware. This will
let us accurately compute all derived state that depends on those.

v2: Reintroduce iCLKIP WARN

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_crt.c      |  3 +
 drivers/gpu/drm/i915/display/intel_dpll.c     | 60 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 24 +++++++-
 .../gpu/drm/i915/display/intel_pch_refclk.c   | 10 ++++
 .../gpu/drm/i915/display/intel_pch_refclk.h   |  1 +
 5 files changed, 94 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
index 6a3893c8ff22..a225af030ad7 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -46,6 +46,7 @@
 #include "intel_gmbus.h"
 #include "intel_hotplug.h"
 #include "intel_pch_display.h"
+#include "intel_pch_refclk.h"
 
 /* Here's the desired hotplug mode */
 #define ADPA_HOTPLUG_BITS (ADPA_CRT_HOTPLUG_PERIOD_128 |		\
@@ -444,6 +445,8 @@ static int hsw_crt_compute_config(struct intel_encoder *encoder,
 	/* FDI must always be 2.7 GHz */
 	pipe_config->port_clock = 135000 * 2;
 
+	adjusted_mode->crtc_clock = lpt_iclkip(pipe_config);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c
index 69dc018385db..cffce8b86d64 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll.c
@@ -938,12 +938,25 @@ static int hsw_crtc_compute_clock(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_encoder *encoder =
 		intel_get_crtc_new_encoder(state, crtc_state);
+	int ret;
 
 	if (DISPLAY_VER(dev_priv) < 11 &&
 	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
 		return 0;
 
-	return intel_compute_shared_dplls(state, crtc, encoder);
+	ret = intel_compute_shared_dplls(state, crtc, encoder);
+	if (ret)
+		return ret;
+
+	/* FIXME this is a mess */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
+		return 0;
+
+	/* CRT dotclock is determined via other means */
+	if (!crtc_state->has_pch_encoder)
+		crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
+	return 0;
 }
 
 static int hsw_crtc_get_shared_dpll(struct intel_atomic_state *state,
@@ -969,8 +982,15 @@ static int dg2_crtc_compute_clock(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_encoder *encoder =
 		intel_get_crtc_new_encoder(state, crtc_state);
+	int ret;
 
-	return intel_mpllb_calc_state(crtc_state, encoder);
+	ret = intel_mpllb_calc_state(crtc_state, encoder);
+	if (ret)
+		return ret;
+
+	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
+	return 0;
 }
 
 static bool ilk_needs_fb_cb_tune(const struct dpll *dpll, int factor)
@@ -1096,6 +1116,7 @@ static int ilk_crtc_compute_clock(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	const struct intel_limit *limit;
 	int refclk = 120000;
+	int ret;
 
 	/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
 	if (!crtc_state->has_pch_encoder)
@@ -1132,7 +1153,14 @@ static int ilk_crtc_compute_clock(struct intel_atomic_state *state,
 	ilk_compute_dpll(crtc_state, &crtc_state->dpll,
 			 &crtc_state->dpll);
 
-	return intel_compute_shared_dplls(state, crtc, NULL);
+	ret = intel_compute_shared_dplls(state, crtc, NULL);
+	if (ret)
+		return ret;
+
+	crtc_state->port_clock = crtc_state->dpll.dot;
+	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
+	return ret;
 }
 
 static int ilk_crtc_get_shared_dpll(struct intel_atomic_state *state,
@@ -1198,6 +1226,13 @@ static int chv_crtc_compute_clock(struct intel_atomic_state *state,
 
 	chv_compute_dpll(crtc_state);
 
+	/* FIXME this is a mess */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
+		return 0;
+
+	crtc_state->port_clock = crtc_state->dpll.dot;
+	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
 	return 0;
 }
 
@@ -1217,6 +1252,13 @@ static int vlv_crtc_compute_clock(struct intel_atomic_state *state,
 
 	vlv_compute_dpll(crtc_state);
 
+	/* FIXME this is a mess */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
+		return 0;
+
+	crtc_state->port_clock = crtc_state->dpll.dot;
+	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
 	return 0;
 }
 
@@ -1259,6 +1301,9 @@ static int g4x_crtc_compute_clock(struct intel_atomic_state *state,
 	i9xx_compute_dpll(crtc_state, &crtc_state->dpll,
 			  &crtc_state->dpll);
 
+	crtc_state->port_clock = crtc_state->dpll.dot;
+	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
 	return 0;
 }
 
@@ -1292,6 +1337,9 @@ static int pnv_crtc_compute_clock(struct intel_atomic_state *state,
 	i9xx_compute_dpll(crtc_state, &crtc_state->dpll,
 			  &crtc_state->dpll);
 
+	crtc_state->port_clock = crtc_state->dpll.dot;
+	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
 	return 0;
 }
 
@@ -1325,6 +1373,9 @@ static int i9xx_crtc_compute_clock(struct intel_atomic_state *state,
 	i9xx_compute_dpll(crtc_state, &crtc_state->dpll,
 			  &crtc_state->dpll);
 
+	crtc_state->port_clock = crtc_state->dpll.dot;
+	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
 	return 0;
 }
 
@@ -1360,6 +1411,9 @@ static int i8xx_crtc_compute_clock(struct intel_atomic_state *state,
 	i8xx_compute_dpll(crtc_state, &crtc_state->dpll,
 			  &crtc_state->dpll);
 
+	crtc_state->port_clock = crtc_state->dpll.dot;
+	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index bfccc96f16fe..09816526c5e4 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -949,6 +949,7 @@ static int
 hsw_ddi_wrpll_compute_dpll(struct intel_atomic_state *state,
 			   struct intel_crtc *crtc)
 {
+	struct drm_i915_private *i915 = to_i915(state->base.dev);
 	struct intel_crtc_state *crtc_state =
 		intel_atomic_get_new_crtc_state(state, crtc);
 	unsigned int p, n2, r2;
@@ -960,6 +961,9 @@ hsw_ddi_wrpll_compute_dpll(struct intel_atomic_state *state,
 		WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
 		WRPLL_DIVIDER_POST(p);
 
+	crtc_state->port_clock = hsw_ddi_wrpll_get_freq(i915, NULL,
+							&crtc_state->dpll_hw_state);
+
 	return 0;
 }
 
@@ -1723,6 +1727,9 @@ static int skl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
 	crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
 	crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
 
+	crtc_state->port_clock = skl_ddi_wrpll_get_freq(i915, NULL,
+							&crtc_state->dpll_hw_state);
+
 	return 0;
 }
 
@@ -2275,11 +2282,20 @@ bxt_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 static int
 bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 {
+	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
 	struct dpll clk_div = {};
+	int ret;
 
 	bxt_ddi_hdmi_pll_dividers(crtc_state, &clk_div);
 
-	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
+	ret = bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
+	if (ret)
+		return ret;
+
+	crtc_state->port_clock = bxt_ddi_pll_get_freq(i915, NULL,
+						      &crtc_state->dpll_hw_state);
+
+	return 0;
 }
 
 static int bxt_compute_dpll(struct intel_atomic_state *state,
@@ -3197,6 +3213,9 @@ static int icl_compute_combo_phy_dpll(struct intel_atomic_state *state,
 
 	icl_calc_dpll_state(dev_priv, &pll_params, &port_dpll->hw_state);
 
+	crtc_state->port_clock = icl_ddi_combo_pll_get_freq(dev_priv, NULL,
+							    &port_dpll->hw_state);
+
 	return 0;
 }
 
@@ -3282,6 +3301,9 @@ static int icl_compute_tc_phy_dplls(struct intel_atomic_state *state,
 	if (ret)
 		return ret;
 
+	crtc_state->port_clock = icl_ddi_mg_pll_get_freq(dev_priv, NULL,
+							 &port_dpll->hw_state);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_pch_refclk.c b/drivers/gpu/drm/i915/display/intel_pch_refclk.c
index 9934c8a9e240..55dfd37cb04e 100644
--- a/drivers/gpu/drm/i915/display/intel_pch_refclk.c
+++ b/drivers/gpu/drm/i915/display/intel_pch_refclk.c
@@ -167,6 +167,15 @@ static void lpt_compute_iclkip(struct iclkip_params *p, int clock)
 	}
 }
 
+int lpt_iclkip(const struct intel_crtc_state *crtc_state)
+{
+	struct iclkip_params p;
+
+	lpt_compute_iclkip(&p, crtc_state->hw.adjusted_mode.crtc_clock);
+
+	return lpt_iclkip_freq(&p);
+}
+
 /* Program iCLKIP clock to the desired frequency */
 void lpt_program_iclkip(const struct intel_crtc_state *crtc_state)
 {
@@ -179,6 +188,7 @@ void lpt_program_iclkip(const struct intel_crtc_state *crtc_state)
 	lpt_disable_iclkip(dev_priv);
 
 	lpt_compute_iclkip(&p, clock);
+	drm_WARN_ON(&dev_priv->drm, lpt_iclkip_freq(&p) != clock);
 
 	/* This should not happen with any sane values */
 	drm_WARN_ON(&dev_priv->drm, SBI_SSCDIVINTPHASE_DIVSEL(p.divsel) &
diff --git a/drivers/gpu/drm/i915/display/intel_pch_refclk.h b/drivers/gpu/drm/i915/display/intel_pch_refclk.h
index 12ab2c75a800..9bcf56629f24 100644
--- a/drivers/gpu/drm/i915/display/intel_pch_refclk.h
+++ b/drivers/gpu/drm/i915/display/intel_pch_refclk.h
@@ -14,6 +14,7 @@ struct intel_crtc_state;
 void lpt_program_iclkip(const struct intel_crtc_state *crtc_state);
 void lpt_disable_iclkip(struct drm_i915_private *dev_priv);
 int lpt_get_iclkip(struct drm_i915_private *dev_priv);
+int lpt_iclkip(const struct intel_crtc_state *crtc_state);
 
 void intel_init_pch_refclk(struct drm_i915_private *dev_priv);
 void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv);
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 07/16] drm/i915: Compute clocks earlier
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (5 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 06/16] drm/i915: Feed the DPLL output freq back into crtc_state Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 08/16] drm/i915: Make M/N checks non-fuzzy Ville Syrjala
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Do the DPLL computation before fastset checks. This should
allow us to get rid of all that horrible fuzzy clock handling
for fastsets. Who knows how many bugs there are caused by our
state not actually matching what the hardware will generate.

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_display.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 59dd66642c5f..ef7454c5b947 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2782,6 +2782,10 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	int ret;
 
+	ret = intel_dpll_crtc_compute_clock(state, crtc);
+	if (ret)
+		return ret;
+
 	ret = intel_crtc_compute_pipe_src(crtc_state);
 	if (ret)
 		return ret;
@@ -7030,10 +7034,6 @@ static int intel_atomic_check(struct drm_device *dev,
 		if (intel_crtc_needs_modeset(new_crtc_state)) {
 			any_ms = true;
 
-			ret = intel_dpll_crtc_compute_clock(state, crtc);
-			if (ret)
-				goto fail;
-
 			intel_release_shared_dplls(state, crtc);
 			continue;
 		}
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 08/16] drm/i915: Make M/N checks non-fuzzy
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (6 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 07/16] drm/i915: Compute clocks earlier Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 09/16] drm/i915: Make all clock " Ville Syrjala
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Now that we no longer fuzz M/N during fastset these should
match exctly.

In order to get a match with what the BIOS does we need to round
M/N down. And we do the opposite rounding when doing the readback.
That gets us pretty much the same thing back.

There can still be slight rounding differences between FDI M/N
vs. the DPLL output so we allow for tiny deviation in
intel_pipe_config_sanity_check().

v2: Tweak rounding/sanity check stuff a bit

Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  | 52 ++++---------------
 .../drm/i915/display/intel_modeset_verify.c   |  6 +--
 2 files changed, 13 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ef7454c5b947..81ed371e8700 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4575,7 +4575,8 @@ int intel_dotclock_calculate(int link_freq,
 	if (!m_n->link_n)
 		return 0;
 
-	return div_u64(mul_u32_u32(m_n->link_m, link_freq), m_n->link_n);
+	return DIV_ROUND_UP_ULL(mul_u32_u32(m_n->link_m, link_freq),
+				m_n->link_n);
 }
 
 int intel_crtc_dotclock(const struct intel_crtc_state *pipe_config)
@@ -5521,47 +5522,15 @@ bool intel_fuzzy_clock_check(int clock1, int clock2)
 	return false;
 }
 
-static bool
-intel_compare_m_n(unsigned int m, unsigned int n,
-		  unsigned int m2, unsigned int n2,
-		  bool exact)
-{
-	if (m == m2 && n == n2)
-		return true;
-
-	if (exact || !m || !n || !m2 || !n2)
-		return false;
-
-	BUILD_BUG_ON(DATA_LINK_M_N_MASK > INT_MAX);
-
-	if (n > n2) {
-		while (n > n2) {
-			m2 <<= 1;
-			n2 <<= 1;
-		}
-	} else if (n < n2) {
-		while (n < n2) {
-			m <<= 1;
-			n <<= 1;
-		}
-	}
-
-	if (n != n2)
-		return false;
-
-	return intel_fuzzy_clock_check(m, m2);
-}
-
 static bool
 intel_compare_link_m_n(const struct intel_link_m_n *m_n,
-		       const struct intel_link_m_n *m2_n2,
-		       bool exact)
+		       const struct intel_link_m_n *m2_n2)
 {
 	return m_n->tu == m2_n2->tu &&
-		intel_compare_m_n(m_n->data_m, m_n->data_n,
-				  m2_n2->data_m, m2_n2->data_n, exact) &&
-		intel_compare_m_n(m_n->link_m, m_n->link_n,
-				  m2_n2->link_m, m2_n2->link_n, exact);
+		m_n->data_m == m2_n2->data_m &&
+		m_n->data_n == m2_n2->data_n &&
+		m_n->link_m == m2_n2->link_m &&
+		m_n->link_n == m2_n2->link_n;
 }
 
 static bool
@@ -5755,8 +5724,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 #define PIPE_CONF_CHECK_M_N(name) do { \
 	if (!intel_compare_link_m_n(&current_config->name, \
-				    &pipe_config->name,\
-				    !fastset)) { \
+				    &pipe_config->name)) { \
 		pipe_config_mismatch(fastset, crtc, __stringify(name), \
 				     "(expected tu %i data %i/%i link %i/%i, " \
 				     "found tu %i, data %i/%i link %i/%i)", \
@@ -5803,9 +5771,9 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
  */
 #define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
 	if (!intel_compare_link_m_n(&current_config->name, \
-				    &pipe_config->name, !fastset) && \
+				    &pipe_config->name) && \
 	    !intel_compare_link_m_n(&current_config->alt_name, \
-				    &pipe_config->name, !fastset)) { \
+				    &pipe_config->name)) { \
 		pipe_config_mismatch(fastset, crtc, __stringify(name), \
 				     "(expected tu %i data %i/%i link %i/%i, " \
 				     "or tu %i data %i/%i link %i/%i, " \
diff --git a/drivers/gpu/drm/i915/display/intel_modeset_verify.c b/drivers/gpu/drm/i915/display/intel_modeset_verify.c
index a91586d77cb6..073607162acc 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_verify.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_verify.c
@@ -94,10 +94,10 @@ static void intel_pipe_config_sanity_check(struct drm_i915_private *dev_priv,
 
 		/*
 		 * FDI already provided one idea for the dotclock.
-		 * Yell if the encoder disagrees.
+		 * Yell if the encoder disagrees. Allow for slight
+		 * rounding differences.
 		 */
-		drm_WARN(&dev_priv->drm,
-			 !intel_fuzzy_clock_check(fdi_dotclock, dotclock),
+		drm_WARN(&dev_priv->drm, abs(fdi_dotclock - dotclock) > 1,
 			 "FDI dotclock and encoder dotclock mismatch, fdi: %i, encoder: %i\n",
 			 fdi_dotclock, dotclock);
 	}
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 09/16] drm/i915: Make all clock checks non-fuzzy
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (7 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 08/16] drm/i915: Make M/N checks non-fuzzy Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 10/16] drm/i915: Set active dpll early for icl+ Ville Syrjala
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Now that we backfeed the actual DPLL frequency into the
compute crtc state all our clocks should come out exact.

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_display.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 81ed371e8700..c98c93500a43 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5808,16 +5808,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	} \
 } while (0)
 
-#define PIPE_CONF_CHECK_CLOCK_FUZZY(name) do { \
-	if (!intel_fuzzy_clock_check(current_config->name, pipe_config->name)) { \
-		pipe_config_mismatch(fastset, crtc, __stringify(name), \
-				     "(expected %i, found %i)", \
-				     current_config->name, \
-				     pipe_config->name); \
-		ret = false; \
-	} \
-} while (0)
-
 #define PIPE_CONF_CHECK_INFOFRAME(name) do { \
 	if (!intel_compare_infoframe(&current_config->infoframes.name, \
 				     &pipe_config->infoframes.name)) { \
@@ -5936,7 +5926,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		PIPE_CONF_CHECK_RECT(pch_pfit.dst);
 
 		PIPE_CONF_CHECK_I(scaler_state.scaler_id);
-		PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate);
+		PIPE_CONF_CHECK_I(pixel_rate);
 
 		PIPE_CONF_CHECK_X(gamma_mode);
 		if (IS_CHERRYVIEW(dev_priv))
@@ -6006,9 +5996,9 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5)
 		PIPE_CONF_CHECK_I(pipe_bpp);
 
-	PIPE_CONF_CHECK_CLOCK_FUZZY(hw.pipe_mode.crtc_clock);
-	PIPE_CONF_CHECK_CLOCK_FUZZY(hw.adjusted_mode.crtc_clock);
-	PIPE_CONF_CHECK_CLOCK_FUZZY(port_clock);
+	PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock);
+	PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock);
+	PIPE_CONF_CHECK_I(port_clock);
 
 	PIPE_CONF_CHECK_I(min_voltage_level);
 
@@ -6050,7 +6040,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 #undef PIPE_CONF_CHECK_BOOL_INCOMPLETE
 #undef PIPE_CONF_CHECK_P
 #undef PIPE_CONF_CHECK_FLAGS
-#undef PIPE_CONF_CHECK_CLOCK_FUZZY
 #undef PIPE_CONF_CHECK_COLOR_LUT
 #undef PIPE_CONF_CHECK_TIMINGS
 #undef PIPE_CONF_CHECK_RECT
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 10/16] drm/i915: Set active dpll early for icl+
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (8 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 09/16] drm/i915: Make all clock " Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 11/16] drm/i915: Nuke fastet state copy hacks Ville Syrjala
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

To make the fastboot checks at least somewhat sensible let's mark
the expected DPLL as the active one right after we finished the
state computation. Otherwise intel_pipe_config_compare() will
always be comparing things against NULL/0.

TODO: This is still not really right. If the previous commit
had to fall back to the other PLL then the comparisong will
now fail. I guess intel_pipe_config_compare() should rather
be comparing port_dplls[] instead. But to do that we really
should just unify every platform to use the port_dplls[]
approach whether they have any need for PLL fallbacks or not.

Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 09816526c5e4..c99ec8da20e0 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3213,6 +3213,9 @@ static int icl_compute_combo_phy_dpll(struct intel_atomic_state *state,
 
 	icl_calc_dpll_state(dev_priv, &pll_params, &port_dpll->hw_state);
 
+	/* this is mainly for the fastset check */
+	icl_set_active_port_dpll(crtc_state, ICL_PORT_DPLL_DEFAULT);
+
 	crtc_state->port_clock = icl_ddi_combo_pll_get_freq(dev_priv, NULL,
 							    &port_dpll->hw_state);
 
@@ -3301,6 +3304,9 @@ static int icl_compute_tc_phy_dplls(struct intel_atomic_state *state,
 	if (ret)
 		return ret;
 
+	/* this is mainly for the fastset check */
+	icl_set_active_port_dpll(crtc_state, ICL_PORT_DPLL_MG_PHY);
+
 	crtc_state->port_clock = icl_ddi_mg_pll_get_freq(dev_priv, NULL,
 							 &port_dpll->hw_state);
 
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 11/16] drm/i915: Nuke fastet state copy hacks
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (9 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 10/16] drm/i915: Set active dpll early for icl+ Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 12/16] drm/i915: Skip intel_modeset_pipe_config_late() if the pipe is not enabled Ville Syrjala
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Now that we no longer do the fuzzy clock and M/N checks we can
get rid of the fastset state copy hacks.

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_display.c | 28 +++-----------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index c98c93500a43..16a4ea183746 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6262,23 +6262,6 @@ static void intel_crtc_check_fastset(const struct intel_crtc_state *old_crtc_sta
 	new_crtc_state->update_pipe = true;
 }
 
-static void intel_crtc_copy_fastset(const struct intel_crtc_state *old_crtc_state,
-				    struct intel_crtc_state *new_crtc_state)
-{
-	/*
-	 * If we're not doing the full modeset we want to
-	 * keep the current M/N values as they may be
-	 * sufficiently different to the computed values
-	 * to cause problems.
-	 *
-	 * FIXME: should really copy more fuzzy state here
-	 */
-	new_crtc_state->fdi_m_n = old_crtc_state->fdi_m_n;
-	new_crtc_state->dp_m_n = old_crtc_state->dp_m_n;
-	new_crtc_state->dp_m2_n2 = old_crtc_state->dp_m2_n2;
-	new_crtc_state->has_drrs = old_crtc_state->has_drrs;
-}
-
 static int intel_crtc_add_planes_to_state(struct intel_atomic_state *state,
 					  struct intel_crtc *crtc,
 					  u8 plane_ids_mask)
@@ -6988,17 +6971,12 @@ static int intel_atomic_check(struct drm_device *dev,
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (intel_crtc_needs_modeset(new_crtc_state)) {
-			any_ms = true;
-
-			intel_release_shared_dplls(state, crtc);
+		if (!intel_crtc_needs_modeset(new_crtc_state))
 			continue;
-		}
 
-		if (!new_crtc_state->update_pipe)
-			continue;
+		any_ms = true;
 
-		intel_crtc_copy_fastset(old_crtc_state, new_crtc_state);
+		intel_release_shared_dplls(state, crtc);
 	}
 
 	if (any_ms && !check_digital_port_conflicts(state)) {
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 12/16] drm/i915: Skip intel_modeset_pipe_config_late() if the pipe is not enabled
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (10 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 11/16] drm/i915: Nuke fastet state copy hacks Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 13/16] drm/i915: Add intel_panel_highest_mode() Ville Syrjala
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

No sense in calling intel_modeset_pipe_config_late() for a disabled
pipe.

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_display.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 16a4ea183746..4f7e119f1cd3 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6918,9 +6918,11 @@ static int intel_atomic_check(struct drm_device *dev,
 		if (!intel_crtc_needs_modeset(new_crtc_state))
 			continue;
 
-		ret = intel_modeset_pipe_config_late(state, crtc);
-		if (ret)
-			goto fail;
+		if (new_crtc_state->hw.enable) {
+			ret = intel_modeset_pipe_config_late(state, crtc);
+			if (ret)
+				goto fail;
+		}
 
 		intel_crtc_check_fastset(old_crtc_state, new_crtc_state);
 	}
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 13/16] drm/i915: Add intel_panel_highest_mode()
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (11 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 12/16] drm/i915: Skip intel_modeset_pipe_config_late() if the pipe is not enabled Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 14/16] drm/i915: Allow M/N change during fastset on bdw+ Ville Syrjala
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Add a function to get the fixed_mode with the highest clock.
The plan is to use this for the link bw calculation on seamless
DRRS panels so that we alwasy end up with the same link params
regardless of the requested refresh rate. This will allow fastset
to do seamless refresh rate changes based on userspace request
instead of having to go for a full modeset.

TODO: the function name isn't great

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_panel.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/display/intel_panel.h |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
index 237a40623dd7..c738de27e49b 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.c
+++ b/drivers/gpu/drm/i915/display/intel_panel.c
@@ -114,6 +114,21 @@ intel_panel_downclock_mode(struct intel_connector *connector,
 	return best_mode;
 }
 
+const struct drm_display_mode *
+intel_panel_highest_mode(struct intel_connector *connector,
+			 const struct drm_display_mode *adjusted_mode)
+{
+	const struct drm_display_mode *fixed_mode, *best_mode = adjusted_mode;
+
+	/* pick the fixed_mode that has the highest clock */
+	list_for_each_entry(fixed_mode, &connector->panel.fixed_modes, head) {
+		if (fixed_mode->clock > best_mode->clock)
+			best_mode = fixed_mode;
+	}
+
+	return best_mode;
+}
+
 int intel_panel_get_modes(struct intel_connector *connector)
 {
 	const struct drm_display_mode *fixed_mode;
diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
index b087c0c3cc6d..eff3ffd3d082 100644
--- a/drivers/gpu/drm/i915/display/intel_panel.h
+++ b/drivers/gpu/drm/i915/display/intel_panel.h
@@ -31,6 +31,9 @@ intel_panel_fixed_mode(struct intel_connector *connector,
 const struct drm_display_mode *
 intel_panel_downclock_mode(struct intel_connector *connector,
 			   const struct drm_display_mode *adjusted_mode);
+const struct drm_display_mode *
+intel_panel_highest_mode(struct intel_connector *connector,
+			 const struct drm_display_mode *adjusted_mode);
 int intel_panel_get_modes(struct intel_connector *connector);
 enum drrs_type intel_panel_drrs_type(struct intel_connector *connector);
 enum drm_mode_status
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 14/16] drm/i915: Allow M/N change during fastset on bdw+
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (12 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 13/16] drm/i915: Add intel_panel_highest_mode() Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 15/16] drm/i915: Use a fixed N value always Ville Syrjala
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx

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

On BDW+ M/N are double buffered and so we can easily reprogram them
during a fastset. So for eDP panels that support seamless DRRS we
can just change these without a full modeset.

For earlier platforms we'd need to play tricks with M1/N1 vs.
M2/N2 during the fastset to make sure we do the switch atomically.
Not sure the added complexity is worth the hassle, so leave it
alone for now.

The slight downside is that we have to keep the link running at
a link rate capable of supporting the highest refresh rate we
want to use. For the moment we just pick the highest mode the
panel reports and calculate the link based on that. This might
need further refinement (eg. if we run into bandwidth
restrictions)...

v2: Only use the high link rate if the platform really supports
    the seamless M/N change uring fastset (ie. bdw+)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  | 13 +++++--
 .../drm/i915/display/intel_display_types.h    |  1 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 37 ++++++++++++++++---
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4f7e119f1cd3..4e33ce635112 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5864,7 +5864,8 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	PIPE_CONF_CHECK_X(lane_lat_optim_mask);
 
 	if (has_double_buffered_m_n(dev_priv)) {
-		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
+		if (!fastset || !pipe_config->seamless_m_n)
+			PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
 	} else {
 		PIPE_CONF_CHECK_M_N(dp_m_n);
 		PIPE_CONF_CHECK_M_N(dp_m2_n2);
@@ -5996,8 +5997,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5)
 		PIPE_CONF_CHECK_I(pipe_bpp);
 
-	PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock);
-	PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock);
+	if (!fastset || !pipe_config->seamless_m_n) {
+		PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock);
+		PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock);
+	}
 	PIPE_CONF_CHECK_I(port_clock);
 
 	PIPE_CONF_CHECK_I(min_voltage_level);
@@ -7137,6 +7140,10 @@ static void intel_pipe_fastset(const struct intel_crtc_state *old_crtc_state,
 	if (DISPLAY_VER(dev_priv) >= 9 ||
 	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
 		hsw_set_linetime_wm(new_crtc_state);
+
+	if (new_crtc_state->seamless_m_n)
+		intel_cpu_transcoder_set_m1_n1(crtc, new_crtc_state->cpu_transcoder,
+					       &new_crtc_state->dp_m_n);
 }
 
 static void commit_pipe_pre_planes(struct intel_atomic_state *state,
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 8b0949b6dc75..95159d1c8ca8 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1129,6 +1129,7 @@ struct intel_crtc_state {
 	/* m2_n2 for eDP downclock */
 	struct intel_link_m_n dp_m2_n2;
 	bool has_drrs;
+	bool seamless_m_n;
 
 	/* PSR is supported but might not be enabled due the lack of enabled planes */
 	bool has_psr;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 75645508080a..7c091c601e30 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1285,21 +1285,45 @@ intel_dp_adjust_compliance_config(struct intel_dp *intel_dp,
 	}
 }
 
+static bool has_seamless_m_n(struct intel_connector *connector)
+{
+	struct drm_i915_private *i915 = to_i915(connector->base.dev);
+
+	/*
+	 * Seamless M/N reprogramming only implemented
+	 * for BDW+ double buffered M/N registers so far.
+	 */
+	return has_double_buffered_m_n(i915) &&
+		intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS;
+}
+
+static int intel_dp_mode_clock(const struct intel_crtc_state *crtc_state,
+			       const struct drm_connector_state *conn_state)
+{
+	struct intel_connector *connector = to_intel_connector(conn_state->connector);
+	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+
+	/* FIXME a bit of a mess wrt clock vs. crtc_clock */
+	if (has_seamless_m_n(connector))
+		return intel_panel_highest_mode(connector, adjusted_mode)->clock;
+	else
+		return adjusted_mode->crtc_clock;
+}
+
 /* Optimize link config in order: max bpp, min clock, min lanes */
 static int
 intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 				  struct intel_crtc_state *pipe_config,
+				  const struct drm_connector_state *conn_state,
 				  const struct link_config_limits *limits)
 {
-	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
-	int bpp, i, lane_count;
+	int bpp, i, lane_count, clock = intel_dp_mode_clock(pipe_config, conn_state);
 	int mode_rate, link_rate, link_avail;
 
 	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
 		int output_bpp = intel_dp_output_bpp(pipe_config->output_format, bpp);
 
-		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
-						   output_bpp);
+		mode_rate = intel_dp_link_required(clock, output_bpp);
 
 		for (i = 0; i < intel_dp->num_common_rates; i++) {
 			link_rate = intel_dp_common_rate(intel_dp, i);
@@ -1599,7 +1623,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	 * Optimize for slow and wide for everything, because there are some
 	 * eDP 1.3 and 1.4 panels don't work well with fast and narrow.
 	 */
-	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
+	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, conn_state, &limits);
 
 	if (ret || joiner_needs_dsc || intel_dp->force_dsc_en) {
 		drm_dbg_kms(&i915->drm, "Try DSC (fallback=%s, joiner=%s, force=%s)\n",
@@ -1887,6 +1911,9 @@ intel_dp_drrs_compute_config(struct intel_connector *connector,
 		intel_panel_downclock_mode(connector, &pipe_config->hw.adjusted_mode);
 	int pixel_clock;
 
+	if (has_seamless_m_n(connector))
+		pipe_config->seamless_m_n = true;
+
 	if (!can_enable_drrs(connector, pipe_config, downclock_mode)) {
 		if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config->cpu_transcoder))
 			intel_zero_m_n(&pipe_config->dp_m2_n2);
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 15/16] drm/i915: Use a fixed N value always
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (13 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 14/16] drm/i915: Allow M/N change during fastset on bdw+ Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 16/16] drm/i915: Round TMDS clock to nearest Ville Syrjala
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Windows/BIOS always uses fixed N values. Let's match that
behaviour.

Allows us to also get rid of that constant_n quirk stuff.

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_display.c | 36 +++++++++-----------
 drivers/gpu/drm/i915/display/intel_display.h |  2 +-
 drivers/gpu/drm/i915/display/intel_dp.c      | 10 +++---
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  3 +-
 drivers/gpu/drm/i915/display/intel_fdi.c     |  2 +-
 5 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4e33ce635112..7e45dd99d03c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2817,19 +2817,11 @@ intel_reduce_m_n_ratio(u32 *num, u32 *den)
 	}
 }
 
-static void compute_m_n(unsigned int m, unsigned int n,
-			u32 *ret_m, u32 *ret_n,
-			bool constant_n)
+static void compute_m_n(u32 *ret_m, u32 *ret_n,
+			u32 m, u32 n, u32 constant_n)
 {
-	/*
-	 * Several DP dongles in particular seem to be fussy about
-	 * too large link M/N values. Give N value as 0x8000 that
-	 * should be acceptable by specific devices. 0x8000 is the
-	 * specified fixed N value for asynchronous clock mode,
-	 * which the devices expect also in synchronous clock mode.
-	 */
 	if (constant_n)
-		*ret_n = DP_LINK_CONSTANT_N_VALUE;
+		*ret_n = constant_n;
 	else
 		*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
 
@@ -2841,22 +2833,28 @@ void
 intel_link_compute_m_n(u16 bits_per_pixel, int nlanes,
 		       int pixel_clock, int link_clock,
 		       struct intel_link_m_n *m_n,
-		       bool constant_n, bool fec_enable)
+		       bool fec_enable)
 {
 	u32 data_clock = bits_per_pixel * pixel_clock;
 
 	if (fec_enable)
 		data_clock = intel_dp_mode_to_fec_clock(data_clock);
 
+	/*
+	 * Windows/BIOS uses fixed M/N values always. Follow suit.
+	 *
+	 * Also several DP dongles in particular seem to be fussy
+	 * about too large link M/N values. Presumably the 20bit
+	 * value used by Windows/BIOS is acceptable to everyone.
+	 */
 	m_n->tu = 64;
-	compute_m_n(data_clock,
-		    link_clock * nlanes * 8,
-		    &m_n->data_m, &m_n->data_n,
-		    constant_n);
+	compute_m_n(&m_n->data_m, &m_n->data_n,
+		    data_clock, link_clock * nlanes * 8,
+		    0x8000000);
 
-	compute_m_n(pixel_clock, link_clock,
-		    &m_n->link_m, &m_n->link_n,
-		    constant_n);
+	compute_m_n(&m_n->link_m, &m_n->link_n,
+		    pixel_clock, link_clock,
+		    0x80000);
 }
 
 static void intel_panel_sanitize_ssc(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 44c88aadfc30..ee02fe4d43a5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -547,7 +547,7 @@ bool has_double_buffered_m_n(struct drm_i915_private *i915);
 void intel_link_compute_m_n(u16 bpp, int nlanes,
 			    int pixel_clock, int link_clock,
 			    struct intel_link_m_n *m_n,
-			    bool constant_n, bool fec_enable);
+			    bool fec_enable);
 u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
 			      u32 pixel_format, u64 modifier);
 enum drm_mode_status
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7c091c601e30..6750a359a555 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1904,7 +1904,7 @@ static bool can_enable_drrs(struct intel_connector *connector,
 static void
 intel_dp_drrs_compute_config(struct intel_connector *connector,
 			     struct intel_crtc_state *pipe_config,
-			     int output_bpp, bool constant_n)
+			     int output_bpp)
 {
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	const struct drm_display_mode *downclock_mode =
@@ -1931,7 +1931,7 @@ intel_dp_drrs_compute_config(struct intel_connector *connector,
 
 	intel_link_compute_m_n(output_bpp, pipe_config->lane_count, pixel_clock,
 			       pipe_config->port_clock, &pipe_config->dp_m2_n2,
-			       constant_n, pipe_config->fec_enable);
+			       pipe_config->fec_enable);
 
 	/* FIXME: abstract this better */
 	if (pipe_config->splitter.enable)
@@ -2006,7 +2006,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	const struct drm_display_mode *fixed_mode;
 	struct intel_connector *connector = intel_dp->attached_connector;
-	bool constant_n = drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_CONSTANT_N);
 	int ret = 0, output_bpp;
 
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && encoder->port != PORT_A)
@@ -2085,7 +2084,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n,
-			       constant_n, pipe_config->fec_enable);
+			       pipe_config->fec_enable);
 
 	/* FIXME: abstract this better */
 	if (pipe_config->splitter.enable)
@@ -2096,8 +2095,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 	intel_vrr_compute_config(pipe_config, conn_state);
 	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
-	intel_dp_drrs_compute_config(connector, pipe_config,
-				     output_bpp, constant_n);
+	intel_dp_drrs_compute_config(connector, pipe_config, output_bpp);
 	intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
 	intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 061b277e5ce7..00e55555091a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -57,7 +57,6 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
-	bool constant_n = drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_CONSTANT_N);
 	int bpp, slots = -EINVAL;
 
 	crtc_state->lane_count = limits->max_lane_count;
@@ -93,7 +92,7 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
 			       adjusted_mode->crtc_clock,
 			       crtc_state->port_clock,
 			       &crtc_state->dp_m_n,
-			       constant_n, crtc_state->fec_enable);
+			       crtc_state->fec_enable);
 	crtc_state->dp_m_n.tu = slots;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_fdi.c b/drivers/gpu/drm/i915/display/intel_fdi.c
index 67d2484afbaa..0dc6414a56c4 100644
--- a/drivers/gpu/drm/i915/display/intel_fdi.c
+++ b/drivers/gpu/drm/i915/display/intel_fdi.c
@@ -256,7 +256,7 @@ int ilk_fdi_compute_config(struct intel_crtc *crtc,
 	pipe_config->fdi_lanes = lane;
 
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
-			       link_bw, &pipe_config->fdi_m_n, false, false);
+			       link_bw, &pipe_config->fdi_m_n, false);
 
 	ret = ilk_check_fdi_lanes(dev, crtc->pipe, pipe_config);
 	if (ret == -EDEADLK)
-- 
2.35.1


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

* [Intel-gfx] [PATCH v2 16/16] drm/i915: Round TMDS clock to nearest
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (14 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 15/16] drm/i915: Use a fixed N value always Ville Syrjala
@ 2022-06-17 16:05 ` Ville Syrjala
  2022-06-17 19:49 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Make fastset not suck and allow seamless M/N changes (rev5) Patchwork
  2022-06-17 20:10 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  17 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2022-06-17 16:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

Use round-to-nearest behavour when calculating the TMDS clock.
Matches what we do for most other clock related things.

Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 3 ++-
 drivers/gpu/drm/i915/display/intel_hdmi.c    | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7e45dd99d03c..1ec9f3d54031 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4585,7 +4585,8 @@ int intel_crtc_dotclock(const struct intel_crtc_state *pipe_config)
 		dotclock = intel_dotclock_calculate(pipe_config->port_clock,
 						    &pipe_config->dp_m_n);
 	else if (pipe_config->has_hdmi_sink && pipe_config->pipe_bpp > 24)
-		dotclock = pipe_config->port_clock * 24 / pipe_config->pipe_bpp;
+		dotclock = DIV_ROUND_CLOSEST(pipe_config->port_clock * 24,
+					     pipe_config->pipe_bpp);
 	else
 		dotclock = pipe_config->port_clock;
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 1ae09431f53a..0b04b3800cd4 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1891,7 +1891,7 @@ int intel_hdmi_tmds_clock(int clock, int bpc, bool ycbcr420_output)
 	 *  1.5x for 12bpc
 	 *  1.25x for 10bpc
 	 */
-	return clock * bpc / 8;
+	return DIV_ROUND_CLOSEST(clock * bpc, 8);
 }
 
 static bool intel_hdmi_source_bpc_possible(struct drm_i915_private *i915, int bpc)
-- 
2.35.1


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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Make fastset not suck and allow seamless M/N changes (rev5)
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (15 preceding siblings ...)
  2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 16/16] drm/i915: Round TMDS clock to nearest Ville Syrjala
@ 2022-06-17 19:49 ` Patchwork
  2022-06-17 20:10 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  17 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2022-06-17 19:49 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Make fastset not suck and allow seamless M/N changes (rev5)
URL   : https://patchwork.freedesktop.org/series/103491/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Make fastset not suck and allow seamless M/N changes (rev5)
  2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
                   ` (16 preceding siblings ...)
  2022-06-17 19:49 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Make fastset not suck and allow seamless M/N changes (rev5) Patchwork
@ 2022-06-17 20:10 ` Patchwork
  17 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2022-06-17 20:10 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

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

== Series Details ==

Series: drm/i915: Make fastset not suck and allow seamless M/N changes (rev5)
URL   : https://patchwork.freedesktop.org/series/103491/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11776 -> Patchwork_103491v5
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_103491v5 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_103491v5, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

Participating hosts (35 -> 34)
------------------------------

  Missing    (1): fi-bdw-samus 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_103491v5:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - fi-bxt-dsi:         [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-bxt-dsi/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-bxt-dsi/igt@i915_module_load@load.html
    - fi-glk-dsi:         [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-glk-dsi/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-glk-dsi/igt@i915_module_load@load.html

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

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

### CI changes ###

#### Issues hit ####

  * boot:
    - fi-snb-2600:        [PASS][5] -> [FAIL][6] ([i915#4338])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-snb-2600/boot.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-snb-2600/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gem:
    - fi-blb-e6850:       NOTRUN -> [DMESG-FAIL][7] ([i915#4528])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-blb-e6850/igt@i915_selftest@live@gem.html
    - fi-pnv-d510:        [PASS][8] -> [DMESG-FAIL][9] ([i915#4528])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-pnv-d510/igt@i915_selftest@live@gem.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-pnv-d510/igt@i915_selftest@live@gem.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       [PASS][10] -> [INCOMPLETE][11] ([i915#5982])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-hsw-4770:        NOTRUN -> [SKIP][12] ([fdo#109271] / [fdo#111827])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_flip@basic-plain-flip@a-edp1:
    - fi-tgl-u2:          [PASS][13] -> [DMESG-WARN][14] ([i915#402])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-tgl-u2/igt@kms_flip@basic-plain-flip@a-edp1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-tgl-u2/igt@kms_flip@basic-plain-flip@a-edp1.html

  * igt@runner@aborted:
    - fi-bxt-dsi:         NOTRUN -> [FAIL][15] ([i915#4312] / [i915#5257])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-bxt-dsi/igt@runner@aborted.html
    - fi-glk-dsi:         NOTRUN -> [FAIL][16] ([i915#4312] / [i915#5257])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-glk-dsi/igt@runner@aborted.html
    - fi-pnv-d510:        NOTRUN -> [FAIL][17] ([fdo#109271] / [i915#2403] / [i915#4312])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-pnv-d510/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0@smem:
    - {fi-ehl-2}:         [DMESG-WARN][18] ([i915#5122]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-ehl-2/igt@gem_exec_suspend@basic-s0@smem.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-ehl-2/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [INCOMPLETE][20] ([i915#3303] / [i915#4785]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@requests:
    - fi-blb-e6850:       [DMESG-FAIL][22] ([i915#4528]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-blb-e6850/igt@i915_selftest@live@requests.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-blb-e6850/igt@i915_selftest@live@requests.html

  * igt@kms_busy@basic@flip:
    - fi-tgl-u2:          [DMESG-WARN][24] ([i915#402]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11776/fi-tgl-u2/igt@kms_busy@basic@flip.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_103491v5/fi-tgl-u2/igt@kms_busy@basic@flip.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4338]: https://gitlab.freedesktop.org/drm/intel/issues/4338
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#5982]: https://gitlab.freedesktop.org/drm/intel/issues/5982


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

  * Linux: CI_DRM_11776 -> Patchwork_103491v5

  CI-20190529: 20190529
  CI_DRM_11776: ac17a5249380aaabe5d1eaebd9b3a2eedc08ccdc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6536: e3de4d32b7a509635fbff4d5131c05a7767699f7 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_103491v5: ac17a5249380aaabe5d1eaebd9b3a2eedc08ccdc @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

014624fafcea drm/i915: Round TMDS clock to nearest
58524d7077d8 drm/i915: Use a fixed N value always
be06321728a3 drm/i915: Allow M/N change during fastset on bdw+
2c672ff7c5c7 drm/i915: Add intel_panel_highest_mode()
8a5309c978ec drm/i915: Skip intel_modeset_pipe_config_late() if the pipe is not enabled
a2cc7b377a57 drm/i915: Nuke fastet state copy hacks
44884e614fb7 drm/i915: Set active dpll early for icl+
0eec88851675 drm/i915: Make all clock checks non-fuzzy
ee84a1a003d7 drm/i915: Make M/N checks non-fuzzy
964dc6e315ff drm/i915: Compute clocks earlier
ba52ccf5b250 drm/i915: Feed the DPLL output freq back into crtc_state
8babf3e09e05 drm/i915: Reassign DPLLs only for crtcs going throug .compute_config()
59b3fd7267b3 drm/i915: Do .crtc_compute_clock() earlier
61a67a86201d drm/i915: Extract has_double_buffered_m_n()
8e103db0e9a3 drm/i915: Shuffle some PLL code around
9c1ce79a778c drm/i915: Relocate intel_crtc_dotclock()

== Logs ==

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

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

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

* Re: [Intel-gfx] [PATCH v2 01/16] drm/i915: Relocate intel_crtc_dotclock()
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 01/16] drm/i915: Relocate intel_crtc_dotclock() Ville Syrjala
@ 2022-06-20  9:01   ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2022-06-20  9:01 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 17 Jun 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_crtc_dotclock() is a bit misplaced. In lieu of a better
> place let's just move it next to its friends in intel_display.c.

With hopes we'll find a better place than intel_display.c for this and
its friends in the future,

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_ddi.c     | 22 --------------------
>  drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
>  2 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 272e1bf6006b..51bf26dcb209 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -323,28 +323,6 @@ static int icl_calc_tbt_pll_link(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> -int intel_crtc_dotclock(const struct intel_crtc_state *pipe_config)
> -{
> -	int dotclock;
> -
> -	if (intel_crtc_has_dp_encoder(pipe_config))
> -		dotclock = intel_dotclock_calculate(pipe_config->port_clock,
> -						    &pipe_config->dp_m_n);
> -	else if (pipe_config->has_hdmi_sink && pipe_config->pipe_bpp > 24)
> -		dotclock = pipe_config->port_clock * 24 / pipe_config->pipe_bpp;
> -	else
> -		dotclock = pipe_config->port_clock;
> -
> -	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> -	    !intel_crtc_has_dp_encoder(pipe_config))
> -		dotclock *= 2;
> -
> -	if (pipe_config->pixel_multiplier)
> -		dotclock /= pipe_config->pixel_multiplier;
> -
> -	return dotclock;
> -}
> -
>  static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
>  {
>  	/* CRT dotclock is determined via other means */
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 90bd26431e31..b24784c4522d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4569,6 +4569,28 @@ int intel_dotclock_calculate(int link_freq,
>  	return div_u64(mul_u32_u32(m_n->link_m, link_freq), m_n->link_n);
>  }
>  
> +int intel_crtc_dotclock(const struct intel_crtc_state *pipe_config)
> +{
> +	int dotclock;
> +
> +	if (intel_crtc_has_dp_encoder(pipe_config))
> +		dotclock = intel_dotclock_calculate(pipe_config->port_clock,
> +						    &pipe_config->dp_m_n);
> +	else if (pipe_config->has_hdmi_sink && pipe_config->pipe_bpp > 24)
> +		dotclock = pipe_config->port_clock * 24 / pipe_config->pipe_bpp;
> +	else
> +		dotclock = pipe_config->port_clock;
> +
> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> +	    !intel_crtc_has_dp_encoder(pipe_config))
> +		dotclock *= 2;
> +
> +	if (pipe_config->pixel_multiplier)
> +		dotclock /= pipe_config->pixel_multiplier;
> +
> +	return dotclock;
> +}
> +
>  /* Returns the currently programmed mode of the given encoder. */
>  struct drm_display_mode *
>  intel_encoder_current_mode(struct intel_encoder *encoder)

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH v2 02/16] drm/i915: Shuffle some PLL code around
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 02/16] drm/i915: Shuffle some PLL code around Ville Syrjala
@ 2022-06-20  9:01   ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2022-06-20  9:01 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 17 Jun 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Shuffle some PLL functions around a bit to avoid ugle
> forward declarations later on. No functional changes.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 176 +++++++++---------
>  1 file changed, 88 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index ddae7e42ac46..bfccc96f16fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -905,37 +905,6 @@ hsw_ddi_calculate_wrpll(int clock /* in Hz */,
>  	*r2_out = best.r2;
>  }
>  
> -static int
> -hsw_ddi_wrpll_compute_dpll(struct intel_atomic_state *state,
> -			   struct intel_crtc *crtc)
> -{
> -	struct intel_crtc_state *crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> -	unsigned int p, n2, r2;
> -
> -	hsw_ddi_calculate_wrpll(crtc_state->port_clock * 1000, &r2, &n2, &p);
> -
> -	crtc_state->dpll_hw_state.wrpll =
> -		WRPLL_PLL_ENABLE | WRPLL_REF_LCPLL |
> -		WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
> -		WRPLL_DIVIDER_POST(p);
> -
> -	return 0;
> -}
> -
> -static struct intel_shared_dpll *
> -hsw_ddi_wrpll_get_dpll(struct intel_atomic_state *state,
> -		       struct intel_crtc *crtc)
> -{
> -	struct intel_crtc_state *crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> -
> -	return intel_find_shared_dpll(state, crtc,
> -				      &crtc_state->dpll_hw_state,
> -				      BIT(DPLL_ID_WRPLL2) |
> -				      BIT(DPLL_ID_WRPLL1));
> -}
> -
>  static int hsw_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
>  				  const struct intel_shared_dpll *pll,
>  				  const struct intel_dpll_hw_state *pll_state)
> @@ -976,6 +945,37 @@ static int hsw_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
>  	return (refclk * n / 10) / (p * r) * 2;
>  }
>  
> +static int
> +hsw_ddi_wrpll_compute_dpll(struct intel_atomic_state *state,
> +			   struct intel_crtc *crtc)
> +{
> +	struct intel_crtc_state *crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +	unsigned int p, n2, r2;
> +
> +	hsw_ddi_calculate_wrpll(crtc_state->port_clock * 1000, &r2, &n2, &p);
> +
> +	crtc_state->dpll_hw_state.wrpll =
> +		WRPLL_PLL_ENABLE | WRPLL_REF_LCPLL |
> +		WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
> +		WRPLL_DIVIDER_POST(p);
> +
> +	return 0;
> +}
> +
> +static struct intel_shared_dpll *
> +hsw_ddi_wrpll_get_dpll(struct intel_atomic_state *state,
> +		       struct intel_crtc *crtc)
> +{
> +	struct intel_crtc_state *crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +
> +	return intel_find_shared_dpll(state, crtc,
> +				      &crtc_state->dpll_hw_state,
> +				      BIT(DPLL_ID_WRPLL2) |
> +				      BIT(DPLL_ID_WRPLL1));
> +}
> +
>  static int
>  hsw_ddi_lcpll_compute_dpll(struct intel_crtc_state *crtc_state)
>  {
> @@ -1618,43 +1618,6 @@ skl_ddi_calculate_wrpll(int clock /* in Hz */,
>  	return 0;
>  }
>  
> -static int skl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
> -{
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> -	struct skl_wrpll_params wrpll_params = {};
> -	u32 ctrl1, cfgcr1, cfgcr2;
> -	int ret;
> -
> -	/*
> -	 * See comment in intel_dpll_hw_state to understand why we always use 0
> -	 * as the DPLL id in this function.
> -	 */
> -	ctrl1 = DPLL_CTRL1_OVERRIDE(0);
> -
> -	ctrl1 |= DPLL_CTRL1_HDMI_MODE(0);
> -
> -	ret = skl_ddi_calculate_wrpll(crtc_state->port_clock * 1000,
> -				      i915->dpll.ref_clks.nssc, &wrpll_params);
> -	if (ret)
> -		return ret;
> -
> -	cfgcr1 = DPLL_CFGCR1_FREQ_ENABLE |
> -		DPLL_CFGCR1_DCO_FRACTION(wrpll_params.dco_fraction) |
> -		wrpll_params.dco_integer;
> -
> -	cfgcr2 = DPLL_CFGCR2_QDIV_RATIO(wrpll_params.qdiv_ratio) |
> -		DPLL_CFGCR2_QDIV_MODE(wrpll_params.qdiv_mode) |
> -		DPLL_CFGCR2_KDIV(wrpll_params.kdiv) |
> -		DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
> -		wrpll_params.central_freq;
> -
> -	crtc_state->dpll_hw_state.ctrl1 = ctrl1;
> -	crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
> -	crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
> -
> -	return 0;
> -}
> -
>  static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
>  				  const struct intel_shared_dpll *pll,
>  				  const struct intel_dpll_hw_state *pll_state)
> @@ -1726,6 +1689,43 @@ static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
>  	return dco_freq / (p0 * p1 * p2 * 5);
>  }
>  
> +static int skl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	struct skl_wrpll_params wrpll_params = {};
> +	u32 ctrl1, cfgcr1, cfgcr2;
> +	int ret;
> +
> +	/*
> +	 * See comment in intel_dpll_hw_state to understand why we always use 0
> +	 * as the DPLL id in this function.
> +	 */
> +	ctrl1 = DPLL_CTRL1_OVERRIDE(0);
> +
> +	ctrl1 |= DPLL_CTRL1_HDMI_MODE(0);
> +
> +	ret = skl_ddi_calculate_wrpll(crtc_state->port_clock * 1000,
> +				      i915->dpll.ref_clks.nssc, &wrpll_params);
> +	if (ret)
> +		return ret;
> +
> +	cfgcr1 = DPLL_CFGCR1_FREQ_ENABLE |
> +		DPLL_CFGCR1_DCO_FRACTION(wrpll_params.dco_fraction) |
> +		wrpll_params.dco_integer;
> +
> +	cfgcr2 = DPLL_CFGCR2_QDIV_RATIO(wrpll_params.qdiv_ratio) |
> +		DPLL_CFGCR2_QDIV_MODE(wrpll_params.qdiv_mode) |
> +		DPLL_CFGCR2_KDIV(wrpll_params.kdiv) |
> +		DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
> +		wrpll_params.central_freq;
> +
> +	crtc_state->dpll_hw_state.ctrl1 = ctrl1;
> +	crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
> +	crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
> +
> +	return 0;
> +}
> +
>  static int
>  skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
>  {
> @@ -2245,26 +2245,6 @@ static int bxt_ddi_set_dpll_hw_state(struct intel_crtc_state *crtc_state,
>  	return 0;
>  }
>  
> -static int
> -bxt_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
> -{
> -	struct dpll clk_div = {};
> -
> -	bxt_ddi_dp_pll_dividers(crtc_state, &clk_div);
> -
> -	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
> -}
> -
> -static int
> -bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
> -{
> -	struct dpll clk_div = {};
> -
> -	bxt_ddi_hdmi_pll_dividers(crtc_state, &clk_div);
> -
> -	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
> -}
> -
>  static int bxt_ddi_pll_get_freq(struct drm_i915_private *i915,
>  				const struct intel_shared_dpll *pll,
>  				const struct intel_dpll_hw_state *pll_state)
> @@ -2282,6 +2262,26 @@ static int bxt_ddi_pll_get_freq(struct drm_i915_private *i915,
>  	return chv_calc_dpll_params(i915->dpll.ref_clks.nssc, &clock);
>  }
>  
> +static int
> +bxt_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
> +{
> +	struct dpll clk_div = {};
> +
> +	bxt_ddi_dp_pll_dividers(crtc_state, &clk_div);
> +
> +	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
> +}
> +
> +static int
> +bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
> +{
> +	struct dpll clk_div = {};
> +
> +	bxt_ddi_hdmi_pll_dividers(crtc_state, &clk_div);
> +
> +	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
> +}
> +
>  static int bxt_compute_dpll(struct intel_atomic_state *state,
>  			    struct intel_crtc *crtc,
>  			    struct intel_encoder *encoder)

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH v2 03/16] drm/i915: Extract has_double_buffered_m_n()
  2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 03/16] drm/i915: Extract has_double_buffered_m_n() Ville Syrjala
@ 2022-06-20  9:05   ` Jani Nikula
  2022-06-20 17:05     ` Ville Syrjälä
  0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2022-06-20  9:05 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 17 Jun 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We have a couple of places that want to make distinction between
> double buffered M/N registers vs. the split M1/N1+M2/N2 registers.
> Add a helper for that.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Mhh. So why a function in intel_display.c instead of a macro in
i915_drv.h? Both are kind of cluttered, but at least in i915_drv.h it
would be among others.

I do think we should have a separate file for display feature check
macros, and move most if not all of the display related HAS_*() stuff
there from i915_drv.h.

So technically this does what it says on the box, and in that sense,

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

but I don't much like the example and precedence this function sets.


> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++-
>  drivers/gpu/drm/i915/display/intel_display.h | 1 +
>  drivers/gpu/drm/i915/display/intel_dp.c      | 3 +--
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b24784c4522d..5559688047b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2798,6 +2798,11 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
>  	return 0;
>  }
>  
> +bool has_double_buffered_m_n(struct drm_i915_private *i915)
> +{
> +	return DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915);
> +}
> +
>  static void
>  intel_reduce_m_n_ratio(u32 *num, u32 *den)
>  {
> @@ -5900,7 +5905,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  	PIPE_CONF_CHECK_I(lane_count);
>  	PIPE_CONF_CHECK_X(lane_lat_optim_mask);
>  
> -	if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) {
> +	if (has_double_buffered_m_n(dev_priv)) {
>  		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
>  	} else {
>  		PIPE_CONF_CHECK_M_N(dp_m_n);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 2feb8ae5d5d4..44c88aadfc30 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -543,6 +543,7 @@ int intel_atomic_add_affected_planes(struct intel_atomic_state *state,
>  				     struct intel_crtc *crtc);
>  u8 intel_calc_active_pipes(struct intel_atomic_state *state,
>  			   u8 active_pipes);
> +bool has_double_buffered_m_n(struct drm_i915_private *i915);
>  void intel_link_compute_m_n(u16 bpp, int nlanes,
>  			    int pixel_clock, int link_clock,
>  			    struct intel_link_m_n *m_n,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 2fac76bcf06d..75645508080a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1842,8 +1842,7 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
>  static bool cpu_transcoder_has_drrs(struct drm_i915_private *i915,
>  				    enum transcoder cpu_transcoder)
>  {
> -	/* M1/N1 is double buffered */
> -	if (DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915))
> +	if (has_double_buffered_m_n(i915))
>  		return true;
>  
>  	return intel_cpu_transcoder_has_m2_n2(i915, cpu_transcoder);

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH v2 03/16] drm/i915: Extract has_double_buffered_m_n()
  2022-06-20  9:05   ` Jani Nikula
@ 2022-06-20 17:05     ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2022-06-20 17:05 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Mon, Jun 20, 2022 at 12:05:42PM +0300, Jani Nikula wrote:
> On Fri, 17 Jun 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We have a couple of places that want to make distinction between
> > double buffered M/N registers vs. the split M1/N1+M2/N2 registers.
> > Add a helper for that.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Mhh. So why a function in intel_display.c instead of a macro in
> i915_drv.h? Both are kind of cluttered, but at least in i915_drv.h it
> would be among others.

Doesn't feel quite right to me to put low level details like this
into the i915 feature stuff. I'd rather keep it next to the
implementation so I have all the relevant details in one place.

For higher level stuff and features named in bspec the HAS_FOO()
stuff feels more appropriate to me.

But maybe that's just me.

> 
> I do think we should have a separate file for display feature check
> macros, and move most if not all of the display related HAS_*() stuff
> there from i915_drv.h.
> 
> So technically this does what it says on the box, and in that sense,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> but I don't much like the example and precedence this function sets.
> 
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++-
> >  drivers/gpu/drm/i915/display/intel_display.h | 1 +
> >  drivers/gpu/drm/i915/display/intel_dp.c      | 3 +--
> >  3 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index b24784c4522d..5559688047b3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -2798,6 +2798,11 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
> >  	return 0;
> >  }
> >  
> > +bool has_double_buffered_m_n(struct drm_i915_private *i915)
> > +{
> > +	return DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915);
> > +}
> > +
> >  static void
> >  intel_reduce_m_n_ratio(u32 *num, u32 *den)
> >  {
> > @@ -5900,7 +5905,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> >  	PIPE_CONF_CHECK_I(lane_count);
> >  	PIPE_CONF_CHECK_X(lane_lat_optim_mask);
> >  
> > -	if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) {
> > +	if (has_double_buffered_m_n(dev_priv)) {
> >  		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
> >  	} else {
> >  		PIPE_CONF_CHECK_M_N(dp_m_n);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index 2feb8ae5d5d4..44c88aadfc30 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -543,6 +543,7 @@ int intel_atomic_add_affected_planes(struct intel_atomic_state *state,
> >  				     struct intel_crtc *crtc);
> >  u8 intel_calc_active_pipes(struct intel_atomic_state *state,
> >  			   u8 active_pipes);
> > +bool has_double_buffered_m_n(struct drm_i915_private *i915);
> >  void intel_link_compute_m_n(u16 bpp, int nlanes,
> >  			    int pixel_clock, int link_clock,
> >  			    struct intel_link_m_n *m_n,
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 2fac76bcf06d..75645508080a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1842,8 +1842,7 @@ intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
> >  static bool cpu_transcoder_has_drrs(struct drm_i915_private *i915,
> >  				    enum transcoder cpu_transcoder)
> >  {
> > -	/* M1/N1 is double buffered */
> > -	if (DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915))
> > +	if (has_double_buffered_m_n(i915))
> >  		return true;
> >  
> >  	return intel_cpu_transcoder_has_m2_n2(i915, cpu_transcoder);
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel

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

end of thread, other threads:[~2022-06-20 17:05 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17 16:04 [Intel-gfx] [PATCH v2 00/16] drm/i915: Make fastset not suck and allow seamless M/N changes Ville Syrjala
2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 01/16] drm/i915: Relocate intel_crtc_dotclock() Ville Syrjala
2022-06-20  9:01   ` Jani Nikula
2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 02/16] drm/i915: Shuffle some PLL code around Ville Syrjala
2022-06-20  9:01   ` Jani Nikula
2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 03/16] drm/i915: Extract has_double_buffered_m_n() Ville Syrjala
2022-06-20  9:05   ` Jani Nikula
2022-06-20 17:05     ` Ville Syrjälä
2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 04/16] drm/i915: Do .crtc_compute_clock() earlier Ville Syrjala
2022-06-17 16:04 ` [Intel-gfx] [PATCH v2 05/16] drm/i915: Reassign DPLLs only for crtcs going throug .compute_config() Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 06/16] drm/i915: Feed the DPLL output freq back into crtc_state Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 07/16] drm/i915: Compute clocks earlier Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 08/16] drm/i915: Make M/N checks non-fuzzy Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 09/16] drm/i915: Make all clock " Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 10/16] drm/i915: Set active dpll early for icl+ Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 11/16] drm/i915: Nuke fastet state copy hacks Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 12/16] drm/i915: Skip intel_modeset_pipe_config_late() if the pipe is not enabled Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 13/16] drm/i915: Add intel_panel_highest_mode() Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 14/16] drm/i915: Allow M/N change during fastset on bdw+ Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 15/16] drm/i915: Use a fixed N value always Ville Syrjala
2022-06-17 16:05 ` [Intel-gfx] [PATCH v2 16/16] drm/i915: Round TMDS clock to nearest Ville Syrjala
2022-06-17 19:49 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Make fastset not suck and allow seamless M/N changes (rev5) Patchwork
2022-06-17 20:10 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.