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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aQiymTFVU7UpcJ1p@kuha.fi.intel.com>
Date: Mon, 3 Nov 2025 15:48:09 +0200
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Chaoyi Chen <chaoyi.chen@...k-chips.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
	Chaoyi Chen <kernel@...kyi.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	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>,
	Andrzej Hajda <andrzej.hajda@...el.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Robert Foss <rfoss@...nel.org>,
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
	Jonas Karlman <jonas@...boo.se>,
	Jernej Skrabec <jernej.skrabec@...il.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>,
	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 v8 03/10] drm/bridge: Implement generic USB Type-C DP HPD
 bridge

> > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > > index a250afd8d662..17257b223a28 100644
> > > --- a/drivers/gpu/drm/bridge/Kconfig
> > > +++ b/drivers/gpu/drm/bridge/Kconfig
> > > @@ -23,13 +23,16 @@ config DRM_AUX_BRIDGE
> > >   	  build bridges chain.
> > >   config DRM_AUX_HPD_BRIDGE
> > > -	tristate
> > > +	tristate "AUX HPD bridge support"
> > Why? No, this is supposed to be selected by other drivers. Users don't
> > know an wouldn't know what is this.
> 
> In v7, I implemented an additional module for selecting this option. But
> Heikki believes that it would be better to merge the two modules into one.

Like I said before, I was merely curious why not just squash the
support into that AUX_PD_HPD_BRIDGE. If that does not make sense, then
so be it - make it a "Display Interface Bridge" driver like you
originally proposed.

> > >   	depends on DRM_BRIDGE && OF
> > >   	select AUXILIARY_BUS
> > >   	help
> > >   	  Simple bridge that terminates the bridge chain and provides HPD
> > >   	  support.
> > > +	  Specifically, if you want a default Type-C DisplayPort HPD bridge for
> > > +	  each port of the Type-C controller, say Y here.
> > > +
> > >   menu "Display Interface Bridges"
> > >   	depends on DRM && DRM_BRIDGE
> > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> > > index c7dc03182e59..2998937444bc 100644
> > > --- a/drivers/gpu/drm/bridge/Makefile
> > > +++ b/drivers/gpu/drm/bridge/Makefile
> > > @@ -1,6 +1,12 @@
> > >   # SPDX-License-Identifier: GPL-2.0
> > >   obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o
> > > -obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += aux-hpd-bridge.o
> > > +
> > > +hpd-bridge-y := aux-hpd-bridge.o
> > > +ifneq ($(CONFIG_TYPEC),)
> > > +hpd-bridge-y += aux-hpd-typec-dp-bridge.o
> > > +endif
> > > +obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += hpd-bridge.o
> > > +
> > >   obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
> > >   obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
> > >   obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
> > > diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> > > index 2e9c702c7087..11ad6dc776c7 100644
> > > --- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> > > +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> > > @@ -12,6 +12,8 @@
> > >   #include <drm/drm_bridge.h>
> > >   #include <drm/bridge/aux-bridge.h>
> > > +#include "aux-hpd-bridge.h"
> > > +
> > >   static DEFINE_IDA(drm_aux_hpd_bridge_ida);
> > >   struct drm_aux_hpd_bridge_data {
> > > @@ -204,7 +206,26 @@ static struct auxiliary_driver drm_aux_hpd_bridge_drv = {
> > >   	.id_table = drm_aux_hpd_bridge_table,
> > >   	.probe = drm_aux_hpd_bridge_probe,
> > >   };
> > > -module_auxiliary_driver(drm_aux_hpd_bridge_drv);
> > > +
> > > +static int drm_aux_hpd_bridge_mod_init(void)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = auxiliary_driver_register(&drm_aux_hpd_bridge_drv);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return drm_aux_hpd_typec_dp_bridge_init();
> > > +}
> > > +
> > > +static void drm_aux_hpd_bridge_mod_exit(void)
> > > +{
> > > +	drm_aux_hpd_typec_dp_bridge_exit();
> > > +	auxiliary_driver_unregister(&drm_aux_hpd_bridge_drv);
> > > +}
> > > +
> > > +module_init(drm_aux_hpd_bridge_mod_init);
> > > +module_exit(drm_aux_hpd_bridge_mod_exit);
> > >   MODULE_AUTHOR("Dmitry Baryshkov <dmitry.baryshkov@...aro.org>");
> > >   MODULE_DESCRIPTION("DRM HPD bridge");
> > > diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.h b/drivers/gpu/drm/bridge/aux-hpd-bridge.h
> > > new file mode 100644
> > > index 000000000000..69364731c2f1
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.h
> > > @@ -0,0 +1,13 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +#ifndef AUX_HPD_BRIDGE_H
> > > +#define AUX_HPD_BRIDGE_H
> > > +
> > > +#if IS_REACHABLE(CONFIG_TYPEC)
> > > +int drm_aux_hpd_typec_dp_bridge_init(void);
> > > +void drm_aux_hpd_typec_dp_bridge_exit(void);
> > > +#else
> > > +static inline int drm_aux_hpd_typec_dp_bridge_init(void) { return 0; }
> > > +static inline void drm_aux_hpd_typec_dp_bridge_exit(void) { }
> > > +#endif /* IS_REACHABLE(CONFIG_TYPEC) */
> > > +
> > > +#endif /* AUX_HPD_BRIDGE_H */
> > > diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> > > new file mode 100644
> > > index 000000000000..6f2a1fca0fc5
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> > > @@ -0,0 +1,47 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +#include <linux/of.h>
> > > +#include <linux/usb/typec_altmode.h>
> > > +#include <linux/usb/typec_dp.h>
> > > +#include <linux/usb/typec_notify.h>
> > > +
> > > +#include <drm/bridge/aux-bridge.h>
> > > +
> > > +#include "aux-hpd-bridge.h"
> > > +
> > > +#if IS_REACHABLE(CONFIG_TYPEC)
> > > +static int drm_typec_bus_event(struct notifier_block *nb,
> > > +			       unsigned long action, void *data)
> > > +{
> > This feels like this should be a part of the Type-C subsystem rather
> > than DRM.
> 
> In v7, this used to be a part of the Type-C subsystem. I'm not sure what
> Heikki thinks about this.

Your original proposal of making the entire TYPEC subsystem depend on
DRM is _not_ going to happen. In general, if I've now understood this
correctly, this thing probable should be a "display interface bridge
driver", similar to what you proposed in the previous version.

Note also that you could make it selected automatically, so there is
no need for user selectable option if that's the preference. Kconfig
and Makefile gives you options on how to do that. For example, maybe
this Kconfig works (or does not, but something like it will):

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index a250afd8d662..7487024ba2ce 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -30,6 +30,15 @@ config DRM_AUX_HPD_BRIDGE
          Simple bridge that terminates the bridge chain and provides HPD
          support.
 
+if DRM_AUX_HPD_BRIDGE
+
+config DRM_AUX_HPD_TYPEC_BRIDGE
+       tristate
+       depends on TYPEC || !TYPEC
+       default TYPEC
+
+endif /* DRM_AUX_HPD_BRIDGE */
+
 menu "Display Interface Bridges"
        depends on DRM && DRM_BRIDGE
 


thanks,

-- 
heikki

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