From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCHv3 05/30] drm/omap: improve DPI clock selection on DRA7xx Date: Wed, 29 Mar 2017 11:36:46 +0300 Message-ID: References: <1490706496-4959-1-git-send-email-tomi.valkeinen@ti.com> <1490706496-4959-6-git-send-email-tomi.valkeinen@ti.com> <4721329.6I6ROrOGic@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1933720291==" Return-path: Received: from fllnx209.ext.ti.com (fllnx209.ext.ti.com [198.47.19.16]) by gabe.freedesktop.org (Postfix) with ESMTPS id B3DD66E001 for ; Wed, 29 Mar 2017 08:37:19 +0000 (UTC) In-Reply-To: <4721329.6I6ROrOGic@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart Cc: Jyri Sarha , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1933720291== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="l0CsSoBdrfEE8wjnvlovFxArI1MARF1KV" --l0CsSoBdrfEE8wjnvlovFxArI1MARF1KV Content-Type: multipart/mixed; boundary="mjhFRCw8EB1ohof9FmxWTcpkqsDGPVqPA"; protected-headers="v1" From: Tomi Valkeinen To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org, Jyri Sarha Message-ID: Subject: Re: [PATCHv3 05/30] drm/omap: improve DPI clock selection on DRA7xx References: <1490706496-4959-1-git-send-email-tomi.valkeinen@ti.com> <1490706496-4959-6-git-send-email-tomi.valkeinen@ti.com> <4721329.6I6ROrOGic@avalon> In-Reply-To: <4721329.6I6ROrOGic@avalon> --mjhFRCw8EB1ohof9FmxWTcpkqsDGPVqPA Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 29/03/17 11:19, Laurent Pinchart wrote: > Hi Tomi, >=20 > Thank you for the patch. >=20 > On Tuesday 28 Mar 2017 16:07:51 Tomi Valkeinen wrote: >> The clock source selection for the LCD outputs is too hardcoded at the= >> moment. For example, LCD3 is set to use PLL2_1, and PLL2 doesn't exist= >> on DRA72x SoCs. >> >> There are quite many ways to configure the clocks, even using HDMI PLL= >> for LCD outputs, but enabling full configuration of the clocks is rath= er >> tricky. >> >> This patch improves the situation a bit by checking if the PLL about t= o >> be used exists, and if not, tries another one. >> >> Signed-off-by: Tomi Valkeinen >> --- >> drivers/gpu/drm/omapdrm/dss/dpi.c | 47 +++++++++++++++++++++++++++---= ------ >> 1 file changed, 37 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c >> b/drivers/gpu/drm/omapdrm/dss/dpi.c index e0b0c5c24c55..0f32d5d078c6 1= 00644 >> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c >> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c >> @@ -67,6 +67,42 @@ static struct dpi_data *dpi_get_data_from_pdev(stru= ct >> platform_device *pdev) return dev_get_drvdata(&pdev->dev); >> } >> >> +static enum dss_clk_source dpi_get_clk_src_dra7xx(enum omap_channel >> channel) >> +{ >> + /* >> + * Possible clock sources: >> + * LCD1: FCK/PLL1_1/HDMI_PLL >> + * LCD2: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_3) >> + * LCD3: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_1) >> + */ >> + >> + switch (channel) { >> + case OMAP_DSS_CHANNEL_LCD: >> + { >> + if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_1)) >> + return DSS_CLK_SRC_PLL1_1; >> + } >=20 > Aren't you missing break statements ? Huh, indeed I am. Thanks! Interesting that we've never hit that bug (this has been in TI kernel for some time). >=20 >> + case OMAP_DSS_CHANNEL_LCD2: >> + { >> + if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3)) >> + return DSS_CLK_SRC_PLL1_3; >> + if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_3)) >> + return DSS_CLK_SRC_PLL2_3; >> + } >> + case OMAP_DSS_CHANNEL_LCD3: >> + { >> + if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_1)) >> + return DSS_CLK_SRC_PLL2_1; >> + if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3)) >> + return DSS_CLK_SRC_PLL1_3; >=20 > What happens if LCD2 is already using PLL1_3 ? Bad things, I guess? =3D) It is a limitation with the current implementation that some combinations are not really possible. To support all the cases, we should somehow share the clocks, and manage the needs of the displays using the same clocks... We have nothing for that. So if you design a board that uses LCD2 and LCD3, and you don't have PLL2, then the driver won't work. Sharing the clock with two displays of exact same pixel clock possibly works even now, but I have not tested it. This patch improves the situation a bit, by allowing all cases where we don't share the clocks, but it doesn't try to fix the sharing problem. Tomi --mjhFRCw8EB1ohof9FmxWTcpkqsDGPVqPA-- --l0CsSoBdrfEE8wjnvlovFxArI1MARF1KV Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJY23IeAAoJEPo9qoy8lh71VTwP/1fUzsacAf1f0Pk3ANENu+gd RsfhSRjsG7F6NvpAIODXcYuW8OsUOmvVUYQKd/UuGIibTIMdr78vl1iLe3bMyK2z c53Fe2ZCNVeVEK0yBRhsRKXInd+96looSTKVnCvs4JjnYyubzqZCxZUKScGFImnb cwh/DDtD5hfXhjON3l7jGUvwRmJYbYD7KDWHN93AlUg3LeBk0Vg5hZvzbU4+AWqQ ManzJ2UdI2TGoLRGso8Jb1pgdIsU75LwAMbZHzDXW2Vjxfwr0LmbiUPExLe27Llq MDzI282sepx40UVCFoVKSkaDwGgUAIXzEmmL+Z63fIONZqR5JtiShfafcesRfbEW IioUNSMUjhZ4d5FvaICXySHNapyAUja2GdYUuI8zPZnw04aQmsvOsYuYqClHCcWN P43mfKOInDrH5l7Hq4EQU8ImtHsQmrGPv5FAkzJvzyl4XQtZBiBOomKNs64KHPl9 MVT6B8dZY2ePCe9W2GMWNMdPU89KFXbxTwX8hTgaK+ng0kSSwbkspWs3YIOvxXk9 CpFFgwfL8+TJGTsmvQgLerelNN9JpgdOlsMzFquSvbuiajhk7aq+dPIANFzFYNDt fn3kXZKUYbaVus2UeydsV3SSTVfuEcLl1yqpuP44c4wt4Rbm3iJO9Lt0TWzm2spZ 5IP/tBwfxS0PxG0QjZLt =fOcm -----END PGP SIGNATURE----- --l0CsSoBdrfEE8wjnvlovFxArI1MARF1KV-- --===============1933720291== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1933720291==--