drm/tegra: dc - Compute shift clock divider in output drivers

The shift clock divider is highly dependent on the type of output, so
push computation of it down into the output drivers. The old code used
to work merely by accident.

Signed-off-by: Thierry Reding <treding@nvidia.com>

drivers/gpu/drm/tegra/dc.c

@@ -619,7 +619,7 @@ static int tegra_dc_set_timings(struct tegra_dc *dc,
 static int tegra_crtc_setup_clk(struct drm_crtc *crtc,
 				struct drm_display_mode *mode)
 {
-	unsigned long pclk = mode->clock * 1000, rate;
+	unsigned long pclk = mode->clock * 1000;
 	struct tegra_dc *dc = to_tegra_dc(crtc);
 	struct tegra_output *output = NULL;
 	struct drm_encoder *encoder;
@@ -637,19 +637,16 @@ static int tegra_crtc_setup_clk(struct drm_crtc *crtc,
 		return -ENODEV;
 
 	/*
-	 * This assumes that the display controller will divide its parent
-	 * clock by 2 to generate the pixel clock.
+	 * This assumes that the parent clock is pll_d_out0 or pll_d2_out
+	 * respectively, each of which divides the base pll_d by 2.
 	 */
-	err = tegra_output_setup_clock(output, dc->clk, pclk * 2);
+	err = tegra_output_setup_clock(output, dc->clk, pclk, &div);
 	if (err < 0) {
 		dev_err(dc->dev, "failed to setup clock: %ld\n", err);
 		return err;
 	}
 
-	rate = clk_get_rate(dc->clk);
-	div = (rate * 2 / pclk) - 2;
-
-	DRM_DEBUG_KMS("rate: %lu, div: %u\n", rate, div);
+	DRM_DEBUG_KMS("rate: %lu, div: %u\n", clk_get_rate(dc->clk), div);
 
 	value = SHIFT_CLK_DIVIDER(div) | PIXEL_CLK_DIVIDER_PCD1;
 	tegra_dc_writel(dc, value, DC_DISP_DISP_CLOCK_CONTROL);

drivers/gpu/drm/tegra/drm.h

@@ -172,7 +172,7 @@ struct tegra_output_ops {
 	int (*enable)(struct tegra_output *output);
 	int (*disable)(struct tegra_output *output);
 	int (*setup_clock)(struct tegra_output *output, struct clk *clk,
-			   unsigned long pclk);
+			   unsigned long pclk, unsigned int *div);
 	int (*check_mode)(struct tegra_output *output,
 			  struct drm_display_mode *mode,
 			  enum drm_mode_status *status);
@@ -230,10 +230,11 @@ static inline int tegra_output_disable(struct tegra_output *output)
 }
 
 static inline int tegra_output_setup_clock(struct tegra_output *output,
-					   struct clk *clk, unsigned long pclk)
+					   struct clk *clk, unsigned long pclk,
+					   unsigned int *div)
 {
 	if (output && output->ops && output->ops->setup_clock)
-		return output->ops->setup_clock(output, clk, pclk);
+		return output->ops->setup_clock(output, clk, pclk, div);
 
 	return output ? -ENOSYS : -EINVAL;
 }

drivers/gpu/drm/tegra/dsi.c

@@ -583,26 +583,39 @@ static int tegra_output_dsi_disable(struct tegra_output *output)
 }
 
 static int tegra_output_dsi_setup_clock(struct tegra_output *output,
-					struct clk *clk, unsigned long pclk)
+					struct clk *clk, unsigned long pclk,
+					unsigned int *divp)
 {
 	struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
 	struct drm_display_mode *mode = &dc->base.mode;
 	unsigned int timeout, mul, div, vrefresh;
 	struct tegra_dsi *dsi = to_dsi(output);
 	unsigned long bclk, plld, value;
-	struct clk *base;
 	int err;
 
 	err = tegra_dsi_get_muldiv(dsi->format, &mul, &div);
 	if (err < 0)
 		return err;
 
+	DRM_DEBUG_KMS("mul: %u, div: %u, lanes: %u\n", mul, div, dsi->lanes);
 	vrefresh = drm_mode_vrefresh(mode);
+	DRM_DEBUG_KMS("vrefresh: %u\n", vrefresh);
 
+	/* compute byte clock */
 	pclk = mode->htotal * mode->vtotal * vrefresh;
 	bclk = (pclk * mul) / (div * dsi->lanes);
-	plld = DIV_ROUND_UP(bclk * 8, 1000000);
-	pclk = (plld * 1000000) / 2;
+
+	/*
+	 * Compute bit clock and round up to the next MHz.
+	 */
+	plld = DIV_ROUND_UP(bclk * 8, 1000000) * 1000000;
+
+	/*
+	 * We divide the frequency by two here, but we make up for that by
+	 * setting the shift clock divider (further below) to half of the
+	 * correct value.
+	 */
+	plld /= 2;
 
 	err = clk_set_parent(clk, dsi->clk_parent);
 	if (err < 0) {
@@ -610,19 +623,25 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output,
 		return err;
 	}
 
-	base = clk_get_parent(dsi->clk_parent);
-
-	/*
-	 * This assumes that the parent clock is pll_d_out0 or pll_d2_out
-	 * respectively, each of which divides the base pll_d by 2.
-	 */
-	err = clk_set_rate(base, pclk * 2);
+	err = clk_set_rate(dsi->clk_parent, plld);
 	if (err < 0) {
 		dev_err(dsi->dev, "failed to set base clock rate to %lu Hz\n",
-			pclk * 2);
+			plld);
 		return err;
 	}
 
