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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0140DC433FE for ; Wed, 19 Jan 2022 11:47:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=eB5lE4BrHJdFUCb172Dy6EOMBTWdr1DIWCMMmlu75r8=; b=uPe4vvW8fat7n6 lx7id7chCVzAu/q12/YoXbWXNNXwAz7Zk2j8gPn6vv9qPDYh8faeZBrQmeq+ZM93HwP4HQEQ/BeVj Ur2dZ85N8dxJmZ3WeXc8zgbyBUF+gGGLk7gEJxVyaLBiVsE5ZHFPzOzFqffG8Q23mqMEPEEoAIsR3 JH+T/ypOVNwhWRRokq1AHWwNT43ShtUeuTO78Uudn4Tvz+I6599e+WCyV2rEMcmMPG71h0kIP38xs BAFyFS1UsL5rV0xdfPGDxOcL3zIO8hqJiS9jYp6qjUIwDfBb0ngHiHSggSo55uwn7AIS/xHJ5hvE/ 3fs/quisyYg2hHduyDaQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nA9Qz-005EJz-OT; Wed, 19 Jan 2022 11:47:25 +0000 Received: from honk.sigxcpu.org ([24.134.29.49]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nA9Qv-005EIG-U8; Wed, 19 Jan 2022 11:47:24 +0000 Received: from localhost (localhost [127.0.0.1]) by honk.sigxcpu.org (Postfix) with ESMTP id F3A14FB03; Wed, 19 Jan 2022 12:47:15 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at honk.sigxcpu.org Received: from honk.sigxcpu.org ([127.0.0.1]) by localhost (honk.sigxcpu.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Y9w4dTYgLJjm; Wed, 19 Jan 2022 12:47:14 +0100 (CET) Date: Wed, 19 Jan 2022 12:47:12 +0100 From: Guido =?iso-8859-1?Q?G=FCnther?= To: Liu Ying Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-phy@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-imx@nxp.com, Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , David Airlie , Daniel Vetter , Kishon Vijay Abraham I , Vinod Koul , Kevin Hilman , Jerome Brunet , Martin Blumenstingl , Heiko Stuebner , Maxime Ripard Subject: Re: [PATCH v2] phy: dphy: Correct clk_pre parameter Message-ID: References: <20220119023714.1498508-1-victor.liu@nxp.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220119023714.1498508-1-victor.liu@nxp.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220119_034722_194626_47E06D24 X-CRM114-Status: GOOD ( 30.48 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org Hi, On Wed, Jan 19, 2022 at 10:37:14AM +0800, Liu Ying wrote: > The D-PHY specification (v1.2) explicitly mentions that the T-CLK-PRE > parameter's unit is Unit Interval(UI) and the minimum value is 8. Also, > kernel doc of the 'clk_pre' member of struct phy_configure_opts_mipi_dphy > mentions that it should be in UI. However, the dphy core driver wrongly > sets 'clk_pre' to 8000, which seems to hint that it's in picoseconds. > And, the kernel doc of the 'clk_pre' member wrongly says the minimum value > is '8 UI', instead of 8. > = > So, let's fix both the dphy core driver and the kernel doc of the 'clk_pr= e' > member to correctly reflect the T-CLK-PRE parameter's unit and the minimum > value according to the D-PHY specification. > = > I'm assuming that all impacted custom drivers shall program values in > TxByteClkHS cycles into hardware for the T-CLK-PRE parameter. The D-PHY > specification mentions that the frequency of TxByteClkHS is exactly 1/8 > the High-Speed(HS) bit rate(each HS bit consumes one UI). So, relevant > custom driver code is changed to program those values as > DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE), then. > = > Note that I've only tested the patch with RM67191 DSI panel on i.MX8mq EV= K. > Help is needed to test with other i.MX8mq, Meson and Rockchip platforms, > as I don't have the hardwares. > = > Fixes: 2ed869990e14 ("phy: Add MIPI D-PHY configuration options") > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: David Airlie > Cc: Daniel Vetter > Cc: Kishon Vijay Abraham I > Cc: Vinod Koul > Cc: Kevin Hilman > Cc: Jerome Brunet > Cc: Martin Blumenstingl > Cc: Heiko Stuebner > Cc: Maxime Ripard > Cc: Guido G=FCnther > Tested-by: Liu Ying # RM67191 DSI panel on i.MX8mq E= VK > Signed-off-by: Liu Ying > --- > v1->v2: > * Use BITS_PER_BYTE macro. (Andrzej) > * Drop dsi argument from ui2bc() in nwl-dsi.c. > = > drivers/gpu/drm/bridge/nwl-dsi.c | 12 +++++------- > drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c | 3 ++- > drivers/phy/phy-core-mipi-dphy.c | 4 ++-- > drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 3 ++- > include/linux/phy/phy-mipi-dphy.h | 2 +- > 5 files changed, 12 insertions(+), 12 deletions(-) > = > diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nw= l-dsi.c > index a7389a0facfb..af07eeb47ca0 100644 > --- a/drivers/gpu/drm/bridge/nwl-dsi.c > +++ b/drivers/gpu/drm/bridge/nwl-dsi.c > @@ -7,6 +7,7 @@ > */ > = > #include > +#include > #include > #include > #include > @@ -196,12 +197,9 @@ static u32 ps2bc(struct nwl_dsi *dsi, unsigned long = long ps) > /* > * ui2bc - UI time periods to byte clock cycles > */ > -static u32 ui2bc(struct nwl_dsi *dsi, unsigned long long ui) > +static u32 ui2bc(unsigned int ui) > { > - u32 bpp =3D mipi_dsi_pixel_format_to_bpp(dsi->format); > - > - return DIV64_U64_ROUND_UP(ui * dsi->lanes, > - dsi->mode.clock * 1000 * bpp); > + return DIV_ROUND_UP(ui, BITS_PER_BYTE); > } > = > /* > @@ -232,12 +230,12 @@ static int nwl_dsi_config_host(struct nwl_dsi *dsi) > } > = > /* values in byte clock cycles */ > - cycles =3D ui2bc(dsi, cfg->clk_pre); > + cycles =3D ui2bc(cfg->clk_pre); > DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_pre: 0x%x\n", cycles); > nwl_dsi_write(dsi, NWL_DSI_CFG_T_PRE, cycles); > cycles =3D ps2bc(dsi, cfg->lpx + cfg->clk_prepare + cfg->clk_zero); > DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap (pre): 0x%x\n", cycles); > - cycles +=3D ui2bc(dsi, cfg->clk_pre); > + cycles +=3D ui2bc(cfg->clk_pre); > DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_post: 0x%x\n", cycles); > nwl_dsi_write(dsi, NWL_DSI_CFG_T_POST, cycles); > cycles =3D ps2bc(dsi, cfg->hs_exit); > diff --git a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c b/drivers/phy/= amlogic/phy-meson-axg-mipi-dphy.c > index cd2332bf0e31..fdbd64c03e12 100644 > --- a/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c > +++ b/drivers/phy/amlogic/phy-meson-axg-mipi-dphy.c > @@ -9,6 +9,7 @@ > = > #include > #include > +#include > #include > #include > #include > @@ -250,7 +251,7 @@ static int phy_meson_axg_mipi_dphy_power_on(struct ph= y *phy) > (DIV_ROUND_UP(priv->config.clk_zero, temp) << 16) | > (DIV_ROUND_UP(priv->config.clk_prepare, temp) << 24)); > regmap_write(priv->regmap, MIPI_DSI_CLK_TIM1, > - DIV_ROUND_UP(priv->config.clk_pre, temp)); > + DIV_ROUND_UP(priv->config.clk_pre, BITS_PER_BYTE)); > = > regmap_write(priv->regmap, MIPI_DSI_HS_TIM, > DIV_ROUND_UP(priv->config.hs_exit, temp) | > diff --git a/drivers/phy/phy-core-mipi-dphy.c b/drivers/phy/phy-core-mipi= -dphy.c > index 288c9c67aa74..ccb4045685cd 100644 > --- a/drivers/phy/phy-core-mipi-dphy.c > +++ b/drivers/phy/phy-core-mipi-dphy.c > @@ -36,7 +36,7 @@ int phy_mipi_dphy_get_default_config(unsigned long pixe= l_clock, > = > cfg->clk_miss =3D 0; > cfg->clk_post =3D 60000 + 52 * ui; > - cfg->clk_pre =3D 8000; > + cfg->clk_pre =3D 8; > cfg->clk_prepare =3D 38000; > cfg->clk_settle =3D 95000; > cfg->clk_term_en =3D 0; > @@ -97,7 +97,7 @@ int phy_mipi_dphy_config_validate(struct phy_configure_= opts_mipi_dphy *cfg) > if (cfg->clk_post < (60000 + 52 * ui)) > return -EINVAL; > = > - if (cfg->clk_pre < 8000) > + if (cfg->clk_pre < 8) > return -EINVAL; > = > if (cfg->clk_prepare < 38000 || cfg->clk_prepare > 95000) > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c b/drivers/p= hy/rockchip/phy-rockchip-inno-dsidphy.c > index 347dc79a18c1..630e01b5c19b 100644 > --- a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c > +++ b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c > @@ -5,6 +5,7 @@ > * Author: Wyon Bi > */ > = > +#include > #include > #include > #include > @@ -364,7 +365,7 @@ static void inno_dsidphy_mipi_mode_enable(struct inno= _dsidphy *inno) > * The value of counter for HS Tclk-pre > * Tclk-pre =3D Tpin_txbyteclkhs * value > */ > - clk_pre =3D DIV_ROUND_UP(cfg->clk_pre, t_txbyteclkhs); > + clk_pre =3D DIV_ROUND_UP(cfg->clk_pre, BITS_PER_BYTE); > = > /* > * The value of counter for HS Tlpx Time > diff --git a/include/linux/phy/phy-mipi-dphy.h b/include/linux/phy/phy-mi= pi-dphy.h > index a877ffee845d..59a5e77ab409 100644 > --- a/include/linux/phy/phy-mipi-dphy.h > +++ b/include/linux/phy/phy-mipi-dphy.h > @@ -42,7 +42,7 @@ struct phy_configure_opts_mipi_dphy { > * the transmitter prior to any associated Data Lane beginning > * the transition from LP to HS mode. > * > - * Minimum value: 8 UI > + * Minimum value: 8 > */ > unsigned int clk_pre; Tested on the Librem 5 (imx8mq) with it's rather picky panel: Tested-by: Guido G=FCnther Cheers -- Guido > = > -- = > 2.25.1 > = _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic