| Message ID | 1344446134-3704-9-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 08 Aug 2012, Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > 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 <paulo.r.zanoni@intel.com> > --- > 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 > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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++) { + 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; + } + 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));