+	/*
+	 * Derive pixel clock from bit clock using the shift clock divider.
+	 * Note that this is only half of what we would expect, but we need
+	 * that to make up for the fact that we divided the bit clock by a
+	 * factor of two above.
+	 *
+	 * It's not clear exactly why this is necessary, but the display is
+	 * not working properly otherwise. Perhaps the PLLs cannot generate
+	 * frequencies sufficiently high.
+	 */
+	*divp = ((8 * mul) / (div * dsi->lanes)) - 2;
+
 	/*
 	 * XXX: Move the below somewhere else so that we don't need to have
 	 * access to the vrefresh in this function?

drivers/gpu/drm/tegra/hdmi.c

@@ -978,10 +978,10 @@ static int tegra_output_hdmi_disable(struct tegra_output *output)
 }
 
 static int tegra_output_hdmi_setup_clock(struct tegra_output *output,
-					 struct clk *clk, unsigned long pclk)
+					 struct clk *clk, unsigned long pclk,
+					 unsigned int *div)
 {
 	struct tegra_hdmi *hdmi = to_hdmi(output);
-	struct clk *base;
 	int err;
 
 	err = clk_set_parent(clk, hdmi->clk_parent);
@@ -990,17 +990,12 @@ static int tegra_output_hdmi_setup_clock(struct tegra_output *output,
 		return err;
 	}
 
-	base = clk_get_parent(hdmi->clk_parent);
-
-	/*
-	 * This assumes that the parent clock is pll_d_out0 or pll_d2_out
-	 * respectively, each of which divides the base pll_d by 2.
-	 */
-	err = clk_set_rate(base, pclk * 2);
+	err = clk_set_rate(hdmi->clk_parent, pclk);
 	if (err < 0)
-		dev_err(output->dev,
-			"failed to set base clock rate to %lu Hz\n",
-			pclk * 2);
+		dev_err(output->dev, "failed to set clock rate to %lu Hz\n",
+			pclk);
+
+	*div = 0;
 
 	return 0;
 }

drivers/gpu/drm/tegra/rgb.c

@@ -159,11 +159,38 @@ static int tegra_output_rgb_disable(struct tegra_output *output)
 }
 
 static int tegra_output_rgb_setup_clock(struct tegra_output *output,
-					struct clk *clk, unsigned long pclk)
+					struct clk *clk, unsigned long pclk,
+					unsigned int *div)
 {
 	struct tegra_rgb *rgb = to_rgb(output);
+	int err;
 
-	return clk_set_parent(clk, rgb->clk_parent);
+	err = clk_set_parent(clk, rgb->clk_parent);
+	if (err < 0) {
+		dev_err(output->dev, "failed to set parent: %d\n", err);
+		return err;
+	}
+
+	/*
+	 * We may not want to change the frequency of the parent clock, since
+	 * it may be a parent for other peripherals. This is due to the fact
+	 * that on Tegra20 there's only a single clock dedicated to display
+	 * (pll_d_out0), whereas later generations have a second one that can
+	 * be used to independently drive a second output (pll_d2_out0).
+	 *
+	 * As a way to support multiple outputs on Tegra20 as well, pll_p is
+	 * typically used as the parent clock for the display controllers.
+	 * But this comes at a cost: pll_p is the parent of several other
+	 * peripherals, so its frequency shouldn't change out of the blue.
+	 *
+	 * The best we can do at this point is to use the shift clock divider
+	 * and hope that the desired frequency can be matched (or at least
+	 * matched sufficiently close that the panel will still work).
+	 */
+
+	*div = ((clk_get_rate(clk) * 2) / pclk) - 2;
+
+	return 0;
 }
 
 static int tegra_output_rgb_check_mode(struct tegra_output *output,

drivers/gpu/drm/tegra/sor.c

@@ -861,13 +861,14 @@ static int tegra_output_sor_disable(struct tegra_output *output)
 }
 
 static int tegra_output_sor_setup_clock(struct tegra_output *output,
-					struct clk *clk, unsigned long pclk)
+					struct clk *clk, unsigned long pclk,
+					unsigned int *div)
 {
 	struct tegra_sor *sor = to_sor(output);
 	int err;
 
 	/* round to next MHz */
-	pclk = DIV_ROUND_UP(pclk / 2, 1000000) * 1000000;
+	pclk = DIV_ROUND_UP(pclk, 1000000) * 1000000;
 
 	err = clk_set_parent(clk, sor->clk_parent);
 	if (err < 0) {
@@ -877,11 +878,12 @@ static int tegra_output_sor_setup_clock(struct tegra_output *output,
 
 	err = clk_set_rate(sor->clk_parent, pclk);
 	if (err < 0) {
-		dev_err(sor->dev, "failed to set base clock rate to %lu Hz\n",
-			pclk * 2);
+		dev_err(sor->dev, "failed to set clock rate to %lu Hz\n", pclk);
 		return err;
 	}
 
+	*div = 0;
+
 	return 0;
 }