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@xxxxxx> >>>>> --- >>>>> 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
