From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B2605C433E2 for ; Tue, 1 Sep 2020 09:46:38 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7ABA32083B for ; Tue, 1 Sep 2020 09:46:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="TKm/Gi8r"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=cerno.tech header.i=@cerno.tech header.b="MJkvAFab"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="HBBaWnBs" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7ABA32083B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cerno.tech Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=BhugjliN44nkjpfVhl2TCq8KclSHxsq2H5QMkeoStFc=; b=TKm/Gi8rCj4ypRXm8K/ILeNX3 JRv919k2IuV39bNgG7U9R1IxpsHEAJnmPS1UHi2rda7OKG4B9lYMQNX4uNCz9gSRvPJlrvSrlLbaA xKgJgZni3STKvagGDZ6UEeVe99ILRf95phw+uv92zN0oIYZAcPFO8kj9iQ6wFEPOU8GasLcAq5k54 oiprjS12eQfFub/Ktc8LdFGM0ZRoi6Cw44MKs8RDz6WD6bT8TzKe6kE2fFCMpIFlN5Mzsx3c1e2ZJ eiW7gr1E8H5WJ7qopgj/Yo7YBamk/T59EFGXgupHRpfIMBM3bEtRlkccpmHLmMDpowpriY7ZAwjhm nxnHNoRcw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kD2qs-0000VH-6z; Tue, 01 Sep 2020 09:45:18 +0000 Received: from wnew3-smtp.messagingengine.com ([64.147.123.17]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kD2qp-0000UZ-VY; Tue, 01 Sep 2020 09:45:16 +0000 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.west.internal (Postfix) with ESMTP id 41796638; Tue, 1 Sep 2020 05:45:13 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Tue, 01 Sep 2020 05:45:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm3; bh=4gcyFzed9X7eNC7JzpnKedk4o4e NcTrAMfkvVcC89lk=; b=MJkvAFabzYnLIuY8R6cMW1TKfX3Z01jyYZqBVsx0iJj kaVPG09Kvm5zLsQBGnseFzZ7CdjPPgek2sIOsHRjaAvsLqg4iERsSzzJ2HzMg56u chvcdPJW914+nOBtkiiNQo17fgf92eLjdIk9f9Sxei8m1bn2bi8kIAeL84/XrD+d MMTWaRvsG4QZnVp738UtbWsW8iE/o13rVP2gwBIZlO2dGyzMm60G23TeELgXiQ0V CEgasM23QQ0xnvG0iR3+ZoJM1U8aq7+P50kU2PXJ3Jj+nfo2szEsPYWTuhrBztxG uhIzPJipzF2fvlE5l5aETJcCfc5BY2nFT/mMA2rFvSA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=4gcyFz ed9X7eNC7JzpnKedk4o4eNcTrAMfkvVcC89lk=; b=HBBaWnBsPg9041RHVLgrHY iU6422JtfjgK93fFnZh6tOcjmm7qYs9RnbcSJl4WJwvDr4elf4CRlo3+vdEf58Lm yMQq15DmAiQCPwYCMbiRhksdHdyiRQKufj3YdWbpfN7xVWKL0Y1JXiCGfWqnx3Fg 4JQROsFW2egQU28hv6aOsv/i7JmOI+XP+Bougqrkqr8sqGdWPdZ3OUK4FqVr05Xv s2Aeb5Ng/zOKAM1+05jB63aVcffSuBdOc/kqRlsILcRyF9sBuXcUfUsu2zdhjP2N BpuRjtTQBjv7n5DPkA5yKOX45YQ0r8Hjtxl6WoTeMuppW0WTT0yKldBlmlhSV5LQ == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduiedrudefjedgvdduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnhepleekgeehhfdutdeljefgleejffehfffgieejhffgueefhfdtveetgeehieeh gedunecukfhppeeltddrkeelrdeikedrjeeinecuvehluhhsthgvrhfuihiivgeptdenuc frrghrrghmpehmrghilhhfrhhomhepmhgrgihimhgvsegtvghrnhhordhtvggthh X-ME-Proxy: Received: from localhost (lfbn-tou-1-1502-76.w90-89.abo.wanadoo.fr [90.89.68.76]) by mail.messagingengine.com (Postfix) with ESMTPA id 5B5F03280059; Tue, 1 Sep 2020 05:45:11 -0400 (EDT) Date: Tue, 1 Sep 2020 11:45:09 +0200 From: Maxime Ripard To: Chanwoo Choi Subject: Re: [PATCH v4 62/78] drm/vc4: hdmi: Adjust HSM clock rate depending on pixel rate Message-ID: <20200901094509.spgxtkfybebo7mmb@gilmour.lan> References: <5919dccdd4a792936e6cb7eb55983c530c9a468d.1594230107.git-series.maxime@cerno.tech> <95172a9a-e861-5e5d-bf51-2ec03c730237@samsung.com> MIME-Version: 1.0 In-Reply-To: <95172a9a-e861-5e5d-bf51-2ec03c730237@samsung.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200901_054516_126684_6CCB67A9 X-CRM114-Status: GOOD ( 33.95 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Tim Gover , Dave Stevenson , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Eric Anholt , bcm-kernel-feedback-list@broadcom.com, Nicolas Saenz Julienne , Phil Elwell , linux-arm-kernel@lists.infradead.org, linux-rpi-kernel@lists.infradead.org Content-Type: multipart/mixed; boundary="===============4421970157287630951==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============4421970157287630951== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="l2cz7keizc6yoqv4" Content-Disposition: inline --l2cz7keizc6yoqv4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Chanwoo, On Tue, Sep 01, 2020 at 01:36:17PM +0900, Chanwoo Choi wrote: > On 7/9/20 2:42 AM, Maxime Ripard wrote: > > The HSM clock needs to be setup at around 101% of the pixel rate. This > > was done previously by setting the clock rate to 163.7MHz at probe time= and > > only check in mode_valid whether the mode pixel clock was under the pix= el > > clock +1% or not. > >=20 > > However, with 4k we need to change that frequency to a higher frequency > > than 163.7MHz, and yet want to have the lowest clock as possible to hav= e a > > decent power saving. > >=20 > > Let's change that logic a bit by setting the clock rate of the HSM clock > > to the pixel rate at encoder_enable time. This would work for the > > BCM2711 that support 4k resolutions and has a clock that can provide it, > > but we still have to take care of a 4k panel plugged on a BCM283x SoCs > > that wouldn't be able to use those modes, so let's define the limit in > > the variant. > >=20 > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/vc4/vc4_hdmi.c | 79 ++++++++++++++++------------------- > > drivers/gpu/drm/vc4/vc4_hdmi.h | 3 +- > > 2 files changed, 41 insertions(+), 41 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_h= dmi.c > > index 17797b14cde4..9f30fab744f2 100644 > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > > @@ -53,7 +53,6 @@ > > #include "vc4_hdmi_regs.h" > > #include "vc4_regs.h" > > =20 > > -#define HSM_CLOCK_FREQ 163682864 > > #define CEC_CLOCK_FREQ 40000 > > =20 > > static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) > > @@ -326,6 +325,7 @@ static void vc4_hdmi_encoder_disable(struct drm_enc= oder *encoder) > > HDMI_WRITE(HDMI_VID_CTL, > > HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE); > > =20 > > + clk_disable_unprepare(vc4_hdmi->hsm_clock); > > clk_disable_unprepare(vc4_hdmi->pixel_clock); > > =20 > > ret =3D pm_runtime_put(&vc4_hdmi->pdev->dev); > > @@ -423,6 +423,7 @@ static void vc4_hdmi_encoder_enable(struct drm_enco= der *encoder) > > struct vc4_hdmi *vc4_hdmi =3D encoder_to_vc4_hdmi(encoder); > > struct vc4_hdmi_encoder *vc4_encoder =3D to_vc4_hdmi_encoder(encoder); > > bool debug_dump_regs =3D false; > > + unsigned long pixel_rate, hsm_rate; > > int ret; > > =20 > > ret =3D pm_runtime_get_sync(&vc4_hdmi->pdev->dev); > > @@ -431,9 +432,8 @@ static void vc4_hdmi_encoder_enable(struct drm_enco= der *encoder) > > return; > > } > > =20 > > - ret =3D clk_set_rate(vc4_hdmi->pixel_clock, > > - mode->clock * 1000 * > > - ((mode->flags & DRM_MODE_FLAG_DBLCLK) ? 2 : 1)); > > + pixel_rate =3D mode->clock * 1000 * ((mode->flags & DRM_MODE_FLAG_DBL= CLK) ? 2 : 1); > > + ret =3D clk_set_rate(vc4_hdmi->pixel_clock, pixel_rate); > > if (ret) { > > DRM_ERROR("Failed to set pixel clock rate: %d\n", ret); > > return; > > @@ -445,6 +445,36 @@ static void vc4_hdmi_encoder_enable(struct drm_enc= oder *encoder) > > return; > > } > > =20 > > + /* > > + * As stated in RPi's vc4 firmware "HDMI state machine (HSM) clock mu= st > > + * be faster than pixel clock, infinitesimally faster, tested in > > + * simulation. Otherwise, exact value is unimportant for HDMI > > + * operation." This conflicts with bcm2835's vc4 documentation, which > > + * states HSM's clock has to be at least 108% of the pixel clock. > > + * > > + * Real life tests reveal that vc4's firmware statement holds up, and > > + * users are able to use pixel clocks closer to HSM's, namely for > > + * 1920x1200@60Hz. So it was decided to have leave a 1% margin between > > + * both clocks. Which, for RPi0-3 implies a maximum pixel clock of > > + * 162MHz. > > + * > > + * Additionally, the AXI clock needs to be at least 25% of > > + * pixel clock, but HSM ends up being the limiting factor. > > + */ > > + hsm_rate =3D max_t(unsigned long, 120000000, (pixel_rate / 100) * 101= ); > > + ret =3D clk_set_rate(vc4_hdmi->hsm_clock, hsm_rate); > > + if (ret) { > > + DRM_ERROR("Failed to set HSM clock rate: %d\n", ret); > > + return; > > + } > > + > > + ret =3D clk_prepare_enable(vc4_hdmi->hsm_clock); > > + if (ret) { > > + DRM_ERROR("Failed to turn on HSM clock: %d\n", ret); > > + clk_disable_unprepare(vc4_hdmi->pixel_clock); > > + return; > > + } >=20 > About vc4_hdmi->hsm_clock instance, usually, we need to enable the clock > with clk_prepare_enable() and then touch the clock like clk_set_rate(). > I think that need to enable the clock before calling clk_set_rate(). >=20 > When I tested this patchset, it is well working because I think that > vc4_hdmi->hsm_clock was already enabled on other side. There's no clear rule here on the ordering (at least enforced by the framework). There's clocks that need to be disabled to change their rate (CLK_SET_RATE_GATE) and some that need to be enabled to change their rate (CLK_SET_RATE_UNGATE). Generally speaking, it seems more logical to me to have first the rate changed and then the clock enabled since it won't create any "hiccup", but I could very well see the opposite to be preferred. Maxime --l2cz7keizc6yoqv4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCX04YJQAKCRDj7w1vZxhR xW70AQD/ZlQ9or7xShXk/2/RckodgJ+4vJLaVsavf7Tsd0WhvAEAiTeK619R18o6 OPxacjLjaE8/DQ+zuJRSY/O8LFoIdgU= =OBfP -----END PGP SIGNATURE----- --l2cz7keizc6yoqv4-- --===============4421970157287630951== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============4421970157287630951==--