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

[Damon Ding <damon.ding@rock-chips.com>]

Hi Sebastian,

On 2024/12/6 2:04, Sebastian Reichel wrote:
> Hello Damon,
> 
> On Thu, Dec 05, 2024 at 09:13:33AM +0800, Damon Ding wrote:
>> Firstly, the term "the HDMI and eDP dynamic switching" can be somewhat
>> misleading, because the eDP usually does not support hot plug. The RK3588
>> eDP is often used as DP, and it actually supports DP 1.2. Therefore, it is
>> better to use the "the HDMI and DP dynamic switching" description.
> 
> The part unclear to me is how the dynamic switching is supposed to
> happen. Looking at the TRM the hotplug detect signals also seem to be
> shared between HDMI and eDP. Can the RK3588S EVB distinguish if HDMI
> or eDP has been plugged, or does this require some user interaction
> to set the right mode?

Indeed, HDMI and eDP share the same pin for hotplug detect. However, 
some users may connect the hotplug detection pin of DP-connector with an 
unexpected pin that can not support the iomux of hotplug detect function 
on RK3588 SoC. This could be due to a flaw in the hardware design, a 
conflict in pin multiplexing, or other factors. Therefore, we support 
the GPIO HDP function for the eDP, as DP also supports this for the same 
reasons.

If the dynamic switching is enabled, HDMI detects the HPD signal through 
  the hotplug detect function pin, while eDP uses one of the available 
GPIO pins to do this.

What's more, if the user connects an HDMI cable first and than connects 
a DP cable as well, despite our clear instruction against using HDMI and 
eDP simultaneously, the status register of GRF will indicate that HDMI 
has been connected. Meanwhile, during the HPD detection process for eDP, 
it will return "connector_status_disconnected". The reverse scenario 
also applies.

> 
>> Indeed, the devm_phy_get(dp->dev, "dp") and devm_of_phy_get_by_index() will
>> help to get the phy reference in .probe() or .bind().
>>
>> However, the phy_set_mode() may be still needed in the HDMI and DP dynamic
>> switching application scenarios. We need the enum phy_mode
>> PHY_MODE_DP/PHY_MODE_HDMI to differentiate the configuration processes in
>> .power_on(), .power_off() and .configure() of struct phy_ops, which will be
>> called in conjunction with plugging in and unplugging an HDMI or DP cable.
> 
> I suppose you could fetch the PHY in power_on() and release it in
> power_off(). But using phy_set_mode() might indeed be better here.
> 

As a future expansion, the .set_mode() can also be helpful in the txffe 
level adjustment for HDMI 2.1. :)

> -- Sebastian

Best regards,
Damon