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

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

On Thu, 22 Dec 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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.

My thoughts exactly. I've just been thinking about Hans' patches, and
whether we should give in and split intel_dsi.c and
intel_dsi_panel_vbt.c differently.

BR,
Jani.

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

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