From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs Date: Fri, 13 Sep 2013 16:54:18 +0300 Message-ID: <20130913135418.GB4531@intel.com> References: <20130908123500.GZ27291@phenom.ffwll.local> <1378724797-8571-1-git-send-email-ville.syrjala@linux.intel.com> <87eh8sna9o.fsf@intel.com> <20130913130632.GT20128@intel.com> <8738p8n88m.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id BEC16E60E3 for ; Fri, 13 Sep 2013 06:54:22 -0700 (PDT) Content-Disposition: inline In-Reply-To: <8738p8n88m.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Sep 13, 2013 at 04:47:53PM +0300, Jani Nikula wrote: > On Fri, 13 Sep 2013, Ville Syrj=E4l=E4 wr= ote: > > On Fri, Sep 13, 2013 at 04:04:03PM +0300, Jani Nikula wrote: > >> On Mon, 09 Sep 2013, ville.syrjala@linux.intel.com wrote: > >> > From: Ville Syrj=E4l=E4 > >> > > >> > Add the 120MHz refernce clock case for PCH DPLLs. > >> > > >> > Also determine the reference clock frequency more accurately by > >> > checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN refclk input > >> > mode. The gen2 code already checked it, but it stil assumed a > >> > fixed 66MHz refclk. Instead we need to consult the VBT for the > >> > real value. > >> > > >> > v2: Fix refclk for SSC panel case > >> > > >> > Signed-off-by: Ville Syrj=E4l=E4 > >> > --- > >> > drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++----= ------- > >> > 1 file changed, 21 insertions(+), 11 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/= i915/intel_display.c > >> > index 754de85..4f07292 100644 > >> > --- a/drivers/gpu/drm/i915/intel_display.c > >> > +++ b/drivers/gpu/drm/i915/intel_display.c > >> > @@ -7324,6 +7324,22 @@ void intel_release_load_detect_pipe(struct dr= m_connector *connector, > >> > mutex_unlock(&crtc->mutex); > >> > } > >> > = > >> > +static int i9xx_pll_refclk(struct drm_device *dev, > >> > + const struct intel_crtc_config *pipe_config) > >> > +{ > >> > + struct drm_i915_private *dev_priv =3D dev->dev_private; > >> > + u32 dpll =3D pipe_config->dpll_hw_state.dpll; > >> > + > >> > + if ((dpll & PLL_REF_INPUT_MASK) =3D=3D PLLB_REF_INPUT_SPREADSPECTR= UMIN) > >> = > >> This seems wrong for at least gen3 and vlv. And it's a bit scary to go > >> change gen2 but oh well... > > > > Why? i8xx_update_pll(), i9xx_update_pll() and ironlake_compute_dpll() > > all have the same logic for setting that bit. > = > My Super Reliable(tm) gen3 spec says reserved for that value > there. *shrug*. Looks like it's there only for pipe B and even then only for certain platforms. But if we managed to program it w/ an invalid value I suspect things fall apart in other ways. > = > > For VLV I agree. But the clock readout there is totally busted anyway, > > so I don't care at this point. > = > Maybe Daniel can copy-paste something along those lines in the commit > message. Or not. *shrug. :) > = > Reviewed-by: Jani Nikula > = > = > > > >> = > >> Jani. > >> = > >> > + return dev_priv->vbt.lvds_ssc_freq * 1000; > >> > + else if (HAS_PCH_SPLIT(dev)) > >> > + return 120000; > >> > + else if (!IS_GEN2(dev)) > >> > + return 96000; > >> > + else > >> > + return 48000; > >> > +} > >> > + > >> > /* Returns the clock of the currently programmed mode of the given = pipe. */ > >> > static void i9xx_crtc_clock_get(struct intel_crtc *crtc, > >> > struct intel_crtc_config *pipe_config) > >> > @@ -7334,6 +7350,7 @@ static void i9xx_crtc_clock_get(struct intel_c= rtc *crtc, > >> > u32 dpll =3D pipe_config->dpll_hw_state.dpll; > >> > u32 fp; > >> > intel_clock_t clock; > >> > + int refclk =3D i9xx_pll_refclk(dev, pipe_config); > >> > = > >> > if ((dpll & DISPLAY_RATE_SELECT_FPA1) =3D=3D 0) > >> > fp =3D pipe_config->dpll_hw_state.fp0; > >> > @@ -7373,9 +7390,9 @@ static void i9xx_crtc_clock_get(struct intel_c= rtc *crtc, > >> > } > >> > = > >> > if (IS_PINEVIEW(dev)) > >> > - pineview_clock(96000, &clock); > >> > + pineview_clock(refclk, &clock); > >> > else > >> > - i9xx_clock(96000, &clock); > >> > + i9xx_clock(refclk, &clock); > >> > } else { > >> > bool is_lvds =3D (pipe =3D=3D 1) && (I915_READ(LVDS) & LVDS_PORT_= EN); > >> > = > >> > @@ -7383,13 +7400,6 @@ static void i9xx_crtc_clock_get(struct intel_= crtc *crtc, > >> > clock.p1 =3D ffs((dpll & DPLL_FPA01_P1_POST_DIV_MASK_I830_LVDS) = >> > >> > DPLL_FPA01_P1_POST_DIV_SHIFT); > >> > clock.p2 =3D 14; > >> > - > >> > - if ((dpll & PLL_REF_INPUT_MASK) =3D=3D > >> > - PLLB_REF_INPUT_SPREADSPECTRUMIN) { > >> > - /* XXX: might not be 66MHz */ > >> > - i9xx_clock(66000, &clock); > >> > - } else > >> > - i9xx_clock(48000, &clock); > >> > } else { > >> > if (dpll & PLL_P1_DIVIDE_BY_TWO) > >> > clock.p1 =3D 2; > >> > @@ -7401,9 +7411,9 @@ static void i9xx_crtc_clock_get(struct intel_c= rtc *crtc, > >> > clock.p2 =3D 4; > >> > else > >> > clock.p2 =3D 2; > >> > - > >> > - i9xx_clock(48000, &clock); > >> > } > >> > + > >> > + i9xx_clock(refclk, &clock); > >> > } > >> > = > >> > pipe_config->adjusted_mode.clock =3D clock.dot; > >> > -- = > >> > 1.8.1.5 > >> > > >> > _______________________________________________ > >> > Intel-gfx mailing list > >> > Intel-gfx@lists.freedesktop.org > >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> = > >> -- = > >> Jani Nikula, Intel Open Source Technology Center > > > > -- = > > Ville Syrj=E4l=E4 > > Intel OTC > = > -- = > Jani Nikula, Intel Open Source Technology Center -- = Ville Syrj=E4l=E4 Intel OTC