> -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] > Sent: Thursday, February 4, 2016 7:28 PM > To: Deepak, M <m.deepak@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: RE: [PATCH] drm/i915/bxt: Additional MIPI clock divider > form B0 stepping onwards > > On Thu, 04 Feb 2016, "Deepak, M" <m.deepak@xxxxxxxxx> wrote: > >> -----Original Message----- > >> From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] > >> Sent: Thursday, February 4, 2016 6:29 PM > >> To: Deepak, M <m.deepak@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >> Cc: Deepak, M <m.deepak@xxxxxxxxx> > >> Subject: Re: [PATCH] drm/i915/bxt: Additional MIPI clock > >> divider form B0 stepping onwards > >> > >> On Wed, 03 Feb 2016, Deepak M <m.deepak@xxxxxxxxx> wrote: > >> > The MIPI clock calculations for the addtional clock are revised > >> > from > >> > B0 stepping onwards, the bit definitions have changed compared to > >> > old stepping. > >> > > >> > v2: Fixing compilation warning. > >> > >> Why did you move and rename everything when it was not needed? > >> > >> BR, > >> Jani. > >> > > [Deepak, M] I have deleted the old macro and added the new as per the > new definitions. With the new bit fields nothing was matching as that of the > old. > > It's not nothing. Plenty of masks and shifts matched, but you had renamed > the defines. > > Besides, please don't move the definitions where they don't belong. We also > have the convention of specifying the bits from highest to lowest. > [Deepak, M] Okay, will fix the macro`s, are there any changes required in the function. > >> > >> > > >> > Signed-off-by: Deepak M <m.deepak@xxxxxxxxx> > >> > --- > >> > drivers/gpu/drm/i915/i915_reg.h | 104 +++++++++++++++++--------- > --- > >> ------ > >> > drivers/gpu/drm/i915/intel_dsi_pll.c | 64 ++++++++++++++------- > >> > 2 files changed, 95 insertions(+), 73 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h > >> > b/drivers/gpu/drm/i915/i915_reg.h index c0bd691..2568f35 100644 > >> > --- a/drivers/gpu/drm/i915/i915_reg.h > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h > >> > @@ -7638,6 +7638,57 @@ enum skl_disp_power_wells { > >> > > >> > /* MIPI DSI registers */ > >> > > >> > +#define BXT_MIPI1_RX_LOWER_SHIFT 16 > >> > +#define BXT_MIPI2_RX_LOWER_SHIFT 0 > >> > +#define BXT_MIPI_RX_LOWER_SHIFT(port) \ > >> > + _MIPI_PORT(port, BXT_MIPI1_RX_LOWER_SHIFT, \ > >> > + BXT_MIPI2_RX_LOWER_SHIFT) > >> > +#define BXT_MIPI1_RX_LOWER_DIVIDER_MASK (3 << 16) > >> > +#define BXT_MIPI2_RX_LOWER_DIVIDER_MASK (3 << 0) > >> > +#define BXT_MIPI_RX_LOWER_DIVIDER_MASK(port) \ > >> > + (3 << BXT_MIPI_RX_LOWER_SHIFT(port)) > >> > +#define BXT_MIPI_RX_LOWER_DIVIDER(port, val) \ > >> > + ((val & 3) << BXT_MIPI_RX_LOWER_SHIFT(port)) > >> > + > >> > +#define BXT_MIPI1_8X_BY3_SHIFT 19 > >> > +#define BXT_MIPI2_8X_BY3_SHIFT 3 > >> > +#define BXT_MIPI_8X_BY3_SHIFT(port) \ > >> > + _MIPI_PORT(port, BXT_MIPI1_8X_BY3_SHIFT, \ > >> > + BXT_MIPI2_8X_BY3_SHIFT) > >> > +#define BXT_MIPI1_8X_BY3_DIVIDER_MASK (3 << 19) > >> > +#define BXT_MIPI2_8X_BY3_DIVIDER_MASK (3 << 3) > >> > +#define BXT_MIPI_8X_BY3_DIVIDER_MASK(port) \ > >> > + (3 << BXT_MIPI_8X_BY3_SHIFT(port)) > >> > +#define BXT_MIPI_8X_BY3_DIVIDER(port, val) \ > >> > + ((val & 3) << BXT_MIPI_8X_BY3_SHIFT(port)) > >> > + > >> > +#define BXT_MIPI1_RX_UPPER_SHIFT 21 > >> > +#define BXT_MIPI2_RX_UPPER_SHIFT 5 > >> > +#define BXT_MIPI_RX_UPPER_SHIFT(port) \ > >> > + _MIPI_PORT(port, BXT_MIPI1_RX_UPPER_SHIFT, \ > >> > + BXT_MIPI2_RX_UPPER_SHIFT) > >> > +#define BXT_MIPI1_RX_UPPER_DIVIDER_MASK (3 << 21) > >> > +#define BXT_MIPI2_RX_UPPER_DIVIDER_MASK (3 << 5) > >> > +#define BXT_MIPI_RX_UPPER_DIVIDER_MASK(port) \ > >> > + (3 << BXT_MIPI_RX_UPPER_SHIFT(port)) > >> > +#define BXT_MIPI_RX_UPPER_DIVIDER(port, val) \ > >> > + ((val & 3) << BXT_MIPI_RX_UPPER_SHIFT(port)) > >> > + > >> > +#define BXT_MIPI1_TX_SHIFT 26 > >> > +#define BXT_MIPI2_TX_SHIFT 10 > >> > +#define BXT_MIPI_TX_SHIFT(port) \ > >> > + _MIPI_PORT(port, BXT_MIPI1_TX_SHIFT, \ > >> > + BXT_MIPI2_TX_SHIFT) > >> > +#define BXT_MIPI1_TX_DIVIDER_MASK (0x3F << 26) > >> > +#define BXT_MIPI2_TX_DIVIDER_MASK (0x3F << 10) > >> > +#define BXT_MIPI_TX_DIVIDER_MASK(port) \ > >> > + (0x3F << BXT_MIPI_TX_SHIFT(port)) > >> > +#define BXT_MIPI_TX_DIVIDER(port, val) \ > >> > + ((val & 0x3F) << BXT_MIPI_TX_SHIFT(port)) > >> > + > >> > +#define RX_DIVIDER_BIT_1_2 0x3 > >> > +#define RX_DIVIDER_BIT_3_4 0xC > >> > + > >> > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A > >> and C only */ > >> > #define _MMIO_MIPI(port, a, c) _MMIO(_MIPI_PORT(port, a, c)) > >> > > >> > @@ -7650,59 +7701,6 @@ enum skl_disp_power_wells { > >> > #define BXT_MIPI_DIV_SHIFT(port) \ > >> > _MIPI_PORT(port, BXT_MIPI1_DIV_SHIFT, \ > >> > BXT_MIPI2_DIV_SHIFT) > >> > -/* Var clock divider to generate TX source. Result must be < 39.5 M */ > >> > -#define BXT_MIPI1_ESCLK_VAR_DIV_MASK (0x3F << 26) > >> > -#define BXT_MIPI2_ESCLK_VAR_DIV_MASK (0x3F << 10) > >> > -#define BXT_MIPI_ESCLK_VAR_DIV_MASK(port) \ > >> > - _MIPI_PORT(port, > >> BXT_MIPI1_ESCLK_VAR_DIV_MASK, \ > >> > - > >> BXT_MIPI2_ESCLK_VAR_DIV_MASK) > >> > - > >> > -#define BXT_MIPI_ESCLK_VAR_DIV(port, val) \ > >> > - (val << BXT_MIPI_DIV_SHIFT(port)) > >> > -/* TX control divider to select actual TX clock output from (8x/var) */ > >> > -#define BXT_MIPI1_TX_ESCLK_SHIFT 21 > >> > -#define BXT_MIPI2_TX_ESCLK_SHIFT 5 > >> > -#define BXT_MIPI_TX_ESCLK_SHIFT(port) \ > >> > - _MIPI_PORT(port, BXT_MIPI1_TX_ESCLK_SHIFT, \ > >> > - BXT_MIPI2_TX_ESCLK_SHIFT) > >> > -#define BXT_MIPI1_TX_ESCLK_FIXDIV_MASK (3 << 21) > >> > -#define BXT_MIPI2_TX_ESCLK_FIXDIV_MASK (3 << 5) > >> > -#define BXT_MIPI_TX_ESCLK_FIXDIV_MASK(port) \ > >> > - _MIPI_PORT(port, > >> BXT_MIPI1_TX_ESCLK_FIXDIV_MASK, \ > >> > - > >> BXT_MIPI2_TX_ESCLK_FIXDIV_MASK) > >> > -#define BXT_MIPI_TX_ESCLK_8XDIV_BY2(port) \ > >> > - (0x0 << BXT_MIPI_TX_ESCLK_SHIFT(port)) > >> > -#define BXT_MIPI_TX_ESCLK_8XDIV_BY4(port) \ > >> > - (0x1 << BXT_MIPI_TX_ESCLK_SHIFT(port)) > >> > -#define BXT_MIPI_TX_ESCLK_8XDIV_BY8(port) \ > >> > - (0x2 << BXT_MIPI_TX_ESCLK_SHIFT(port)) > >> > -/* RX control divider to select actual RX clock output from 8x*/ > >> > -#define BXT_MIPI1_RX_ESCLK_SHIFT 19 > >> > -#define BXT_MIPI2_RX_ESCLK_SHIFT 3 > >> > -#define BXT_MIPI_RX_ESCLK_SHIFT(port) \ > >> > - _MIPI_PORT(port, BXT_MIPI1_RX_ESCLK_SHIFT, \ > >> > - BXT_MIPI2_RX_ESCLK_SHIFT) > >> > -#define BXT_MIPI1_RX_ESCLK_FIXDIV_MASK (3 << 19) > >> > -#define BXT_MIPI2_RX_ESCLK_FIXDIV_MASK (3 << 3) > >> > -#define BXT_MIPI_RX_ESCLK_FIXDIV_MASK(port) \ > >> > - (3 << BXT_MIPI_RX_ESCLK_SHIFT(port)) > >> > -#define BXT_MIPI_RX_ESCLK_8X_BY2(port) \ > >> > - (1 << BXT_MIPI_RX_ESCLK_SHIFT(port)) > >> > -#define BXT_MIPI_RX_ESCLK_8X_BY3(port) \ > >> > - (2 << BXT_MIPI_RX_ESCLK_SHIFT(port)) > >> > -#define BXT_MIPI_RX_ESCLK_8X_BY4(port) \ > >> > - (3 << BXT_MIPI_RX_ESCLK_SHIFT(port)) > >> > -/* BXT-A WA: Always prog DPHY dividers to 00 */ > >> > -#define BXT_MIPI1_DPHY_DIV_SHIFT 16 > >> > -#define BXT_MIPI2_DPHY_DIV_SHIFT 0 > >> > -#define BXT_MIPI_DPHY_DIV_SHIFT(port) \ > >> > - _MIPI_PORT(port, BXT_MIPI1_DPHY_DIV_SHIFT, \ > >> > - BXT_MIPI2_DPHY_DIV_SHIFT) > >> > -#define BXT_MIPI_1_DPHY_DIVIDER_MASK (3 << 16) > >> > -#define BXT_MIPI_2_DPHY_DIVIDER_MASK (3 << 0) > >> > -#define BXT_MIPI_DPHY_DIVIDER_MASK(port) \ > >> > - (3 << BXT_MIPI_DPHY_DIV_SHIFT(port)) > >> > - > >> > /* BXT MIPI mode configure */ > >> > #define _BXT_MIPIA_TRANS_HACTIVE 0x6B0F8 > >> > #define _BXT_MIPIC_TRANS_HACTIVE 0x6B8F8 > >> > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c > >> > b/drivers/gpu/drm/i915/intel_dsi_pll.c > >> > index bb5e95a..7435115 100644 > >> > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c > >> > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c > >> > @@ -362,35 +362,59 @@ static void vlv_dsi_reset_clocks(struct > >> > intel_encoder *encoder, enum port port) > >> > /* Program BXT Mipi clocks and dividers */ static void > >> > bxt_dsi_program_clocks(struct drm_device *dev, enum port port) { > >> > - u32 tmp; > >> > - u32 divider; > >> > - u32 dsi_rate; > >> > - u32 pll_ratio; > >> > struct drm_i915_private *dev_priv = dev->dev_private; > >> > + u32 tmp; > >> > + u32 dsi_rate = 0; > >> > + u32 pll_ratio = 0; > >> > + u32 rx_div; > >> > + u32 tx_div; > >> > + u32 rx_div_upper; > >> > + u32 rx_div_lower; > >> > + u32 mipi_8by3_divider; > >> > > >> > /* Clear old configurations */ > >> > tmp = I915_READ(BXT_MIPI_CLOCK_CTL); > >> > - tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(port)); > >> > - tmp &= ~(BXT_MIPI_RX_ESCLK_FIXDIV_MASK(port)); > >> > - tmp &= ~(BXT_MIPI_ESCLK_VAR_DIV_MASK(port)); > >> > - tmp &= ~(BXT_MIPI_DPHY_DIVIDER_MASK(port)); > >> > + tmp &= ~(BXT_MIPI_8X_BY3_DIVIDER_MASK(port)); > >> > + tmp &= ~(BXT_MIPI_RX_LOWER_DIVIDER_MASK(port)); > >> > + tmp &= ~(BXT_MIPI_RX_UPPER_DIVIDER_MASK(port)); > >> > + tmp &= ~(BXT_MIPI_TX_DIVIDER_MASK(port)); > >> > > >> > /* Get the current DSI rate(actual) */ > >> > pll_ratio = I915_READ(BXT_DSI_PLL_CTL) & > >> > - BXT_DSI_PLL_RATIO_MASK; > >> > + BXT_DSI_PLL_RATIO_MASK; > >> > + > >> > + /* To get 8X clock, divide ref_freq * pll ratio by 2 as per bspec > >> > +*/ > >> > dsi_rate = (BXT_REF_CLOCK_KHZ * pll_ratio) / 2; > >> > > >> > - /* Max possible output of clock is 39.5 MHz, program value -1 */ > >> > - divider = (dsi_rate / BXT_MAX_VAR_OUTPUT_KHZ) - 1; > >> > - tmp |= BXT_MIPI_ESCLK_VAR_DIV(port, divider); > >> > + /* > >> > + * tx clock should be <= 20MHz and the div value must be > >> > + * subtracted by 1 as per bspec > >> > + */ > >> > + tx_div = DIV_ROUND_UP(dsi_rate, 20000) - 1; > >> > + /* > >> > + * rx clock should be <= 150MHz and the div value must be > >> > + * subtracted by 1 as per bspec > >> > + */ > >> > + rx_div = DIV_ROUND_UP(dsi_rate, 150000) - 1; > >> > > >> > /* > >> > - * Tx escape clock must be as close to 20MHz possible, but should > >> > - * not exceed it. Hence select divide by 2 > >> > + * rx divider value needs to be updated in the > >> > + * two differnt bit fields in the register hence splitting the > >> > + * rx divider value accordingly > >> > */ > >> > - tmp |= BXT_MIPI_TX_ESCLK_8XDIV_BY2(port); > >> > + rx_div_lower = rx_div & RX_DIVIDER_BIT_1_2; > >> > + rx_div_upper = (rx_div & RX_DIVIDER_BIT_3_4) >> 2; > >> > + > >> > + /* As per bpsec program the 8/3X clock divider to the below value */ > >> > + if (dev_priv->vbt.dsi.config->is_cmd_mode) > >> > + mipi_8by3_divider = 0x2; > >> > + else > >> > + mipi_8by3_divider = 0x3; > >> > > >> > - tmp |= BXT_MIPI_RX_ESCLK_8X_BY3(port); > >> > + tmp |= BXT_MIPI_8X_BY3_DIVIDER(port, mipi_8by3_divider); > >> > + tmp |= BXT_MIPI_TX_DIVIDER(port, tx_div); > >> > + tmp |= BXT_MIPI_RX_LOWER_DIVIDER(port, rx_div_lower); > >> > + tmp |= BXT_MIPI_RX_UPPER_DIVIDER(port, rx_div_upper); > >> > > >> > I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp); } @@ -512,10 +536,10 > >> @@ static > >> > void bxt_dsi_reset_clocks(struct intel_encoder *encoder, enum port > >> > port) > >> > > >> > /* Clear old configurations */ > >> > tmp = I915_READ(BXT_MIPI_CLOCK_CTL); > >> > - tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(port)); > >> > - tmp &= ~(BXT_MIPI_RX_ESCLK_FIXDIV_MASK(port)); > >> > - tmp &= ~(BXT_MIPI_ESCLK_VAR_DIV_MASK(port)); > >> > - tmp &= ~(BXT_MIPI_DPHY_DIVIDER_MASK(port)); > >> > + tmp &= ~(BXT_MIPI_8X_BY3_DIVIDER_MASK(port)); > >> > + tmp &= ~(BXT_MIPI_RX_LOWER_DIVIDER_MASK(port)); > >> > + tmp &= ~(BXT_MIPI_RX_UPPER_DIVIDER_MASK(port)); > >> > + tmp &= ~(BXT_MIPI_TX_DIVIDER_MASK(port)); > >> > I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp); > >> > I915_WRITE(MIPI_EOT_DISABLE(port), CLOCKSTOP); } > >> > >> -- > >> Jani Nikula, Intel Open Source Technology Center > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
