Re: [PATCH 07/12] drm/i915/bxt: Program Tx Rx and Dphy clocks

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> BXT DSI clocks are different than previous platforms. So adding a
> new function to program following clocks and dividers:
> 1. Program variable divider to generate input to Tx clock divider
>    (Output value must be < 39.5Mhz)
> 2. Select divide by 2 option to get < 20Mhz for Tx clock
> 3. Program 8by3 divider to generate Rx clock
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |   51 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.c     |    3 ++
>  drivers/gpu/drm/i915/intel_dsi.h     |    1 +
>  drivers/gpu/drm/i915/intel_dsi_pll.c |   35 +++++++++++++++++++++++
>  4 files changed, 90 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 5eeb2b7..f96f049 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7387,6 +7387,57 @@ enum skl_disp_power_wells {
>  #define GEN9_FUSE_PG1_ENABLED                          (1 << 26)
>  #define GEN9_FUSE_PG0_ENABLED                          (1 << 27)
>  
> +/* BXT MIPI clock controls */
> +#define BXT_MAX_VAR_OUTPUT_KHZ                  39500
> +#define BXT_MIPI_CLOCK_CTL                      0x46090
> +#define BXT_MIPI_DIV_SHIFT                      16

Same comments here, above and below, as in previous patches about
conventions for defining registers.

> +
> +/* Var clock divider to generate TX source. Result must be < 39.5 M */
> +#define BXT_MIPI1_ESCLK_VAR_DIV_MASK            (0x3F << 26)
> +/* BXT MIPI clock controls */
> +#define BXT_MAX_VAR_OUTPUT_KHZ                  39500
> +#define BXT_MIPI_CLOCK_CTL                      0x46090
> +#define BXT_MIPI_DIV_SHIFT                      16
> +
> +/* 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(check)    \
> +		(0x3F << ((!check) * BXT_MIPI_DIV_SHIFT + 10))

check?

Ugh, I hate the confusion between pipe and port for DSI. Yes, I'm
confused too.

Whichever it is, I'd prefer not using !check, because it conflates pipe
B and port C, adding to the confusion.

> +#define BXT_MIPI_ESCLK_VAR_DIV(check, val)            \
> +		(val << ((!check) * BXT_MIPI_DIV_SHIFT + 10))
> +
> +/* TX control divider to select actual TX clock output from (8x/var) */
> +#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(check)  \
> +		(3 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
> +
> +#define BXT_MIPI_TX_ESCLK_8XDIV_BY2(check)    \
> +		(0x0 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
> +#define BXT_MIPI_TX_ESCLK_8XDIV_BY4(check)    \
> +		(0x1 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
> +#define BXT_MIPI_TX_ESCLK_8XDIV_BY8(check)    \
> +		(0x2 << ((!check) * BXT_MIPI_DIV_SHIFT + 5))
> +
> +/* RX control divider to select actual RX clock output from 8x*/
> +#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(check)  \
> +		(3 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
> +#define BXT_MIPI_RX_ESCLK_8X_BY2(check)       \
> +		(1 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
> +#define BXT_MIPI_RX_ESCLK_8X_BY3(check)       \
> +		(2 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
> +#define BXT_MIPI_RX_ESCLK_8X_BY4(check)       \
> +		(3 << ((!check) * BXT_MIPI_DIV_SHIFT + 3))
> +
> +/* BXT: Always prog DPHY dividers to 00 */
> +#define BXT_MIPI_1_DPHY_DIVIDER_MASK          (3 << 16)
> +#define BXT_MIPI_2_DPHY_DIVIDER_MASK          (3 << 0)
> +#define BXT_MIPI_DPHY_DIVIDER_MASK(check)               \
> +		(3 << ((!check) * BXT_MIPI_DIV_SHIFT))
> +
>  /* 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.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3a1bb04..729faf6 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -503,6 +503,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  		tmp = I915_READ(DSPCLK_GATE_D);
>  		tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
>  		I915_WRITE(DSPCLK_GATE_D, tmp);
> +	} else if (IS_BROXTON(dev)) {
> +		/* Program Tx Rx and Dphy clocks */
> +		bxt_dsi_program_clocks(dev, intel_dsi->ports);

intel_dsi->ports is a bit mask of ports, e.g. (1 << PORT_A) | (1 <<
PORT_B). I don't think this is what you mean or want to use here.

>  	}
>  
>  	/* put device in ready state */
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 759983e..af5a09f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -124,6 +124,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>  extern void intel_enable_dsi_pll(struct intel_encoder *encoder);
>  extern void intel_disable_dsi_pll(struct intel_encoder *encoder);
>  extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp);
> +extern void bxt_dsi_program_clocks(struct drm_device *dev, int pipe);
>  
>  struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 0cbcf32..9a8a35d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -369,6 +369,41 @@ u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)
>  	return pclk;
>  }
>  
> +/* Program BXT Mipi clocks and dividers */
> +void bxt_dsi_program_clocks(struct drm_device *dev, int pipe)

That should probably be enum pipe or enum port...

> +{
> +	u32 tmp = 0;
> +	u32 divider = 0;
> +	u32 dsi_rate = 0;
> +	u32 pll_ratio = 0;

Unnecessary initializations.

> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/* Clear old configurations */
> +	tmp = I915_READ(BXT_MIPI_CLOCK_CTL);
> +	tmp &= ~(BXT_MIPI_TX_ESCLK_FIXDIV_MASK(pipe));
> +	tmp &= ~(BXT_MIPI_RX_ESCLK_FIXDIV_MASK(pipe));
> +	tmp &= ~(BXT_MIPI_ESCLK_VAR_DIV_MASK(pipe));
> +	tmp &= ~(BXT_MIPI_DPHY_DIVIDER_MASK(pipe));
> +
> +	/* Get the current DSI rate(actual) */
> +	pll_ratio = I915_READ(BXT_DSI_PLL_CTL) &
> +		BXT_DSI_MASK_PLL_RATIO;
> +	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(pipe, divider);
> +
> +	/* Tx escape clock should be >=20MHz, so select divide by 2 */
> +	tmp |= BXT_MIPI_TX_ESCLK_8XDIV_BY2(pipe);
> +
> +	/* Rx escape clock, select fix divide by 3 clock */
> +	tmp |= BXT_MIPI_RX_ESCLK_8X_BY3(pipe);
> +
> +	/* Do the honors */
> +	I915_WRITE(BXT_MIPI_CLOCK_CTL, tmp);
> +}
> +
>  static bool bxt_configure_dsi_pll(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx