> -----Original Message----- > From: Nikula, Jani > Sent: Wednesday, January 18, 2017 7:41 PM > To: Chauhan, Madhav <madhav.chauhan@xxxxxxxxx>; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Shankar, Uma <uma.shankar@xxxxxxxxx>; Mukherjee, Indranil > <indranil.mukherjee@xxxxxxxxx>; Kamath, Sunil <sunil.kamath@xxxxxxxxx>; > Saarinen, Jani <jani.saarinen@xxxxxxxxx>; Conselvan De Oliveira, Ander > <ander.conselvan.de.oliveira@xxxxxxxxx>; Konduru, Chandra > <chandra.konduru@xxxxxxxxx>; Kumar, Shobhit > <shobhit.kumar@xxxxxxxxx>; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Deepak > M <m.deepak@xxxxxxxxx>; Chauhan, Madhav > <madhav.chauhan@xxxxxxxxx> > 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@xxxxxxxxx> wrote: > > On Mon, 02 Jan 2017, Madhav Chauhan <madhav.chauhan@xxxxxxxxx> > wrote: > >> From: Deepak M <m.deepak@xxxxxxxxx> > >> > >> 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@xxxxxxxxx> > >> Signed-off-by: Madhav Chauhan <madhav.chauhan@xxxxxxxxx> > >> --- > >> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
