| Message ID | 1497644132-30887-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Sex, 2017-06-16 às 13:15 -0700, Rodrigo Vivi escreveu: > Paulo noticed that we were missing few bits clear > before writing values back to the register on > these RMW MMIO operations. > > Fixes: cf54ca8bc567 ("drm/i915/cnl: Implement voltage swing > sequence.") > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++ > drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++ > 2 files changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index bd535f1..d4edc79 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1764,8 +1764,11 @@ enum skl_disp_power_wells { > _CNL_PORT_TX_DW2 > _LN0_AE, \ > _CNL_PORT_TX_DW2 > _LN0_F) > #define SWING_SEL_UPPER(x) ((x >> 3) << 15) > +#define SWING_SEL_UPPER_MASK (1 << 15) > #define SWING_SEL_LOWER(x) ((x & 0x7) << 11) > +#define SWING_SEL_LOWER_MASK (0x7 << 11) > #define RCOMP_SCALAR(x) ((x) << 0) > +#define RCOMP_SCALAR_MASK (0xFF << 0) > > #define _CNL_PORT_TX_DW4_GRP_AE 0x162350 > #define _CNL_PORT_TX_DW4_GRP_B 0x1623D0 > @@ -1795,8 +1798,11 @@ enum skl_disp_power_wells { > _CNL_PORT_TX_DW4 > _LN0_F) > #define LOADGEN_SELECT (1 << 31) > #define POST_CURSOR_1(x) ((x) << 12) > +#define POST_CURSOR_1_MASK (0x3F << 12) > #define POST_CURSOR_2(x) ((x) << 6) > +#define POST_CURSOR_2_MASK (0x3F << 6) > #define CURSOR_COEFF(x) ((x) << 0) > +#define POST_CURSOR_COEFF_MASK (0x3F << 6) s/POST_CURSOR_COEFF_MASK/CURSOR_COEFF_MASK/, there's no "post" in the name of this field. With that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> As a side note, I really think all these CNL definitions lack the name of the registers as prefix. I worry about future namespace collision. But that's a discussion for another patch. > > #define _CNL_PORT_TX_DW5_GRP_AE 0x162354 > #define _CNL_PORT_TX_DW5_GRP_B 0x1623D4 > @@ -1825,7 +1831,9 @@ enum skl_disp_power_wells { > #define TX_TRAINING_EN (1 << 31) > #define TAP3_DISABLE (1 << 29) > #define SCALING_MODE_SEL(x) ((x) << 18) > +#define SCALING_MODE_SEL_MASK (0x7 << 18) > #define RTERM_SELECT(x) ((x) << 3) > +#define RTERM_SELECT_MASK (0x7 << 3) > > #define _CNL_PORT_TX_DW7_GRP_AE 0x16235C > #define _CNL_PORT_TX_DW7_GRP_B 0x1623DC > @@ -1852,6 +1860,7 @@ enum skl_disp_power_wells { > _CNL_PORT_TX_DW7 > _LN0_AE, \ > _CNL_PORT_TX_DW7 > _LN0_F) > #define N_SCALAR(x) ((x) << 24) > +#define N_SCALAR_MASK (0x7F << 24) > > /* The spec defines this only for BXT PHY0, but lets assume that > this > * would exist for PHY1 too if it had a second channel. > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index db80938..e66947a 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1813,11 +1813,14 @@ static void cnl_ddi_vswing_program(struct > drm_i915_private *dev_priv, > > /* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */ > val = I915_READ(CNL_PORT_TX_DW5_LN0(port)); > + val &= ~(SCALING_MODE_SEL_MASK); > val |= SCALING_MODE_SEL(2); > I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val); > > /* Program PORT_TX_DW2 */ > val = I915_READ(CNL_PORT_TX_DW2_LN0(port)); > + val &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK | > + RCOMP_SCALAR_MASK); > val |= > SWING_SEL_UPPER(ddi_translations[level].dw2_swing_sel); > val |= > SWING_SEL_LOWER(ddi_translations[level].dw2_swing_sel); > /* Rcomp scalar is fixed as 0x98 for every table entry */ > @@ -1828,6 +1831,8 @@ static void cnl_ddi_vswing_program(struct > drm_i915_private *dev_priv, > /* We cannot write to GRP. It would overrite individual > loadgen */ > for (ln = 0; ln < 4; ln++) { > val = I915_READ(CNL_PORT_TX_DW4_LN(port, ln)); > + val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK | > + POST_CURSOR_COEFF_MASK); > val |= > POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1); > val |= > POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2); > val |= > CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff); > @@ -1837,12 +1842,14 @@ static void cnl_ddi_vswing_program(struct > drm_i915_private *dev_priv, > /* Program PORT_TX_DW5 */ > /* All DW5 values are fixed for every table entry */ > val = I915_READ(CNL_PORT_TX_DW5_LN0(port)); > + val &= ~(RTERM_SELECT_MASK); > val |= RTERM_SELECT(6); > val |= TAP3_DISABLE; > I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val); > > /* Program PORT_TX_DW7 */ > val = I915_READ(CNL_PORT_TX_DW7_LN0(port)); > + val &= ~N_SCALAR_MASK; > val |= N_SCALAR(ddi_translations[level].dw7_n_scalar); > I915_WRITE(CNL_PORT_TX_DW7_GRP(port), val); > }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index bd535f1..d4edc79 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1764,8 +1764,11 @@ enum skl_disp_power_wells { _CNL_PORT_TX_DW2_LN0_AE, \ _CNL_PORT_TX_DW2_LN0_F) #define SWING_SEL_UPPER(x) ((x >> 3) << 15) +#define SWING_SEL_UPPER_MASK (1 << 15) #define SWING_SEL_LOWER(x) ((x & 0x7) << 11) +#define SWING_SEL_LOWER_MASK (0x7 << 11) #define RCOMP_SCALAR(x) ((x) << 0) +#define RCOMP_SCALAR_MASK (0xFF << 0) #define _CNL_PORT_TX_DW4_GRP_AE 0x162350 #define _CNL_PORT_TX_DW4_GRP_B 0x1623D0 @@ -1795,8 +1798,11 @@ enum skl_disp_power_wells { _CNL_PORT_TX_DW4_LN0_F) #define LOADGEN_SELECT (1 << 31) #define POST_CURSOR_1(x) ((x) << 12) +#define POST_CURSOR_1_MASK (0x3F << 12) #define POST_CURSOR_2(x) ((x) << 6) +#define POST_CURSOR_2_MASK (0x3F << 6) #define CURSOR_COEFF(x) ((x) << 0) +#define POST_CURSOR_COEFF_MASK (0x3F << 6) #define _CNL_PORT_TX_DW5_GRP_AE 0x162354 #define _CNL_PORT_TX_DW5_GRP_B 0x1623D4 @@ -1825,7 +1831,9 @@ enum skl_disp_power_wells { #define TX_TRAINING_EN (1 << 31) #define TAP3_DISABLE (1 << 29) #define SCALING_MODE_SEL(x) ((x) << 18) +#define SCALING_MODE_SEL_MASK (0x7 << 18) #define RTERM_SELECT(x) ((x) << 3) +#define RTERM_SELECT_MASK (0x7 << 3) #define _CNL_PORT_TX_DW7_GRP_AE 0x16235C #define _CNL_PORT_TX_DW7_GRP_B 0x1623DC @@ -1852,6 +1860,7 @@ enum skl_disp_power_wells { _CNL_PORT_TX_DW7_LN0_AE, \ _CNL_PORT_TX_DW7_LN0_F) #define N_SCALAR(x) ((x) << 24) +#define N_SCALAR_MASK (0x7F << 24) /* The spec defines this only for BXT PHY0, but lets assume that this * would exist for PHY1 too if it had a second channel. diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index db80938..e66947a 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1813,11 +1813,14 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv, /* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */ val = I915_READ(CNL_PORT_TX_DW5_LN0(port)); + val &= ~(SCALING_MODE_SEL_MASK); val |= SCALING_MODE_SEL(2); I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val); /* Program PORT_TX_DW2 */ val = I915_READ(CNL_PORT_TX_DW2_LN0(port)); + val &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK | + RCOMP_SCALAR_MASK); val |= SWING_SEL_UPPER(ddi_translations[level].dw2_swing_sel); val |= SWING_SEL_LOWER(ddi_translations[level].dw2_swing_sel); /* Rcomp scalar is fixed as 0x98 for every table entry */ @@ -1828,6 +1831,8 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv, /* We cannot write to GRP. It would overrite individual loadgen */ for (ln = 0; ln < 4; ln++) { val = I915_READ(CNL_PORT_TX_DW4_LN(port, ln)); + val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK | + POST_CURSOR_COEFF_MASK); val |= POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1); val |= POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2); val |= CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff); @@ -1837,12 +1842,14 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv, /* Program PORT_TX_DW5 */ /* All DW5 values are fixed for every table entry */ val = I915_READ(CNL_PORT_TX_DW5_LN0(port)); + val &= ~(RTERM_SELECT_MASK); val |= RTERM_SELECT(6); val |= TAP3_DISABLE; I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val); /* Program PORT_TX_DW7 */ val = I915_READ(CNL_PORT_TX_DW7_LN0(port)); + val &= ~N_SCALAR_MASK; val |= N_SCALAR(ddi_translations[level].dw7_n_scalar); I915_WRITE(CNL_PORT_TX_DW7_GRP(port), val); }
Paulo noticed that we were missing few bits clear before writing values back to the register on these RMW MMIO operations. Fixes: cf54ca8bc567 ("drm/i915/cnl: Implement voltage swing sequence.") Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 9 +++++++++ drivers/gpu/drm/i915/intel_ddi.c | 7 +++++++ 2 files changed, 16 insertions(+)