On Wed, Dec 08, 2021 at 05:51:43PM +0100, Heiko Stübner wrote: > Hi Sascha, > > Am Mittwoch, 8. Dezember 2021, 16:12:30 CET schrieb Sascha Hauer: > > On the rk3568 we have this (simplified) situation: > > > > .--------. .-----. .---------. > > -| hpll |--.--| /n |----|dclk_vop0|- > > `--------´ | `-----´ `---------´ > > | .-----. .---------. > > `--| /m |----|dclk_vop1|- > > | `-----´ `---------´ > > | .---------. > > `-------------|hdmi_ref |- > > `---------´ > > > > hpll is the PLL that drives the HDMI reference clock and the pixel > > clocks for the different CRTCs (dclk_vop0/1). Between the pixel clocks > > and the hpll there are programmable dividers whereas the HDMI reference > > clock is directly connected to the hpll. > > > > For the HDMI output to work the pixel clock must be the same as the HDMI > > reference clock, hence the dividers must be programmed to 1. Normally a > > rate change on dclk_vop0/1 propagates through to the hpll and the clock > > framework picks a suitable combination of hpll and divider settings. by > > accident it picks a divider setting of 1 for the standard 1080p case, > > but other divider settings for most other resolutions leaving the HDMI > > port non working. > > > > This patch is not a solution, it merely puts the finger in the wound. We > > leave out the divider for the composite clock for dclk_vop0 which then > > leaves the divider at the bootloader default setting of 1. I assume > > the divider is disturbing only for the HDMI case, but needed for other > > outputs. Any thoughts how this can be handled? > > I'm not even sure if/how the common clock framework keeps track of > diverging wishes to parent-rates :-) . I don't think the common clock framework tries to keep track of that. > > But I do see two direct issues in the _existing_ code. > > dclk_vop0/1 uses CLK_SET_RATE_PARENT so is allowed to change > the rates of its parent clock(s). > > Its parent clocks are not only hpll but can also be vpll, gpll and cpll. > So this can cause even more mayhem, if the ccf for example decides > to select the gpll and then change its rate,which may result in a lot > of peripherals getting their rates changed under them ;-) . Right, we can only allow the CLK_SET_RATE_PARENT parent flag on the dclk clocks when the parent is HPLL. Since we can't be sure that HPLL is the parent we have to remove the flag. > > On the other hand I see in the clock driver that hdmi-ref is not allowed > to change its parent rate, so can only select between hpll and hpll_ph0 > (1/2 the rate?). > > So I guess, one way could be: > - add CLK_SET_RATE_PARENT to the hdmi-ref clock > - drop CLK_SET_RATE_PARENT from the dclks > - make sure hdmi-clock is set before the dclk That solves it for the HDMI case. I can imagine that for a LVDS user the CLK_SET_RATE_PARENT flag on the dclks is quite handy to get a PLL frequency suitable for the display. Otherwise he would have to set a suitable PLL frequency using assigned-clock-rates in the device tree. That's still possible so this might be a good compromise. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
