Message ID | 20221226095745.19757-2-a-bhatia1@ti.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp831729wrt; Mon, 26 Dec 2022 01:59:10 -0800 (PST) X-Google-Smtp-Source: AMrXdXtPIu6mOa5ccTp2CxLAZGwegAmdf6MP/rO0kRWaoiK9VtKv3qAlurGtKu599c+kNLPRkJHh X-Received: by 2002:a17:906:d963:b0:83d:97fd:d421 with SMTP id rp3-20020a170906d96300b0083d97fdd421mr14093064ejb.38.1672048750269; Mon, 26 Dec 2022 01:59:10 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672048750; cv=none; d=google.com; s=arc-20160816; b=mgfnN7KBEe5KbXexIpFw4KM3migqxRzqDZhkhwgneyngn6gbHW1pM9IsTVNoD7CJ8i fYplxeDcAqL1bKM4TCG9ijxs1/2Fnc1Sh6wdqBkdmLy3RdJetW+HQ5hm/j0DCXZPs6SY U4ZN/WVm94t/SjGA+TlxkILARxOp8vyZkdYmVKuf5UwrmJuUUKPu4OtSsgSdW3aWXVgg GaeTKNaL00855sN6/oUw4o+BFDlq4heZjToOqFk7mRhhBZR6qXm+rwoJlq3YswBt9+JP hQ/BDRhAE9hMwAJ0V1xQg9ddTGwyMi8iXWtxwYux+SDEPkOu8kc7/pdSfaXGiXXDPYWY qADQ== 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=JxQrW/7JjDKzWfRlSNStXWL3EAObEqOtkObrTh1wfmw=; b=xfy7sJyu9chhMnn5qtxgIY1tdj/He2zRU/mZ9thaRerWEnJ1wnkNwJB8BmbO5cBL1M 76L1+RFGNs8iEkbu2C2D9EUAtTJBpPGvM3fQQfW9HKUqWs2ItRWIXnHokeP9Dkdee1BC BuotTY/s/CalBc8zHMx8xs9Ze2U8k/RuWjiZs5TthrNhPegsV874ChE3W+hcITdGFb74 zo0L8BOVXMyTRhZaBi/mRhvoADkPmZC73jGsqo71il2UlzQf5HAnc5JY0MMJqnvmEsU6 RWIkS27eU5xeS1WmIe5q38EosdX00moaYyRzwnruPQLZd56hA9kyd7CkHoXwMV0QdFfP zxTQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b=KkPTznZD; 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=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id oz31-20020a1709077d9f00b007c0b03b23b5si8546320ejc.542.2022.12.26.01.58.46; Mon, 26 Dec 2022 01:59:10 -0800 (PST) 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=@ti.com header.s=ti-com-17Q1 header.b=KkPTznZD; 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=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231789AbiLZJ6G (ORCPT <rfc822;eddaouddi.ayoub@gmail.com> + 99 others); Mon, 26 Dec 2022 04:58:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39534 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229500AbiLZJ6E (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 26 Dec 2022 04:58:04 -0500 Received: from lelv0143.ext.ti.com (lelv0143.ext.ti.com [198.47.23.248]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0636525FF; Mon, 26 Dec 2022 01:58:02 -0800 (PST) Received: from fllv0035.itg.ti.com ([10.64.41.0]) by lelv0143.ext.ti.com (8.15.2/8.15.2) with ESMTP id 2BQ9vmxU036128; Mon, 26 Dec 2022 03:57:48 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1672048668; bh=JxQrW/7JjDKzWfRlSNStXWL3EAObEqOtkObrTh1wfmw=; h=From:To:CC:Subject:Date:In-Reply-To:References; b=KkPTznZDXIvrWMHoyy7qRz4kPc2nIrdoUciqKGf/ExhN3OP6HcwsoKDEOea+Mbf2J FztuOm1e4Av7DrpEm936xA9uSsgld/3A9dbhRYOHkF/6ES6/WvJfIbYStGOIpyyzRW jw492YUbjwRb+vjft7iQ12NpsDkk29P2p9zJeIbc= Received: from DFLE105.ent.ti.com (dfle105.ent.ti.com [10.64.6.26]) by fllv0035.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 2BQ9vm8q126041 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 26 Dec 2022 03:57:48 -0600 Received: from DFLE112.ent.ti.com (10.64.6.33) by DFLE105.ent.ti.com (10.64.6.26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.16; Mon, 26 Dec 2022 03:57:47 -0600 Received: from lelv0326.itg.ti.com (10.180.67.84) by DFLE112.ent.ti.com (10.64.6.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.16 via Frontend Transport; Mon, 26 Dec 2022 03:57:48 -0600 Received: from localhost (ileaxei01-snat2.itg.ti.com [10.180.69.6]) by lelv0326.itg.ti.com (8.15.2/8.15.2) with ESMTP id 2BQ9vlvx003621; Mon, 26 Dec 2022 03:57:47 -0600 From: Aradhya Bhatia <a-bhatia1@ti.com> To: Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> CC: Tomi Valkeinen <tomba@kernel.org>, Samuel Holland <samuel@sholland.org>, Maxime Ripard <mripard@kernel.org>, Linux Clock List <linux-clk@vger.kernel.org>, Devicetree List <devicetree@vger.kernel.org>, Linux Kernel List <linux-kernel@vger.kernel.org>, Nishanth Menon <nm@ti.com>, Vignesh Raghavendra <vigneshr@ti.com>, Devarsh Thakkar <devarsht@ti.com>, Jai Luthra <j-luthra@ti.com>, Aradhya Bhatia <a-bhatia1@ti.com> Subject: [PATCH 1/2] dt-bindings: clock: fixed-factor: Add TI AM62 SoC OLDI clock Date: Mon, 26 Dec 2022 15:27:44 +0530 Message-ID: <20221226095745.19757-2-a-bhatia1@ti.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20221226095745.19757-1-a-bhatia1@ti.com> References: <20221226095745.19757-1-a-bhatia1@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS 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?1753270190570735690?= X-GMAIL-MSGID: =?utf-8?q?1753270190570735690?= |
Series |
Re-introduce parent clock-rate set for fixed-factor clock
|
|
Commit Message
Aradhya Bhatia
Dec. 26, 2022, 9:57 a.m. UTC
Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum
list.
"ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI
display subsystem request a pixel clock for itself and a corresponding
serial clock for its OLDI Transmitters. The serial clock is 7 times the
pixel clock. This clock needs the clock set rate request to be
propagated to the parent clock provider.
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
---
Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 +
1 file changed, 1 insertion(+)
Comments
On Mon, 26 Dec 2022 15:27:44 +0530, Aradhya Bhatia wrote: > Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum > list. > > "ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI > display subsystem request a pixel clock for itself and a corresponding > serial clock for its OLDI Transmitters. The serial clock is 7 times the > pixel clock. This clock needs the clock set rate request to be > propagated to the parent clock provider. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 + > 1 file changed, 1 insertion(+) > Acked-by: Rob Herring <robh@kernel.org>
Quoting Aradhya Bhatia (2022-12-26 01:57:44) > Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum > list. > > "ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI > display subsystem request a pixel clock for itself and a corresponding > serial clock for its OLDI Transmitters. The serial clock is 7 times the > pixel clock. This clock needs the clock set rate request to be > propagated to the parent clock provider. > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > --- > Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > index 8f71ab300470..0696237530f7 100644 > --- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > @@ -14,6 +14,7 @@ properties: > compatible: > enum: > - fixed-factor-clock > + - ti,k3-am62-oldi-clk-div I don't see this compatible anywhere in the kernel tree. Is there a patch that adds a node using this? I wonder why the display subsystem can't add this fixed factor clk directly in the driver. Does the OLDI Transmitter send a clk to the display subsystem? I'm asking all these questions because we got rid of vendor compatibles here in hopes of simplifying the logic. Maybe the problem can be approached differently, but I don't know all the details.
Hi Stephen, Thanks for taking a look at the patch. On 12-Jan-23 01:14, Stephen Boyd wrote: > Quoting Aradhya Bhatia (2022-12-26 01:57:44) >> Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum >> list. >> >> "ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI >> display subsystem request a pixel clock for itself and a corresponding >> serial clock for its OLDI Transmitters. The serial clock is 7 times the >> pixel clock. This clock needs the clock set rate request to be >> propagated to the parent clock provider. >> >> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >> --- >> Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >> index 8f71ab300470..0696237530f7 100644 >> --- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >> @@ -14,6 +14,7 @@ properties: >> compatible: >> enum: >> - fixed-factor-clock >> + - ti,k3-am62-oldi-clk-div > > I don't see this compatible anywhere in the kernel tree. Is there a > patch that adds a node using this? I wonder why the display subsystem > can't add this fixed factor clk directly in the driver. Does the OLDI > Transmitter send a clk to the display subsystem? > > I'm asking all these questions because we got rid of vendor compatibles > here in hopes of simplifying the logic. Maybe the problem can be > approached differently, but I don't know all the details. +--------+ +------------------+ | | | | | PLL +---+----+------------->| OLDI Transmitter | | | | | | | +--------+ | | +------------------+ | | | | +------------------+ | | | | | +------------->| OLDI Transmitter | | | | | +------------------+ | | +------------------+ | +----------+ | | | | /7 | | Display | +-->| Clock +--->| Sub-System (DSS) | | Div | | | +----------+ +------------------+ This is how the the clock architecture for DSS looks like. The clock divider is not a part of DSS, but outside it. The clock request flow is initiated by the DSS driver because it has the required timing parameter information. It requests a certain pixel frequency. But the frequency required by the OLDI TXes is 7 times that pixel frequency. (Just for clarification, in some cases, the OLDI TX does require only 3.5 times the pixel frequency, but in those situations there is another divider in-front of OLDI TX that gets activated with a signal and divides the incoming frequency by 2, thereby requiring the PLL to still generate a 7x frequency.) Hence, the idea is that the clock divider is able to propagate the set rate request back to PLL, asking for a frequency 7 times more than the DSS's asking rate. If this is something less than ideal and should not go up, then I can implement a new clock device with a separate but similar clock driver. Let me know what you think! Regards Aradhya
On 16/01/2023 11:51, Aradhya Bhatia wrote: > Hi Stephen, > > Thanks for taking a look at the patch. > > On 12-Jan-23 01:14, Stephen Boyd wrote: >> Quoting Aradhya Bhatia (2022-12-26 01:57:44) >>> Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum >>> list. >>> >>> "ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI >>> display subsystem request a pixel clock for itself and a corresponding >>> serial clock for its OLDI Transmitters. The serial clock is 7 times the >>> pixel clock. This clock needs the clock set rate request to be >>> propagated to the parent clock provider. >>> >>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >>> --- >>> Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >>> b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >>> index 8f71ab300470..0696237530f7 100644 >>> --- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >>> @@ -14,6 +14,7 @@ properties: >>> compatible: >>> enum: >>> - fixed-factor-clock >>> + - ti,k3-am62-oldi-clk-div >> >> I don't see this compatible anywhere in the kernel tree. Is there a >> patch that adds a node using this? I wonder why the display subsystem >> can't add this fixed factor clk directly in the driver. Does the OLDI >> Transmitter send a clk to the display subsystem? >> >> I'm asking all these questions because we got rid of vendor compatibles >> here in hopes of simplifying the logic. Maybe the problem can be >> approached differently, but I don't know all the details. > > > +--------+ +------------------+ > | | | | > | PLL +---+----+------------->| OLDI Transmitter | > | | | | | | > +--------+ | | +------------------+ > | | > | | +------------------+ > | | | | > | +------------->| OLDI Transmitter | > | | | > | +------------------+ > | > | +------------------+ > | +----------+ | | > | | /7 | | Display | > +-->| Clock +--->| Sub-System (DSS) | > | Div | | | > +----------+ +------------------+ > > This is how the the clock architecture for DSS looks like. > > The clock divider is not a part of DSS, but outside it. > > The clock request flow is initiated by the DSS driver because it has the > required timing parameter information. It requests a certain pixel > frequency. But the frequency required by the OLDI TXes is 7 times > that pixel frequency. > > (Just for clarification, in some cases, the OLDI TX does require only > 3.5 times the pixel frequency, but in those situations there is another > divider in-front of OLDI TX that gets activated with a signal and > divides the incoming frequency by 2, thereby requiring the PLL to still > generate a 7x frequency.) > > Hence, the idea is that the clock divider is able to propagate the set > rate request back to PLL, asking for a frequency 7 times more than the > DSS's asking rate. > > If this is something less than ideal and should not go up, then I can > implement a new clock device with a separate but similar clock driver. > > Let me know what you think! As a clarification I would also add to the above that on other TI SoCs with DSS, and also for the second video port on AM62, the clock framework provides DSS a clock using the pclk frequency. Tomi
Quoting Tomi Valkeinen (2023-01-17 01:40:24) > On 16/01/2023 11:51, Aradhya Bhatia wrote: > > Hi Stephen, > > > > Thanks for taking a look at the patch. > > > > On 12-Jan-23 01:14, Stephen Boyd wrote: > >> Quoting Aradhya Bhatia (2022-12-26 01:57:44) > >>> Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum > >>> list. > >>> > >>> "ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI > >>> display subsystem request a pixel clock for itself and a corresponding > >>> serial clock for its OLDI Transmitters. The serial clock is 7 times the > >>> pixel clock. This clock needs the clock set rate request to be > >>> propagated to the parent clock provider. > >>> > >>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > >>> --- > >>> Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > >>> b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > >>> index 8f71ab300470..0696237530f7 100644 > >>> --- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > >>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > >>> @@ -14,6 +14,7 @@ properties: > >>> compatible: > >>> enum: > >>> - fixed-factor-clock > >>> + - ti,k3-am62-oldi-clk-div > >> > >> I don't see this compatible anywhere in the kernel tree. Is there a > >> patch that adds a node using this? I wonder why the display subsystem > >> can't add this fixed factor clk directly in the driver. Does the OLDI > >> Transmitter send a clk to the display subsystem? > >> > >> I'm asking all these questions because we got rid of vendor compatibles > >> here in hopes of simplifying the logic. Maybe the problem can be > >> approached differently, but I don't know all the details. > > > > > > +--------+ +------------------+ > > | | | | > > | PLL +---+----+------------->| OLDI Transmitter | > > | | | | | | > > +--------+ | | +------------------+ > > | | > > | | +------------------+ > > | | | | > > | +------------->| OLDI Transmitter | > > | | | > > | +------------------+ > > | > > | +------------------+ > > | +----------+ | | > > | | /7 | | Display | > > +-->| Clock +--->| Sub-System (DSS) | > > | Div | | | > > +----------+ +------------------+ > > > > This is how the the clock architecture for DSS looks like. > > > > The clock divider is not a part of DSS, but outside it. The divider is fixed as well? And presumably inside the SoC? > > > > The clock request flow is initiated by the DSS driver because it has the > > required timing parameter information. It requests a certain pixel > > frequency. But the frequency required by the OLDI TXes is 7 times > > that pixel frequency. > > > > (Just for clarification, in some cases, the OLDI TX does require only > > 3.5 times the pixel frequency, but in those situations there is another > > divider in-front of OLDI TX that gets activated with a signal and > > divides the incoming frequency by 2, thereby requiring the PLL to still > > generate a 7x frequency.) > > > > Hence, the idea is that the clock divider is able to propagate the set > > rate request back to PLL, asking for a frequency 7 times more than the > > DSS's asking rate. Got it. Can the PLL driver provide a pll_div_7 clk that is used for the DSS pixel clk? > > > > If this is something less than ideal and should not go up, then I can > > implement a new clock device with a separate but similar clock driver. > > > > Let me know what you think! > > As a clarification I would also add to the above that on other TI SoCs > with DSS, and also for the second video port on AM62, the clock > framework provides DSS a clock using the pclk frequency. > Are you saying that adding a fixed div-7 clk in the DSS driver is wrong?
Hi Stephen, On 26-Jan-23 05:36, Stephen Boyd wrote: > Quoting Tomi Valkeinen (2023-01-17 01:40:24) >> On 16/01/2023 11:51, Aradhya Bhatia wrote: >>> Hi Stephen, >>> >>> Thanks for taking a look at the patch. >>> >>> On 12-Jan-23 01:14, Stephen Boyd wrote: >>>> Quoting Aradhya Bhatia (2022-12-26 01:57:44) >>>>> Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum >>>>> list. >>>>> >>>>> "ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI >>>>> display subsystem request a pixel clock for itself and a corresponding >>>>> serial clock for its OLDI Transmitters. The serial clock is 7 times the >>>>> pixel clock. This clock needs the clock set rate request to be >>>>> propagated to the parent clock provider. >>>>> >>>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> >>>>> --- >>>>> Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >>>>> b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >>>>> index 8f71ab300470..0696237530f7 100644 >>>>> --- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >>>>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml >>>>> @@ -14,6 +14,7 @@ properties: >>>>> compatible: >>>>> enum: >>>>> - fixed-factor-clock >>>>> + - ti,k3-am62-oldi-clk-div >>>> >>>> I don't see this compatible anywhere in the kernel tree. Is there a >>>> patch that adds a node using this? I wonder why the display subsystem >>>> can't add this fixed factor clk directly in the driver. Does the OLDI >>>> Transmitter send a clk to the display subsystem? >>>> >>>> I'm asking all these questions because we got rid of vendor compatibles >>>> here in hopes of simplifying the logic. Maybe the problem can be >>>> approached differently, but I don't know all the details. >>> >>> >>> +--------+ +------------------+ >>> | | | | >>> | PLL +---+----+------------->| OLDI Transmitter | >>> | | | | | | >>> +--------+ | | +------------------+ >>> | | >>> | | +------------------+ >>> | | | | >>> | +------------->| OLDI Transmitter | >>> | | | >>> | +------------------+ >>> | >>> | +------------------+ >>> | +----------+ | | >>> | | /7 | | Display | >>> +-->| Clock +--->| Sub-System (DSS) | >>> | Div | | | >>> +----------+ +------------------+ >>> >>> This is how the the clock architecture for DSS looks like. >>> >>> The clock divider is not a part of DSS, but outside it. > > The divider is fixed as well? And presumably inside the SoC? Yes, and yes! > >>> >>> The clock request flow is initiated by the DSS driver because it has the >>> required timing parameter information. It requests a certain pixel >>> frequency. But the frequency required by the OLDI TXes is 7 times >>> that pixel frequency. >>> >>> (Just for clarification, in some cases, the OLDI TX does require only >>> 3.5 times the pixel frequency, but in those situations there is another >>> divider in-front of OLDI TX that gets activated with a signal and >>> divides the incoming frequency by 2, thereby requiring the PLL to still >>> generate a 7x frequency.) >>> >>> Hence, the idea is that the clock divider is able to propagate the set >>> rate request back to PLL, asking for a frequency 7 times more than the >>> DSS's asking rate. > > Got it. Can the PLL driver provide a pll_div_7 clk that is used for the > DSS pixel clk? > The PLL driver can not map the clock divider and hence can't provide the pll_div_7 clock directly to DSS. >>> >>> If this is something less than ideal and should not go up, then I can >>> implement a new clock device with a separate but similar clock driver. >>> >>> Let me know what you think! >> >> As a clarification I would also add to the above that on other TI SoCs >> with DSS, and also for the second video port on AM62, the clock >> framework provides DSS a clock using the pclk frequency. >> > > Are you saying that adding a fixed div-7 clk in the DSS driver is wrong? Yes. All variants of DSS accept a pixel clock and it would be wrong to implement a fixed div-7 in the DSS driver. All that said, I now understand that the new compatible shouldn't go there. I will implement a new driver and post it. =) Regards Aradhya
Quoting Aradhya Bhatia (2023-02-05 21:34:53) > Hi Stephen, > > On 26-Jan-23 05:36, Stephen Boyd wrote: > > Quoting Tomi Valkeinen (2023-01-17 01:40:24) > >> On 16/01/2023 11:51, Aradhya Bhatia wrote: > >>> Hi Stephen, > >>> > >>> Thanks for taking a look at the patch. > >>> > >>> On 12-Jan-23 01:14, Stephen Boyd wrote: > >>>> Quoting Aradhya Bhatia (2022-12-26 01:57:44) > >>>>> Add "ti,k3-am62-oldi-clk-div" to the fixed factor clock compatible enum > >>>>> list. > >>>>> > >>>>> "ti,k3-am62-oldi-clk-div" is a fixed-factor clock that helps the TI > >>>>> display subsystem request a pixel clock for itself and a corresponding > >>>>> serial clock for its OLDI Transmitters. The serial clock is 7 times the > >>>>> pixel clock. This clock needs the clock set rate request to be > >>>>> propagated to the parent clock provider. > >>>>> > >>>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com> > >>>>> --- > >>>>> Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml | 1 + > >>>>> 1 file changed, 1 insertion(+) > >>>>> > >>>>> diff --git > >>>>> a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > >>>>> b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > >>>>> index 8f71ab300470..0696237530f7 100644 > >>>>> --- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > >>>>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml > >>>>> @@ -14,6 +14,7 @@ properties: > >>>>> compatible: > >>>>> enum: > >>>>> - fixed-factor-clock > >>>>> + - ti,k3-am62-oldi-clk-div > >>>> > >>>> I don't see this compatible anywhere in the kernel tree. Is there a > >>>> patch that adds a node using this? I wonder why the display subsystem > >>>> can't add this fixed factor clk directly in the driver. Does the OLDI > >>>> Transmitter send a clk to the display subsystem? > >>>> > >>>> I'm asking all these questions because we got rid of vendor compatibles > >>>> here in hopes of simplifying the logic. Maybe the problem can be > >>>> approached differently, but I don't know all the details. > >>> > >>> > >>> +--------+ +------------------+ > >>> | | | | > >>> | PLL +---+----+------------->| OLDI Transmitter | > >>> | | | | | | > >>> +--------+ | | +------------------+ > >>> | | > >>> | | +------------------+ > >>> | | | | > >>> | +------------->| OLDI Transmitter | > >>> | | | > >>> | +------------------+ > >>> | > >>> | +------------------+ > >>> | +----------+ | | > >>> | | /7 | | Display | > >>> +-->| Clock +--->| Sub-System (DSS) | > >>> | Div | | | > >>> +----------+ +------------------+ > >>> > >>> This is how the the clock architecture for DSS looks like. > >>> > >>> The clock divider is not a part of DSS, but outside it. > > > > The divider is fixed as well? And presumably inside the SoC? > Yes, and yes! Ok. > > > > >>> > >>> The clock request flow is initiated by the DSS driver because it has the > >>> required timing parameter information. It requests a certain pixel > >>> frequency. But the frequency required by the OLDI TXes is 7 times > >>> that pixel frequency. > >>> > >>> (Just for clarification, in some cases, the OLDI TX does require only > >>> 3.5 times the pixel frequency, but in those situations there is another > >>> divider in-front of OLDI TX that gets activated with a signal and > >>> divides the incoming frequency by 2, thereby requiring the PLL to still > >>> generate a 7x frequency.) > >>> > >>> Hence, the idea is that the clock divider is able to propagate the set > >>> rate request back to PLL, asking for a frequency 7 times more than the > >>> DSS's asking rate. > > > > Got it. Can the PLL driver provide a pll_div_7 clk that is used for the > > DSS pixel clk? > > > The PLL driver can not map the clock divider and hence can't provide the > pll_div_7 clock directly to DSS. Isn't the divider fixed? So what is there to map? > > >>> > >>> If this is something less than ideal and should not go up, then I can > >>> implement a new clock device with a separate but similar clock driver. > >>> > >>> Let me know what you think! > >> > >> As a clarification I would also add to the above that on other TI SoCs > >> with DSS, and also for the second video port on AM62, the clock > >> framework provides DSS a clock using the pclk frequency. > >> > > > > Are you saying that adding a fixed div-7 clk in the DSS driver is wrong? > Yes. All variants of DSS accept a pixel clock and it would be wrong to > implement a fixed div-7 in the DSS driver. The reason being that it has an input of a pixel clk divided by 7? That's why I suggested having the PLL provide a clk output that is divided by 7. > > All that said, I now understand that the new compatible shouldn't go > there. I will implement a new driver and post it. =) > The block diagram above shows the fixed divider living outside of the PLL or display subsystem or OLDI transmitter(s). It could just as well be drawn where the PLL has two outputs, one divided by 7 and sent as the pixel clk and the other not divided by 7. For all I know the PLL and the fixed divider are shipped by the same hardware engineer, so it looks totally arbitrary. Isn't it easier to _not_ create a struct device, _not_ create a different device node, and simply make the PLL have another output clk, so that we don't need to implement this as a new compatible string and update the basic clk type code? To clarify, I don't understand why this must be implemented through devicetree. We already have a driver for the PLL and that knows what SoC it is, so why can't we add the fixed divider there?
diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml index 8f71ab300470..0696237530f7 100644 --- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.yaml @@ -14,6 +14,7 @@ properties: compatible: enum: - fixed-factor-clock + - ti,k3-am62-oldi-clk-div "#clock-cells": const: 0