From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751504AbeAEKD3 (ORCPT + 1 other); Fri, 5 Jan 2018 05:03:29 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:37845 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750857AbeAEKD0 (ORCPT ); Fri, 5 Jan 2018 05:03:26 -0500 Date: Fri, 5 Jan 2018 11:03:14 +0100 From: Maxime Ripard To: Jonathan Liu Cc: David Airlie , Chen-Yu Tsai , linux-kernel , dri-devel , linux-arm-kernel , linux-sunxi Subject: Re: [PATCH v2 1/3] drm/sun4i: hdmi: Check for unset best_parent in sun4i_tmds_determine_rate Message-ID: <20180105100314.3j5zm4ushv4tfky5@flea.lan> References: <20171226111227.4526-1-net147@gmail.com> <20171226111227.4526-2-net147@gmail.com> <20180104195650.vmbooz3pwjy77wt7@flea.lan> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fygjyq34ptjl6l4b" Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20171215 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: --fygjyq34ptjl6l4b Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 05, 2018 at 09:44:39AM +1100, Jonathan Liu wrote: > Hi Maxime, >=20 > On 5 January 2018 at 06:56, Maxime Ripard > wrote: > > On Tue, Dec 26, 2017 at 10:12:25PM +1100, Jonathan Liu wrote: > >> We should check if the best match has been set before comparing it. > >> > >> Fixes: 9c5681011a0c ("drm/sun4i: Add HDMI support") > >> Signed-off-by: Jonathan Liu > >> --- > >> drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c b/drivers/gpu= /drm/sun4i/sun4i_hdmi_tmds_clk.c > >> index dc332ea56f6c..4d235e5ea31c 100644 > >> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c > >> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c > >> @@ -102,7 +102,7 @@ static int sun4i_tmds_determine_rate(struct clk_hw= *hw, > >> goto out; > >> } > >> > >> - if (abs(rate - rounded / i) < > >> + if (!best_parent || abs(rate - rounded /= i) < > > > > Why is that causing any issue? > > > > If best_parent is set to 0... > > > >> abs(rate - best_parent / best_div)) { > > > > ... the value returned here is going to be rate, which is going to be > > higher than the first part of the comparison meaning ... > > > >> best_parent =3D rounded; > > > > ... that best_parent is going to be set there. >=20 > Consider the following: > rate =3D 83500000 > rounded =3D ideal * 2 >=20 > It is possible that if "rounded =3D clk_hw_round_rate(parent, ideal)" > gives high enough values that the condition "abs(rate - rounded / i) < > abs(rate - best_parent / best_div)" is never met. >=20 > Then you can end up with: > req->rate =3D 0 > req->best_parent_rate =3D 0 > req->best_parent_hw =3D ... >=20 > Also, the sun4i_tmds_calc_divider function has a similar check. Ok. That explanation must be part of your commit log, or at least which problem you're trying to address and in which situation it will trigger. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --fygjyq34ptjl6l4b Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEE0VqZU19dR2zEVaqr0rTAlCFNr3QFAlpPTWEACgkQ0rTAlCFN r3TsUA/9HXArZb38Cdwg54K5lq7FwnkzwJPYt4M4y8aQ+tdsQZelS1cWORYaFcUY Y8gQ3gTuc31GvtJTa6esTVCO3FSmB9bczxCVBpyLC2dqP+ZhFwmwyUtSg/V8s6Rv bq81tgnX0oA9/Uj3cYYNoaqJ8ZeaFYj8K21i8YqlyJX0tdPvb4qneVzcUyvovr6B 858gDerr2d7H44FYaFCqD9z31+BrhEMgzu6l8ogPWAOdbn+hpxEwNKpHVQd18XxD ttufhFzvZ9/9Mbm3y3er1FDW9FWajA8QvgD7q/3IGriCw/laXATlYhsBRNjqIXfR fWhtsfpdIYZ6TlREqYGkV8oj2SyMRjFA+9dCQXEJ7qsbhBi1D1izy6NQ4v16egUd ZyUpinnqYULHD8JVfgTRkHR+B6sXLYyaZVcFYKzTln6UyHOK5sOTuW72yH+GBT/c NFdlwsBFlD80JOEXSLIDv1kTj7BVOdxaSleCLWG/oFGKRcD80rgAyDYcBy33yfHf a3G75pzyg1/DCSxkY/fX3aXM4qnLLHgMzxh0Cl4gvlPUQIBbe5zw5ynPBUfgwz7j IFNCXORHFliPvAnc4VySCIZz8Mfch+f5pA3iG3LxtiRmOIMYvmRm62HduypVFN5s 6ZXksWGZwiCFHNp/GU10b4rLAfRg3TIc8W6oUkV4ZNtbEJQ3X14= =9pEt -----END PGP SIGNATURE----- --fygjyq34ptjl6l4b--