| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ccaa0f30-6b18-4d44-bda5-e90e1b087ecc@rock-chips.com>
Date: Tue, 23 Sep 2025 11:33:19 +0800
From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Chaoyi Chen <kernel@...kyi.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>, Heiko Stuebner
<heiko@...ech.de>, Sandy Huang <hjc@...k-chips.com>,
Andy Yan <andy.yan@...k-chips.com>,
Yubing Zhang <yubing.zhang@...k-chips.com>,
Frank Wang <frank.wang@...k-chips.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Amit Sunil Dhamne <amitsd@...gle.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Dragan Simic <dsimic@...jaro.org>, Johan Jonker <jbx6244@...il.com>,
Diederik de Haas <didi.debian@...ow.org>,
Peter Robinson <pbrobinson@...il.com>, linux-usb@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-phy@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v4 5/7] drm/rockchip: cdn-dp: Add multiple bridges to
support PHY port selection
On 9/23/2025 11:22 AM, Dmitry Baryshkov wrote:
> On Tue, Sep 23, 2025 at 10:09:38AM +0800, Chaoyi Chen wrote:
>> On 9/23/2025 9:50 AM, Dmitry Baryshkov wrote:
>>
>>> On Mon, Sep 22, 2025 at 09:20:37AM +0800, Chaoyi Chen wrote:
>>>> From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
>>>>
>>>> The RK3399 has two USB/DP combo PHY and one CDN-DP controller. And
>>>> the CDN-DP can be switched to output to one of the PHYs. If both ports
>>>> are plugged into DP, DP will select the first port for output.
>>>>
>>>> This patch adds support for multiple bridges, enabling users to flexibly
>>>> select the output port. For each PHY port, a separate encoder and bridge
>>>> are registered.
>>>>
>>>> The change is based on the DRM AUX HPD bridge, rather than the
>>>> extcon approach. This requires the DT to correctly describe the
>>>> connections between the PHY, USB connector, and DP controller.
>>>> And cdn_dp_parse_hpd_bridge_dt() will parses it and determines
>>>> whether to register one or two bridges.
>>>>
>>>> Since there is only one DP controller, only one of the PHY ports can
>>>> output at a time. The key is how to switch between different PHYs,
>>>> which is handled by cdn_dp_switch_port() and cdn_dp_enable().
>>>>
>>>> There are two cases:
>>>>
>>>> 1. Neither bridge is enabled. In this case, both bridges can
>>>> independently read the EDID, and the PHY port may switch before
>>>> reading the EDID.
>>>>
>>>> 2. One bridge is already enabled. In this case, other bridges are not
>>>> allowed to read the EDID.
>>>>
>>>> Since the scenario of two ports plug in at the same time is rare,
>>>> I don't have a board which support two TypeC connector to test this.
>>>> Therefore, I tested forced switching on a single PHY port, as well as
>>>> output using a fake PHY port alongside a real PHY port.
>>>>
>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@...k-chips.com>
>>>> ---
>>>> drivers/gpu/drm/rockchip/Kconfig | 1 +
>>>> drivers/gpu/drm/rockchip/cdn-dp-core.c | 398 +++++++++++++++++++++----
>>>> drivers/gpu/drm/rockchip/cdn-dp-core.h | 23 +-
>>>> 3 files changed, 366 insertions(+), 56 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
>>>> index faf50d872be3..3a6266279323 100644
>>>> --- a/drivers/gpu/drm/rockchip/Kconfig
>>>> +++ b/drivers/gpu/drm/rockchip/Kconfig
>>>> @@ -55,6 +55,7 @@ config ROCKCHIP_CDN_DP
>>>> select DRM_DISPLAY_HELPER
>>>> select DRM_BRIDGE_CONNECTOR
>>>> select DRM_DISPLAY_DP_HELPER
>>>> + select DRM_AUX_HPD_BRIDGE
>>>> help
>>>> This selects support for Rockchip SoC specific extensions
>>>> for the cdn DP driver. If you want to enable Dp on
>>>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> index 1e27301584a4..784f5656fcc4 100644
>>>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>>>> @@ -27,16 +27,17 @@
>>>> #include "cdn-dp-core.h"
>>>> #include "cdn-dp-reg.h"
>>>> -static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge)
>>>> +static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port,
>>>> + struct cdn_dp_port *port);
>>>> +
>>>> +static inline struct cdn_dp_bridge *bridge_to_dp_bridge(struct drm_bridge *bridge)
>>>> {
>>>> - return container_of(bridge, struct cdn_dp_device, bridge);
>>>> + return container_of(bridge, struct cdn_dp_bridge, bridge);
>>>> }
>>>> -static inline struct cdn_dp_device *encoder_to_dp(struct drm_encoder *encoder)
>>>> +static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge)
>>>> {
>>>> - struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
>>>> -
>>>> - return container_of(rkencoder, struct cdn_dp_device, encoder);
>>>> + return bridge_to_dp_bridge(bridge)->parent;
>>>> }
>>>> #define GRF_SOC_CON9 0x6224
>>>> @@ -191,14 +192,27 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
>>>> static struct cdn_dp_port *cdn_dp_connected_port(struct cdn_dp_device *dp)
>>>> {
>>>> struct cdn_dp_port *port;
>>>> - int i, lanes;
>>>> + int i, lanes[MAX_PHY];
>>>> for (i = 0; i < dp->ports; i++) {
>>>> port = dp->port[i];
>>>> - lanes = cdn_dp_get_port_lanes(port);
>>>> - if (lanes)
>>>> + lanes[i] = cdn_dp_get_port_lanes(port);
>>>> + if (!dp->hpd_bridge_valid)
>>>> return port;
>>>> }
>>>> +
>>>> + if (dp->hpd_bridge_valid) {
>>>> + /* If more than one port is available, pick the last active port */
>>>> + if (dp->active_port > 0 && lanes[dp->active_port])
>>>> + return dp->port[dp->active_port];
>>>> +
>>>> + /* If the last active port is not available, pick an available port in order */
>>>> + for (i = 0; i < dp->bridge_count; i++) {
>>>> + if (lanes[i])
>>>> + return dp->port[i];
>>>> + }
>>>> + }
>>>> +
>>>> return NULL;
>>>> }
>>>> @@ -239,10 +253,11 @@ static enum drm_connector_status
>>>> cdn_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
>>>> {
>>>> struct cdn_dp_device *dp = bridge_to_dp(bridge);
>>>> + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
>>>> enum drm_connector_status status = connector_status_disconnected;
>>>> mutex_lock(&dp->lock);
>>>> - if (dp->connected)
>>>> + if (dp_bridge->connected && dp->connected)
>>>> status = connector_status_connected;
>>>> mutex_unlock(&dp->lock);
>>>> @@ -253,10 +268,36 @@ static const struct drm_edid *
>>>> cdn_dp_bridge_edid_read(struct drm_bridge *bridge, struct drm_connector *connector)
>>>> {
>>>> struct cdn_dp_device *dp = bridge_to_dp(bridge);
>>>> - const struct drm_edid *drm_edid;
>>>> + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
>>>> + struct cdn_dp_port *port = dp->port[dp_bridge->id];
>>>> + struct cdn_dp_port *prev_port;
>>>> + const struct drm_edid *drm_edid = NULL;
>>>> + int i, ret;
>>>> mutex_lock(&dp->lock);
>>>> +
>>>> + /* More than one port is available */
>>>> + if (dp->bridge_count > 1 && !port->phy_enabled) {
>>>> + for (i = 0; i < dp->bridge_count; i++) {
>>>> + /* Another port already enable */
>>>> + if (dp->bridge_list[i] != dp_bridge && dp->bridge_list[i]->enabled)
>>>> + goto unlock;
>>>> + /* Find already enabled port */
>>>> + if (dp->port[i]->phy_enabled)
>>>> + prev_port = dp->port[i];
>>>> + }
>>>> +
>>>> + /* Switch to current port */
>>>> + if (prev_port) {
>>>> + ret = cdn_dp_switch_port(dp, prev_port, port);
>>>> + if (ret)
>>>> + goto unlock;
>>>> + }
>>>> + }
>>>> +
>>>> drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
>>> So... If I try reading EDID for the PHY 2 while PHY 1 is enabled, will
>>> it return NULL, even if there is a monitor there? It totally feels like
>>> this is one of the rare cases when caching EDIDs might make sense.
>> Of course. I did consider using cache, but if the monitor changes, then caching the EDID doesn't seem to be of much use…
> Yes... It might still be better to invalidate the cache on the plug
> event rather than always reporting empty EDID when another monitor is
> enabled.
Hmm, I will add cache in v5.
>
--
Best,
Chaoyi
Powered by blists - more mailing lists