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

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

On Wed, 18 Jan 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 02 Jan 2017, Madhav Chauhan <madhav.chauhan@intel.com> wrote:
>> From: Deepak M <m.deepak@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@intel.com>
>> Signed-off-by: Madhav Chauhan <madhav.chauhan@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.

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx