All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/2] drm: show "all" bridges in debugfs
@ 2025-02-26 21:23 Luca Ceresoli
  2025-02-26 21:23 ` [PATCH v8 1/2] drm/bridge: move bridges_show logic from drm_debugfs.c Luca Ceresoli
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Luca Ceresoli @ 2025-02-26 21:23 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: Jani Nikula, Dmitry Baryshkov, Thomas Petazzoni, dri-devel,
	linux-kernel, Luca Ceresoli

This series adds a /sys/kernel/debug/dri/bridges file showing all bridges
between drm_bridge_add() and drm_bridge_remove(), which might not be bound
to any encoder and thus not visible anywhere in debugfs.

It also cleans up the DRM bridge debugfs code by moving code to
drm_bridge.c.

Luca

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v8:
- moved more code to drm_bridge.c, which makes adding '#if CONFIG_DEBUG_FS' unnecessary
- a small fix to a harmless bug
- Link to v7: https://lore.kernel.org/r/20250225-drm-debugfs-show-all-bridges-v7-0-8826037ada37@bootlin.com

This series was initially part of v6 of this other series:
- Link to v6: https://lore.kernel.org/dri-devel/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/

---
Luca Ceresoli (2):
      drm/bridge: move bridges_show logic from drm_debugfs.c
      drm/debugfs: add top-level 'bridges' file showing all added bridges

 drivers/gpu/drm/drm_bridge.c  | 70 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_debugfs.c | 38 +----------------------
 drivers/gpu/drm/drm_drv.c     |  2 ++
 include/drm/drm_bridge.h      |  3 ++
 4 files changed, 76 insertions(+), 37 deletions(-)
---
base-commit: b439ab75b6382f5c34aec6e87435cf7e58e72a35
change-id: 20250225-drm-debugfs-show-all-bridges-642e870b71e7

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v8 1/2] drm/bridge: move bridges_show logic from drm_debugfs.c
  2025-02-26 21:23 [PATCH v8 0/2] drm: show "all" bridges in debugfs Luca Ceresoli
@ 2025-02-26 21:23 ` Luca Ceresoli
  2025-02-27  8:56   ` Jani Nikula
  2025-02-26 21:23 ` [PATCH v8 2/2] drm/debugfs: add top-level 'bridges' file showing all added bridges Luca Ceresoli
  2025-03-12  9:59 ` [PATCH v8 0/2] drm: show "all" bridges in debugfs Louis Chauvet
  2 siblings, 1 reply; 6+ messages in thread
From: Luca Ceresoli @ 2025-02-26 21:23 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: Jani Nikula, Dmitry Baryshkov, Thomas Petazzoni, dri-devel,
	linux-kernel, Luca Ceresoli

In preparation to expose more info about bridges in debugfs, which will
require more insight into drm_bridge data structures, move the bridges_show
code to drm_bridge.c.

Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changed in v8:
 - add the file in drm_bridge.c, which avois the added #if CONFIG_DEBUG_FS

This patch was added in v7.
---
 drivers/gpu/drm/drm_bridge.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_debugfs.c | 38 +-------------------------------------
 include/drm/drm_bridge.h      |  2 ++
 3 files changed, 45 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 241a384ebce39b4a3db58c208af27960904fc662..a6bf1a565e3c3a8d24de60448972849f6d86ba72 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -21,6 +21,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include <linux/debugfs.h>
 #include <linux/err.h>
 #include <linux/media-bus-format.h>
 #include <linux/module.h>
@@ -1335,6 +1336,47 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
 EXPORT_SYMBOL(of_drm_find_bridge);
 #endif
 
+static int encoder_bridges_show(struct seq_file *m, void *data)
+{
+	struct drm_encoder *encoder = m->private;
+	struct drm_printer p = drm_seq_file_printer(m);
+	struct drm_bridge *bridge;
+	unsigned int idx = 0;
+
+	drm_for_each_bridge_in_chain(encoder, bridge) {
+		drm_printf(&p, "bridge[%u]: %ps\n", idx++, bridge->funcs);
+		drm_printf(&p, "\ttype: [%d] %s\n",
+			   bridge->type,
+			   drm_get_connector_type_name(bridge->type));
+
+		if (bridge->of_node)
+			drm_printf(&p, "\tOF: %pOFfc\n", bridge->of_node);
+
+		drm_printf(&p, "\tops: [0x%x]", bridge->ops);
+		if (bridge->ops & DRM_BRIDGE_OP_DETECT)
+			drm_puts(&p, " detect");
+		if (bridge->ops & DRM_BRIDGE_OP_EDID)
+			drm_puts(&p, " edid");
+		if (bridge->ops & DRM_BRIDGE_OP_HPD)
+			drm_puts(&p, " hpd");
+		if (bridge->ops & DRM_BRIDGE_OP_MODES)
+			drm_puts(&p, " modes");
+		if (bridge->ops & DRM_BRIDGE_OP_HDMI)
+			drm_puts(&p, " hdmi");
+		drm_puts(&p, "\n");
+	}
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(encoder_bridges);
+
+void drm_bridge_debugfs_encoder_params(struct dentry *root,
+				       struct drm_encoder *encoder)
+{
+	/* bridges list */
+	debugfs_create_file("bridges", 0444, root, encoder, &encoder_bridges_fops);
+}
+
 MODULE_AUTHOR("Ajay Kumar <ajaykumar.rs@samsung.com>");
 MODULE_DESCRIPTION("DRM bridge infrastructure");
 MODULE_LICENSE("GPL and additional rights");
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 6b2178864c7ee12db9aa1f562e106b2f604439f8..3dfd8b34dceb7a5b8f11e3072a1eaef430869722 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -740,40 +740,6 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
 	crtc->debugfs_entry = NULL;
 }
 
-static int bridges_show(struct seq_file *m, void *data)
-{
-	struct drm_encoder *encoder = m->private;
-	struct drm_printer p = drm_seq_file_printer(m);
-	struct drm_bridge *bridge;
-	unsigned int idx = 0;
-
-	drm_for_each_bridge_in_chain(encoder, bridge) {
-		drm_printf(&p, "bridge[%u]: %ps\n", idx++, bridge->funcs);
-		drm_printf(&p, "\ttype: [%d] %s\n",
-			   bridge->type,
-			   drm_get_connector_type_name(bridge->type));
-
-		if (bridge->of_node)
-			drm_printf(&p, "\tOF: %pOFfc\n", bridge->of_node);
-
-		drm_printf(&p, "\tops: [0x%x]", bridge->ops);
-		if (bridge->ops & DRM_BRIDGE_OP_DETECT)
-			drm_puts(&p, " detect");
-		if (bridge->ops & DRM_BRIDGE_OP_EDID)
-			drm_puts(&p, " edid");
-		if (bridge->ops & DRM_BRIDGE_OP_HPD)
-			drm_puts(&p, " hpd");
-		if (bridge->ops & DRM_BRIDGE_OP_MODES)
-			drm_puts(&p, " modes");
-		if (bridge->ops & DRM_BRIDGE_OP_HDMI)
-			drm_puts(&p, " hdmi");
-		drm_puts(&p, "\n");
-	}
-
-	return 0;
-}
-DEFINE_SHOW_ATTRIBUTE(bridges);
-
 void drm_debugfs_encoder_add(struct drm_encoder *encoder)
 {
 	struct drm_minor *minor = encoder->dev->primary;
@@ -789,9 +755,7 @@ void drm_debugfs_encoder_add(struct drm_encoder *encoder)
 
 	encoder->debugfs_entry = root;
 
-	/* bridges list */
-	debugfs_create_file("bridges", 0444, root, encoder,
-			    &bridges_fops);
+	drm_bridge_debugfs_encoder_params(root, encoder);
 
 	if (encoder->funcs && encoder->funcs->debugfs_init)
 		encoder->funcs->debugfs_init(encoder, root);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..0890acfe04b99b1ccbbff10b507cb8c2b2705e06 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1108,4 +1108,6 @@ static inline struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
 }
 #endif
 
+void drm_bridge_debugfs_encoder_params(struct dentry *root, struct drm_encoder *encoder);
+
 #endif

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v8 2/2] drm/debugfs: add top-level 'bridges' file showing all added bridges
  2025-02-26 21:23 [PATCH v8 0/2] drm: show "all" bridges in debugfs Luca Ceresoli
  2025-02-26 21:23 ` [PATCH v8 1/2] drm/bridge: move bridges_show logic from drm_debugfs.c Luca Ceresoli
@ 2025-02-26 21:23 ` Luca Ceresoli
  2025-02-27  8:59   ` Jani Nikula
  2025-03-12  9:59 ` [PATCH v8 0/2] drm: show "all" bridges in debugfs Louis Chauvet
  2 siblings, 1 reply; 6+ messages in thread
From: Luca Ceresoli @ 2025-02-26 21:23 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: Jani Nikula, Dmitry Baryshkov, Thomas Petazzoni, dri-devel,
	linux-kernel, Luca Ceresoli

The global bridges_list holding all the bridges between drm_bridge_add()
and drm_bridge_remove() cannot be inspected via debugfs. Add a file showing
it.

To avoid code duplication, move the code printing a bridge info to a common
function.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changed in v8:
- add the file in drm_bridge.c, which avois the added #if CONFIG_DEBUG_FS
- fix incorrect (but harmless) idx increment in
  drm_bridge_debugfs_show_bridge()

Changed in v7:
- move implementation to drm_bridge.c to avoid exporting bridge_list and
  bridge_mutex

This patch was added in v6.
---
 drivers/gpu/drm/drm_bridge.c | 72 ++++++++++++++++++++++++++++++--------------
 drivers/gpu/drm/drm_drv.c    |  2 ++
 include/drm/drm_bridge.h     |  1 +
 3 files changed, 53 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index a6bf1a565e3c3a8d24de60448972849f6d86ba72..9c6e35d41ed54a14d5745e684a341c907ed84d6b 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1336,6 +1336,49 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
 EXPORT_SYMBOL(of_drm_find_bridge);
 #endif
 
+static void drm_bridge_debugfs_show_bridge(struct drm_printer *p,
+					   struct drm_bridge *bridge,
+					   unsigned int idx)
+{
+	drm_printf(p, "bridge[%u]: %ps\n", idx, bridge->funcs);
+	drm_printf(p, "\ttype: [%d] %s\n",
+		   bridge->type,
+		   drm_get_connector_type_name(bridge->type));
+
+	if (bridge->of_node)
+		drm_printf(p, "\tOF: %pOFfc\n", bridge->of_node);
+
+	drm_printf(p, "\tops: [0x%x]", bridge->ops);
+	if (bridge->ops & DRM_BRIDGE_OP_DETECT)
+		drm_puts(p, " detect");
+	if (bridge->ops & DRM_BRIDGE_OP_EDID)
+		drm_puts(p, " edid");
+	if (bridge->ops & DRM_BRIDGE_OP_HPD)
+		drm_puts(p, " hpd");
+	if (bridge->ops & DRM_BRIDGE_OP_MODES)
+		drm_puts(p, " modes");
+	if (bridge->ops & DRM_BRIDGE_OP_HDMI)
+		drm_puts(p, " hdmi");
+	drm_puts(p, "\n");
+}
+
+static int allbridges_show(struct seq_file *m, void *data)
+{
+	struct drm_printer p = drm_seq_file_printer(m);
+	struct drm_bridge *bridge;
+	unsigned int idx = 0;
+
+	mutex_lock(&bridge_lock);
+
+	list_for_each_entry(bridge, &bridge_list, list)
+		drm_bridge_debugfs_show_bridge(&p, bridge, idx++);
+
+	mutex_unlock(&bridge_lock);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(allbridges);
+
 static int encoder_bridges_show(struct seq_file *m, void *data)
 {
 	struct drm_encoder *encoder = m->private;
@@ -1343,33 +1386,18 @@ static int encoder_bridges_show(struct seq_file *m, void *data)
 	struct drm_bridge *bridge;
 	unsigned int idx = 0;
 
-	drm_for_each_bridge_in_chain(encoder, bridge) {
-		drm_printf(&p, "bridge[%u]: %ps\n", idx++, bridge->funcs);
-		drm_printf(&p, "\ttype: [%d] %s\n",
-			   bridge->type,
-			   drm_get_connector_type_name(bridge->type));
-
-		if (bridge->of_node)
-			drm_printf(&p, "\tOF: %pOFfc\n", bridge->of_node);
-
-		drm_printf(&p, "\tops: [0x%x]", bridge->ops);
-		if (bridge->ops & DRM_BRIDGE_OP_DETECT)
-			drm_puts(&p, " detect");
-		if (bridge->ops & DRM_BRIDGE_OP_EDID)
-			drm_puts(&p, " edid");
-		if (bridge->ops & DRM_BRIDGE_OP_HPD)
-			drm_puts(&p, " hpd");
-		if (bridge->ops & DRM_BRIDGE_OP_MODES)
-			drm_puts(&p, " modes");
-		if (bridge->ops & DRM_BRIDGE_OP_HDMI)
-			drm_puts(&p, " hdmi");
-		drm_puts(&p, "\n");
-	}
+	drm_for_each_bridge_in_chain(encoder, bridge)
+		drm_bridge_debugfs_show_bridge(&p, bridge, idx++);
 
 	return 0;
 }
 DEFINE_SHOW_ATTRIBUTE(encoder_bridges);
 
+void drm_bridge_debugfs_params(struct dentry *root)
+{
+	debugfs_create_file("bridges", 0444, root, NULL, &allbridges_fops);
+}
+
 void drm_bridge_debugfs_encoder_params(struct dentry *root,
 				       struct drm_encoder *encoder)
 {
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 3cf440eee8a2ab3de134d925db8f1d2ce68062b7..22e8cd0a6a37a0ac25535e9d570da25571b0b2bc 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -38,6 +38,7 @@
 #include <linux/xarray.h>
 
 #include <drm/drm_accel.h>
+#include <drm/drm_bridge.h>
 #include <drm/drm_cache.h>
 #include <drm/drm_client_event.h>
 #include <drm/drm_color_mgmt.h>
@@ -1120,6 +1121,7 @@ static int __init drm_core_init(void)
 	}
 
 	drm_debugfs_root = debugfs_create_dir("dri", NULL);
+	drm_bridge_debugfs_params(drm_debugfs_root);
 
 	ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
 	if (ret < 0)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 0890acfe04b99b1ccbbff10b507cb8c2b2705e06..2a99d70865571f24db0ca75c758cfd09d3a5d459 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1108,6 +1108,7 @@ static inline struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
 }
 #endif
 
+void drm_bridge_debugfs_params(struct dentry *root);
 void drm_bridge_debugfs_encoder_params(struct dentry *root, struct drm_encoder *encoder);
 
 #endif

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v8 1/2] drm/bridge: move bridges_show logic from drm_debugfs.c
  2025-02-26 21:23 ` [PATCH v8 1/2] drm/bridge: move bridges_show logic from drm_debugfs.c Luca Ceresoli
