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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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,
>>>   };
>>>
> 
>