Re: [PATCH v2 13/15] phy: starfive: Add jh7110-inno-hdmi-phy driver

[Icenowy Zheng]

在 2026-09-07一的 08:02 +0200，Maud Spierings写道：
> Hi Dominique and Icenowy,
> 
> On 9/7/26 06:01, Icenowy Zheng wrote:
> > 在 2026-09-06日的 17:16 -0400，Dominique Belhachemi写道：
> > > On Sun, Sep 6, 2026 at 11:19 AM Icenowy Zheng <uwu@xxxxxxxxxx>
> > > wrote:
> > > > 
> > > > 在 2026-09-06日的 16:59 +0200，Maud Spierings写道：
> > > > > On 9/6/26 07:39, Dominique Belhachemi wrote:
> > > > > > On Sun, Aug 30, 2026 at 10:17 AM Maud Spierings
> > > > > > <
> > > > > > maud_spierings@xxxxxxxxx <mailto:maud_spierings@xxxxxxxxx>>
> > > > > > wrote:
> > > > > > 
> > > > > >      I was still having some glitching happening on the
> > > > > > display,
> > > > > > but
> > > > > > I've
> > > > > >      found the way to fix that, the question is what is
> > > > > > actually
> > > > > >      happening here.
> > > > > > 
> > > > > >          0x29590020 <- 0x00000005
> > > > > >      This one I have no idea, it is 0x00000009 with this
> > > > > > patch
> > > > > > series but
> > > > > >      with the vendor kernel I get the value above. When I
> > > > > > hook
> > > > > > up my
> > > > > >      external
> > > > > >      display (regular 1440p) this becomes 0x0000000D on the
> > > > > > vendor
> > > > > > kernel.
> > > > > > 
> > > > > >      But I can't find this register being written to
> > > > > > anywhere
> > > > > > there?
> > > > > > 
> > > > > > 
> > > > > > Maybe this needs to be swapped?
> > > > > > 
> > > > > > drivers/gpu/drm/bridge/inno-hdmi.c
> > > > > >     -#define v_HSYNC_POLARITY(n)          ((n) << 3)
> > > > > >     -#define v_VSYNC_POLARITY(n)          ((n) << 2)
> > > > > >     +#define v_HSYNC_POLARITY(n)          ((n) << 2)
> > > > > >     +#define v_VSYNC_POLARITY(n)          ((n) << 3)
> > > > 
> > > > Very weirdly, the original definition here matches current
> > > > mainline
> > > > inno-hdmi.c, but the changed definition matches JH7110 vendor
> > > > inno_hdmi.h [1].
> > > > 
> > > 
> > > The vendor code is correct and matches the RK3128 TRM.
> > 
> > Thanks for the tips on documentation, and I verified this.
> > 
> > It seems that Rockchip people made this always wrong, even with
> > their
> > pre-DRM display driver... [1]
> > 
> > BTW I checked the Innosilicon dGPU driver code, and its
> > g3_ne_hdmi.h
> > source file also contains the definition of BIT(3) as VSYNC. (It's
> > quite weird that most logic of that driver is in some .o_shipped
> > blob,
> > but fortunately the g3 logic might be too new to be closed down)
> > 
> > Thanks,
> > Icenowy
> > 
> > [1]
> > https://github.com/rockchip-linux/kernel/blob/release-4.4/drivers/video/rockchip/hdmi/rockchip-hdmiv1/rockchip_hdmiv1_hw.h#L161
> > 
> > > 
> > > HDMI_reg08
> > >      Bit  Attr  Reset  Description
> > >      3    RW    0x0    vs_polarity   VSYNC polarity   1'b0:
> > > Negative
> > > 1'b1: Positive
> > >      2    RW    0x0    hs_polarity   HSYNC polarity   1'b0:
> > > Negative
> > > 1'b1: Positive
> > > 
> > > Nobody noticed this so far because in 720p/1080p both polarities
> > > are
> > > positive, but Maud's 3:2 panel has differing H/V polarity.
> > > 
> > > Best
> > > -Dominique
> 
> This also solves the devmem behaviour I saw, or well, doesn't realy 
> explain it but does show what is actually happening. As Icenowy noted
> (maybe that was on telegram only?) the inno-hdmi regs are 8bit
> instead 
> of 32 bit.
> 
> What is actually happening is:
> 
> devmem 0x29590000 b reads 0x29590000 as expected
> but
> devmem 0x29590004 b reads 0x29590001
> devmem 0x29590008 b reads 0x29590002
> ....
> devmem 0x29590020 b reads 0x29590008
> 
> For some reason the addressing is weird?
> 
> With this change, the assigned clock change and removing the 
> v_REG_CLK_INV | v_REG_CLK_SOURCE_SYS write to HDMI_SYS_CTRL, the
> display 
> comes up from the start as it should!

Well I checked the RK3128 TRM for these two bits, and their field
descriptions are -- "reserved".

What can I react...

Thanks,
Icenowy

> 
> Kind regards,
> Maud