| 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: <3n6gvt2lcf24smt6njiqoqt5h4733z36hwvc2zfyft62d25uu5@vwl26mmsfzbz>
Date: Tue, 23 Sep 2025 06:22:03 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Chaoyi Chen <chaoyi.chen@...k-chips.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 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.
--
With best wishes
Dmitry
Powered by blists - more mailing lists