Message ID | 20230502010759.17282-4-aford173@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp282028vqo; Mon, 1 May 2023 18:13:19 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5ZX8FX8zedjBDFSKXzLZFLr8+zjcbkqtustdQ4eWQDiWoOhpx3U+zOIiTxoXJts0kk8Xpk X-Received: by 2002:a17:90a:46c1:b0:24e:1584:483f with SMTP id x1-20020a17090a46c100b0024e1584483fmr2851987pjg.43.1682989998842; Mon, 01 May 2023 18:13:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1682989998; cv=none; d=google.com; s=arc-20160816; b=FZcOoLfzL6jlSkN3HsOMCPurZJw/v/ou0Wh3JZaLBC5pFsgZitGyotGnM1Y0Fd+asC 84FUdPj3WyH3P7Oji6DizacjF4ZBWijzOFBmyqePJPaRTJPvdJSdjdIkkSqAqA4etrZH BEaOX6FB9+aLkZGIBWXnLnuRZ5yOErEAT5kDnJ4LKwH+YCSmTHIH3xYN5w8Tpg5ZVV5l BACh2TAAzhIE20WDMbmVJY6qNm+PP5JAOz6v0cwqtLoV8m60jSys4tiGguA0U94nLQ85 PDCb66GDlqRoqmQkS0U47OQESRCQcqJlmp80OoQCK7WEOWAUAT2JDXL8lBVW5065GK9p BnYA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=HhrMWjFte7goMFkthyl+3kO5kW7x7TcIvC2UAKwMf6E=; b=f/E9cQ3CjHQ5AshmLs0kpxuSGap7RbxS1vT7GhdvxZ6U4vVvCQTl2W9gpcO1mjp82F SGOfm7wWzSPgY4C74vORbErFjY+1uglvcg5QKnaiwf6aUIFQR6iUMuY/gfMZP43FuMor W196E9uOYaedvOw7aqiMx1fC8Yis1Va6BAhKGs8QATu0C46okI0dhsKcuB7hGZvlwjsr Yl3Da0xakLXpOBHA130rBv4vaeiPFI5MpJt4hgANg8cMF7uglRkCjOauSMx260jrHAGE Ye0is2tN20psFMlMxCRrD54hxcumGXS2X0xNQYOvl5N4U5IgLWdF8HFxwNYqCpgXovzu ZlJg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=ID1JuvXX; 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: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q9-20020a17090aa00900b00246e9abad4bsi32315416pjp.85.2023.05.01.18.13.05; Mon, 01 May 2023 18:13:18 -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=ID1JuvXX; 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 S233349AbjEBBIW (ORCPT <rfc822;rbbytesnap@gmail.com> + 99 others); Mon, 1 May 2023 21:08:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54186 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233245AbjEBBIO (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 1 May 2023 21:08:14 -0400 Received: from mail-io1-xd36.google.com (mail-io1-xd36.google.com [IPv6:2607:f8b0:4864:20::d36]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7EF5F35A8 for <linux-kernel@vger.kernel.org>; Mon, 1 May 2023 18:08:13 -0700 (PDT) Received: by mail-io1-xd36.google.com with SMTP id ca18e2360f4ac-76937f5f9c5so44393739f.1 for <linux-kernel@vger.kernel.org>; Mon, 01 May 2023 18:08:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682989693; x=1685581693; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=HhrMWjFte7goMFkthyl+3kO5kW7x7TcIvC2UAKwMf6E=; b=ID1JuvXXaLMCf95WTEkk2TJJKto2G0IwIyP5dukwdxsKvv6+k9wvTf0ZW/8rB6Iq9G Zr9cdfS0Zm+DAwfcYuSXj3csRRYTcq+dqgwfOOiWevPX6vSU8C8yq8DhL6Mn2SwZ5k5m 6Gx+4MXd56NJr9DFfDZ3Wm0SsfSgXsUDx1sNzDzfAcWZ/0G9P7CtwcwnBPAG2kUe/fV+ Q+t++OufgZxqNj1XmNls0xVCt/cYMiFq9CPQQv6tKwAnU1EdD4xWF+TPj+sSLixJZmH6 cYS+sGVy/YHVhOYzIx2a1YkrWkqPdWwquZmf0cUr4AGdwbap/KgrtTcXTnJ947vaX1Jy UZ3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682989693; x=1685581693; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HhrMWjFte7goMFkthyl+3kO5kW7x7TcIvC2UAKwMf6E=; b=YLEl3Wed4gEkPD6RjVu7EalPvagC8oaatcANFLL0aXi9TAaqpwwQeMZQSImlTv1AGq rCbWesWiKc4cP6kj9TDLN7el/XLvz0jK1C67XTBFKNqQc2CtMdDBmeGBzeOVSQVz25z5 3jREaWf5NiiEQ5C5pcyR00oRST4OBopDiqoD+j7c9rE3pDzUHIFRa1OMdW/kKZuLrdD2 urLKDWZcw4Poq8CGNU3Zc7RGg+pfYWwuToMWPuTaP4XsUgHHJi5Qp1n+MQ4BSN/2Dk8C NFwTvV4Ow78CU2rDslpOuQk9Ni58pBvPt48hAgA3/VFvqNhV7a1Y8hPoSx4SSd/i0Vqw xgEQ== X-Gm-Message-State: AC+VfDytJ1QpWbhDP6LzWDuz0/Uv6Ua/adnD+SUrgFxZE7mN539riy7N EPSrmGf0XhQ/iZju/0fKwYQ= X-Received: by 2002:a6b:f007:0:b0:766:41fa:e26c with SMTP id w7-20020a6bf007000000b0076641fae26cmr11441794ioc.10.1682989692703; Mon, 01 May 2023 18:08:12 -0700 (PDT) Received: from aford-B741.lan ([2601:447:d001:897f:8257:a536:d7fc:1919]) by smtp.gmail.com with ESMTPSA id f16-20020a056638329000b0040fb5d5429fsm4836329jav.131.2023.05.01.18.08.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 18:08:12 -0700 (PDT) From: Adam Ford <aford173@gmail.com> To: dri-devel@lists.freedesktop.org Cc: marex@denx.de, aford@beaconembedded.com, Adam Ford <aford173@gmail.com>, Chen-Yu Tsai <wenst@chromium.org>, Andrzej Hajda <andrzej.hajda@intel.com>, Neil Armstrong <neil.armstrong@linaro.org>, Robert Foss <rfoss@kernel.org>, Laurent Pinchart <Laurent.pinchart@ideasonboard.com>, Jonas Karlman <jonas@kwiboo.se>, Jernej Skrabec <jernej.skrabec@gmail.com>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, Inki Dae <inki.dae@samsung.com>, Jagan Teki <jagan@amarulasolutions.com>, Marek Szyprowski <m.szyprowski@samsung.com>, linux-kernel@vger.kernel.org Subject: [PATCH V3 3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically Date: Mon, 1 May 2023 20:07:55 -0500 Message-Id: <20230502010759.17282-4-aford173@gmail.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230502010759.17282-1-aford173@gmail.com> References: <20230502010759.17282-1-aford173@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1764742920802051018?= X-GMAIL-MSGID: =?utf-8?q?1764742920802051018?= |
Series |
drm: bridge: samsung-dsim: Support variable clocking
|
|
Commit Message
Adam Ford
May 2, 2023, 1:07 a.m. UTC
Make the pll-clock-frequency optional. If it's present, use it to maintain backwards compatibility with existing hardware. If it is absent, read clock rate of "sclk_mipi" to determine the rate. Signed-off-by: Adam Ford <aford173@gmail.com> Tested-by: Chen-Yu Tsai <wenst@chromium.org> --- drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
Comments
On 02.05.23 03:07, Adam Ford wrote: > Make the pll-clock-frequency optional. If it's present, use it > to maintain backwards compatibility with existing hardware. If it > is absent, read clock rate of "sclk_mipi" to determine the rate. > > Signed-off-by: Adam Ford <aford173@gmail.com> > Tested-by: Chen-Yu Tsai <wenst@chromium.org> Tested on Kontron BL i.MX8MM with SN65DSI84 and ADV7535 bridges. Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Am Dienstag, 2. Mai 2023, 03:07:55 CEST schrieb Adam Ford: > Make the pll-clock-frequency optional. If it's present, use it > to maintain backwards compatibility with existing hardware. If it > is absent, read clock rate of "sclk_mipi" to determine the rate. > > Signed-off-by: Adam Ford <aford173@gmail.com> > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > --- > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > b/drivers/gpu/drm/bridge/samsung-dsim.c index bf4b33d2de76..2dc02a9e37c0 > 100644 > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > @@ -1726,12 +1726,20 @@ static int samsung_dsim_parse_dt(struct samsung_dsim > *dsi) { > struct device *dev = dsi->dev; > struct device_node *node = dev->of_node; > + struct clk *pll_clk; > int ret; > > ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency", > &dsi->pll_clk_rate); > - if (ret < 0) > - return ret; > + > + /* If it doesn't exist, read it from the clock instead of failing */ > + if (ret < 0) { > + pll_clk = devm_clk_get(dev, "sclk_mipi"); > + if (!IS_ERR(pll_clk)) > + dsi->pll_clk_rate = clk_get_rate(pll_clk); > + else > + return PTR_ERR(pll_clk); > + } > Now that 'samsung,pll-clock-frequency' is optional the error in samsung_dsim_of_read_u32() should be changed. Otherwise you will get > /soc@0/bus@32c00000/dsi@32e10000: failed to get 'samsung,pll-clock- frequency' property Best regards, Alexander > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock- frequency", > &dsi->burst_clk_rate);
On Thu, May 4, 2023 at 4:21 AM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Am Dienstag, 2. Mai 2023, 03:07:55 CEST schrieb Adam Ford: > > Make the pll-clock-frequency optional. If it's present, use it > > to maintain backwards compatibility with existing hardware. If it > > is absent, read clock rate of "sclk_mipi" to determine the rate. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > > --- > > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > b/drivers/gpu/drm/bridge/samsung-dsim.c index bf4b33d2de76..2dc02a9e37c0 > > 100644 > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > @@ -1726,12 +1726,20 @@ static int samsung_dsim_parse_dt(struct samsung_dsim > > *dsi) { > > struct device *dev = dsi->dev; > > struct device_node *node = dev->of_node; > > + struct clk *pll_clk; > > int ret; > > > > ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency", > > &dsi->pll_clk_rate); > > - if (ret < 0) > > - return ret; > > + > > + /* If it doesn't exist, read it from the clock instead of failing */ > > + if (ret < 0) { > > + pll_clk = devm_clk_get(dev, "sclk_mipi"); > > + if (!IS_ERR(pll_clk)) > > + dsi->pll_clk_rate = clk_get_rate(pll_clk); > > + else > > + return PTR_ERR(pll_clk); > > + } > > > > Now that 'samsung,pll-clock-frequency' is optional the error in > samsung_dsim_of_read_u32() should be changed. Otherwise you will get > > /soc@0/bus@32c00000/dsi@32e10000: failed to get 'samsung,pll-clock- > frequency' property I'll change the message from err to info with a message that reads "no samsung,pll-clock-frequency, using pixel clock" Does that work? adam > > Best regards, > Alexander > > > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock- > frequency", > > &dsi->burst_clk_rate); > > > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > http://www.tq-group.com/ > >
Hi Adam, Am Donnerstag, 4. Mai 2023, 14:00:08 CEST schrieb Adam Ford: > On Thu, May 4, 2023 at 4:21 AM Alexander Stein > > <alexander.stein@ew.tq-group.com> wrote: > > Am Dienstag, 2. Mai 2023, 03:07:55 CEST schrieb Adam Ford: > > > Make the pll-clock-frequency optional. If it's present, use it > > > to maintain backwards compatibility with existing hardware. If it > > > is absent, read clock rate of "sclk_mipi" to determine the rate. > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > > > --- > > > > > > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++++++-- > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > > b/drivers/gpu/drm/bridge/samsung-dsim.c index bf4b33d2de76..2dc02a9e37c0 > > > 100644 > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > > @@ -1726,12 +1726,20 @@ static int samsung_dsim_parse_dt(struct > > > samsung_dsim *dsi) { > > > > > > struct device *dev = dsi->dev; > > > struct device_node *node = dev->of_node; > > > > > > + struct clk *pll_clk; > > > > > > int ret; > > > > > > ret = samsung_dsim_of_read_u32(node, > > > "samsung,pll-clock-frequency", > > > > > > &dsi->pll_clk_rate); > > > > > > - if (ret < 0) > > > - return ret; > > > + > > > + /* If it doesn't exist, read it from the clock instead of failing > > > */ > > > + if (ret < 0) { > > > + pll_clk = devm_clk_get(dev, "sclk_mipi"); > > > + if (!IS_ERR(pll_clk)) > > > + dsi->pll_clk_rate = clk_get_rate(pll_clk); > > > + else > > > + return PTR_ERR(pll_clk); > > > + } > > > > Now that 'samsung,pll-clock-frequency' is optional the error in > > samsung_dsim_of_read_u32() should be changed. Otherwise you will get > > > > > /soc@0/bus@32c00000/dsi@32e10000: failed to get 'samsung,pll-clock- > > > > frequency' property > > I'll change the message from err to info with a message that reads "no > samsung,pll-clock-frequency, using pixel clock" > > Does that work? Having just a info is totally fine with me. Thanks. Although your suggested message somehow implies (to me) using pixel clock is just a fallback. I'm a bit concerned some might think "samsung,pll-clock- frequency" should be provided in DT. But this might just be me. Best regards, Alexander > adam > > > Best regards, > > Alexander > > > > > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock- > > > > frequency", > > > > > &dsi->burst_clk_rate); > > > > -- > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > Amtsgericht München, HRB 105018 > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > http://www.tq-group.com/
On Thu, May 4, 2023 at 7:40 AM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hi Adam, > > Am Donnerstag, 4. Mai 2023, 14:00:08 CEST schrieb Adam Ford: > > On Thu, May 4, 2023 at 4:21 AM Alexander Stein > > > > <alexander.stein@ew.tq-group.com> wrote: > > > Am Dienstag, 2. Mai 2023, 03:07:55 CEST schrieb Adam Ford: > > > > Make the pll-clock-frequency optional. If it's present, use it > > > > to maintain backwards compatibility with existing hardware. If it > > > > is absent, read clock rate of "sclk_mipi" to determine the rate. > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > > > > --- > > > > > > > > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++++++-- > > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > > > b/drivers/gpu/drm/bridge/samsung-dsim.c index bf4b33d2de76..2dc02a9e37c0 > > > > 100644 > > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > > > @@ -1726,12 +1726,20 @@ static int samsung_dsim_parse_dt(struct > > > > samsung_dsim *dsi) { > > > > > > > > struct device *dev = dsi->dev; > > > > struct device_node *node = dev->of_node; > > > > > > > > + struct clk *pll_clk; > > > > > > > > int ret; > > > > > > > > ret = samsung_dsim_of_read_u32(node, > > > > "samsung,pll-clock-frequency", > > > > > > > > &dsi->pll_clk_rate); > > > > > > > > - if (ret < 0) > > > > - return ret; > > > > + > > > > + /* If it doesn't exist, read it from the clock instead of failing > > > > */ > > > > + if (ret < 0) { > > > > + pll_clk = devm_clk_get(dev, "sclk_mipi"); > > > > + if (!IS_ERR(pll_clk)) > > > > + dsi->pll_clk_rate = clk_get_rate(pll_clk); > > > > + else > > > > + return PTR_ERR(pll_clk); > > > > + } > > > > > > Now that 'samsung,pll-clock-frequency' is optional the error in > > > samsung_dsim_of_read_u32() should be changed. Otherwise you will get > > > > > > > /soc@0/bus@32c00000/dsi@32e10000: failed to get 'samsung,pll-clock- > > > > > > frequency' property > > > > I'll change the message from err to info with a message that reads "no > > samsung,pll-clock-frequency, using pixel clock" > > > > Does that work? > > Having just a info is totally fine with me. Thanks. > Although your suggested message somehow implies (to me) using pixel clock is > just a fallback. I'm a bit concerned some might think "samsung,pll-clock- > frequency" should be provided in DT. But this might just be me. Oops, I got the PLL and burst burst clock confused. I think both burst-clock and pll clock messages should get updates. The pll clock should say something like "samsung,pll-clock-frequency not defined, using sclk_mipi" The imx8m n/m/p have the sclk_mipi defined in the device tree, and this patch allows them to not have to manually set the pll clock since it can be read. This allows to people to change the frequency of the PLL in in one place and let the driver read it instead of having to set the value in two places for the same clock. For the burst clock, I'd like to propose "samsung,burst-clock-frequency not defined, using pixel clock" Does that work for you? adam > frequency > > Best regards, > Alexander > > > adam > > > > > Best regards, > > > Alexander > > > > > > > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock- > > > > > > frequency", > > > > > > > &dsi->burst_clk_rate); > > > > > > -- > > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > > Amtsgericht München, HRB 105018 > > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > > http://www.tq-group.com/ > > > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > http://www.tq-group.com/ > >
Am Donnerstag, 4. Mai 2023, 14:57:01 CEST schrieb Adam Ford: > On Thu, May 4, 2023 at 7:40 AM Alexander Stein > > <alexander.stein@ew.tq-group.com> wrote: > > Hi Adam, > > > > Am Donnerstag, 4. Mai 2023, 14:00:08 CEST schrieb Adam Ford: > > > On Thu, May 4, 2023 at 4:21 AM Alexander Stein > > > > > > <alexander.stein@ew.tq-group.com> wrote: > > > > Am Dienstag, 2. Mai 2023, 03:07:55 CEST schrieb Adam Ford: > > > > > Make the pll-clock-frequency optional. If it's present, use it > > > > > to maintain backwards compatibility with existing hardware. If it > > > > > is absent, read clock rate of "sclk_mipi" to determine the rate. > > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > > > > > --- > > > > > > > > > > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++++++-- > > > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > > > > b/drivers/gpu/drm/bridge/samsung-dsim.c index > > > > > bf4b33d2de76..2dc02a9e37c0 > > > > > 100644 > > > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > > > > @@ -1726,12 +1726,20 @@ static int samsung_dsim_parse_dt(struct > > > > > samsung_dsim *dsi) { > > > > > > > > > > struct device *dev = dsi->dev; > > > > > struct device_node *node = dev->of_node; > > > > > > > > > > + struct clk *pll_clk; > > > > > > > > > > int ret; > > > > > > > > > > ret = samsung_dsim_of_read_u32(node, > > > > > "samsung,pll-clock-frequency", > > > > > > > > > > &dsi->pll_clk_rate); > > > > > > > > > > - if (ret < 0) > > > > > - return ret; > > > > > + > > > > > + /* If it doesn't exist, read it from the clock instead of > > > > > failing > > > > > */ > > > > > + if (ret < 0) { > > > > > + pll_clk = devm_clk_get(dev, "sclk_mipi"); > > > > > + if (!IS_ERR(pll_clk)) > > > > > + dsi->pll_clk_rate = clk_get_rate(pll_clk); > > > > > + else > > > > > + return PTR_ERR(pll_clk); > > > > > + } > > > > > > > > Now that 'samsung,pll-clock-frequency' is optional the error in > > > > samsung_dsim_of_read_u32() should be changed. Otherwise you will get > > > > > > > > > /soc@0/bus@32c00000/dsi@32e10000: failed to get 'samsung,pll-clock- > > > > > > > > frequency' property > > > > > > I'll change the message from err to info with a message that reads "no > > > samsung,pll-clock-frequency, using pixel clock" > > > > > > Does that work? > > > > Having just a info is totally fine with me. Thanks. > > Although your suggested message somehow implies (to me) using pixel clock > > is just a fallback. I'm a bit concerned some might think > > "samsung,pll-clock- frequency" should be provided in DT. But this might > > just be me. > > Oops, I got the PLL and burst burst clock confused. I think both > burst-clock and pll clock messages should get updates. > > The pll clock should say something like "samsung,pll-clock-frequency > not defined, using sclk_mipi" > > The imx8m n/m/p have the sclk_mipi defined in the device tree, and > this patch allows them to not have > to manually set the pll clock since it can be read. This allows to > people to change the frequency of the PLL in > in one place and let the driver read it instead of having to set the > value in two places for the same clock. That's why I would like to make it sound less error-like. How about "Using sclk_mipi for pll clock frequency"? > For the burst clock, I'd like to propose > "samsung,burst-clock-frequency not defined, using pixel clock" Similar to above how about "Using pixel clock for burst clock frequency"? > Does that work for you? But I'm okay with both ways. Up to you. Thanks and best regards, Alexander > > frequency > > > > > > Best regards, > > Alexander > > > > > adam > > > > > > > Best regards, > > > > Alexander > > > > > > > > > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock- > > > > > > > > frequency", > > > > > > > > > &dsi->burst_clk_rate); > > > > > > > > -- > > > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > > > Amtsgericht München, HRB 105018 > > > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > > > http://www.tq-group.com/ > > > > -- > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > Amtsgericht München, HRB 105018 > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > http://www.tq-group.com/
On Thu, May 4, 2023 at 8:18 AM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Am Donnerstag, 4. Mai 2023, 14:57:01 CEST schrieb Adam Ford: > > On Thu, May 4, 2023 at 7:40 AM Alexander Stein > > > > <alexander.stein@ew.tq-group.com> wrote: > > > Hi Adam, > > > > > > Am Donnerstag, 4. Mai 2023, 14:00:08 CEST schrieb Adam Ford: > > > > On Thu, May 4, 2023 at 4:21 AM Alexander Stein > > > > > > > > <alexander.stein@ew.tq-group.com> wrote: > > > > > Am Dienstag, 2. Mai 2023, 03:07:55 CEST schrieb Adam Ford: > > > > > > Make the pll-clock-frequency optional. If it's present, use it > > > > > > to maintain backwards compatibility with existing hardware. If it > > > > > > is absent, read clock rate of "sclk_mipi" to determine the rate. > > > > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > > > > > > --- > > > > > > > > > > > > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++++++-- > > > > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c > > > > > > b/drivers/gpu/drm/bridge/samsung-dsim.c index > > > > > > bf4b33d2de76..2dc02a9e37c0 > > > > > > 100644 > > > > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > > > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > > > > > @@ -1726,12 +1726,20 @@ static int samsung_dsim_parse_dt(struct > > > > > > samsung_dsim *dsi) { > > > > > > > > > > > > struct device *dev = dsi->dev; > > > > > > struct device_node *node = dev->of_node; > > > > > > > > > > > > + struct clk *pll_clk; > > > > > > > > > > > > int ret; > > > > > > > > > > > > ret = samsung_dsim_of_read_u32(node, > > > > > > "samsung,pll-clock-frequency", > > > > > > > > > > > > &dsi->pll_clk_rate); > > > > > > > > > > > > - if (ret < 0) > > > > > > - return ret; > > > > > > + > > > > > > + /* If it doesn't exist, read it from the clock instead of > > > > > > failing > > > > > > */ > > > > > > + if (ret < 0) { > > > > > > + pll_clk = devm_clk_get(dev, "sclk_mipi"); > > > > > > + if (!IS_ERR(pll_clk)) > > > > > > + dsi->pll_clk_rate = clk_get_rate(pll_clk); > > > > > > + else > > > > > > + return PTR_ERR(pll_clk); > > > > > > + } > > > > > > > > > > Now that 'samsung,pll-clock-frequency' is optional the error in > > > > > samsung_dsim_of_read_u32() should be changed. Otherwise you will get > > > > > > > > > > > /soc@0/bus@32c00000/dsi@32e10000: failed to get 'samsung,pll-clock- > > > > > > > > > > frequency' property > > > > > > > > I'll change the message from err to info with a message that reads "no > > > > samsung,pll-clock-frequency, using pixel clock" > > > > > > > > Does that work? > > > > > > Having just a info is totally fine with me. Thanks. > > > Although your suggested message somehow implies (to me) using pixel clock > > > is just a fallback. I'm a bit concerned some might think > > > "samsung,pll-clock- frequency" should be provided in DT. But this might > > > just be me. > > > > Oops, I got the PLL and burst burst clock confused. I think both > > burst-clock and pll clock messages should get updates. > > > > The pll clock should say something like "samsung,pll-clock-frequency > > not defined, using sclk_mipi" > > > > The imx8m n/m/p have the sclk_mipi defined in the device tree, and > > this patch allows them to not have > > to manually set the pll clock since it can be read. This allows to > > people to change the frequency of the PLL in > > in one place and let the driver read it instead of having to set the > > value in two places for the same clock. > > That's why I would like to make it sound less error-like. > How about "Using sclk_mipi for pll clock frequency"? > > > For the burst clock, I'd like to propose > > "samsung,burst-clock-frequency not defined, using pixel clock" > > Similar to above how about "Using pixel clock for burst clock frequency"? I like that. > > > Does that work for you? > > But I'm okay with both ways. Up to you. I'll wait another day or two to see if anyone else has any feedback, and I'll submit V4 with some other items addressed too. Thank you for your review! adam > > Thanks and best regards, > Alexander > > > > > frequency > > > > > > > > > Best regards, > > > Alexander > > > > > > > adam > > > > > > > > > Best regards, > > > > > Alexander > > > > > > > > > > > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock- > > > > > > > > > > frequency", > > > > > > > > > > > &dsi->burst_clk_rate); > > > > > > > > > > -- > > > > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > > > > Amtsgericht München, HRB 105018 > > > > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > > > > http://www.tq-group.com/ > > > > > > -- > > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > > > Amtsgericht München, HRB 105018 > > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > > > http://www.tq-group.com/ > > > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > http://www.tq-group.com/ > >
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c index bf4b33d2de76..2dc02a9e37c0 100644 --- a/drivers/gpu/drm/bridge/samsung-dsim.c +++ b/drivers/gpu/drm/bridge/samsung-dsim.c @@ -1726,12 +1726,20 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi) { struct device *dev = dsi->dev; struct device_node *node = dev->of_node; + struct clk *pll_clk; int ret; ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency", &dsi->pll_clk_rate); - if (ret < 0) - return ret; + + /* If it doesn't exist, read it from the clock instead of failing */ + if (ret < 0) { + pll_clk = devm_clk_get(dev, "sclk_mipi"); + if (!IS_ERR(pll_clk)) + dsi->pll_clk_rate = clk_get_rate(pll_clk); + else + return PTR_ERR(pll_clk); + } ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency", &dsi->burst_clk_rate);