| Message ID | 20171114194759.24541-3-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 14, 2017 at 11:47:54AM -0800, Rodrigo Vivi wrote: > No functional change. Just starting the wrpll fixes > with a clean-up to make units a bit more clear. > > Cc: Mika Kahola <mika.kahola@intel.com> > Cc: Manasi Navare <manasi.d.navare@intel.com> > Cc: James Ausmus <james.ausmus@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index 61c684ac47af..db7afd314462 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -2198,11 +2198,11 @@ static void cnl_wrpll_params_populate(struct skl_wrpll_params *params, uint32_t > } > > static bool > -cnl_ddi_calculate_wrpll(int clock /* in Hz */, Now the clock sent is already in KHz, should we have that in the comment either in the argument or afe_clock calculation? Apart from this nitpick, looks good. Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> > +cnl_ddi_calculate_wrpll(int clock, > struct drm_i915_private *dev_priv, > struct skl_wrpll_params *wrpll_params) > { > - uint64_t afe_clock = clock * 5 / KHz(1); /* clocks in kHz */ > + uint64_t afe_clock = clock * 5; > unsigned int dco_min = 7998 * KHz(1); > unsigned int dco_max = 10000 * KHz(1); > unsigned int dco_mid = (dco_min + dco_max) / 2; > @@ -2255,7 +2255,7 @@ static bool cnl_ddi_hdmi_pll_dividers(struct intel_crtc *crtc, > > cfgcr0 = DPLL_CFGCR0_HDMI_MODE; > > - if (!cnl_ddi_calculate_wrpll(clock * 1000, dev_priv, &wrpll_params)) > + if (!cnl_ddi_calculate_wrpll(clock, dev_priv, &wrpll_params)) > return false; > > cfgcr0 |= DPLL_CFGCR0_DCO_FRACTION(wrpll_params.dco_fraction) | > -- > 2.13.6 >
On Tue, 14 Nov 2017, Manasi Navare <manasi.d.navare@intel.com> wrote: > On Tue, Nov 14, 2017 at 11:47:54AM -0800, Rodrigo Vivi wrote: >> No functional change. Just starting the wrpll fixes >> with a clean-up to make units a bit more clear. >> >> Cc: Mika Kahola <mika.kahola@intel.com> >> Cc: Manasi Navare <manasi.d.navare@intel.com> >> Cc: James Ausmus <james.ausmus@intel.com> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c >> index 61c684ac47af..db7afd314462 100644 >> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c >> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c >> @@ -2198,11 +2198,11 @@ static void cnl_wrpll_params_populate(struct skl_wrpll_params *params, uint32_t >> } >> >> static bool >> -cnl_ddi_calculate_wrpll(int clock /* in Hz */, > > Now the clock sent is already in KHz, should we have that in the comment either > in the argument or afe_clock calculation? > Apart from this nitpick, looks good. We use kHz for clocks all over the place, IMO only any deviation from that needs to be documented. BR, Jani. > > Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> > >> +cnl_ddi_calculate_wrpll(int clock, >> struct drm_i915_private *dev_priv, >> struct skl_wrpll_params *wrpll_params) >> { >> - uint64_t afe_clock = clock * 5 / KHz(1); /* clocks in kHz */ >> + uint64_t afe_clock = clock * 5; >> unsigned int dco_min = 7998 * KHz(1); >> unsigned int dco_max = 10000 * KHz(1); >> unsigned int dco_mid = (dco_min + dco_max) / 2; >> @@ -2255,7 +2255,7 @@ static bool cnl_ddi_hdmi_pll_dividers(struct intel_crtc *crtc, >> >> cfgcr0 = DPLL_CFGCR0_HDMI_MODE; >> >> - if (!cnl_ddi_calculate_wrpll(clock * 1000, dev_priv, &wrpll_params)) >> + if (!cnl_ddi_calculate_wrpll(clock, dev_priv, &wrpll_params)) >> return false; >> >> cfgcr0 |= DPLL_CFGCR0_DCO_FRACTION(wrpll_params.dco_fraction) | >> -- >> 2.13.6 >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 61c684ac47af..db7afd314462 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2198,11 +2198,11 @@ static void cnl_wrpll_params_populate(struct skl_wrpll_params *params, uint32_t } static bool -cnl_ddi_calculate_wrpll(int clock /* in Hz */, +cnl_ddi_calculate_wrpll(int clock, struct drm_i915_private *dev_priv, struct skl_wrpll_params *wrpll_params) { - uint64_t afe_clock = clock * 5 / KHz(1); /* clocks in kHz */ + uint64_t afe_clock = clock * 5; unsigned int dco_min = 7998 * KHz(1); unsigned int dco_max = 10000 * KHz(1); unsigned int dco_mid = (dco_min + dco_max) / 2; @@ -2255,7 +2255,7 @@ static bool cnl_ddi_hdmi_pll_dividers(struct intel_crtc *crtc, cfgcr0 = DPLL_CFGCR0_HDMI_MODE; - if (!cnl_ddi_calculate_wrpll(clock * 1000, dev_priv, &wrpll_params)) + if (!cnl_ddi_calculate_wrpll(clock, dev_priv, &wrpll_params)) return false; cfgcr0 |= DPLL_CFGCR0_DCO_FRACTION(wrpll_params.dco_fraction) |
No functional change. Just starting the wrpll fixes with a clean-up to make units a bit more clear. Cc: Mika Kahola <mika.kahola@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Cc: James Ausmus <james.ausmus@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)