On Wed, Sep 28, 2016 at 02:32:19PM +0300, Ander Conselvan De Oliveira wrote: > On Mon, 2016-09-26 at 11:30 +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > DPLL_SDVO_HIGH_SPEED must be set for SDVO/HDMI/DP, but nowhere is it > > forbidden to set it for LVDS/CRT as well. So let's also set it on > > CRT to make it possible to share the DPLL between HDMI and CRT. > > > > What that bit apparently does is enable the x5 clock to the port, > > which then pumps out the bits on both edges of the clock. The DAC > > doesn't need that clock since it's not pumping out bits, but I don't > > think it hurts to have the DPLL output that clock anyway. > > > > This is fairly important on IVB since it has only two DPLLs with three > > pipes. So trying to drive three or more PCH ports with three pipes > > is only possible when at least one of the DPLLs gets shared between > > two of the pipes. > > > > SNB doesn't really need to do this since it has only two pipes. It could > > be done to avoid enabling the second DPLL at all in certain cases, but > > I'm not sure that's such a huge win. So let's not do it for SNB, at > > least for now. On ILK it never makes sense as the DPLLs can't be shared. > > > > v2: Just always enable the high speed clock to keep things simple (Daniel) > > Beef up the commit message a bit (Daniel) > > > > Cc: Nick Yamane <nick.diego@xxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Cc: stable@xxxxxxxxxxxxxxx > > Tested-by: Nick Yamane <nick.diego@xxxxxxxxx> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97204 > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index e5ad1010c8b1..45ff5007544c 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -9512,6 +9512,24 @@ static void ironlake_compute_dpll(struct intel_crtc > > *intel_crtc, > > if (intel_crtc_has_dp_encoder(crtc_state)) > > dpll |= DPLL_SDVO_HIGH_SPEED; > > > > + /* > > + * The high speed IO clock is only really required for > > + * SDVO/HDMI/DP, but we also enable it for CRT to make it > > + * possible to share the DPLL between CRT and HDMI. Enabling > > + * the clock needlessly does no real harm, except use up a > > + * bit of power potentially. > > I guess we could have a smarter way to check if two configurations are > compatible than the current memcmp(). I.e., a platform hook that takes two PLL > configs and either returns a merged configuration that satisfy both or signal > failure. That way we could only enable the high speed clock for CRT if really > necessary. > > But meh, sounds like too much work for very little gain. > > The documentation indeed doesn't say anything about not enabling this with CRT, > so > > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@xxxxxxxxx> Thanks. Pushed to dinq. > > > + * > > + * We'll limit this to IVB with 3 pipes, since it has only two > > + * DPLLs and so DPLL sharing is the only way to get three pipes > > + * driving PCH ports at the same time. On SNB we could do this, > > + * and potentially avoid enabling the second DPLL, but it's not > > + * clear if it''s a win or loss power wise. No point in doing > > + * this on ILK at all since it has a fixed DPLL<->pipe mapping. > > + */ > > + if (INTEL_INFO(dev_priv)->num_pipes == 3 && > > + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) > > + dpll |= DPLL_SDVO_HIGH_SPEED; > > + > > /* compute bitmask from p1 value */ > > dpll |= (1 << (crtc_state->dpll.p1 - 1)) << > > DPLL_FPA01_P1_POST_DIV_SHIFT; > > /* also FPA1 */ -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
