Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Madhav Chauhan <madhav.chauhan@intel.com>
Cc: jani.nikula@intel.com, ander.conselvan.de.oliveira@intel.com,
	intel-gfx@lists.freedesktop.org, Deepak M <m.deepak@intel.com>,
	shobhit.kumar@intel.com
Subject: Re: [GLK MIPI DSI V2 2/9] drm/i915/glk: Program new MIPI DSI PHY registers for GLK
Date: Thu, 22 Dec 2016 14:02:20 +0200	[thread overview]
Message-ID: <20161222120220.GI31595@intel.com> (raw)
In-Reply-To: <1481792500-30863-3-git-send-email-madhav.chauhan@intel.com>

On Thu, Dec 15, 2016 at 02:31:33PM +0530, Madhav Chauhan wrote:
> From: Deepak M <m.deepak@intel.com>
> 
> Program the clk lane and tlpx time count registers
> to configure DSI PHY.
> 
> v2: Addressed Jani's Review comments(renamed bit field macros)
> 
> Signed-off-by: Deepak M <m.deepak@intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8e47b59..03858f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8550,6 +8550,24 @@ enum {
>  #define  LP_BYTECLK_SHIFT				0
>  #define  LP_BYTECLK_MASK				(0xffff << 0)
>  
> +#define _MIPIA_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb0a4)
> +#define _MIPIC_TLPX_TIME_COUNT		(dev_priv->mipi_mmio_base + 0xb8a4)
> +#define MIPI_TLPX_TIME_COUNT(port)	 _MMIO_MIPI(port, _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
> +#define  GLK_DPHY_TLPX_TIME_CNT_SHIFT			0
> +#define  GLK_DPHY_TLPX_TIME_CNT_MASK			(0xff << 0)
> +
> +#define _MIPIA_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb098)
> +#define _MIPIC_CLK_LANE_TIMING		(dev_priv->mipi_mmio_base + 0xb898)
> +#define MIPI_CLK_LANE_TIMING(port)	 _MMIO_MIPI(port, _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
> +#define  GLK_MIPI_CLK_LANE_HS_PREP_SHIFT			0
> +#define  GLK_MIPI_CLK_LANE_HS_PREP_MASK			(0xff << 0)
> +#define  GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT			8
> +#define  GLK_MIPI_CLK_LANE_HS_ZERO_MASK			(0xff00 << 0)
> +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT			16
> +#define  GLK_MIPI_CLK_LANE_HS_TRAIL_MASK			(0xff0000 << 0)
> +#define  GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT			24
> +#define  GLK_MIPI_CLK_LANE_HS_EXIT_MASK			(0xff000000 << 0)
> +
>  /* bits 31:0 */
>  #define _MIPIA_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb064)
>  #define _MIPIC_LP_GEN_DATA		(dev_priv->mipi_mmio_base + 0xb864)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 6b63355..b78c686 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1123,6 +1123,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> +	struct mipi_config *mipi_config = dev_priv->vbt.dsi.config;
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	enum port port;
>  	unsigned int bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> @@ -1278,6 +1279,20 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder,
>  		 */
>  		I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
>  
> +		if (IS_GEMINILAKE(dev_priv)) {
> +			I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
> +					intel_dsi->lp_byte_clk);
> +			val = ((mipi_config->ths_prepare <<
> +				GLK_MIPI_CLK_LANE_HS_PREP_SHIFT) |
> +				(mipi_config->ths_prepare_hszero <<
> +				GLK_MIPI_CLK_LANE_HS_ZERO_SHIFT) |
> +				(mipi_config->ths_trail <<
> +				GLK_MIPI_CLK_LANE_HS_TRAIL_SHIFT) |
> +				(mipi_config->ths_exit <<
> +				GLK_MIPI_CLK_LANE_HS_EXIT_SHIFT));

I don't think anything else here uses the mipi_config block directly.
Rather everything gets duplicated in intel_dsi. This here looks
like the same (or similar) thing to dphy_reg. Probably should
reuse that, or split it up into its component pieces if we want
to keep duplicated information between the mipi_config and intel_dsi.

> +			I915_WRITE(MIPI_CLK_LANE_TIMING(port), val);
> +		}
> +
>  		/* the bw essential for transmitting 16 long packets containing
>  		 * 252 bytes meant for dcs write memory command is programmed in
>  		 * this register in terms of byte clocks. based on dsi transfer
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx