[Intel-gfx] [GLK MIPI DSI V3 1/7] drm/i915/glk: Program dphy param reg for GLK

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Wednesday, January 18, 2017 7:41 PM
> To: Chauhan, Madhav <madhav.chauhan at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Shankar, Uma <uma.shankar at intel.com>; Mukherjee, Indranil
> <indranil.mukherjee at intel.com>; Kamath, Sunil <sunil.kamath at intel.com>;
> Saarinen, Jani <jani.saarinen at intel.com>; Conselvan De Oliveira, Ander
> <ander.conselvan.de.oliveira at intel.com>; Konduru, Chandra
> <chandra.konduru at intel.com>; Kumar, Shobhit
> <shobhit.kumar at intel.com>; Syrjala, Ville <ville.syrjala at intel.com>; Deepak
> M <m.deepak at intel.com>; Chauhan, Madhav
> <madhav.chauhan at intel.com>
> Subject: Re: [GLK MIPI DSI V3 1/7] drm/i915/glk: Program dphy param reg for
> GLK
> 
> On Wed, 18 Jan 2017, Jani Nikula <jani.nikula at intel.com> wrote:
> > On Mon, 02 Jan 2017, Madhav Chauhan <madhav.chauhan at intel.com>
> wrote:
> >> From: Deepak M <m.deepak at intel.com>
> >>
> >> For GEMINILAKE, dphy param reg values are programmed in terms of HS
> >> byte clock count while for legacy platforms in terms of HS ddr clk
> >> count.
> >
> > No need to call everything before this one "legacy".
> >
> >> Signed-off-by: Deepak M <m.deepak at intel.com>
> >> Signed-off-by: Madhav Chauhan <madhav.chauhan at intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 33
> >> +++++++++++++++++++++++-------
> >>  1 file changed, 26 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> >> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> >> index 8f683b8..8059cbb 100644
> >> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> >> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> >> @@ -695,16 +695,26 @@ struct drm_panel *vbt_panel_init(struct
> intel_dsi *intel_dsi, u16 panel_id)
> >>  	/* count values in UI = (ns value) * (bitrate / (2 * 10^6))
> >>  	 *
> >>  	 * Since txddrclkhs_i is 2xUI, all the count values programmed in
> >> -	 * DPHY param register are divided by 2
> >> +	 * DPHY param register are divided by 2 except GEMINILAKE where it
> is
> >> +	 * programmed in terms of HS byte clock so divided by 8
> >
> > Would you say these two hold?
> >
> > 1) HSDDR = 2 * HS
> >
> > 2) HS byte clock = HS / 8
> >
> > So it would seem to me either the existing code or your patch is
> > wrong. (Or I'm seriously confused.)
> >
> > If the register is in terms of clock *cycles*, not frequency, should
> > the HSDDR based clock (pre-GLK) actually have *twice* the clock
> > cycles, not *half*? Making the existing code wrong?
> >
> > The existing code could use some serious cleanup to make it readable.
> > :(
> 
> Additionally, I think you could use a variable to handle this, to not add so
> much duplicated code.

Old Comments are not very clear to justify this calculation.
Went through dphy/IP spec and here is my explanation
1. As per DPHY spec
	DDR Clock period = 1UI + 1UI = 2UI
2. 1UI in terms of time:
	1UI (sec) = 1/(Bitrate *10^3)	// bitrate is in KHZ
	1UI (nsec) = (1*10^9)/(Bitrate*10^3) = 10^6/Bitrate
3. DDR clock period is twice of UI period as per 1 = (2*10^6)/Bitrate
4. ths_prepare_ns (via mipi_config) is to be programmed in terms *DDR clock count*  to dphy register

	DDR clock count = ths_prepare_ns /((2*10^6)/Bitrate)) = (ths_prepare_ns*Bitrate)/(2*10^6)
> 
> BR,
> Jani.
> 
> 
> 
> >
> > BR,
> > Jani.
> >
> >
> >
> >>  	 *
> >>  	 * prepare count
> >>  	 */
> >>  	ths_prepare_ns = max(mipi_config->ths_prepare,
> >>  			     mipi_config->tclk_prepare);
> >> -	prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den, ui_num *
> 2);
> >> +	if (IS_GEMINILAKE(dev_priv))
> >> +		prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den,
> ui_num * 8);
> >> +	else
> >> +		prepare_cnt = DIV_ROUND_UP(ths_prepare_ns * ui_den,
> ui_num * 2);
> >>
> >>  	/* exit zero count */
> >> -	exit_zero_cnt = DIV_ROUND_UP(
> >> +	if (IS_GEMINILAKE(dev_priv))
> >> +		exit_zero_cnt = DIV_ROUND_UP(
> >> +				(ths_prepare_hszero - ths_prepare_ns) *
> ui_den,
> >> +				ui_num * 8
> >> +				);
> >> +	else
> >> +		exit_zero_cnt = DIV_ROUND_UP(
> >>  				(ths_prepare_hszero - ths_prepare_ns) *
> ui_den,
> >>  				ui_num * 2
> >>  				);
> >> @@ -719,13 +729,22 @@ struct drm_panel *vbt_panel_init(struct
> intel_dsi *intel_dsi, u16 panel_id)
> >>  		exit_zero_cnt += 1;
> >>
> >>  	/* clk zero count */
> >> -	clk_zero_cnt = DIV_ROUND_UP(
> >> -			(tclk_prepare_clkzero -	ths_prepare_ns)
> >> -			* ui_den, 2 * ui_num);
> >> +	if (IS_GEMINILAKE(dev_priv))
> >> +		clk_zero_cnt = DIV_ROUND_UP(
> >> +				(tclk_prepare_clkzero -	ths_prepare_ns)
> >> +				* ui_den, 8 * ui_num);
> >> +	else
> >> +		clk_zero_cnt = DIV_ROUND_UP(
> >> +				(tclk_prepare_clkzero -	ths_prepare_ns)
> >> +				* ui_den, 2 * ui_num);
> >>
> >>  	/* trail count */
> >>  	tclk_trail_ns = max(mipi_config->tclk_trail, mipi_config->ths_trail);
> >> -	trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, 2 * ui_num);
> >> +
> >> +	if (IS_GEMINILAKE(dev_priv))
> >> +		trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, 8 *
> ui_num);
> >> +	else
> >> +		trail_cnt = DIV_ROUND_UP(tclk_trail_ns * ui_den, 2 *
> ui_num);
> >>
> >>  	if (prepare_cnt > PREPARE_CNT_MAX ||
> >>  		exit_zero_cnt > EXIT_ZERO_CNT_MAX ||
> 
> --
> Jani Nikula, Intel Open Source Technology Center