Re: [PATCH v1 3/6] drm/mediatek: mtk_dpi: Use an array for pixclk factor calculation

[AngeloGioacchino Del Regno]

Il 22/11/24 07:23, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
>
> On Fri, 2024-11-22 at 11:54 +0800, CK Hu wrote:
> > Hi, Angelo:
> >
> > On Wed, 2024-11-20 at 13:44 +0100, AngeloGioacchino Del Regno wrote:
> > > External email : Please do not click links or open attachments until you have verified the sender or the content.
> > >
> > >
> > > Setting the TVD PLL clock requires to multiply the target pixel
> > > clock by a specific constant factor to achieve the target PLL
> > > frequency, and this is done to reduce jitter to acceptable levels.
> > >
> > > On all MediaTek SoCs, the factor is not retrieved by any real kind
> > > of calculation but rather by checking if the target pixel clock
> > > is less than a specified frequency, hence assigning a function
> > > pointer for just a bunch of if branches does enlarge the code
> > > size for little reason.
> > >
> > > Remove all SoC-specific functions, add a structure `mtk_dpi_factor`
> > > that holds a clock frequency and corresponding PLL factor, and
> > > declare the constraints for each SoC in form of an array of said
> > > structure.
> > > Instead of function pointers, this structure (and its size) is then
> > > assigned to each SoC's platform data.
> > >
> > > The "calculation" is then performed with a new static function
> > > mtk_dpi_calculate_factor(dpi, mode_clk) that iterates through all
> > > of the entries of the aforementioned array and returns the right
> > > factor.
> > >
> > > If no factor is found, the lowest possible factor is returned,
> > > mimicking the same flow as all of the old per-SoC calculation
> > > functions.
> > >
> > > This commit brings no functional change.
>
> [snip]
>
> > >   static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
> > >                                      struct drm_display_mode *mode)
> > >   {
> > > @@ -529,7 +550,7 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
> > >          unsigned int factor;
> > >
> > >          /* let pll_rate can fix the valid range of tvdpll (1G~2GHz) */
> > > -       factor = dpi->conf->cal_factor(mode->clock);
> > > +       factor = mtk_dpi_calculate_factor(dpi, mode_clk);
>
> mode_clk is defined in next patch.
> keep mode->clock in this patch to keep my reviewed-by tag.

Oh! Nice catch!
That happened during the final cleanup, heh :-)

Thanks btw, I will fix that in v2.

Cheers,
Angelo

> Regards,
> CK
>
> > >          drm_display_mode_to_videomode(mode, &vm);
> > >          pll_rate = vm.pixelclock * factor;
> > >
> > > @@ -964,48 +985,6 @@ static const struct component_ops mtk_dpi_component_ops = {
> > >          .unbind = mtk_dpi_unbind,
> > >   };