On 2023-05-19 22:37:34, Dmitry Baryshkov wrote:
<snip>
> >>>>>> + ret = cfg_hnd->ops->calc_clk_rate(msm_host);
> >>>>>
> >>>>> I am not too sure what we are gaining by this.
> >>>>>
> >>>>> Its not that we are replacing dsi_get_pclk_rate().
> >>>>>
> >>>>> We are moving the dsi_get_pclk_rate() from the calc_clk_rate() to
> >>>>> the msm_dsi_host_get_phy_clk_req().
> >>>>>
> >>>>> Also, with this change, dsi_calc_clk_rate_6g() looks kind of empty
> >>>>> to stand on its own.
> >>>>>
> >>>>> The original intention of the calc_clk_rate() op seems to be
> >>>>> calculate and store all the clocks (byte, pixel and esc).
> >>>>>
> >>>>> Why change that behavior by breaking it up?
> >>>>
> >>>> Unification between platforms. Both v2 and 6g platforms call
> >>>> dsi_calc_pclk(). Let's just move it to a common code path.
> >>>
> >>> Hi Dmitry,
> >>>
> >>> I think what Abhinav means here is that the meaning and functionality
> >>> of calc_clk_rate() changes with this patch.
> >>>
> >>> Before, calc_clk_rate() does *all* the clk_rate calculations and
> >>> assignments. But after this change, it will only calculate and assign
> >>> the escape clk rate.
> >>>
> >>> I agree with Abhinav that this change renders the calc_clk_rate() op
> >>> misleading as it will not calculate all of the clock rates anymore.
> >>
> >> Would it make sense if I rename it to calc_other_clock_rates()?
> >>
> >
> > Not really. I would rather still have it separate and drop this patch.
> >
> > So even if pixel clock calculation looks common today between v2 and 6g,
> > lets say tomorrow there is a 7g or 8g which needs some other math there,
> > I think this is the right place where it should stay so that we
> > calculate all clocks together.
>
> Ack.
Unfortunate, but okay. Then don't forget to send the following hunk of
this patch in isolation:
- pclk_bpp = (u64)dsi_get_pclk_rate(msm_host->mode, is_bonded_dsi) * bpp;
+ pclk_bpp = msm_host->pixel_clk_rate * bpp;
- Marijn
> >> Moving pclk calculation to the core code emphasises that pclk
> >> calculation is common between v2 and 6g hosts.
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Jessica Zhang
> >>>
> >>>>
> >>>>>
> >>>>>> if (ret) {
> >>>>>> pr_err("%s: unable to calc clk rate, %d\n", __func__, ret);
> >>>>>> return;
> >>>>
> >>>> --
> >>>> With best wishes
> >>>> Dmitry
> >>>>
> >>
>
> --
> With best wishes
> Dmitry
>