@ 2025-02-27  8:56   ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2025-02-27  8:56 UTC (permalink / raw)
  To: Luca Ceresoli, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: Dmitry Baryshkov, Thomas Petazzoni, dri-devel, linux-kernel,
	Luca Ceresoli

On Wed, 26 Feb 2025, Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> In preparation to expose more info about bridges in debugfs, which will
> require more insight into drm_bridge data structures, move the bridges_show
> code to drm_bridge.c.
>
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>
> ---
>
> Changed in v8:
>  - add the file in drm_bridge.c, which avois the added #if CONFIG_DEBUG_FS
>
> This patch was added in v7.
> ---
>  drivers/gpu/drm/drm_bridge.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_debugfs.c | 38 +-------------------------------------
>  include/drm/drm_bridge.h      |  2 ++
>  3 files changed, 45 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 241a384ebce39b4a3db58c208af27960904fc662..a6bf1a565e3c3a8d24de60448972849f6d86ba72 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -21,6 +21,7 @@
>   * DEALINGS IN THE SOFTWARE.
>   */
>  
> +#include <linux/debugfs.h>
>  #include <linux/err.h>
>  #include <linux/media-bus-format.h>
>  #include <linux/module.h>
> @@ -1335,6 +1336,47 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
>  EXPORT_SYMBOL(of_drm_find_bridge);
>  #endif
>  
> +static int encoder_bridges_show(struct seq_file *m, void *data)
> +{
> +	struct drm_encoder *encoder = m->private;
> +	struct drm_printer p = drm_seq_file_printer(m);
> +	struct drm_bridge *bridge;
> +	unsigned int idx = 0;
> +
> +	drm_for_each_bridge_in_chain(encoder, bridge) {
> +		drm_printf(&p, "bridge[%u]: %ps\n", idx++, bridge->funcs);
> +		drm_printf(&p, "\ttype: [%d] %s\n",
> +			   bridge->type,
> +			   drm_get_connector_type_name(bridge->type));
> +
> +		if (bridge->of_node)
> +			drm_printf(&p, "\tOF: %pOFfc\n", bridge->of_node);
> +
> +		drm_printf(&p, "\tops: [0x%x]", bridge->ops);
> +		if (bridge->ops & DRM_BRIDGE_OP_DETECT)
> +			drm_puts(&p, " detect");
> +		if (bridge->ops & DRM_BRIDGE_OP_EDID)
> +			drm_puts(&p, " edid");
> +		if (bridge->ops & DRM_BRIDGE_OP_HPD)
> +			drm_puts(&p, " hpd");
> +		if (bridge->ops & DRM_BRIDGE_OP_MODES)
> +			drm_puts(&p, " modes");
> +		if (bridge->ops & DRM_BRIDGE_OP_HDMI)
> +			drm_puts(&p, " hdmi");
> +		drm_puts(&p, "\n");
> +	}
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(encoder_bridges);
> +
> +void drm_bridge_debugfs_encoder_params(struct dentry *root,
> +				       struct drm_encoder *encoder)
> +{
> +	/* bridges list */
> +	debugfs_create_file("bridges", 0444, root, encoder, &encoder_bridges_fops);

The function could look at encoder->debugfs_entry instead of requiring
the root parameter, but that's neither here nor there.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> +}
> +
>  MODULE_AUTHOR("Ajay Kumar <ajaykumar.rs@samsung.com>");
>  MODULE_DESCRIPTION("DRM bridge infrastructure");
>  MODULE_LICENSE("GPL and additional rights");
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 6b2178864c7ee12db9aa1f562e106b2f604439f8..3dfd8b34dceb7a5b8f11e3072a1eaef430869722 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -740,40 +740,6 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
>  	crtc->debugfs_entry = NULL;
>  }
>  
> -static int bridges_show(struct seq_file *m, void *data)
> -{
> -	struct drm_encoder *encoder = m->private;
> -	struct drm_printer p = drm_seq_file_printer(m);
> -	struct drm_bridge *bridge;
> -	unsigned int idx = 0;
> -
> -	drm_for_each_bridge_in_chain(encoder, bridge) {
> -		drm_printf(&p, "bridge[%u]: %ps\n", idx++, bridge->funcs);
> -		drm_printf(&p, "\ttype: [%d] %s\n",
> -			   bridge->type,
> -			   drm_get_connector_type_name(bridge->type));
> -
> -		if (bridge->of_node)
> -			drm_printf(&p, "\tOF: %pOFfc\n", bridge->of_node);
> -
> -		drm_printf(&p, "\tops: [0x%x]", bridge->ops);
> -		if (bridge->ops & DRM_BRIDGE_OP_DETECT)
> -			drm_puts(&p, " detect");
> -		if (bridge->ops & DRM_BRIDGE_OP_EDID)
> -			drm_puts(&p, " edid");
> -		if (bridge->ops & DRM_BRIDGE_OP_HPD)
> -			drm_puts(&p, " hpd");
> -		if (bridge->ops & DRM_BRIDGE_OP_MODES)
> -			drm_puts(&p, " modes");
> -		if (bridge->ops & DRM_BRIDGE_OP_HDMI)
> -			drm_puts(&p, " hdmi");
> -		drm_puts(&p, "\n");
> -	}
> -
> -	return 0;
> -}
> -DEFINE_SHOW_ATTRIBUTE(bridges);
> -
>  void drm_debugfs_encoder_add(struct drm_encoder *encoder)
>  {
>  	struct drm_minor *minor = encoder->dev->primary;
> @@ -789,9 +755,7 @@ void drm_debugfs_encoder_add(struct drm_encoder *encoder)
>  
>  	encoder->debugfs_entry = root;
>  
> -	/* bridges list */
> -	debugfs_create_file("bridges", 0444, root, encoder,
> -			    &bridges_fops);
> +	drm_bridge_debugfs_encoder_params(root, encoder);
>  
>  	if (encoder->funcs && encoder->funcs->debugfs_init)
>  		encoder->funcs->debugfs_init(encoder, root);
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..0890acfe04b99b1ccbbff10b507cb8c2b2705e06 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -1108,4 +1108,6 @@ static inline struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
>  }
>  #endif
>  
> +void drm_bridge_debugfs_encoder_params(struct dentry *root, struct drm_encoder *encoder);
> +
>  #endif

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v8 2/2] drm/debugfs: add top-level 'bridges' file showing all added bridges
  2025-02-26 21:23 ` [PATCH v8 2/2] drm/debugfs: add top-level 'bridges' file showing all added bridges Luca Ceresoli
@ 2025-02-27  8:59   ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2025-02-27  8:59 UTC (permalink / raw)
  To: Luca Ceresoli, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: Dmitry Baryshkov, Thomas Petazzoni, dri-devel, linux-kernel,
	Luca Ceresoli

On Wed, 26 Feb 2025, Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:
> The global bridges_list holding all the bridges between drm_bridge_add()
> and drm_bridge_remove() cannot be inspected via debugfs. Add a file showing
> it.
>
> To avoid code duplication, move the code printing a bridge info to a common
> function.

Going forward, please separate refactoring (extracting the function)
from the functional changes (adding the new debugfs) to independent
patches. It's just easier to review.

Anyway, I reviewed this one, so no need to roll another version.

And thanks for doing this; I believe the end result is better.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>
> ---
>
> Changed in v8:
> - add the file in drm_bridge.c, which avois the added #if CONFIG_DEBUG_FS
> - fix incorrect (but harmless) idx increment in
>   drm_bridge_debugfs_show_bridge()
>
> Changed in v7:
> - move implementation to drm_bridge.c to avoid exporting bridge_list and
>   bridge_mutex
>
> This patch was added in v6.
> ---
>  drivers/gpu/drm/drm_bridge.c | 72 ++++++++++++++++++++++++++++++--------------
>  drivers/gpu/drm/drm_drv.c    |  2 ++
>  include/drm/drm_bridge.h     |  1 +
>  3 files changed, 53 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index a6bf1a565e3c3a8d24de60448972849f6d86ba72..9c6e35d41ed54a14d5745e684a341c907ed84d6b 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1336,6 +1336,49 @@ struct drm_bridge *of_drm_find_bridge(struct device_node *np)
>  EXPORT_SYMBOL(of_drm_find_bridge);
>  #endif
>  
> +static void drm_bridge_debugfs_show_bridge(struct drm_printer *p,
> +					   struct drm_bridge *bridge,
> +					   unsigned int idx)
> +{
> +	drm_printf(p, "bridge[%u]: %ps\n", idx, bridge->funcs);
> +	drm_printf(p, "\ttype: [%d] %s\n",
> +		   bridge->type,
> +		   drm_get_connector_type_name(bridge->type));
> +
> +	if (bridge->of_node)
> +		drm_printf(p, "\tOF: %pOFfc\n", bridge->of_node);
> +
> +	drm_printf(p, "\tops: [0x%x]", bridge->ops);
> +	if (bridge->ops & DRM_BRIDGE_OP_DETECT)
> +		drm_puts(p, " detect");
> +	if (bridge->ops & DRM_BRIDGE_OP_EDID)
> +		drm_puts(p, " edid");
> +	if (bridge->ops & DRM_BRIDGE_OP_HPD)
> +		drm_puts(p, " hpd");
> +	if (bridge->ops & DRM_BRIDGE_OP_MODES)
> +		drm_puts(p, " modes");
> +	if (bridge->ops & DRM_BRIDGE_OP_HDMI)
> +		drm_puts(p, " hdmi");
> +	drm_puts(p, "\n");
> +}
> +
> +static int allbridges_show(struct seq_file *m, void *data)
> +{
> +	struct drm_printer p = drm_seq_file_printer(m);
> +	struct drm_bridge *bridge;
> +	unsigned int idx = 0;
> +
> +	mutex_lock(&bridge_lock);
> +
> +	list_for_each_entry(bridge, &bridge_list, list)
> +		drm_bridge_debugfs_show_bridge(&p, bridge, idx++);
> +
> +	mutex_unlock(&bridge_lock);
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(allbridges);
> +
>  static int encoder_bridges_show(struct seq_file *m, void *data)
>  {
>  	struct drm_encoder *encoder = m->private;
> @@ -1343,33 +1386,18 @@ static int encoder_bridges_show(struct seq_file *m, void *data)
>  	struct drm_bridge *bridge;
>  	unsigned int idx = 0;
>  
> -	drm_for_each_bridge_in_chain(encoder, bridge) {
> -		drm_printf(&p, "bridge[%u]: %ps\n", idx++, bridge->funcs);
> -		drm_printf(&p, "\ttype: [%d] %s\n",
> -			   bridge->type,
> -			   drm_get_connector_type_name(bridge->type));
> -
> -		if (bridge->of_node)
> -			drm_printf(&p, "\tOF: %pOFfc\n", bridge->of_node);
> -
> -		drm_printf(&p, "\tops: [0x%x]", bridge->ops);
> -		if (bridge->ops & DRM_BRIDGE_OP_DETECT)
> -			drm_puts(&p, " detect");
> -		if (bridge->ops & DRM_BRIDGE_OP_EDID)
> -			drm_puts(&p, " edid");
> -		if (bridge->ops & DRM_BRIDGE_OP_HPD)
> -			drm_puts(&p, " hpd");
> -		if (bridge->ops & DRM_BRIDGE_OP_MODES)
> -			drm_puts(&p, " modes");
> -		if (bridge->ops & DRM_BRIDGE_OP_HDMI)
> -			drm_puts(&p, " hdmi");
> -		drm_puts(&p, "\n");
> -	}
> +	drm_for_each_bridge_in_chain(encoder, bridge)
> +		drm_bridge_debugfs_show_bridge(&p, bridge, idx++);
>  
>  	return 0;
>  }
>  DEFINE_SHOW_ATTRIBUTE(encoder_bridges);
>  
> +void drm_bridge_debugfs_params(struct dentry *root)
> +{
> +	debugfs_create_file("bridges", 0444, root, NULL, &allbridges_fops);
> +}
> +
>  void drm_bridge_debugfs_encoder_params(struct dentry *root,
>  				       struct drm_encoder *encoder)
>  {
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 3cf440eee8a2ab3de134d925db8f1d2ce68062b7..22e8cd0a6a37a0ac25535e9d570da25571b0b2bc 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -38,6 +38,7 @@
>  #include <linux/xarray.h>
>  
>  #include <drm/drm_accel.h>
> +#include <drm/drm_bridge.h>
>  #include <drm/drm_cache.h>
>  #include <drm/drm_client_event.h>
>  #include <drm/drm_color_mgmt.h>
> @@ -1120,6 +1121,7 @@ static int __init drm_core_init(void)
>  	}
>  
>  	drm_debugfs_root = debugfs_create_dir("dri", NULL);
> +	drm_bridge_debugfs_params(drm_debugfs_root);
>  
>  	ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
>  	if (ret < 0)
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 0890acfe04b99b1ccbbff10b507cb8c2b2705e06..2a99d70865571f24db0ca75c758cfd09d3a5d459 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -1108,6 +1108,7 @@ static inline struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
>  }
>  #endif
>  
> +void drm_bridge_debugfs_params(struct dentry *root);
>  void drm_bridge_debugfs_encoder_params(struct dentry *root, struct drm_encoder *encoder);
>  
>  #endif

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v8 0/2] drm: show "all" bridges in debugfs
  2025-02-26 21:23 [PATCH v8 0/2] drm: show "all" bridges in debugfs Luca Ceresoli
  2025-02-26 21:23 ` [PATCH v8 1/2] drm/bridge: move bridges_show logic from drm_debugfs.c Luca Ceresoli
  2025-02-26 21:23 ` [PATCH v8 2/2] drm/debugfs: add top-level 'bridges' file showing all added bridges Luca Ceresoli
@ 2025-03-12  9:59 ` Louis Chauvet
  2 siblings, 0 replies; 6+ messages in thread
