Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Peng Fan <peng.fan@nxp.com>
To:     Adam Ford <aford173@gmail.com>,
        "linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>
CC:     "aford@beaconembedded.com" <aford@beaconembedded.com>,
        Abel Vesa <abelvesa@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        dl-linux-imx <linux-imx@nxp.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] clk: imx: composite-8m:  Fix clock divisions greater than
 8
Thread-Topic: [PATCH] clk: imx: composite-8m:  Fix clock divisions greater
 than 8
Thread-Index: AQHZcJcyD+u9fdjl0kGyfYxulAld2K8urL9w
Date:   Mon, 17 Apr 2023 00:55:06 +0000
Message-ID: <DU0PR04MB9417F545EDAF40730E1C872D889C9@DU0PR04MB9417.eurprd04.prod.outlook.com>
References: <20230416191051.106513-1-aford173@gmail.com>
In-Reply-To: <20230416191051.106513-1-aford173@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?d63a+braDFa7zC/VTam8DHHItKjBCI8/RKc4yfWi168R9THFco6qTgQPKVTr?=
 =?us-ascii?Q?1c/UrceUFX7bWbZhZidF7HKUVuDaHmBEPEAjXOWBLYLQdEGhMGJ/WtGQt/w9?=
 =?us-ascii?Q?N+Si/aTvBdJEhEXRICaqxtjMUKkMkINiBz9rfa9YZrDMvClI6oxTBg9g0J0H?=
 =?us-ascii?Q?aGiIlZ5aBw4sUA+SKtPNN3VR8WJSfkZPwHQ8VVx5TDqO6pBCsez9wxjlsILD?=
 =?us-ascii?Q?NtnyJlneRORCOZitgcBkVfn8M3rpME3/x+rf6EkSOjpBd26RF6GSO8ik0ukI?=
 =?us-ascii?Q?EuNoS7iER9URsX9UlswxoSux4tUOZgMLadxvDdvdBnvKj4tnqI7h1k/rs7s1?=
 =?us-ascii?Q?hoOxlEytGVqgF7y/0mnLGf1iMayQNpBLFWlWDJbGMbR4EjpSs1r56vaeKjiw?=
 =?us-ascii?Q?Nf3ZP/GF7W3VcQUYp0Gc6IhRstr6Lrsik0T/NqP/UgHVAQkrAdmgTn5lz+0i?=
 =?us-ascii?Q?kPTnNwBn1Vy6eeQNbYOnTDEiBkPoiDaOjrmDgSHUdhRzwqYvwCU0BTGry7jS?=
 =?us-ascii?Q?ovCxaD4VeUCKIBG6ZHtFL3IECnsxqWgZmQLUAXcCVFdxtRJyqUAtDSMQKus6?=
 =?us-ascii?Q?AT7e7TmpX6UpjZFuqHeyhxaSjPT52WlxM1L24hOzpECqri4QIyvmsXFa4hiz?=
 =?us-ascii?Q?/KwlKzJchtKZtcBYuNua+i8tQBJtxHsyuGKbcaKPFrxUsmpI6icK2Rbaa259?=
 =?us-ascii?Q?IHYn9TVQA2eXHBMPQqeD+BBHbRdbQ6ClyuVONkyT7z+viPm5b5d34CZrw2P4?=
 =?us-ascii?Q?f+1Qq1Opro2ZqLRCJbUOMSRn1X+8OzqtH2vnjExHieyvrQKTjlkj0acKcf4p?=
 =?us-ascii?Q?nu0i4C7hpOpFIbr7ZkDld4a9zLki8Rgg7yD0htOikObFCvFFIJVASeOYsVtf?=
 =?us-ascii?Q?qLuBUfkAdnjapqAXAybvS8Fe2bsS0MOBFooW48MdrH+MaHkSlRfUGXYwP+gf?=
 =?us-ascii?Q?GMYDgmG1Ra7qvJ5KqhXNRol0cWrq5AyP8etD071gnBPrueZaOPlb3D82qkZ6?=
 =?us-ascii?Q?yTVT/SyfwZzymdAF6GcFhDXKdPxleGtgSLlCLsUCtr5Tu9v2eepNiqFysbqq?=
 =?us-ascii?Q?WmVQy7/Xx4IjhRAEbYjlxxFBrQGZhx8cI2w4V+mEQEAbgggUEBdyMAx5Qt8h?=
 =?us-ascii?Q?2Ghsz0sTzIusmh8sERc2Yjir5njMqTIUCAHQSztJBw3odEU11sA1Yc6Te07S?=
 =?us-ascii?Q?ByqNQ/E7Owc1cBSHlZ4bZAqFw7UuVi+ZgBR59H596gS90bRN4mxGXDh02SrA?=
 =?us-ascii?Q?Z/TMRhwnIxNNfDukaWx9hMfmMa75MLrzNOT55fmvNaSRzLO2y+B4gDuYhxM5?=
 =?us-ascii?Q?RSUCBXBrwi4YYEZx4DVaUIai9RntJELofOFhsVkcL0I8xRY8M8ZkWvdOtMli?=
 =?us-ascii?Q?aF/W9U0c9Vj/+GJ6v7N6EAt/qMAozDnr3fX5dwbn3KdJ8yPfqm79klxZAxbG?=
 =?us-ascii?Q?TH9EOnJT3Keqz3MsfVbz9w2hAqtgUI0PnzIBpbvPNcuJTdph3TRRQyoC8F+a?=
 =?us-ascii?Q?UGKt5IkoTqp7Vp7JMjpNOCsIrwoDbbkYqZimbgjUj/JTzacXJepv9DadjTcf?=
 =?us-ascii?Q?IzUea4zF+LyG1EIXcak=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DU0PR04MB9417.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e4431e39-9fe0-498c-2db7-08db3ede62bc
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Apr 2023 00:55:06.3102
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 01S42ANH0fA4u6SqMPFTDyduXF8As7lZsHmdATuRwaainN7aRf0IufQljZc6GdgxLg3hsVgU1CeLAQWCwKwwXw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8404
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> Subject: [PATCH] clk: imx: composite-8m: Fix clock divisions greater than=
 8
