Re: [PATCH 1/2] drm/bridge: samsung-dsim: Set P divider based on min/max of fin pll

[Frieder Schrempf]

Hi Adam,

On 12.12.23 04:32, Adam Ford wrote:
> The P divider should be set based on the min and max values of
> the fin pll which may vary between different platforms.
> These ranges are defined per platform, but hard-coded values
> were used instead which resulted in a smaller range available
> on the i.MX8M[MNP] than what was possible.
> 
> Fixes: 846307185f0f ("drm/bridge: samsung-dsim: update PLL reference clock")
> Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
> 
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index be5914caa17d..239d253a7d71 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -573,8 +573,8 @@ static unsigned long samsung_dsim_pll_find_pms(struct samsung_dsim *dsi,
>  	u16 _m, best_m;
>  	u8 _s, best_s;
>  
> -	p_min = DIV_ROUND_UP(fin, (12 * MHZ));
> -	p_max = fin / (6 * MHZ);
> +	p_min = DIV_ROUND_UP(fin, (driver_data->pll_fin_max * MHZ));
> +	p_max = fin / (driver_data->pll_fin_min * MHZ);

I did some tinkering with the PLL settings the other day and this is
literally one of the things I came up with.

So I'm happy to provide:

Reviewed-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
Tested-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>

Regarding the P divider, I'm also wondering if there is an upper limit
for the p-value (not for the resulting fin_pll) that we should take into
account, too. The problem is that we have conflicts in the documentation
(again) so we don't really know what the correct limit would be.

There are the following ranges given in the RMs:

* 1..63 (i.MX8MM RM 13.7.8.18.4)
* 1..33 (i.MX8MM RM 13.7.10.1)
* 1..63 (i.MX8MP RM 13.2.3.1.5.2)
* 1..63 (i.MX8MP RM 13.7.2.4)

Unfortunately there are similar discrepancies for the other parameters
and limits.

Thanks
Frieder

>  
>  	for (_p = p_min; _p <= p_max; ++_p) {
>  		for (_s = 0; _s <= 5; ++_s) {