Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <6bdf9e39-e938-4644-b0ce-37191e1c00d2@kontron.de>
Date:   Tue, 12 Dec 2023 09:25:12 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] drm/bridge: samsung-dsim: Set P divider based on
 min/max of fin pll
Content-Language: en-US, de-DE
To:     Adam Ford <aford173@gmail.com>, dri-devel@lists.freedesktop.org
Cc:     Maxime Ripard <mripard@kernel.org>,
        Neil Armstrong <neil.armstrong@linaro.org>,
        Robert Foss <rfoss@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Jonas Karlman <jonas@kwiboo.se>,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
        aford@beaconembedded.com,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Marco Felsch <m.felsch@pengutronix.de>,
        Michael Tretter <m.tretter@pengutronix.de>,
        Jagan Teki <jagan@amarulasolutions.com>,
        Andrzej Hajda <andrzej.hajda@intel.com>,
        linux-kernel@vger.kernel.org,
        Marek Szyprowski <m.szyprowski@samsung.com>
References: <20231212033259.189718-1-aford173@gmail.com>
From:   Frieder Schrempf <frieder.schrempf@kontron.de>
In-Reply-To: <20231212033259.189718-1-aford173@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bXd2VVdYTVVZWG95RVZ6REJ1UjhkUWxjUW5nTjlKa0FrMC9BTC9MVUdQampU?=
 =?utf-8?B?WXlmRTliSGhRbkpjUVpEb0R1aW5TSURNbXZiUS9Jb3UweGpYay8vS2lhSWRa?=
 =?utf-8?B?VE1NQjRRTk5TRGRINVJoMHNBeTJtYnFVT3FSM3FFUGZmbStGMzhqMFJVQlJp?=
 =?utf-8?B?UjJxdlZ5NlI2cWZ5bzlaaEY5YklmaXhwcTkwQWRCSGl1ZTJvOVBuRDl4YlV1?=
 =?utf-8?B?TFNrZGg1V25XOG1NRHhBY0E0ZGZkRGNPbGgzcmUyMjcwVS9NTWdvM3AyNlVP?=
 =?utf-8?B?RlAxUi9sc2cwNVpFMjN4WXFRcndNT3kzb0g5QVFLT3BXb3lRaUJDS0dHdDBU?=
 =?utf-8?B?R0NnTTRRSjArTVVUSW4wK0tDUnlHT00wT1NrNW92R0dZd1d6U1JQTmp1N1ps?=
 =?utf-8?B?dmtZMk9URzJKU0Y2dk5tckxMazJDZ2h4RXh0Mmc2Zzk3ZXpaMnQreEZmdmQx?=
 =?utf-8?B?U3lIWTdvVWVqM00yNU9MdmFTSUJHL0t1M2dZejcrWmxOQXNYZFRsUEJabkE3?=
 =?utf-8?B?a2N5S1AwSWlrOExpMUxoT0tDNWZRaVZ5emJLdjNqWXZTNjZZNmxUL3JDVG4x?=
 =?utf-8?B?WUlaUlJSbGJNT0RhNGtRNi9kdDhJMUw5TkJNbnczVS9WVk40WnJ5RVFVNjAx?=
 =?utf-8?B?WldnL2NYYUxCZ0tsQytaR2ZDK3lOVC80TWdOUDFHZHQ5MEI3RHNaUFNTbmxG?=
 =?utf-8?B?dVRUTkFlby9OY3VKUGdiUlBDS0RqeGlNZHduR0ZuazZ2aFVjdDJoT1NCVDY0?=
 =?utf-8?B?bUJZeHhhUGNyMXAvUVZEUHZKamtJSjArcExTd0pxc0t6QXh2VWlta3E2K1N3?=
 =?utf-8?B?YTYvcmVIbW9ZWVJjdE55ZzBYTFk1OEZkbDR5eUc1V2Zud2hXOHRVUnQxMkk3?=
 =?utf-8?B?R0VnYVhwSVBCNEdhWnZBQjlvNjJjZDA4dTRCalY0dXpaNm90d1JLMnBNcjRi?=
 =?utf-8?B?ZXdwRGZFbVVUUzhQSlVSOFhzNUxtR3JSMXlVWFpPdEJYbWYweFhkR1oxL2ZC?=
 =?utf-8?B?NEk4dEZ5ZmI5Nlh6VGFjNjNvdnFYSFpQeTlHclZJVWgzVGZnMFVXS2pjaFRE?=
 =?utf-8?B?SFlUTTJQRjY4Q0JXVkI0SERBVFZpQVZvQVVhNHRYOFdFTnQ2Z25BbDhmdmJC?=
 =?utf-8?B?dGcyYTBXcnZFSHA0Y3AyclFLSGErTDMzQmlSZnpsTFRDMDN5RG9FaC9jUnRm?=
 =?utf-8?B?b3AwM3NrUXRhalVzSTB5UFowd2Y2b3Y3TVFFSnJKMmdRWk9tMnlMUGFRbnlT?=
 =?utf-8?B?QWtQNjEydmV2ZkUvdVZIVjNuUHl1MUhyN2h5eUtlZXZNVkM3c2FiSWRoWkl3?=
 =?utf-8?B?Q3UwSmVqdnc1cnN2UmgwU1lwaDIrQ1FpSmhvT2RPZlpNaVJBWUFrSHp3M3BR?=
 =?utf-8?B?eXhHbzI1WWtnYi91SWJuTlFndDJFa1Q4SGZwRmRCM2JSZ2FqeHRJTnByajAx?=
 =?utf-8?B?dFN6WWZJbGhPbWZNVFBSMHhTRkU2NjB6enJEOXMvMjMvSEV1ekVVZzBzU3dk?=
 =?utf-8?B?QkY3VUEvR01lT2xRZ29YSzMreGFyWWE2eVlCNnNlMHZ0Q3pYU0FYU3VaeU5L?=
 =?utf-8?B?cDA5UVREVlhaYXVSUDNaOWh0S0ZsaW1TL2ppa1oxdEdmbVlzZHNodDN4MVkr?=
 =?utf-8?B?d0tseS91cWRyOGpBWXY2NGdNT0xEMFd4SzgxZ25jdlRmVmsrSmdhV0dZTW51?=
 =?utf-8?B?ZnFMVFM0d21Vb1BVK25jVHVYRUU4WFlwTW41QVBiVnM5cmptVmViTHV0M1Aw?=
 =?utf-8?B?OSt3RzhTaGw4VHVRemJFamFmaGFGemF2MjhRaHhmS0IrbjI0eXEzWnA4a3BR?=
 =?utf-8?B?aGdkeTFKazROalk3ODBSU2YycVFLc0FpeTgxSTBPTWM1T1V0cW01aHhSd0RM?=
 =?utf-8?B?dlp0cEpBeGZRQVpOcm5zYkhnZGQxbnpUUkVMc0l3Qkk3ajk4S2k3YTJySHF3?=
 =?utf-8?B?R3BiN3RxRitodmJOMkxkUkdaTnUwWnViUWduV1VLZ3hjVFJRN2FtTkFEMDFt?=
 =?utf-8?B?a0hmdkJDQk1COU9rbkFaUkRsMGpicHZKR3hmb1dKUnBzakp3RlFuMEkxMGpn?=
 =?utf-8?B?NmYvdWdMK04zMk1iWlNzTzV2cGNKVFd1dlZRUFl1SlM1OVlBOFJ5ZXZLdUYz?=
 =?utf-8?B?VG1uVUVUR2VWcFJza1MzTjc3Zm10UnJweXdTMEdoMmFEWmxNWkNXUEVpaVRW?=
 =?utf-8?Q?x7QovrlsmJ7VzypY1IgJrkOg1zR4I0lxP/9JcOGhM48M?=
X-OriginatorOrg: kontron.de
X-MS-Exchange-CrossTenant-Network-Message-Id: ae2ed0ac-2f76-4250-3f3a-08dbfaebdef7
X-MS-Exchange-CrossTenant-AuthSource: PA4PR10MB5681.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Dec 2023 08:25:16.9330
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8c9d3c97-3fd9-41c8-a2b1-646f3942daf1
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PWQ23BwlmW+e0hZfOKb6YMNR2T1KWh+fpZiQNwtRSNbd1j4mKIUcu1aJDMpiMdbp0qeO5KfYHD1fpqPtVKd+ratAQdz5ASfPJbIsq1GjP/Y=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS2PR10MB7763
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,
	SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Tue, 12 Dec 2023 00:36:14 -0800 (PST)

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@gmail.com>
> 
> 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@kontron.de>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>

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) {