>=20
> When adding imx8m_clk_divider_determine_rate to let the imx8mm and
> imx8mn get finer granulatiry of video clocks, it accidentally broke the
> imx8mp ability to divide clocks by more than 8 on clocks when the
> CLK_SET_RATE_PARENT flag is not set.
>=20
> On the imx8mp, the CLK_SET_RATE_PARENT flag cannot be set on either
> media_disp1_pix or media_disp2_pix, because they both share the
> video_pll as a common clock, and if two displays are used, the parent clo=
ck
> needs to be something that both child clocks can divide.
>=20
> imx8m_clk_divider_determine_rate ends up calling clk_divider_bestdiv
> which uses the value of 'width' to setup the maximum divisor.  The clk-
> composite-8m driver sets the with to 3 which means the maximum divisor is
> 8, but these clocks can divide by up to 64.
>=20
> Currently, if the video_pll is set to 1039500000, the slowest clock rate
> achievable is 129937500 which breaks a whole bunch of lower resolution
> and refresh options for both media_disp1_pix and media_disp2_pix.
>=20
> By changing the 'width' variable to PCG_DIV_WIDTH, the maximum divisor
> becomes 64 which allows the clocks to divide down more, so the lower
> resolutions and refresh rates are achievable again.
>=20
> Fixes: 156e96ff2172 ("clk: imx: composite-8m: Add support to
> determine_rate")
> Signed-off-by: Adam Ford <aford173@gmail.com>
>=20
> diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-
> composite-8m.c
> index 6883a8199b6c..805e6aada83d 100644
> --- a/drivers/clk/imx/clk-composite-8m.c
> +++ b/drivers/clk/imx/clk-composite-8m.c
> @@ -47,6 +47,7 @@ static unsigned long
> imx8m_clk_composite_divider_recalc_rate(struct clk_hw *hw,
>  				   divider->flags, PCG_DIV_WIDTH);
>  }
>=20
> +
>  static int imx8m_clk_composite_compute_dividers(unsigned long rate,
>  						unsigned long parent_rate,
>  						int *prediv, int *postdiv)
> @@ -215,12 +216,12 @@ struct clk_hw *__imx8m_clk_hw_composite(const
> char *name,
>  		mux_ops =3D &imx8m_clk_composite_mux_ops;
>  	} else if (composite_flags & IMX_COMPOSITE_BUS) {
>  		div->shift =3D PCG_PREDIV_SHIFT;
> -		div->width =3D PCG_PREDIV_WIDTH;
> +		div->width =3D PCG_DIV_WIDTH;
This is wrong. Pre divider width is 3, not 6.

>  		divider_ops =3D &imx8m_clk_composite_divider_ops;
>  		mux_ops =3D &imx8m_clk_composite_mux_ops;
>  	} else {
>  		div->shift =3D PCG_PREDIV_SHIFT;
> -		div->width =3D PCG_PREDIV_WIDTH;
> +		div->width =3D PCG_DIV_WIDTH;
Ditto.

You need find the other way to resolve the regression.

Regards,
Peng.

>  		divider_ops =3D &imx8m_clk_composite_divider_ops;
>  		mux_ops =3D &clk_mux_ops;
>  		if (!(composite_flags & IMX_COMPOSITE_FW_MANAGED))
> --
> 2.39.2