From: Louis Chauvet @ 2025-03-12  9:59 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Luca Ceresoli
  Cc: Jani Nikula, Dmitry Baryshkov, Thomas Petazzoni, dri-devel,
	linux-kernel


On Wed, 26 Feb 2025 22:23:51 +0100, Luca Ceresoli wrote:
> This series adds a /sys/kernel/debug/dri/bridges file showing all bridges
> between drm_bridge_add() and drm_bridge_remove(), which might not be bound
> to any encoder and thus not visible anywhere in debugfs.
> 
> It also cleans up the DRM bridge debugfs code by moving code to
> drm_bridge.c.
> 
> [...]

Applied, thanks!

[1/2] drm/bridge: move bridges_show logic from drm_debugfs.c
      commit: 9497c5a0f7c26ff81f11df738a94c6b80f890c0a
[2/2] drm/debugfs: add top-level 'bridges' file showing all added bridges
      commit: eff0347e7c228335e9ff64aaf02c66957803af6a

Best regards,
-- 
Louis Chauvet <louis.chauvet@bootlin.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-03-12  9:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-26 21:23 [PATCH v8 0/2] drm: show "all" bridges in debugfs Luca Ceresoli
2025-02-26 21:23 ` [PATCH v8 1/2] drm/bridge: move bridges_show logic from drm_debugfs.c Luca Ceresoli
2025-02-27  8:56   ` Jani Nikula
2025-02-26 21:23 ` [PATCH v8 2/2] drm/debugfs: add top-level 'bridges' file showing all added bridges Luca Ceresoli
2025-02-27  8:59   ` Jani Nikula
2025-03-12  9:59 ` [PATCH v8 0/2] drm: show "all" bridges in debugfs Louis Chauvet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.