Re: [PATCH v1 04/10] phy: phy-rockchip-samsung-hdptx: Add support for eDP mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Heiko,

On 2024/11/27 19:04, Heiko Stübner wrote:
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.

I apologize for reopening the discussion about the phy-types setting.

With the .set_mode() of struct phy_ops, the HDMI and eDP dynamic switching can be achieved, which just depends on the right setting of enum phy_mode in include/linux/phy/phy.h. So the previous way of configuring phy mode may be also good.

And other phys may want to support dynamic switching too, like the Rockchip USBDP combo phy.







Best regards,
Damon








Best regards,
Damon



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux