Re: [PATCH v3] drm/i915/bxt: BUNs related to port PLL

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Jul 06, 2015 at 01:38:44PM +0530, Kannan, Vandana wrote:
> Hi Daniel,
> 
> Is there any other change required in this patch to consider before merge?
> Please let me know.

Oh I was kinda waiting for an ack from Imre. Merged now anyway.

Btw for next time around please spell out BUN - not everyone knows what
this means, especially outside of intel gfx teams (product teams,
integration, customers, ...). And we write changelogs also for those
external people. Maybe "Implement latest Bspec updates for ..." or
similar. I've changed the summary for this patch.
-Daniel

> 
> - Vandana
> 
> On 7/3/2015 10:23 AM, Kannan, Vandana wrote:
> >Hi,
> >
> >Any other review comments on this patch? Do let me know.
> >Siva and Sonika have given their R-b.
> >
> >Thanks,
> >Vandana
> >
> >On 7/1/2015 4:41 PM, Sivakumar Thulasimani wrote:
> >>thanks for the changes.
> >>
> >>Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >>
> >>
> >>
> >>On 7/1/2015 5:02 PM, Vandana Kannan wrote:
> >>>This patch contains changes based on 2 updates to the spec:
> >>>Port PLL VCO restriction raised up to 6700.
> >>>Port PLL now needs DCO amp override enable for all VCO frequencies.
> >>>
> >>>v2: Sonika's review comment addressed
> >>>    - dcoampovr_en_h variable not required
> >>>Based on a discussion with Siva, the following changes have been made.
> >>>    - replace dco_amp var with #define BXT_DCO_AMPLITUDE
> >>>    - set pll10 in a single assignment
> >>>
> >>>v3:
> >>>Move DCO amplitude default value to i915_reg.h. Suggested by Siva.
> >>>
> >>>Signed-off-by: Vandana Kannan<vandana.kannan@intel.com>
> >>>Reviewed-by: Sonika Jindal<sonika.jindal@intel.com>  [v2]
> >>>---
> >>>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
> >>>  drivers/gpu/drm/i915/intel_ddi.c     | 15 +++++----------
> >>>  drivers/gpu/drm/i915/intel_display.c |  2 +-
> >>>  3 files changed, 7 insertions(+), 11 deletions(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >>>b/drivers/gpu/drm/i915/i915_reg.h
> >>>index ac985c5..d1b8928 100644
> >>>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>>@@ -1211,6 +1211,7 @@ enum skl_disp_power_wells {
> >>>  #define  PORT_PLL_LOCK_THRESHOLD_MASK    0xe
> >>>  /* PORT_PLL_10_A */
> >>>  #define  PORT_PLL_DCO_AMP_OVR_EN_H    (1<<27)
> >>>+#define  PORT_PLL_DCO_AMP_DEFAULT    15
> >>>  #define  PORT_PLL_DCO_AMP_MASK        0x3c00
> >>>  #define  PORT_PLL_DCO_AMP(x)        (x<<10)
> >>>  #define _PORT_PLL_BASE(port)        _PORT3(port, _PORT_PLL_0_A,    \
> >>>diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> >>>b/drivers/gpu/drm/i915/intel_ddi.c
> >>>index 42c1487..9c05cc0 100644
> >>>--- a/drivers/gpu/drm/i915/intel_ddi.c
> >>>+++ b/drivers/gpu/drm/i915/intel_ddi.c
> >>>@@ -1455,7 +1455,7 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> >>>      struct bxt_clk_div clk_div = {0};
> >>>      int vco = 0;
> >>>      uint32_t prop_coef, int_coef, gain_ctl, targ_cnt;
> >>>-    uint32_t dcoampovr_en_h, dco_amp, lanestagger;
> >>>+    uint32_t lanestagger;
> >>>
> >>>      if (intel_encoder->type == INTEL_OUTPUT_HDMI) {
> >>>          intel_clock_t best_clock;
> >>>@@ -1494,9 +1494,7 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> >>>          vco = clock * 10 / 2 * clk_div.p1 * clk_div.p2;
> >>>      }
> >>>
> >>>-    dco_amp = 15;
> >>>-    dcoampovr_en_h = 0;
> >>>-    if (vco >= 6200000 && vco <= 6480000) {
> >>>+    if (vco >= 6200000 && vco <= 6700000) {
> >>>          prop_coef = 4;
> >>>          int_coef = 9;
> >>>          gain_ctl = 3;
> >>>@@ -1507,8 +1505,6 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> >>>          int_coef = 11;
> >>>          gain_ctl = 3;
> >>>          targ_cnt = 9;
> >>>-        if (vco >= 4800000 && vco < 5400000)
> >>>-            dcoampovr_en_h = 1;
> >>>      } else if (vco == 5400000) {
> >>>          prop_coef = 3;
> >>>          int_coef = 8;
> >>>@@ -1550,10 +1546,9 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
> >>>
> >>>      crtc_state->dpll_hw_state.pll8 = targ_cnt;
> >>>
> >>>-    if (dcoampovr_en_h)
> >>>-        crtc_state->dpll_hw_state.pll10 = PORT_PLL_DCO_AMP_OVR_EN_H;
> >>>-
> >>>-    crtc_state->dpll_hw_state.pll10 |= PORT_PLL_DCO_AMP(dco_amp);
> >>>+    crtc_state->dpll_hw_state.pll10 =
> >>>+        PORT_PLL_DCO_AMP(PORT_PLL_DCO_AMP_DEFAULT)
> >>>+        | PORT_PLL_DCO_AMP_OVR_EN_H;
> >>>
> >>>      crtc_state->dpll_hw_state.pcsdw12 =
> >>>          LANESTAGGER_STRAP_OVRD | lanestagger;
> >>>diff --git a/drivers/gpu/drm/i915/intel_display.c
> >>>b/drivers/gpu/drm/i915/intel_display.c
> >>>index eb665d7..e04be45 100644
> >>>--- a/drivers/gpu/drm/i915/intel_display.c
> >>>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>>@@ -409,7 +409,7 @@ static const intel_limit_t intel_limits_chv = {
> >>>  static const intel_limit_t intel_limits_bxt = {
> >>>      /* FIXME: find real dot limits */
> >>>      .dot = { .min = 0, .max = INT_MAX },
> >>>-    .vco = { .min = 4800000, .max = 6480000 },
> >>>+    .vco = { .min = 4800000, .max = 6700000 },
> >>>      .n = { .min = 1, .max = 1 },
> >>>      .m1 = { .min = 2, .max = 2 },
> >>>      /* FIXME: find real m2 limits */
> >>
> >>--
> >>regards,
> >>Sivakumar
> >>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx