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]

 



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.

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