Hi Michael, Am Dienstag, 13. Juli 2021, 10:44:00 CEST schrieb Michael Riesch: > The HDMI TX block in the RK3568 requires two power supplies, which have > to be enabled in some cases (at least on the RK3568 EVB1 the voltages > VDDA0V9_IMAGE and VCCA1V8_IMAGE are disabled by default). It would be > great if this was considered by the driver and the device tree binding. > I am not sure, though, whether this is a RK3568 specific or > rockchip_dw_hdmi specific thing. Maybe it can even enter the Synopsis DW > HDMI driver. I do remember that this discussion happened many years back already. And yes the supplies are needed for all but back then there was opposition as these are supposedly phy-related supplies, not for the dw-hdmi itself. [There are variants with an external phy, like on the rk3328] See discussion on [0] [0] https://dri-devel.freedesktop.narkive.com/pen2zWo1/patch-v3-1-2-drm-bridge-dw-hdmi-support-optional-supply-regulators > On 7/7/21 2:03 PM, Benjamin Gaignard wrote: > > Define a new compatible for rk3568 HDMI. > > This version of HDMI hardware block needs two new clocks hclk_vio and hclk > > to provide phy reference clocks. > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx> > > --- > > version 2: > > - Add the clocks needed for the phy. > > > > .../bindings/display/rockchip/rockchip,dw-hdmi.yaml | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml > > index 75cd9c686e985..cb8643b3a8b84 100644 > > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml > > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml > > @@ -23,6 +23,7 @@ properties: > > - rockchip,rk3288-dw-hdmi > > - rockchip,rk3328-dw-hdmi > > - rockchip,rk3399-dw-hdmi > > + - rockchip,rk3568-dw-hdmi > > > > reg-io-width: > > const: 4 > > @@ -51,8 +52,11 @@ properties: > > - vpll > > - enum: > > - grf > > + - hclk_vio > > + - vpll > > + - enum: > > + - hclk > > - vpll > > - - const: vpll > > The description and documentation of the clocks are somewhat misleading > IMHO. This is not caused by your patches, of course. But maybe this is a > chance to clean them up a bit. > > It seems that the CEC clock is an optional clock of the dw-hdmi driver. > Shouldn't it be documented in the synopsys,dw-hdmi.yaml? > > Also, it would be nice if the clocks hclk_vio and hclk featured a > description in the binding. > > BTW, I am not too familiar with the syntax here, but shouldn't items in > clocks and items in clock-names be aligned (currently, there is a plain > list vs. an enum structure)? > > Best regards, > Michael > > > > > ddc-i2c-bus: > > $ref: /schemas/types.yaml#/definitions/phandle > > >
