Received: by 2002:a05:6358:9144:b0:117:f937:c515 with SMTP id r4csp10810974rwr; Fri, 12 May 2023 13:24:33 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6SzbhWfaSMlerxJ1cNAcrWiFI24uJRiLU7FvOdRUq0dR/EPofbmGvW/h62Abn0oIpPqnvw X-Received: by 2002:a05:6a20:1587:b0:101:5171:c880 with SMTP id h7-20020a056a20158700b001015171c880mr17705002pzj.62.1683923073442; Fri, 12 May 2023 13:24:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683923073; cv=none; d=google.com; s=arc-20160816; b=DZw6Vr1PyTixtTY7gruO+Bml/KJlzIhcAxmcuqSbCS9DpqJPaP5GYoZVLMPECrQI8J Mfj788BA79pGM41td2aBVk2BXD4/5KcW1eT1rVVwfCHawW8nLDKcuHQ2Bsmn0bG47eLm MtoQy/02JdUtJCkgEARtXXZzwq/2X9wrQ5RBv9wKEpun/pSbhK9sy2DpmGceZOWioFpX 9Eap5ktlTGHMv8wnQjbVcooY+i4RJQBuuMSyI12Yl8XZHoUzR/rnaoAouKo+35J9U45h fkp+y3CcSfmhxpVuC0iFrvzscwaRtlK9Sz/XYUbQzQ1ZjuoupXKZkrQh6SrpYNxPR+mw bIRg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=7yplfwTnAjJ22mVVSMi+sjEOrdV8REJgqS175ygmB04=; b=APk2WYrZWjkT4AsMx9IVkTP/i1NZuq+2dgGk/IPmboPxXz82W/Dec6UKMSZeGFJyha o9EKXcOXKSPOjwrclG9XSUipgnUJ2DBFYqAuyCF2JESAkBsPSEBGTa/mMLBPbsnyPLHe 65NME7jfKKVTC15SDquiHQcqR3hLBF19tpfo7Y/BzRrFId4l/dygSKRy6ssUD7wGX3AX IqkELv+3pKFSsmHGvDABZ7N/RbQ2cfr3Wd0TNKgN2yt9XJaLRFoXlg8m/F9tUYYFcvuM axRx+CQttumQdrkT407AKr6NbEh8ZRRTUNhorAMNWQ6DSAz/cAFr0/sDV6G0czcK3K19 soAA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=XYrLQ3At; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l71-20020a63914a000000b005308b2910c5si3851719pge.338.2023.05.12.13.24.19; Fri, 12 May 2023 13:24:33 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=XYrLQ3At; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238606AbjELUBO (ORCPT + 99 others); Fri, 12 May 2023 16:01:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55412 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230373AbjELUBM (ORCPT ); Fri, 12 May 2023 16:01:12 -0400 Received: from mail-pg1-x52f.google.com (mail-pg1-x52f.google.com [IPv6:2607:f8b0:4864:20::52f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8C5C56A7A for ; Fri, 12 May 2023 13:01:11 -0700 (PDT) Received: by mail-pg1-x52f.google.com with SMTP id 41be03b00d2f7-5144043d9d1so7115286a12.3 for ; Fri, 12 May 2023 13:01:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683921671; x=1686513671; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=7yplfwTnAjJ22mVVSMi+sjEOrdV8REJgqS175ygmB04=; b=XYrLQ3AtF9CTcvXNCZr2McUhiCgzvBlvtfKpfEtzCzr7M/9SdlGiwmYFyiMGWmXkLR O6yLLKCZUQGF8136SBBcoAin4klJw1LJICoeupy5+3P5kGqp9HBA5TzmymO5Z8XLWvio uYh/oI73BViBbBgiUc6U4/F7EQ+4a9dA1L3EWzi1mteNH2cjBeCpft7mIGYndBqSmBGu fbuwtFer5utI4YDnx0PeCxSdFxX6npMi0rCg5io9AsJQ9zDlmPMTYhwR1NShxTFUAofd q6BDwHneNCKh3S6Iicww8sTL9Rnfd860g7+3bnBScOPUAvUr1uNdNPdXoYrEvCfXon5w DBQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683921671; x=1686513671; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7yplfwTnAjJ22mVVSMi+sjEOrdV8REJgqS175ygmB04=; b=JEA9ZCug8X/KbfbY0zoZPJfd5+ZOTei4+8+vscTguwKeWTGe7AuWEgI/dWBmRkPr/4 9YBjdNOTfZ5m8bOvobDI3Aq6nCsKK1QvaMb6IQdVmeNKZv/MMTWdKghr5QROMR1RhAul Ar4WbhLmZlxwbEFEiGshOFxYJ9RfkM35ZMn9xLDAvsn8y59fHkHS4Dr/6O/MlAPypBKR Ju9jbziSJewYvmuAloorZNYh+AoARoLY7APvM9NjW5E0sJ3xzJCMmKeCzG5SJ8DszUe5 ClFwpWpvOMV5d4nYgpfIQTyrvGEwKQDA7wBrIY4g4mM6+11TwEV1r5WRUS5NwsNrDaMG rN6g== X-Gm-Message-State: AC+VfDx5Vx9B4zVT86FSPPMuNTxRdBijt4CO0tpAvsEhS7LKflKOLHB3 bSvVY7tlDfIX7pWX0y4S+228pbDOy0yHprpjMTKaCu0n X-Received: by 2002:a17:90b:38c2:b0:252:8b33:52cc with SMTP id nn2-20020a17090b38c200b002528b3352ccmr7274696pjb.16.1683921670658; Fri, 12 May 2023 13:01:10 -0700 (PDT) MIME-Version: 1.0 References: <20230506192453.725621-1-aford173@gmail.com> <20230506192453.725621-6-aford173@gmail.com> <275064c0e6c814d8e8fda6bcf70d6e8c3bdc3011.camel@pengutronix.de> In-Reply-To: <275064c0e6c814d8e8fda6bcf70d6e8c3bdc3011.camel@pengutronix.de> From: Adam Ford Date: Fri, 12 May 2023 15:00:59 -0500 Message-ID: Subject: Re: [PATCH V5 5/6] drm: bridge: samsung-dsim: Dynamically configure DPHY timing To: Lucas Stach Cc: dri-devel@lists.freedesktop.org, Marek Vasut , Neil Armstrong , Jernej Skrabec , Robert Foss , Jonas Karlman , aford@beaconembedded.com, Frieder Schrempf , linux-kernel@vger.kernel.org, Michael Walle , Laurent Pinchart , Andrzej Hajda , Chen-Yu Tsai , Marek Szyprowski , Jagan Teki Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,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: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 12, 2023 at 2:37=E2=80=AFPM Lucas Stach wrote: > > Hi Adam, > > Am Samstag, dem 06.05.2023 um 14:24 -0500 schrieb Adam Ford: > > The DPHY timings are currently hard coded. Since the input > > clock can be variable, the phy timings need to be variable > > too. Add an additional variable to the driver data to enable > > this feature to prevent breaking boards that don't support it. > > > > The phy_mipi_dphy_get_default_config function configures the > > DPHY timings in pico-seconds, and a small macro converts those > > timings into clock cycles based on the pixel clock rate. > > > This week I finally had some time to take a deeper look at this series > and test it on some of my systems. Thanks for testing this! > > This patch causes issues when the burst clock rate is fixed by > supplying the DT entry. Instead of describing the issue below, I'm > attaching the patch that makes things work on my system. Oops, sorry about that. > > I would appreciate if you could test this one on your side. Feel free > to squash it into yours if you find it working properly. I reviewed your patch, and it looks like it makes a lot of sense. If it works, I'll squash them together and add your name to the sign-off. > > Also I would almost bet that dynamic_dphy is working on the Exynos > boards with that fix added. So if anyone with access to those boards > would like to give it a shot, we may be able to get rid of the > hardcoded PHY parameters altogether, which would be a nice cleanup. I wondered the same thing, but I didn't want to create more work for Marek S and since there was so much churn getting the original driver ported, I thought it would be the safest thing to try to give the imx8m m/n/p the features without breaking the Exynos. Marek S - Do you want me to post this file without the extra checks to see if it still works with Exynos? adam > > Regards, > Lucas > > > Signed-off-by: Adam Ford > > Tested-by: Chen-Yu Tsai > > Tested-by: Frieder Schrempf > > Reviewed-by: Frieder Schrempf > > Tested-by: Michael Walle > > --- > > drivers/gpu/drm/bridge/samsung-dsim.c | 74 ++++++++++++++++++++++++--- > > include/drm/bridge/samsung-dsim.h | 1 + > > 2 files changed, 68 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/br= idge/samsung-dsim.c > > index 08266303c261..d19a5c87b749 100644 > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > @@ -218,6 +218,8 @@ > > > > #define OLD_SCLK_MIPI_CLK_NAME "pll_clk" > > > > +#define PS_TO_CYCLE(ps, hz) DIV64_U64_ROUND_CLOSEST(((ps) * (hz)), 100= 0000000000ULL) > > + > > static const char *const clk_names[5] =3D { > > "bus_clk", > > "sclk_mipi", > > @@ -487,6 +489,7 @@ static const struct samsung_dsim_driver_data imx8mm= _dsi_driver_data =3D { > > .m_min =3D 64, > > .m_max =3D 1023, > > .min_freq =3D 1050, > > + .dynamic_dphy =3D 1, > > }; > > > > static const struct samsung_dsim_driver_data * > > @@ -698,13 +701,50 @@ static void samsung_dsim_set_phy_ctrl(struct sams= ung_dsim *dsi) > > const struct samsung_dsim_driver_data *driver_data =3D dsi->drive= r_data; > > const unsigned int *reg_values =3D driver_data->reg_values; > > u32 reg; > > + struct drm_display_mode *m =3D &dsi->mode; > > + int bpp =3D mipi_dsi_pixel_format_to_bpp(dsi->format); > > + struct phy_configure_opts_mipi_dphy cfg; > > + int clk_prepare, lpx, clk_zero, clk_post, clk_trail; > > + int hs_exit, hs_prepare, hs_zero, hs_trail; > > + unsigned long long clock_in_hz =3D m->clock * 1000; > > > > if (driver_data->has_freqband) > > return; > > > > + /* The dynamic_phy has the ability to adjust PHY Timing settings = */ > > + if (driver_data->dynamic_dphy) { > > + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->l= anes, &cfg); > > + > > + /* > > + * TODO: > > + * The tech reference manual for i.MX8M Mini/Nano/Plus > > + * doesn't state what the definition of the PHYTIMING > > + * bits are beyond their address and bit position. > > + * After reviewing NXP's downstream code, it appears > > + * that the various PHYTIMING registers take the number > > + * of cycles and use various dividers on them. This > > + * calculation does not result in an exact match to the > > + * downstream code, but it is very close, and it appears > > + * to sync at a variety of resolutions. If someone > > + * can get a more accurate mathematical equation needed > > + * for these registers, this should be updated. > > + */ > > + > > + lpx =3D PS_TO_CYCLE(cfg.lpx, clock_in_hz); > > + hs_exit =3D PS_TO_CYCLE(cfg.hs_exit, clock_in_hz); > > + clk_prepare =3D PS_TO_CYCLE(cfg.clk_prepare, clock_in_hz)= ; > > + clk_zero =3D PS_TO_CYCLE(cfg.clk_zero, clock_in_hz); > > + clk_post =3D PS_TO_CYCLE(cfg.clk_post, clock_in_hz); > > + clk_trail =3D PS_TO_CYCLE(cfg.clk_trail, clock_in_hz); > > + hs_prepare =3D PS_TO_CYCLE(cfg.hs_prepare, clock_in_hz); > > + hs_zero =3D PS_TO_CYCLE(cfg.hs_zero, clock_in_hz); > > + hs_trail =3D PS_TO_CYCLE(cfg.hs_trail, clock_in_hz); > > + } > > + > > /* B D-PHY: D-PHY Master & Slave Analog Block control */ > > reg =3D reg_values[PHYCTRL_ULPS_EXIT] | reg_values[PHYCTRL_VREG_L= P] | > > reg_values[PHYCTRL_SLEW_UP]; > > + > > samsung_dsim_write(dsi, DSIM_PHYCTRL_REG, reg); > > > > /* > > @@ -712,7 +752,11 @@ static void samsung_dsim_set_phy_ctrl(struct samsu= ng_dsim *dsi) > > * T HS-EXIT: Time that the transmitter drives LP-11 following a = HS > > * burst > > */ > > - reg =3D reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT]= ; > > + if (driver_data->dynamic_dphy) > > + reg =3D DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT= (hs_exit); > > + else > > + reg =3D reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_= HS_EXIT]; > > + > > samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg); > > > > /* > > @@ -728,10 +772,17 @@ static void samsung_dsim_set_phy_ctrl(struct sams= ung_dsim *dsi) > > * T CLK-TRAIL: Time that the transmitter drives the HS-0 state a= fter > > * the last payload clock bit of a HS transmission burst > > */ > > - reg =3D reg_values[PHYTIMING_CLK_PREPARE] | > > - reg_values[PHYTIMING_CLK_ZERO] | > > - reg_values[PHYTIMING_CLK_POST] | > > - reg_values[PHYTIMING_CLK_TRAIL]; > > + if (driver_data->dynamic_dphy) { > > + reg =3D DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) | > > + DSIM_PHYTIMING1_CLK_ZERO(clk_zero) | > > + DSIM_PHYTIMING1_CLK_POST(clk_post) | > > + DSIM_PHYTIMING1_CLK_TRAIL(clk_trail); > > + } else { > > + reg =3D reg_values[PHYTIMING_CLK_PREPARE] | > > + reg_values[PHYTIMING_CLK_ZERO] | > > + reg_values[PHYTIMING_CLK_POST] | > > + reg_values[PHYTIMING_CLK_TRAIL]; > > + } > > > > samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg); > > > > @@ -744,8 +795,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsu= ng_dsim *dsi) > > * T HS-TRAIL: Time that the transmitter drives the flipped diffe= rential > > * state after last payload data bit of a HS transmission bu= rst > > */ > > - reg =3D reg_values[PHYTIMING_HS_PREPARE] | reg_values[PHYTIMING_H= S_ZERO] | > > - reg_values[PHYTIMING_HS_TRAIL]; > > + > > + if (driver_data->dynamic_dphy) { > > + reg =3D DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) | > > + DSIM_PHYTIMING2_HS_ZERO(hs_zero) | > > + DSIM_PHYTIMING2_HS_TRAIL(hs_trail); > > + } else { > > + reg =3D reg_values[PHYTIMING_HS_PREPARE] | > > + reg_values[PHYTIMING_HS_ZERO] | > > + reg_values[PHYTIMING_HS_TRAIL]; > > + } > > + > > samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg); > > } > > > > diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/sam= sung-dsim.h > > index a1a5b2b89a7a..76ea8a1720cc 100644 > > --- a/include/drm/bridge/samsung-dsim.h > > +++ b/include/drm/bridge/samsung-dsim.h > > @@ -62,6 +62,7 @@ struct samsung_dsim_driver_data { > > const unsigned int *reg_values; > > u16 m_min; > > u16 m_max; > > + bool dynamic_dphy; > > }; > > > > struct samsung_dsim_host_ops { >