[Freedreno] [PATCH] drm/msm/dsi: simplify pixel clk rate handling

[Marijn Suijten]

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
>