Re: [PATCH] drm/i915/bxt: Additional MIPI clock divider form B0 stepping onwards

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux