Re: [PATCH 4/4] drm/i915: Add support for Generic MIPI panel driver

Damien Lespiau
2014-05-20T20:55:18Z

On Tue, May 20, 2014 at 09:46:01PM +0530, Shobhit Kumar wrote:
> >- UI is a period, so is homogeneous to time (s), but ui_num being in
> > s^-1 and ui_den a constant, ui_num/ui_den looks like a frequency. Or
> > could it be that UI = ui_den / ui_num? would be confusing, but the
> > code below would make more sense. In which case could we have UI =
> > ui_num / ui_den?
>
> I just kept ui_num and ui_den separately to take care of precision
> loss, but I see how it is adding to confusion. Actually it is ui_den
> / ui_num and we have all computations as 1/UI so it works. I think I
> will compute UI directly as UI = (NS_KHZ_RATIO * 1000) /bitrate and
> divide by 1000 wherever we use to maintain precision. Sounds ok ?

I think just exchanging the two variable names (ui_num and ui_den)
should be less work for you and should be enough. It's really just about
having ui_num being the UI numerator so the reader is not too surprised

> >>+ /* B044 */
> >>+ intel_dsi->hs_to_lp_count =
> >>+ CEIL_DIV(
> >>+ 4 * tlpx_ui + prepare_cnt * 2 +
> >>+ exit_zero_cnt * 2 + 10,
> >>+ 8);
> >
> >The previous was before I tried to look at the spec too closely. Mind
> >explaining why we don't look at the HS to LP switch count? ie why HS to
> >LP switch cound is always smaller than the LP to HS one?
>
> Because LP to HS uses exit_zero_count which is generally higher than
> clk_zero_count. So just directly used LP to HS which amounts to
> saying that switching from HS to LP takes lesser time than switching
> from LP to HS. I can/should add code to compute max of the two.

This could go to a separate task if you don't have time right now,

Thanks for your answers!

--
Damien