[Intel-gfx] [PATCH] drm/i915: Fix TV encoder clock computation

[Ville Syrjala]

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

The TV encoder has its own special clocking strategy,
which means we can't just use intel_crtc_dotclock() to
figure out what the resulting dotclock will be given
the actual DPLL port_clock. Additionally the DPLL can't
always generate exactly the frequency we initially asked
for. This results in us computing a bogus dotclock/etc.,
and it won't match the readout which is handled by the
encoder itself properly. Naturally the state checker
becomes unhappy with the mismatch.

To do this sanely we'll need to move the DPLL computation
into encoder->compute_config() so that all the derived
state gets correctly computed based on the actual DPLL
output frequency. Start doing that just for the TV encoder
initally as intel_crtc_dotclock() should be able to handle
other encoder types well enough. Though eventually this
should be done for all encoder types rather than
doing it from intel_crtc_compute_config().

With this we actually do some of the DPLL state computation
twice, but we can skip the second actual .find_dpll() search
by flagging .clock_set=true after we've done it once. We also
still need to avoid clobbering the correct
adjusted_mode.crtc_clock set up by encoder->compute_config()
when called a second time from intel_crtc_compute_config().

Fixes: 665a7b04092c ("drm/i915: Feed the DPLL output freq back into crtc_state")
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll.c |  8 ++++++--
 drivers/gpu/drm/i915/display/intel_tv.c   | 23 +++++++++++++++++------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c
index 52f2fe1735da..b15ba78d64d6 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll.c
@@ -1302,7 +1302,9 @@ static int g4x_crtc_compute_clock(struct intel_atomic_state *state,
 			  &crtc_state->dpll);
 
 	crtc_state->port_clock = crtc_state->dpll.dot;
-	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+	/* FIXME this is a mess */
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_TVOUT))
+		crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
 
 	return 0;
 }
@@ -1374,7 +1376,9 @@ static int i9xx_crtc_compute_clock(struct intel_atomic_state *state,
 			  &crtc_state->dpll);
 
 	crtc_state->port_clock = crtc_state->dpll.dot;
-	crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
+	/* FIXME this is a mess */
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_TVOUT))
+		crtc_state->hw.adjusted_mode.crtc_clock = intel_crtc_dotclock(crtc_state);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_tv.c b/drivers/gpu/drm/i915/display/intel_tv.c
index 9379f3463344..dcf89d701f0f 100644
--- a/drivers/gpu/drm/i915/display/intel_tv.c
+++ b/drivers/gpu/drm/i915/display/intel_tv.c
@@ -39,6 +39,7 @@
 #include "intel_crtc.h"
 #include "intel_de.h"
 #include "intel_display_types.h"
+#include "intel_dpll.h"
 #include "intel_hotplug.h"
 #include "intel_tv.h"
 
@@ -982,10 +983,10 @@ intel_tv_mode_vdisplay(const struct tv_mode *tv_mode)
 
 static void
 intel_tv_mode_to_mode(struct drm_display_mode *mode,
-		      const struct tv_mode *tv_mode)
+		      const struct tv_mode *tv_mode,
+		      int clock)
 {
-	mode->clock = tv_mode->clock /
-		(tv_mode->oversample >> !tv_mode->progressive);
+	mode->clock = clock / (tv_mode->oversample >> !tv_mode->progressive);
 
 	/*
 	 * tv_mode horizontal timings:
@@ -1143,7 +1144,7 @@ intel_tv_get_config(struct intel_encoder *encoder,
 	xsize = tmp >> 16;
 	ysize = tmp & 0xffff;
 
-	intel_tv_mode_to_mode(&mode, &tv_mode);
+	intel_tv_mode_to_mode(&mode, &tv_mode, pipe_config->port_clock);
 
 	drm_dbg_kms(&dev_priv->drm, "TV mode: " DRM_MODE_FMT "\n",
 		    DRM_MODE_ARG(&mode));
@@ -1184,6 +1185,9 @@ intel_tv_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_state *pipe_config,
 			struct drm_connector_state *conn_state)
 {
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(pipe_config->uapi.state);
+	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_tv_connector_state *tv_conn_state =
 		to_intel_tv_connector_state(conn_state);
@@ -1192,6 +1196,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
 		&pipe_config->hw.adjusted_mode;
 	int hdisplay = adjusted_mode->crtc_hdisplay;
 	int vdisplay = adjusted_mode->crtc_vdisplay;
+	int ret;
 
 	if (!tv_mode)
 		return -EINVAL;
@@ -1206,7 +1211,13 @@ intel_tv_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->port_clock = tv_mode->clock;
 
-	intel_tv_mode_to_mode(adjusted_mode, tv_mode);
+	ret = intel_dpll_crtc_compute_clock(state, crtc);
+	if (ret)
+		return ret;
+
+	pipe_config->clock_set = true;
+
+	intel_tv_mode_to_mode(adjusted_mode, tv_mode, pipe_config->port_clock);
 	drm_mode_set_crtcinfo(adjusted_mode, 0);
 
 	if (intel_tv_source_too_wide(dev_priv, hdisplay) ||
@@ -1804,7 +1815,7 @@ intel_tv_get_modes(struct drm_connector *connector)
 		 * about the actual timings of the mode. We
 		 * do ignore the margins though.
 		 */
-		intel_tv_mode_to_mode(mode, tv_mode);
+		intel_tv_mode_to_mode(mode, tv_mode, tv_mode->clock);
 		if (count == 0) {
 			drm_dbg_kms(&dev_priv->drm, "TV mode: " DRM_MODE_FMT "\n",
 				    DRM_MODE_ARG(mode));
-- 
2.35.1