Hi Damon,
Am Mittwoch, 27. November 2024, 12:00:10 CET schrieb Damon Ding:
> Hi Heiko:
>
> On 2024/11/27 17:29, Heiko Stübner wrote:
> > Hi Damon,
> >
> > Am Mittwoch, 27. November 2024, 08:51:51 CET schrieb Damon Ding:
> >> Add basic support for RBR/HBR/HBR2 link rates, and the voltage swing and
> >> pre-emphasis configurations of each link rate have been verified according
> >> to the eDP 1.3 requirements.
> >>
> >> Signed-off-by: Damon Ding <damon.ding@xxxxxxxxxxxxxx>
> >> ---
> >
> > [ ... huge block of DP phy support ...]
> >
> > yes that block was huge, but I also don't see a way to split that up in a
> > useful way, so it should be fine.
> >
>
> As for the huge block of DP phy support, I will try to use the existing
> rk_hdptx_multi_reg_write() to set regs in next version, maybe the way
> can make the codes more concise.
I actually did like the the dp-side of the phy code.
That you need to add all the DP stuff can't be helped and I actually find
real functions nicer than having anonymous register writes.
I.e. the hdmi-side with its register lists does write "magic" values to
registers.
So personally I'd just leave the dp-functions as is please, until someone
does complain (I was not trying to complain, just mentioned why I cut
it from the reply :-) )
Thanks
Heiko
> >> +static int rk_hdptx_phy_set_mode(struct phy *phy, enum phy_mode mode,
> >> + int submode)
> >> +{
> >> + return 0;
> >> +}
> >
> > I think it might make sense to go the same way as the DCPHY and also
> > naneng combophy, to use #phy-cells = 1 to select the phy-mode via DT .
> >
> > See [0] for Sebastians initial suggestion regarding the DC-PHY.
> > The naneng combophy already uses that scheme of mode-selection too.
> >
> > There is of course the issue of backwards-compatibility, but that can be
> > worked around in the binding with something like:
> >
> > '#phy-cells':
> > enum: [0, 1]
> > description: |
> > If #phy-cells is 0, PHY mode is set to PHY_TYPE_HDMI
> > If #phy-cells is 1 mode is set in the PHY cells. Supported modes are:
> > - PHY_TYPE_HDMI
> > - PHY_TYPE_DP
> > See include/dt-bindings/phy/phy.h for constants.
> >
> > PHY_TYPE_HDMI needs to be added to include/dt-bindings/phy/phy.h
> > but PHY_TYPE_DP is already there.
> >
> > That way we would standardize on one form of accessing phy-types
> > on rk3588 :-) .
> >
> > Also see the Mediatek CSI rx phy doing this too already [1]
> >
> >
> > Heiko
> >
> > [0] https://lore.kernel.org/linux-rockchip/udad4qf3o7kt45nuz6gxsvsmprh4rnyfxfogopmih6ucznizih@7oj2jrnlfonz/
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/phy/mediatek,mt8365-csi-rx.yaml
> >
>
> It is really a nice way to separate HDMI and DP modes.
>
> >
> >
> >
> >
>
> Best regards,
> Damon
>
>
