On Wed, 08 Aug 2012, Paulo Zanoni <[email protected]> wrote:
> From: Paulo Zanoni <[email protected]>
>
> The current situation is: we use WR PLL1 for everything, so if we have
> 2 HDMI outputs they will share the same PLL. As a consequence, when
> you set a mode on HDMI2, HDMI1 will change its refresh rate. If the
> modes are too different, setting a mode on HDMI2 may make the HDMI1
> screen blank (with one of those error messages from your monitor).
>
> So now we at least try to use WR PLL 2. This will improve the case
> where we have 2 HDMI outputs and don't keep crazily changing ports,
> but it's still not the perfect solution:
>
> - We need to select PORT_CLK_SEL_NONE when we stop using a port, so we
> will be able to reuse its PLL.
> - We need to move the whole DDI PLL selection code to a place where we
> can properly fail and return an error code, possibly undoing the
> mode set. Right now, instead of failing we're hijacking WR PLL 2.
>
> This patch is not the perfect solution, but it's at least better than
> the current one. Future patches will fix the remaining problems.
>
> Signed-off-by: Paulo Zanoni <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 41
> ++++++++++++++++++++++++++++++++++------
> 1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index db242cf..80b9902 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -659,7 +659,10 @@ void intel_ddi_mode_set(struct drm_encoder *encoder,
> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> int port = intel_hdmi->ddi_port;
> int pipe = intel_crtc->pipe;
> - int p, n2, r2;
> + uint32_t pll_reg[] = {WRPLL_CTL1, WRPLL_CTL2};
> + uint32_t pll_sel[] = {PORT_CLK_SEL_WRPLL1, PORT_CLK_SEL_WRPLL2};
> + int p, n2, r2, pll;
> + bool used;
> u32 temp, i;
>
> /* On Haswell, we need to enable the clocks and prepare DDI function to
> @@ -667,6 +670,35 @@ void intel_ddi_mode_set(struct drm_encoder *encoder,
> */
> DRM_DEBUG_KMS("Preparing HDMI DDI mode for Haswell on port %c, pipe
> %c\n", port_name(port), pipe_name(pipe));
>
> + for (pll = 0; pll < 2; pll++) {
Personally I'd prefer to use ARRAY_SIZE() even for small arrays like
this. YMMV.
> + used = false;
> + for (i = PORT_A; i <= PORT_E; i++) {
> + if (i == port)
> + continue;
> +
> + if (I915_READ(PORT_CLK_SEL(i)) == pll_sel[pll]) {
> + used = true;
> + break;
> + }
> + }
> + if (!used)
> + break;
> + }
I wonder if the logic (esp. the use of the "used" variable) could be
simplified by putting the inner for loop into a function. But this is
just bikeshedding, really.
> + if (pll == 2) {
> + /* FIXME: just claiming failure and returning from this function
> + * won't help us at all. So instead we hijack WR PLL 2 and hope
> + * we don't break the other output (if the refresh rates are
> + * similar we may survive). We should definitely move the PLL
> + * code to somewhere else, where we may be able to properly
> + * fail. Also, we should write code to select PORT_CLK_SEL_NONE
> + * when we stop using the port, so other ports will be able to
> + * reuse the WR PLL. */
> + DRM_ERROR("No WR PLL available\n");
> + pll = 1;
> + }
> +
> + DRM_DEBUG_KMS("Using WR PLL %d\n", pll+1);
> +
> for (i = 0; i < ARRAY_SIZE(wrpll_tmds_clock_table); i++)
> if (crtc->mode.clock <= wrpll_tmds_clock_table[i].clock)
> break;
> @@ -688,9 +720,7 @@ void intel_ddi_mode_set(struct drm_encoder *encoder,
> I915_WRITE(LCPLL_CTL,
> temp & ~LCPLL_PLL_DISABLE);
>
> - /* Configure WR PLL 1, program the correct divider values for
> - * the desired frequency and wait for warmup */
> - I915_WRITE(WRPLL_CTL1,
> + I915_WRITE(pll_reg[pll],
> WRPLL_PLL_ENABLE |
> WRPLL_PLL_SELECT_LCPLL_2700 |
> WRPLL_DIVIDER_REFERENCE(r2) |
> @@ -702,8 +732,7 @@ void intel_ddi_mode_set(struct drm_encoder *encoder,
> /* Use WRPLL1 clock to drive the output to the port, and tell the pipe
> to use
> * this port for connection.
> */
> - I915_WRITE(PORT_CLK_SEL(port),
> - PORT_CLK_SEL_WRPLL1);
> + I915_WRITE(PORT_CLK_SEL(port), pll_sel[pll]);
> I915_WRITE(PIPE_CLK_SEL(pipe),
> PIPE_CLK_SEL_PORT(port));
>
> --
> 1.7.11.2
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
