[PATCH 0/4] drm/dp_mst: Add DP MST debugfs nodes for all drivers

[PATCH 0/4] drm/dp_mst: Add DP MST debugfs nodes for all drivers

[Lyude Paul]

This is the next version of my patch series for teaching DRM how to
automatically create debugfs nodes for drivers with MST topologies. This
was originally intended just for nouveau, but has since been expanded to
all DRM drivers.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Stone <daniel@fooishbar.org>

Lyude Paul (4):
  drm/debugfs: Add support for dynamic debugfs initialization
  drm/dp_mst: Pass entire connector to drm_dp_mst_topology_mgr_init()
  drm/dp_mst: Add dp_mst_status debugfs node for all drivers
  drm/i915: Remove i915_drm_dp_mst_status

 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |   2 +-
 drivers/gpu/drm/drm_debugfs.c                 | 173 +++++++++++++++++-
 drivers/gpu/drm/drm_dp_mst_topology.c         | 114 +++++++++++-
 drivers/gpu/drm/drm_drv.c                     |   3 +
 drivers/gpu/drm/drm_internal.h                |   5 +
 drivers/gpu/drm/i915/i915_debugfs.c           |  32 ----
 drivers/gpu/drm/i915/intel_dp.c               |   2 +-
 drivers/gpu/drm/i915/intel_dp_mst.c           |   6 +-
 drivers/gpu/drm/i915/intel_drv.h              |   3 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |   6 +-
 drivers/gpu/drm/radeon/radeon_dp_mst.c        |   2 +-
 include/drm/drm_debugfs.h                     |  27 +++
 include/drm/drm_dp_mst_helper.h               |  17 +-
 include/drm/drm_file.h                        |   4 +
 14 files changed, 342 insertions(+), 54 deletions(-)

-- 
2.17.1

[PATCH 1/4] drm/debugfs: Add support for dynamic debugfs initialization

[Lyude Paul]

Currently all debugfs related initialization for the DRM core happens in
drm_debugfs_init(), which is called when registering the minor device.
While this works fine for features such as atomic modesetting and GEM,
this doesn't work at all for resources like DP MST topology managers
which can potentially be created both before and after the minor
device has been registered.

So, in order to add driver-wide debugfs files for MST we'll need to be
able to handle debugfs initialization for such resources. We do so by
introducing drm_debugfs_callback_register() and
drm_debugfs_callback_unregister(). These functions allow driver-agnostic
parts of DRM to add additional debugfs initialization callbacks at any
point during a DRM driver's lifetime.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Stone <daniel@fooishbar.org>
---
 drivers/gpu/drm/drm_debugfs.c  | 173 +++++++++++++++++++++++++++++++--
 drivers/gpu/drm/drm_drv.c      |   3 +
 drivers/gpu/drm/drm_internal.h |   5 +
 include/drm/drm_debugfs.h      |  27 +++++
 include/drm/drm_file.h         |   4 +
 5 files changed, 203 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 6f28fe58f169..a53a454b167f 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -39,6 +39,13 @@
 
 #if defined(CONFIG_DEBUG_FS)
 
+struct drm_debugfs_callback {
+	void (*init)(void *data);
+	void (*cleanup_cb)(void *data);
+	void *data;
+	struct list_head list;
+};
+
 /***************************************************
  * Initialization, etc.
  **************************************************/
@@ -67,6 +74,113 @@ static const struct file_operations drm_debugfs_fops = {
 	.release = single_release,
 };
 
+/**
+ * drm_debugfs_register_callback - Register a callback for initializing
+ *                                 dynamic driver-agnostic debugfs files
+ * @minor: device minor number
+ * @init: The callback to invoke to perform the debugfs initialization
+ * @cleanup_cb: The callback to invoke to cleanup any resources for the
+ * callback
+ * @data: Data to pass to @init and @cleanup_cb
+ * @out: Where to store the pointer to the callback struct
+ *
+ * Register an initialization callback with debugfs. This callback can be used
+ * to creating debugfs nodes for DRM resources that might get created before
+ * the debugfs node for @minor has been created.
+ *
+ * When a callback is registered with this function before the debugfs root
+ * has been created, the callback's execution will be delayed until all other
+ * debugfs nodes (including those owned by the DRM device's driver) have been
+ * instantiated. If a callback is registered with this function after the
+ * debugfs root has been created, @init and @cleanup_cb will be executed
+ * immediately without creating a &struct drm_debugfs_callback.
+ *
+ * In the event that debugfs creation for the device fails; all registered
+ * debugfs callbacks will have their @cleanup_cb callbacks invoked without
+ * having their @init callbacks invoked. This is to ensure that no resources
+ * are leaked during initialization of debugfs, even if the initialization
+ * process fails. Likewise; any callbacks that are registered after DRM has
+ * failed to initialize it's debugfs files will have their @cleanup_cb
+ * callbacks invoked immediately and all of their respective resources
+ * destroyed.
+ *
+ * Implementations of @cleanup_cb should clean up all resources for the
+ * callback, with the exception of freeing the memory for @out. Freeing @out
+ * will be handled by the DRM core automatically.
+ *
+ * Users of this function should take care to add a symmetric call to
+ * @drm_debugfs_unregister_callback to handle destroying a registered callback
+ * in case the resources for the user of this function are destroyed before
+ * debugfs root is initialized.
+ *
+ */
+int
+drm_debugfs_register_callback(struct drm_minor *minor,
+			      void (*init)(void *),
+			      void (*cleanup_cb)(void *),
+			      void *data, struct drm_debugfs_callback **out)
+{
+	int ret = 0;
+
+	mutex_lock(&minor->debugfs_callback_lock);
+	if (minor->debugfs_callbacks_done) {
+		/* debugfs is already setup, so just handle the callback
+		 * immediately
+		 */
+		if (minor->debugfs_root)
+			(*init)(data);
+		(*cleanup_cb)(data);
+		goto out_unlock;
+	}
+
+	*out = kzalloc(sizeof(**out), GFP_KERNEL);
+	if (!*out) {
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
+
+	(*out)->init = init;
+	(*out)->cleanup_cb = cleanup_cb;
+	(*out)->data = data;
+	list_add(&(*out)->list, &minor->debugfs_callback_list);
+
+out_unlock:
+	mutex_unlock(&minor->debugfs_callback_lock);
+	return ret;
+}
+EXPORT_SYMBOL(drm_debugfs_register_callback);
+
+/**
+ * drm_debugfs_unregister_callback - Unregister and release the resources
+ *                                   associated with a debugfs init callback
+ * @minor: device minor number
+ * @cb: A pointer to the &struct drm_debugfs_callback struct returned by
+ * drm_debugfs_register_callback(). May be NULL
+ *
+ * Unregisters a &struct drm_debugfs_callback struct with debugfs and destroys
+ * all of it's associated resources. This includes a call to the callback's
+ * @cleanup_cb implementation.
+ *
+ * Once the debugfs root is initialized or has failed initialization, all
+ * registered callbacks are automatically destroyed. If this function is
+ * called after that point; it will automatically be a no-op.
+ */
+void
+drm_debugfs_unregister_callback(struct drm_minor *minor,
+				struct drm_debugfs_callback *cb)
+{
+	mutex_lock(&minor->debugfs_callback_lock);
+	/* We don't have to do anything if we've already completed any
+	 * registered callbacks, as they will have already been destroyed
+	 */
+	if (!minor->debugfs_callbacks_done) {
+		cb->cleanup_cb(cb->data);
+		list_del(&cb->list);
+		kfree(cb);
+	}
+	mutex_unlock(&minor->debugfs_callback_lock);
+}
+EXPORT_SYMBOL(drm_debugfs_unregister_callback);
 
 /**
  * drm_debugfs_create_files - Initialize a given set of debugfs files for DRM
@@ -126,12 +240,24 @@ int drm_debugfs_create_files(const struct drm_info_list *files, int count,
 }
 EXPORT_SYMBOL(drm_debugfs_create_files);
 
+int drm_debugfs_alloc(struct drm_minor *minor)
+{
+	INIT_LIST_HEAD(&minor->debugfs_callback_list);
+	mutex_init(&minor->debugfs_callback_lock);
+	return 0;
+}
+
 int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		     struct dentry *root)
 {
 	struct drm_device *dev = minor->dev;
+	struct drm_debugfs_callback *pos, *tmp;
 	char name[64];
-	int ret;
+	int ret = 0;
+
+	/* Don't allow any more callbacks to be registered while we setup */
+	mutex_lock(&minor->debugfs_callback_lock);
+	minor->debugfs_callbacks_done = true;
 
 	INIT_LIST_HEAD(&minor->debugfs_list);
 	mutex_init(&minor->debugfs_lock);
@@ -139,7 +265,8 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	minor->debugfs_root = debugfs_create_dir(name, root);
 	if (!minor->debugfs_root) {
 		DRM_ERROR("Cannot create /sys/kernel/debug/dri/%s\n", name);
-		return -1;
+		ret = -1;
+		goto out_unlock;
 	}
 
 	ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
@@ -148,14 +275,14 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		debugfs_remove(minor->debugfs_root);
 		minor->debugfs_root = NULL;
 		DRM_ERROR("Failed to create core drm debugfs files\n");
-		return ret;
+		goto out_unlock;
 	}
 
 	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
 		ret = drm_atomic_debugfs_init(minor);
 		if (ret) {
 			DRM_ERROR("Failed to create atomic debugfs files\n");
-			return ret;
+			goto out_unlock;
 		}
 	}
 
@@ -163,13 +290,13 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		ret = drm_framebuffer_debugfs_init(minor);
 		if (ret) {
 			DRM_ERROR("Failed to create framebuffer debugfs file\n");
-			return ret;
+			goto out_unlock;
 		}
 
 		ret = drm_client_debugfs_init(minor);
 		if (ret) {
 			DRM_ERROR("Failed to create client debugfs file\n");
-			return ret;
+			goto out_unlock;
 		}
 	}
 
@@ -178,10 +305,23 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		if (ret) {
 			DRM_ERROR("DRM: Driver failed to initialize "
 				  "/sys/kernel/debug/dri.\n");
-			return ret;
+			goto out_unlock;
 		}
 	}
-	return 0;
+
+out_unlock:
+	/* Execute any delayed callbacks if we succeeded, or just clean them
+	 * up without running them if we failed
+	 */
+	list_for_each_entry_safe(pos, tmp, &minor->debugfs_callback_list,
+				 list) {
+		if (!ret)
+			pos->init(pos->data);
+		pos->cleanup_cb(pos->data);
+		kfree(pos);
+	}
+	mutex_unlock(&minor->debugfs_callback_lock);
+	return ret;
 }
 
 
@@ -223,14 +363,29 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor)
 
 int drm_debugfs_cleanup(struct drm_minor *minor)
 {
+	struct drm_debugfs_callback *pos, *tmp;
+
+	mutex_lock(&minor->debugfs_callback_lock);
+	if (!minor->debugfs_callbacks_done) {
+		list_for_each_entry_safe(pos, tmp,
+					 &minor->debugfs_callback_list,
+					 list) {
+			pos->cleanup_cb(pos->data);
+			kfree(pos);
+		}
+	}
+	minor->debugfs_callbacks_done = true;
+
 	if (!minor->debugfs_root)
-		return 0;
+		goto out;
 
 	drm_debugfs_remove_all_files(minor);
 
 	debugfs_remove_recursive(minor->debugfs_root);
 	minor->debugfs_root = NULL;
 
+out:
+	mutex_unlock(&minor->debugfs_callback_lock);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index ea4941da9b27..7041b3137229 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -118,6 +118,9 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
 
 	minor->type = type;
 	minor->dev = dev;
+	r = drm_debugfs_alloc(minor);
+	if (r)
+		goto err_free;
 
 	idr_preload(GFP_KERNEL);
 	spin_lock_irqsave(&drm_minor_lock, flags);
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 40179c5fc6b8..d6394246967d 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -118,6 +118,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
 
 /* drm_debugfs.c drm_debugfs_crc.c */
 #if defined(CONFIG_DEBUG_FS)
+int drm_debugfs_alloc(struct drm_minor *minor);
 int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		     struct dentry *root);
 int drm_debugfs_cleanup(struct drm_minor *minor);
@@ -127,6 +128,10 @@ int drm_debugfs_crtc_add(struct drm_crtc *crtc);
 void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
 int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
 #else
+static inline int drm_debugfs_alloc(struct drm_minor *minor)
+{
+	return 0;
+}
 static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 				   struct dentry *root)
 {
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
index ac0f75df1ac9..6ac45d96fcd1 100644
--- a/include/drm/drm_debugfs.h
+++ b/include/drm/drm_debugfs.h
@@ -77,12 +77,23 @@ struct drm_info_node {
 	struct dentry *dent;
 };
 
+struct drm_debugfs_callback;
+
 #if defined(CONFIG_DEBUG_FS)
 int drm_debugfs_create_files(const struct drm_info_list *files,
 			     int count, struct dentry *root,
 			     struct drm_minor *minor);
 int drm_debugfs_remove_files(const struct drm_info_list *files,
 			     int count, struct drm_minor *minor);
+
+int drm_debugfs_register_callback(struct drm_minor *minor,
+				  void (*init)(void *),
+				  void (*cleanup_cb)(void *),
+				  void *data,
+				  struct drm_debugfs_callback **out);
+void drm_debugfs_unregister_callback(struct drm_minor *minor,
+				     struct drm_debugfs_callback *cb);
+
 #else
 static inline int drm_debugfs_create_files(const struct drm_info_list *files,
 					   int count, struct dentry *root,
@@ -96,6 +107,22 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
 {
 	return 0;
 }
+
+static inline int
+drm_debugfs_register_callback(struct drm_minor *minor,
+			      void (*init)(void *),
+			      void (*cleanup_cb)(void *),
+			      void *data,
+			      struct drm_debugfs_callback **out)
+{
+	return 0;
+}
+
+static inline void
+drm_debugfs_unregister_callback(struct drm_minor *minor,
+				struct drm_debugfs_callback *cb)
+{
+}
 #endif
 
 #endif /* _DRM_DEBUGFS_H_ */
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 26485acc51d7..180052b51712 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -74,6 +74,10 @@ struct drm_minor {
 
 	struct dentry *debugfs_root;
 
+	bool debugfs_callbacks_done;
+	struct list_head debugfs_callback_list;
+	struct mutex debugfs_callback_lock;
+
 	struct list_head debugfs_list;
 	struct mutex debugfs_lock; /* Protects debugfs_list. */
 };
-- 
2.17.1

[PATCH 2/4] drm/dp_mst: Pass entire connector to drm_dp_mst_topology_mgr_init()

[Lyude Paul]

There's no actual reason we pass the connector ID instead of a pointer
to the connector itself, and we're going to need the entire connector
(but only temporarily) in order to name MST debugfs folders properly
since connector IDs can't be looked up until the driver has been
registered with userspace which happens after debugfs init.

So, just pass the entire drm_connector struct instead of just it's id.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c   | 2 +-
 drivers/gpu/drm/drm_dp_mst_topology.c                     | 8 +++++---
 drivers/gpu/drm/i915/intel_dp.c                           | 2 +-
 drivers/gpu/drm/i915/intel_dp_mst.c                       | 6 ++++--
 drivers/gpu/drm/i915/intel_drv.h                          | 3 ++-
 drivers/gpu/drm/nouveau/dispnv50/disp.c                   | 6 +++---
 drivers/gpu/drm/radeon/radeon_dp_mst.c                    | 2 +-
 include/drm/drm_dp_mst_helper.h                           | 3 ++-
 8 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 9a300732ba37..60da7e8fcca7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -503,6 +503,6 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
 		&aconnector->dm_dp_aux.aux,
 		16,
 		4,
-		aconnector->connector_id);
+		&aconnector->base);
 }
 
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 7780567aa669..edba17073c7a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3166,9 +3166,11 @@ EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
  * Return 0 for success, or negative error code on failure
  */
 int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
-				 struct drm_device *dev, struct drm_dp_aux *aux,
+				 struct drm_device *dev,
+				 struct drm_dp_aux *aux,
 				 int max_dpcd_transaction_bytes,
-				 int max_payloads, int conn_base_id)
+				 int max_payloads,
+				 struct drm_connector *connector)
 {
 	struct drm_dp_mst_topology_state *mst_state;
 
@@ -3186,7 +3188,7 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 	mgr->aux = aux;
 	mgr->max_dpcd_transaction_bytes = max_dpcd_transaction_bytes;
 	mgr->max_payloads = max_payloads;
-	mgr->conn_base_id = conn_base_id;
+	mgr->conn_base_id = connector->base.id;
 	if (max_payloads + 1 > sizeof(mgr->payload_mask) * 8 ||
 	    max_payloads + 1 > sizeof(mgr->vcpi_mask) * 8)
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cd0f649b57a5..3688df38dbe7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6247,7 +6247,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	    (port == PORT_B || port == PORT_C ||
 	     port == PORT_D || port == PORT_F))
 		intel_dp_mst_encoder_init(intel_dig_port,
-					  intel_connector->base.base.id);
+					  &intel_connector->base);
 
 	if (!intel_edp_init_connector(intel_dp, intel_connector)) {
 		intel_dp_aux_fini(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 7e3e01607643..6c07c29235df 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -583,7 +583,8 @@ intel_dp_create_fake_mst_encoders(struct intel_digital_port *intel_dig_port)
 }
 
 int
-intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_base_id)
+intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port,
+			  struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 	struct drm_device *dev = intel_dig_port->base.base.dev;
@@ -595,7 +596,8 @@ intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_ba
 	/* create encoders */
 	intel_dp_create_fake_mst_encoders(intel_dig_port);
 	ret = drm_dp_mst_topology_mgr_init(&intel_dp->mst_mgr, dev,
-					   &intel_dp->aux, 16, 3, conn_base_id);
+					   &intel_dp->aux, 16, 3,
+					   connector);
 	if (ret) {
 		intel_dp->can_mst = false;
 		return ret;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8fc61e96754f..af7a6111ff74 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1749,7 +1749,8 @@ bool intel_digital_port_connected(struct intel_encoder *encoder);
 int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
 
 /* intel_dp_mst.c */
-int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
+int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port,
+			      struct drm_connector *connector);
 void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
 /* vlv_dsi.c */
 void vlv_dsi_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 7139b962c6fd..918f2519859b 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1226,7 +1226,7 @@ nv50_mstm_del(struct nv50_mstm **pmstm)
 
 static int
 nv50_mstm_new(struct nouveau_encoder *outp, struct drm_dp_aux *aux, int aux_max,
-	      int conn_base_id, struct nv50_mstm **pmstm)
+	      struct drm_connector *connector, struct nv50_mstm **pmstm)
 {
 	const int max_payloads = hweight8(outp->dcb->heads);
 	struct drm_device *dev = outp->base.base.dev;
@@ -1250,7 +1250,7 @@ nv50_mstm_new(struct nouveau_encoder *outp, struct drm_dp_aux *aux, int aux_max,
 	mstm->mgr.cbs = &nv50_mstm;
 
 	ret = drm_dp_mst_topology_mgr_init(&mstm->mgr, dev, aux, aux_max,
-					   max_payloads, conn_base_id);
+					   max_payloads, connector);
 	if (ret)
 		return ret;
 
@@ -1496,7 +1496,7 @@ nv50_sor_create(struct drm_connector *connector, struct dcb_output *dcbe)
 		if ((data = nvbios_dp_table(bios, &ver, &hdr, &cnt, &len)) &&
 		    ver >= 0x40 && (nvbios_rd08(bios, data + 0x08) & 0x04)) {
 			ret = nv50_mstm_new(nv_encoder, &nv_connector->aux, 16,
-					    nv_connector->base.base.id,
+					    &nv_connector->base,
 					    &nv_encoder->dp.mstm);
 			if (ret)
 				return ret;
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index f920be236cc9..3c66b1d26f88 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -664,7 +664,7 @@ radeon_dp_mst_init(struct radeon_connector *radeon_connector)
 	radeon_connector->mst_mgr.cbs = &mst_cbs;
 	return drm_dp_mst_topology_mgr_init(&radeon_connector->mst_mgr, dev,
 					    &radeon_connector->ddc_bus->aux, 16, 6,
-					    radeon_connector->base.base.id);
+					    &radeon_connector->base);
 }
 
 int
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 7f78d26a0766..ef8ba093ae8a 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -573,7 +573,8 @@ struct drm_dp_mst_topology_mgr {
 int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 				 struct drm_device *dev, struct drm_dp_aux *aux,
 				 int max_dpcd_transaction_bytes,
-				 int max_payloads, int conn_base_id);
+				 int max_payloads,
+				 struct drm_connector *connector);
 
 void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr);
 
-- 
2.17.1

[PATCH 3/4] drm/dp_mst: Add dp_mst_status debugfs node for all drivers

[Lyude Paul]

Originally I was just going to be adding dp_mst_status for nouveau until
Daniel Stone pointed out that we should probably just make this so it's
magically added for every DRM driver that's using the DRM DP MST
helpers. So, let's do that!

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Stone <daniel@fooishbar.org>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 106 ++++++++++++++++++++++++++
 include/drm/drm_dp_mst_helper.h       |  14 ++++
 2 files changed, 120 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index edba17073c7a..c12c2bfc3411 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -27,6 +27,7 @@
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/i2c.h>
+#include <linux/debugfs.h>
 #include <drm/drm_dp_mst_helper.h>
 #include <drm/drmP.h>
 
@@ -3154,6 +3155,104 @@ struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_a
 }
 EXPORT_SYMBOL(drm_atomic_get_mst_topology_state);
 
+#ifdef CONFIG_DEBUG_FS
+static int drm_dp_mst_debugfs_state_show(struct seq_file *m, void *data)
+{
+	drm_dp_mst_dump_topology(m, m->private);
+	return 0;
+}
+
+static int drm_dp_mst_debugfs_state_open(struct inode *inode,
+					 struct file *file)
+{
+	struct drm_dp_mst_topology_mgr *mgr = inode->i_private;
+
+	return single_open(file, drm_dp_mst_debugfs_state_show, mgr);
+}
+
+static const struct file_operations drm_dp_mst_debugfs_state_fops = {
+	.owner = THIS_MODULE,
+	.open = drm_dp_mst_debugfs_state_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+struct drm_dp_mst_debugfs_init_data {
+	struct drm_dp_mst_topology_mgr *mgr;
+	char *connector_name;
+};
+
+static void
+drm_dp_mst_debugfs_init(void *data)
+{
+	struct drm_dp_mst_debugfs_init_data *init_data = data;
+	struct drm_dp_mst_topology_mgr *mgr = init_data->mgr;
+	struct drm_minor *minor = mgr->dev->primary;
+	struct dentry *root;
+	bool put_ref = false;
+
+	/* Create the dp_mst directory for this device if it doesn't exist
+	 * already
+	 */
+	root = debugfs_lookup("dp_mst", minor->debugfs_root);
+	if (root) {
+		put_ref = true;
+	} else {
+		root = debugfs_create_dir("dp_mst", minor->debugfs_root);
+		if (!root || IS_ERR(root))
+			return;
+	}
+
+	mgr->debugfs = debugfs_create_dir(init_data->connector_name, root);
+	if (!mgr->debugfs)
+		goto out_dput;
+
+	debugfs_create_file("state", 0444, mgr->debugfs, mgr,
+			    &drm_dp_mst_debugfs_state_fops);
+
+out_dput:
+	if (put_ref)
+		dput(root);
+}
+
+static void
+drm_dp_mst_debugfs_cleanup_cb(void *data)
+{
+	struct drm_dp_mst_debugfs_init_data *init_data = data;
+
+	init_data->mgr->debugfs_init_cb = NULL;
+	kfree(init_data->connector_name);
+	kfree(init_data);
+}
+
+static void
+drm_dp_mst_debugfs_register(struct drm_dp_mst_topology_mgr *mgr,
+			    struct drm_connector *connector)
+{
+	struct drm_dp_mst_debugfs_init_data *init_data;
+
+	if (!connector)
+		return;
+
+	init_data = kmalloc(sizeof(*init_data), GFP_KERNEL);
+	if (!init_data)
+		return;
+
+	init_data->mgr = mgr;
+	init_data->connector_name = kstrdup(connector->name, GFP_KERNEL);
+	if (!init_data->connector_name) {
+		kfree(init_data);
+		return;
+	}
+
+	drm_debugfs_register_callback(mgr->dev->primary,
+				      drm_dp_mst_debugfs_init,
+				      drm_dp_mst_debugfs_cleanup_cb,
+				      init_data, &mgr->debugfs_init_cb);
+}
+#endif
+
 /**
  * drm_dp_mst_topology_mgr_init - initialise a topology manager
  * @mgr: manager struct to initialise
@@ -3214,6 +3313,9 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
 	drm_atomic_private_obj_init(&mgr->base,
 				    &mst_state->base,
 				    &mst_state_funcs);
+#ifdef CONFIG_DEBUG_FS
+	drm_dp_mst_debugfs_register(mgr, connector);
+#endif
 
 	return 0;
 }
@@ -3225,6 +3327,10 @@ EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init);
  */
 void drm_dp_mst_topology_mgr_destroy(struct drm_dp_mst_topology_mgr *mgr)
 {
+#ifdef CONFIG_DEBUG_FS
+	drm_debugfs_unregister_callback(mgr->dev->primary,
+					mgr->debugfs_init_cb);
+#endif
 	flush_work(&mgr->work);
 	flush_work(&mgr->destroy_connector_work);
 	mutex_lock(&mgr->payload_lock);
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index ef8ba093ae8a..c70b81cd78b1 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -25,6 +25,7 @@
 #include <linux/types.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_atomic.h>
+#include <drm/drm_debugfs.h>
 
 struct drm_dp_mst_branch;
 
@@ -568,6 +569,19 @@ struct drm_dp_mst_topology_mgr {
 	 * avoid locking inversion.
 	 */
 	struct work_struct destroy_connector_work;
+#ifdef CONFIG_DEBUG_FS
+	/**
+	 * @debugfs_init_cb: Pending debugfs callback for initializing the
+	 * debugfs files for this topology.
+	 */
+	struct drm_debugfs_callback *debugfs_init_cb;
+
+	/**
+	 * @debugfs_entry: dentry for dp_mst_status located in connector's
+	 * debugfs directory.
+	 */
+	struct dentry *debugfs;
+#endif
 };
 
 int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
-- 
2.17.1

[PATCH 4/4] drm/i915: Remove i915_drm_dp_mst_status

[Lyude Paul]

Now that DRM can create these debugfs nodes automatically; this isn't
needed.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Stone <daniel@fooishbar.org>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 32 -----------------------------
 1 file changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f9ce35da4123..5014828ca022 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3534,37 +3534,6 @@ static int i915_drrs_status(struct seq_file *m, void *unused)
 	return 0;
 }
 
-static int i915_dp_mst_info(struct seq_file *m, void *unused)
-{
-	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct drm_device *dev = &dev_priv->drm;
-	struct intel_encoder *intel_encoder;
-	struct intel_digital_port *intel_dig_port;
-	struct drm_connector *connector;
-	struct drm_connector_list_iter conn_iter;
-
-	drm_connector_list_iter_begin(dev, &conn_iter);
-	drm_for_each_connector_iter(connector, &conn_iter) {
-		if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort)
-			continue;
-
-		intel_encoder = intel_attached_encoder(connector);
-		if (!intel_encoder || intel_encoder->type == INTEL_OUTPUT_DP_MST)
-			continue;
-
-		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
-		if (!intel_dig_port->dp.can_mst)
-			continue;
-
-		seq_printf(m, "MST Source Port %c\n",
-			   port_name(intel_dig_port->base.port));
-		drm_dp_mst_dump_topology(m, &intel_dig_port->dp.mst_mgr);
-	}
-	drm_connector_list_iter_end(&conn_iter);
-
-	return 0;
-}
-
 static ssize_t i915_displayport_test_active_write(struct file *file,
 						  const char __user *ubuf,
 						  size_t len, loff_t *offp)
@@ -4733,7 +4702,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_rcs_topology", i915_rcs_topology, 0},
 	{"i915_shrinker_info", i915_shrinker_info, 0},
 	{"i915_shared_dplls_info", i915_shared_dplls_info, 0},
-	{"i915_dp_mst_info", i915_dp_mst_info, 0},
 	{"i915_wa_registers", i915_wa_registers, 0},
 	{"i915_ddb_info", i915_ddb_info, 0},
 	{"i915_sseu_status", i915_sseu_status, 0},
-- 
2.17.1

Re: [Intel-gfx] [PATCH 2/4] drm/dp_mst: Pass entire connector to drm_dp_mst_topology_mgr_init()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 25999 bytes --]

Hi Lyude,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.19-rc1 next-20180828]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Lyude-Paul/drm-dp_mst-Add-DP-MST-debugfs-nodes-for-all-drivers/20180828-083742
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/net/mac80211.h:977: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.is_valid_ack_signal' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.status_driver_data' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info'
   include/linux/mod_devicetable.h:763: warning: Function parameter or member 'driver_data' not described in 'typec_device_id'
   kernel/sched/fair.c:3371: warning: Function parameter or member 'flags' not described in 'attach_entity_load_avg'
   arch/x86/include/asm/atomic.h:84: warning: Excess function parameter 'i' description in 'arch_atomic_sub_and_test'
   arch/x86/include/asm/atomic.h:84: warning: Excess function parameter 'v' description in 'arch_atomic_sub_and_test'
   arch/x86/include/asm/atomic.h:96: warning: Excess function parameter 'v' description in 'arch_atomic_inc'
   arch/x86/include/asm/atomic.h:109: warning: Excess function parameter 'v' description in 'arch_atomic_dec'
   arch/x86/include/asm/atomic.h:124: warning: Excess function parameter 'v' description in 'arch_atomic_dec_and_test'
   arch/x86/include/asm/atomic.h:138: warning: Excess function parameter 'v' description in 'arch_atomic_inc_and_test'
   arch/x86/include/asm/atomic.h:153: warning: Excess function parameter 'i' description in 'arch_atomic_add_negative'
   arch/x86/include/asm/atomic.h:153: warning: Excess function parameter 'v' description in 'arch_atomic_add_negative'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf'
   include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
   include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip'
   include/linux/iio/hw-consumer.h:1: warning: no structured comments found
   include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry'
   drivers/pci/pci.c:218: warning: Excess function parameter 'p' description in 'pci_dev_str_match_path'
   include/linux/regulator/driver.h:227: warning: Function parameter or member 'resume' not described in 'regulator_ops'
   drivers/regulator/core.c:4479: warning: Excess function parameter 'state' description in 'regulator_suspend'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb'
   drivers/slimbus/stream.c:1: warning: no structured comments found
   drivers/target/target_core_device.c:1: warning: no structured comments found
   drivers/usb/dwc3/gadget.c:510: warning: Excess function parameter 'dwc' description in 'dwc3_gadget_start_config'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/bus.c:1: warning: no structured comments found
   drivers/usb/typec/bus.c:268: warning: Function parameter or member 'mode' not described in 'typec_match_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1: warning: no structured comments found
   include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family'
   fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete'
   fs/file_table.c:1: warning: no structured comments found
   fs/libfs.c:477: warning: Excess function parameter 'available' description in 'simple_write_end'
   fs/posix_acl.c:646: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:646: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:646: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:183: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:254: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_invalidate_range_start_gfx'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:302: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2986: warning: Excess function parameter 'dev' description in 'amdgpu_vm_get_task_info'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2987: warning: Function parameter or member 'adev' not described in 'amdgpu_vm_get_task_info'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2987: warning: Excess function parameter 'dev' description in 'amdgpu_vm_get_task_info'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
   include/drm/drm_panel.h:98: warning: Function parameter or member 'link' not described in 'drm_panel'
   drivers/gpu/drm/drm_dp_mst_topology.c:3174: warning: Excess function parameter 'conn_base_id' description in 'drm_dp_mst_topology_mgr_init'
>> drivers/gpu/drm/drm_dp_mst_topology.c:3175: warning: Function parameter or member 'connector' not described in 'drm_dp_mst_topology_mgr_init'
   drivers/gpu/drm/drm_dp_mst_topology.c:3175: warning: Excess function parameter 'conn_base_id' description in 'drm_dp_mst_topology_mgr_init'
   drivers/gpu/drm/i915/i915_vma.h:49: warning: cannot understand function prototype: 'struct i915_vma '
   drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found
   drivers/gpu/drm/i915/intel_guc_fwif.h:553: warning: cannot understand function prototype: 'struct guc_log_buffer_state '
   drivers/gpu/drm/i915/i915_trace.h:1: warning: no structured comments found
   include/linux/skbuff.h:860: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'list' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'skb_mstamp' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:238: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.len' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.head' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'adj_list.upper' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'adj_list.lower' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'switchdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
   sound/soc/soc-core.c:2918: warning: Excess function parameter 'legacy_dai_naming' description in 'snd_soc_register_dais'
   Documentation/admin-guide/cgroup-v2.rst:1485: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/admin-guide/cgroup-v2.rst:1487: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/admin-guide/cgroup-v2.rst:1488: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/core-api/boot-time-mm.rst:78: ERROR: Error in "kernel-doc" directive:
   unknown option: "nodocs".

vim +3175 drivers/gpu/drm/drm_dp_mst_topology.c

3f3353b7 Pandiyan, Dhinakaran 2017-04-20  3156  
ad7f8a1f Dave Airlie          2014-06-05  3157  /**
ad7f8a1f Dave Airlie          2014-06-05  3158   * drm_dp_mst_topology_mgr_init - initialise a topology manager
ad7f8a1f Dave Airlie          2014-06-05  3159   * @mgr: manager struct to initialise
ad7f8a1f Dave Airlie          2014-06-05  3160   * @dev: device providing this structure - for i2c addition.
ad7f8a1f Dave Airlie          2014-06-05  3161   * @aux: DP helper aux channel to talk to this device
ad7f8a1f Dave Airlie          2014-06-05  3162   * @max_dpcd_transaction_bytes: hw specific DPCD transaction limit
ad7f8a1f Dave Airlie          2014-06-05  3163   * @max_payloads: maximum number of payloads this GPU can source
ad7f8a1f Dave Airlie          2014-06-05  3164   * @conn_base_id: the connector object ID the MST device is connected to.
ad7f8a1f Dave Airlie          2014-06-05  3165   *
ad7f8a1f Dave Airlie          2014-06-05  3166   * Return 0 for success, or negative error code on failure
ad7f8a1f Dave Airlie          2014-06-05  3167   */
ad7f8a1f Dave Airlie          2014-06-05  3168  int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr,
41ea4701 Lyude Paul           2018-08-27  3169  				 struct drm_device *dev,
41ea4701 Lyude Paul           2018-08-27  3170  				 struct drm_dp_aux *aux,
ad7f8a1f Dave Airlie          2014-06-05  3171  				 int max_dpcd_transaction_bytes,
41ea4701 Lyude Paul           2018-08-27  3172  				 int max_payloads,
41ea4701 Lyude Paul           2018-08-27  3173  				 struct drm_connector *connector)
ad7f8a1f Dave Airlie          2014-06-05 @3174  {
a4370c77 Ville Syrjälä        2017-07-12 @3175  	struct drm_dp_mst_topology_state *mst_state;
a4370c77 Ville Syrjälä        2017-07-12  3176  
ad7f8a1f Dave Airlie          2014-06-05  3177  	mutex_init(&mgr->lock);
ad7f8a1f Dave Airlie          2014-06-05  3178  	mutex_init(&mgr->qlock);
ad7f8a1f Dave Airlie          2014-06-05  3179  	mutex_init(&mgr->payload_lock);
6b8eeca6 Dave Airlie          2015-06-15  3180  	mutex_init(&mgr->destroy_connector_lock);
ad7f8a1f Dave Airlie          2014-06-05  3181  	INIT_LIST_HEAD(&mgr->tx_msg_downq);
6b8eeca6 Dave Airlie          2015-06-15  3182  	INIT_LIST_HEAD(&mgr->destroy_connector_list);
ad7f8a1f Dave Airlie          2014-06-05  3183  	INIT_WORK(&mgr->work, drm_dp_mst_link_probe_work);
ad7f8a1f Dave Airlie          2014-06-05  3184  	INIT_WORK(&mgr->tx_work, drm_dp_tx_work);
6b8eeca6 Dave Airlie          2015-06-15  3185  	INIT_WORK(&mgr->destroy_connector_work, drm_dp_destroy_connector_work);
ad7f8a1f Dave Airlie          2014-06-05  3186  	init_waitqueue_head(&mgr->tx_waitq);
ad7f8a1f Dave Airlie          2014-06-05  3187  	mgr->dev = dev;
ad7f8a1f Dave Airlie          2014-06-05  3188  	mgr->aux = aux;
ad7f8a1f Dave Airlie          2014-06-05  3189  	mgr->max_dpcd_transaction_bytes = max_dpcd_transaction_bytes;
ad7f8a1f Dave Airlie          2014-06-05  3190  	mgr->max_payloads = max_payloads;
41ea4701 Lyude Paul           2018-08-27  3191  	mgr->conn_base_id = connector->base.id;
4d6a10da Imre Deak            2016-01-29  3192  	if (max_payloads + 1 > sizeof(mgr->payload_mask) * 8 ||
4d6a10da Imre Deak            2016-01-29  3193  	    max_payloads + 1 > sizeof(mgr->vcpi_mask) * 8)
4d6a10da Imre Deak            2016-01-29  3194  		return -EINVAL;
ad7f8a1f Dave Airlie          2014-06-05  3195  	mgr->payloads = kcalloc(max_payloads, sizeof(struct drm_dp_payload), GFP_KERNEL);
ad7f8a1f Dave Airlie          2014-06-05  3196  	if (!mgr->payloads)
ad7f8a1f Dave Airlie          2014-06-05  3197  		return -ENOMEM;
ad7f8a1f Dave Airlie          2014-06-05  3198  	mgr->proposed_vcpis = kcalloc(max_payloads, sizeof(struct drm_dp_vcpi *), GFP_KERNEL);
ad7f8a1f Dave Airlie          2014-06-05  3199  	if (!mgr->proposed_vcpis)
ad7f8a1f Dave Airlie          2014-06-05  3200  		return -ENOMEM;
ad7f8a1f Dave Airlie          2014-06-05  3201  	set_bit(0, &mgr->payload_mask);
441388a8 Imre Deak            2016-01-29  3202  	if (test_calc_pbn_mode() < 0)
441388a8 Imre Deak            2016-01-29  3203  		DRM_ERROR("MST PBN self-test failed\n");
441388a8 Imre Deak            2016-01-29  3204  
a4370c77 Ville Syrjälä        2017-07-12  3205  	mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
a4370c77 Ville Syrjälä        2017-07-12  3206  	if (mst_state == NULL)
3f3353b7 Pandiyan, Dhinakaran 2017-04-20  3207  		return -ENOMEM;
a4370c77 Ville Syrjälä        2017-07-12  3208  
a4370c77 Ville Syrjälä        2017-07-12  3209  	mst_state->mgr = mgr;
3f3353b7 Pandiyan, Dhinakaran 2017-04-20  3210  
3f3353b7 Pandiyan, Dhinakaran 2017-04-20  3211  	/* max. time slots - one slot for MTP header */
a4370c77 Ville Syrjälä        2017-07-12  3212  	mst_state->avail_slots = 63;
a4370c77 Ville Syrjälä        2017-07-12  3213  
a4370c77 Ville Syrjälä        2017-07-12  3214  	drm_atomic_private_obj_init(&mgr->base,
a4370c77 Ville Syrjälä        2017-07-12  3215  				    &mst_state->base,
a4370c77 Ville Syrjälä        2017-07-12  3216  				    &mst_state_funcs);
3f3353b7 Pandiyan, Dhinakaran 2017-04-20  3217  
ad7f8a1f Dave Airlie          2014-06-05  3218  	return 0;
ad7f8a1f Dave Airlie          2014-06-05  3219  }
ad7f8a1f Dave Airlie          2014-06-05  3220  EXPORT_SYMBOL(drm_dp_mst_topology_mgr_init);
ad7f8a1f Dave Airlie          2014-06-05  3221  

:::::: The code at line 3175 was first introduced by commit
:::::: a4370c777406c2810e37fafd166ccddecdb2a60c drm/atomic: Make private objs proper objects

:::::: TO: Ville Syrjälä <ville.syrjala@linux.intel.com>
:::::: CC: Ville Syrjälä <ville.syrjala@linux.intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6587 bytes --]

Re: [PATCH 3/4] drm/dp_mst: Add dp_mst_status debugfs node for all drivers

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 20773 bytes --]

Hi Lyude,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.19-rc1 next-20180828]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Lyude-Paul/drm-dp_mst-Add-DP-MST-debugfs-nodes-for-all-drivers/20180828-083742
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/net/mac80211.h:977: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.is_valid_ack_signal' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'status.status_driver_data' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info'
   include/net/mac80211.h:977: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
   net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info'
   include/linux/mod_devicetable.h:763: warning: Function parameter or member 'driver_data' not described in 'typec_device_id'
   kernel/sched/fair.c:3371: warning: Function parameter or member 'flags' not described in 'attach_entity_load_avg'
   arch/x86/include/asm/atomic.h:84: warning: Excess function parameter 'i' description in 'arch_atomic_sub_and_test'
   arch/x86/include/asm/atomic.h:84: warning: Excess function parameter 'v' description in 'arch_atomic_sub_and_test'
   arch/x86/include/asm/atomic.h:96: warning: Excess function parameter 'v' description in 'arch_atomic_inc'
   arch/x86/include/asm/atomic.h:109: warning: Excess function parameter 'v' description in 'arch_atomic_dec'
   arch/x86/include/asm/atomic.h:124: warning: Excess function parameter 'v' description in 'arch_atomic_dec_and_test'
   arch/x86/include/asm/atomic.h:138: warning: Excess function parameter 'v' description in 'arch_atomic_inc_and_test'
   arch/x86/include/asm/atomic.h:153: warning: Excess function parameter 'i' description in 'arch_atomic_add_negative'
   arch/x86/include/asm/atomic.h:153: warning: Excess function parameter 'v' description in 'arch_atomic_add_negative'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf'
   include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf'
   include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
   include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip'
   include/linux/iio/hw-consumer.h:1: warning: no structured comments found
   include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry'
   drivers/pci/pci.c:218: warning: Excess function parameter 'p' description in 'pci_dev_str_match_path'
   include/linux/regulator/driver.h:227: warning: Function parameter or member 'resume' not described in 'regulator_ops'
   drivers/regulator/core.c:4479: warning: Excess function parameter 'state' description in 'regulator_suspend'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb'
   arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb'
   drivers/slimbus/stream.c:1: warning: no structured comments found
   drivers/target/target_core_device.c:1: warning: no structured comments found
   drivers/usb/dwc3/gadget.c:510: warning: Excess function parameter 'dwc' description in 'dwc3_gadget_start_config'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/bus.c:1: warning: no structured comments found
   drivers/usb/typec/bus.c:268: warning: Function parameter or member 'mode' not described in 'typec_match_altmode'
   drivers/usb/typec/class.c:1497: warning: Excess function parameter 'drvdata' description in 'typec_port_register_altmode'
   drivers/usb/typec/class.c:1: warning: no structured comments found
   include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family'
   fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete'
   fs/file_table.c:1: warning: no structured comments found
   fs/libfs.c:477: warning: Excess function parameter 'available' description in 'simple_write_end'
   fs/posix_acl.c:646: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:646: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode'
   fs/posix_acl.c:646: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:183: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:254: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_invalidate_range_start_gfx'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:302: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_invalidate_range_start_hsa'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2986: warning: Excess function parameter 'dev' description in 'amdgpu_vm_get_task_info'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2987: warning: Function parameter or member 'adev' not described in 'amdgpu_vm_get_task_info'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2987: warning: Excess function parameter 'dev' description in 'amdgpu_vm_get_task_info'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
   include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
   include/drm/drm_panel.h:98: warning: Function parameter or member 'link' not described in 'drm_panel'
   drivers/gpu/drm/drm_dp_mst_topology.c:3273: warning: Excess function parameter 'conn_base_id' description in 'drm_dp_mst_topology_mgr_init'
>> include/drm/drm_dp_mst_helper.h:586: warning: Function parameter or member 'debugfs' not described in 'drm_dp_mst_topology_mgr'
   drivers/gpu/drm/drm_dp_mst_topology.c:3274: warning: Function parameter or member 'connector' not described in 'drm_dp_mst_topology_mgr_init'
   drivers/gpu/drm/drm_dp_mst_topology.c:3274: warning: Excess function parameter 'conn_base_id' description in 'drm_dp_mst_topology_mgr_init'
   drivers/gpu/drm/i915/i915_vma.h:49: warning: cannot understand function prototype: 'struct i915_vma '
   drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found
   drivers/gpu/drm/i915/intel_guc_fwif.h:553: warning: cannot understand function prototype: 'struct guc_log_buffer_state '
   drivers/gpu/drm/i915/i915_trace.h:1: warning: no structured comments found
   include/linux/skbuff.h:860: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'list' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'skb_mstamp' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:860: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:238: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.len' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.head' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:509: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'adj_list.upper' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'adj_list.lower' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'switchdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
   include/linux/netdevice.h:2018: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
   sound/soc/soc-core.c:2918: warning: Excess function parameter 'legacy_dai_naming' description in 'snd_soc_register_dais'
   Documentation/admin-guide/cgroup-v2.rst:1485: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/admin-guide/cgroup-v2.rst:1487: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/admin-guide/cgroup-v2.rst:1488: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/core-api/boot-time-mm.rst:78: ERROR: Error in "kernel-doc" directive:
   unknown option: "nodocs".

vim +586 include/drm/drm_dp_mst_helper.h

ad7f8a1f Dave Airlie 2014-06-05 @586  

:::::: The code at line 586 was first introduced by commit
:::::: ad7f8a1f9ced7f049f9b66d588723f243a7034cd drm/helper: add Displayport multi-stream helper (v0.6)

:::::: TO: Dave Airlie <airlied@redhat.com>
:::::: CC: Dave Airlie <airlied@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6587 bytes --]

Re: [PATCH 1/4] drm/debugfs: Add support for dynamic debugfs initialization

[Daniel Vetter]

On Mon, Aug 27, 2018 at 08:36:24PM -0400, Lyude Paul wrote:
> Currently all debugfs related initialization for the DRM core happens in
> drm_debugfs_init(), which is called when registering the minor device.
> While this works fine for features such as atomic modesetting and GEM,
> this doesn't work at all for resources like DP MST topology managers
> which can potentially be created both before and after the minor
> device has been registered.
> 
> So, in order to add driver-wide debugfs files for MST we'll need to be
> able to handle debugfs initialization for such resources. We do so by
> introducing drm_debugfs_callback_register() and
> drm_debugfs_callback_unregister(). These functions allow driver-agnostic
> parts of DRM to add additional debugfs initialization callbacks at any
> point during a DRM driver's lifetime.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Stone <daniel@fooishbar.org>

So the debugfs lifetime rules are indeed silly and broken. I'm not sure
this is what we want to do though:

- Thanks to Noralf's cleanup you don't need a debugfs cleanup callback
  anymore really, it will auto-clean-up on device unregistration.

- For stuff tied to connectors we have the connector->register/unregister
  callbacks. Imo connector-related debugfs stuff, like for mst, should be
  put there.

- debugfs_init is a dead idea, as you point out it's fundamentally racy.

- Dropping the silly notion that we have to duplicate all debugfs entries
  per-minor would be really sweet (last time I checked there's not a
  single debugfs file that's actually different per minor).

Cheers, Daniel

> ---
>  drivers/gpu/drm/drm_debugfs.c  | 173 +++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/drm_drv.c      |   3 +
>  drivers/gpu/drm/drm_internal.h |   5 +
>  include/drm/drm_debugfs.h      |  27 +++++
>  include/drm/drm_file.h         |   4 +
>  5 files changed, 203 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 6f28fe58f169..a53a454b167f 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -39,6 +39,13 @@
>  
>  #if defined(CONFIG_DEBUG_FS)
>  
> +struct drm_debugfs_callback {
> +	void (*init)(void *data);
> +	void (*cleanup_cb)(void *data);
> +	void *data;
> +	struct list_head list;
> +};
> +
>  /***************************************************
>   * Initialization, etc.
>   **************************************************/
> @@ -67,6 +74,113 @@ static const struct file_operations drm_debugfs_fops = {
>  	.release = single_release,
>  };
>  
> +/**
> + * drm_debugfs_register_callback - Register a callback for initializing
> + *                                 dynamic driver-agnostic debugfs files
> + * @minor: device minor number
> + * @init: The callback to invoke to perform the debugfs initialization
> + * @cleanup_cb: The callback to invoke to cleanup any resources for the
> + * callback
> + * @data: Data to pass to @init and @cleanup_cb
> + * @out: Where to store the pointer to the callback struct
> + *
> + * Register an initialization callback with debugfs. This callback can be used
> + * to creating debugfs nodes for DRM resources that might get created before
> + * the debugfs node for @minor has been created.
> + *
> + * When a callback is registered with this function before the debugfs root
> + * has been created, the callback's execution will be delayed until all other
> + * debugfs nodes (including those owned by the DRM device's driver) have been
> + * instantiated. If a callback is registered with this function after the
> + * debugfs root has been created, @init and @cleanup_cb will be executed
> + * immediately without creating a &struct drm_debugfs_callback.
> + *
> + * In the event that debugfs creation for the device fails; all registered
> + * debugfs callbacks will have their @cleanup_cb callbacks invoked without
> + * having their @init callbacks invoked. This is to ensure that no resources
> + * are leaked during initialization of debugfs, even if the initialization
> + * process fails. Likewise; any callbacks that are registered after DRM has
> + * failed to initialize it's debugfs files will have their @cleanup_cb
> + * callbacks invoked immediately and all of their respective resources
> + * destroyed.
> + *
> + * Implementations of @cleanup_cb should clean up all resources for the
> + * callback, with the exception of freeing the memory for @out. Freeing @out
> + * will be handled by the DRM core automatically.
> + *
> + * Users of this function should take care to add a symmetric call to
> + * @drm_debugfs_unregister_callback to handle destroying a registered callback
> + * in case the resources for the user of this function are destroyed before
> + * debugfs root is initialized.
> + *
> + */
> +int
> +drm_debugfs_register_callback(struct drm_minor *minor,
> +			      void (*init)(void *),
> +			      void (*cleanup_cb)(void *),
> +			      void *data, struct drm_debugfs_callback **out)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&minor->debugfs_callback_lock);
> +	if (minor->debugfs_callbacks_done) {
> +		/* debugfs is already setup, so just handle the callback
> +		 * immediately
> +		 */
> +		if (minor->debugfs_root)
> +			(*init)(data);
> +		(*cleanup_cb)(data);
> +		goto out_unlock;
> +	}
> +
> +	*out = kzalloc(sizeof(**out), GFP_KERNEL);
> +	if (!*out) {
> +		ret = -ENOMEM;
> +		goto out_unlock;
> +	}
> +
> +	(*out)->init = init;
> +	(*out)->cleanup_cb = cleanup_cb;
> +	(*out)->data = data;
> +	list_add(&(*out)->list, &minor->debugfs_callback_list);
> +
> +out_unlock:
> +	mutex_unlock(&minor->debugfs_callback_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_debugfs_register_callback);
> +
> +/**
> + * drm_debugfs_unregister_callback - Unregister and release the resources
> + *                                   associated with a debugfs init callback
> + * @minor: device minor number
> + * @cb: A pointer to the &struct drm_debugfs_callback struct returned by
> + * drm_debugfs_register_callback(). May be NULL
> + *
> + * Unregisters a &struct drm_debugfs_callback struct with debugfs and destroys
> + * all of it's associated resources. This includes a call to the callback's
> + * @cleanup_cb implementation.
> + *
> + * Once the debugfs root is initialized or has failed initialization, all
> + * registered callbacks are automatically destroyed. If this function is
> + * called after that point; it will automatically be a no-op.
> + */
> +void
> +drm_debugfs_unregister_callback(struct drm_minor *minor,
> +				struct drm_debugfs_callback *cb)
> +{
> +	mutex_lock(&minor->debugfs_callback_lock);
> +	/* We don't have to do anything if we've already completed any
> +	 * registered callbacks, as they will have already been destroyed
> +	 */
> +	if (!minor->debugfs_callbacks_done) {
> +		cb->cleanup_cb(cb->data);
> +		list_del(&cb->list);
> +		kfree(cb);
> +	}
> +	mutex_unlock(&minor->debugfs_callback_lock);
> +}
> +EXPORT_SYMBOL(drm_debugfs_unregister_callback);
>  
>  /**
>   * drm_debugfs_create_files - Initialize a given set of debugfs files for DRM
> @@ -126,12 +240,24 @@ int drm_debugfs_create_files(const struct drm_info_list *files, int count,
>  }
>  EXPORT_SYMBOL(drm_debugfs_create_files);
>  
> +int drm_debugfs_alloc(struct drm_minor *minor)
> +{
> +	INIT_LIST_HEAD(&minor->debugfs_callback_list);
> +	mutex_init(&minor->debugfs_callback_lock);
> +	return 0;
> +}
> +
>  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		     struct dentry *root)
>  {
>  	struct drm_device *dev = minor->dev;
> +	struct drm_debugfs_callback *pos, *tmp;
>  	char name[64];
> -	int ret;
> +	int ret = 0;
> +
> +	/* Don't allow any more callbacks to be registered while we setup */
> +	mutex_lock(&minor->debugfs_callback_lock);
> +	minor->debugfs_callbacks_done = true;
>  
>  	INIT_LIST_HEAD(&minor->debugfs_list);
>  	mutex_init(&minor->debugfs_lock);
> @@ -139,7 +265,8 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  	minor->debugfs_root = debugfs_create_dir(name, root);
>  	if (!minor->debugfs_root) {
>  		DRM_ERROR("Cannot create /sys/kernel/debug/dri/%s\n", name);
> -		return -1;
> +		ret = -1;
> +		goto out_unlock;
>  	}
>  
>  	ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
> @@ -148,14 +275,14 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		debugfs_remove(minor->debugfs_root);
>  		minor->debugfs_root = NULL;
>  		DRM_ERROR("Failed to create core drm debugfs files\n");
> -		return ret;
> +		goto out_unlock;
>  	}
>  
>  	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>  		ret = drm_atomic_debugfs_init(minor);
>  		if (ret) {
>  			DRM_ERROR("Failed to create atomic debugfs files\n");
> -			return ret;
> +			goto out_unlock;
>  		}
>  	}
>  
> @@ -163,13 +290,13 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		ret = drm_framebuffer_debugfs_init(minor);
>  		if (ret) {
>  			DRM_ERROR("Failed to create framebuffer debugfs file\n");
> -			return ret;
> +			goto out_unlock;
>  		}
>  
>  		ret = drm_client_debugfs_init(minor);
>  		if (ret) {
>  			DRM_ERROR("Failed to create client debugfs file\n");
> -			return ret;
> +			goto out_unlock;
>  		}
>  	}
>  
> @@ -178,10 +305,23 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		if (ret) {
>  			DRM_ERROR("DRM: Driver failed to initialize "
>  				  "/sys/kernel/debug/dri.\n");
> -			return ret;
> +			goto out_unlock;
>  		}
>  	}
> -	return 0;
> +
> +out_unlock:
> +	/* Execute any delayed callbacks if we succeeded, or just clean them
> +	 * up without running them if we failed
> +	 */
> +	list_for_each_entry_safe(pos, tmp, &minor->debugfs_callback_list,
> +				 list) {
> +		if (!ret)
> +			pos->init(pos->data);
> +		pos->cleanup_cb(pos->data);
> +		kfree(pos);
> +	}
> +	mutex_unlock(&minor->debugfs_callback_lock);
> +	return ret;
>  }
>  
>  
> @@ -223,14 +363,29 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor)
>  
>  int drm_debugfs_cleanup(struct drm_minor *minor)
>  {
> +	struct drm_debugfs_callback *pos, *tmp;
> +
> +	mutex_lock(&minor->debugfs_callback_lock);
> +	if (!minor->debugfs_callbacks_done) {
> +		list_for_each_entry_safe(pos, tmp,
> +					 &minor->debugfs_callback_list,
> +					 list) {
> +			pos->cleanup_cb(pos->data);
> +			kfree(pos);
> +		}
> +	}
> +	minor->debugfs_callbacks_done = true;
> +
>  	if (!minor->debugfs_root)
> -		return 0;
> +		goto out;
>  
>  	drm_debugfs_remove_all_files(minor);
>  
>  	debugfs_remove_recursive(minor->debugfs_root);
>  	minor->debugfs_root = NULL;
>  
> +out:
> +	mutex_unlock(&minor->debugfs_callback_lock);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index ea4941da9b27..7041b3137229 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -118,6 +118,9 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
>  
>  	minor->type = type;
>  	minor->dev = dev;
> +	r = drm_debugfs_alloc(minor);
> +	if (r)
> +		goto err_free;
>  
>  	idr_preload(GFP_KERNEL);
>  	spin_lock_irqsave(&drm_minor_lock, flags);
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 40179c5fc6b8..d6394246967d 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -118,6 +118,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
>  
>  /* drm_debugfs.c drm_debugfs_crc.c */
>  #if defined(CONFIG_DEBUG_FS)
> +int drm_debugfs_alloc(struct drm_minor *minor);
>  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		     struct dentry *root);
>  int drm_debugfs_cleanup(struct drm_minor *minor);
> @@ -127,6 +128,10 @@ int drm_debugfs_crtc_add(struct drm_crtc *crtc);
>  void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
>  int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
>  #else
> +static inline int drm_debugfs_alloc(struct drm_minor *minor)
> +{
> +	return 0;
> +}
>  static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  				   struct dentry *root)
>  {
> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> index ac0f75df1ac9..6ac45d96fcd1 100644
> --- a/include/drm/drm_debugfs.h
> +++ b/include/drm/drm_debugfs.h
> @@ -77,12 +77,23 @@ struct drm_info_node {
>  	struct dentry *dent;
>  };
>  
> +struct drm_debugfs_callback;
> +
>  #if defined(CONFIG_DEBUG_FS)
>  int drm_debugfs_create_files(const struct drm_info_list *files,
>  			     int count, struct dentry *root,
>  			     struct drm_minor *minor);
>  int drm_debugfs_remove_files(const struct drm_info_list *files,
>  			     int count, struct drm_minor *minor);
> +
> +int drm_debugfs_register_callback(struct drm_minor *minor,
> +				  void (*init)(void *),
> +				  void (*cleanup_cb)(void *),
> +				  void *data,
> +				  struct drm_debugfs_callback **out);
> +void drm_debugfs_unregister_callback(struct drm_minor *minor,
> +				     struct drm_debugfs_callback *cb);
> +
>  #else
>  static inline int drm_debugfs_create_files(const struct drm_info_list *files,
>  					   int count, struct dentry *root,
> @@ -96,6 +107,22 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
>  {
>  	return 0;
>  }
> +
> +static inline int
> +drm_debugfs_register_callback(struct drm_minor *minor,
> +			      void (*init)(void *),
> +			      void (*cleanup_cb)(void *),
> +			      void *data,
> +			      struct drm_debugfs_callback **out)
> +{
> +	return 0;
> +}
> +
> +static inline void
> +drm_debugfs_unregister_callback(struct drm_minor *minor,
> +				struct drm_debugfs_callback *cb)
> +{
> +}
>  #endif
>  
>  #endif /* _DRM_DEBUGFS_H_ */
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 26485acc51d7..180052b51712 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -74,6 +74,10 @@ struct drm_minor {
>  
>  	struct dentry *debugfs_root;
>  
> +	bool debugfs_callbacks_done;
> +	struct list_head debugfs_callback_list;
> +	struct mutex debugfs_callback_lock;
> +
>  	struct list_head debugfs_list;
>  	struct mutex debugfs_lock; /* Protects debugfs_list. */
>  };
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/4] drm/debugfs: Add support for dynamic debugfs initialization

[Lyude Paul]

On Fri, 2018-08-31 at 10:53 +0200, Daniel Vetter wrote:
> On Mon, Aug 27, 2018 at 08:36:24PM -0400, Lyude Paul wrote:
> > Currently all debugfs related initialization for the DRM core happens in
> > drm_debugfs_init(), which is called when registering the minor device.
> > While this works fine for features such as atomic modesetting and GEM,
> > this doesn't work at all for resources like DP MST topology managers
> > which can potentially be created both before and after the minor
> > device has been registered.
> > 
> > So, in order to add driver-wide debugfs files for MST we'll need to be
> > able to handle debugfs initialization for such resources. We do so by
> > introducing drm_debugfs_callback_register() and
> > drm_debugfs_callback_unregister(). These functions allow driver-agnostic
> > parts of DRM to add additional debugfs initialization callbacks at any
> > point during a DRM driver's lifetime.
> > 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Daniel Stone <daniel@fooishbar.org>
> 
> So the debugfs lifetime rules are indeed silly and broken. I'm not sure
> this is what we want to do though:
> 
> - Thanks to Noralf's cleanup you don't need a debugfs cleanup callback
>   anymore really, it will auto-clean-up on device unregistration.
This is true, but the cleanup there is more-so to handle the theoretical case
that a driver uninitializes an MST topology mgr before the rest of debugfs is
torn down.
> 
> - For stuff tied to connectors we have the connector->register/unregister
>   callbacks. Imo connector-related debugfs stuff, like for mst, should be
>   put there.

Since this would tie the lifetime of the debugfs files we make to the lifetime
of the connector, as it should be, we'd be able to pull off a much nicer
debugfs hierarchy:

/sys/kernel/debug/dri/0
   DP-1
      edid_override
      dp_mst
         status
   DP-2
      edid_override
      dp_mst
         status
   DP-3
      edid_override
      dp_mst
         status

The only thing I don't like about that approach though is that now we've given
up on the idea of these debugfs nodes being added to drivers using MST
automatically, since drivers wanting this would have to add calls into
late_register/early_unregister.

Since I think this would probably be useful for more then just connectors,
what if we added some sort of similar dynamic initialization system that could
be used per-DRM object? That way drivers would still get the debugfs files for
free, and we'd get a nice hierarchy and a sane way for DRM helpers to add
additional debugfs files to drivers for free. Assumably, such a system would
be added in addition to a device-wide dynamic init system like the one this
patch adds.
> 
> - debugfs_init is a dead idea, as you point out it's fundamentally racy.
> 
> - Dropping the silly notion that we have to duplicate all debugfs entries
>   per-minor would be really sweet (last time I checked there's not a
>   single debugfs file that's actually different per minor).

Yeah I'm down for this as well, I've never once seen anything actually use the
minor debugfs files. Plus, we're supposed to be able to do whatever we want in
debugfs anyway! So I don't see the harm in just gutting the duplicate minor
debugfs stuff entirely
> 
> Cheers, Daniel
> 
> > ---
> >  drivers/gpu/drm/drm_debugfs.c  | 173 +++++++++++++++++++++++++++++++--
> >  drivers/gpu/drm/drm_drv.c      |   3 +
> >  drivers/gpu/drm/drm_internal.h |   5 +
> >  include/drm/drm_debugfs.h      |  27 +++++
> >  include/drm/drm_file.h         |   4 +
> >  5 files changed, 203 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> > index 6f28fe58f169..a53a454b167f 100644
> > --- a/drivers/gpu/drm/drm_debugfs.c
> > +++ b/drivers/gpu/drm/drm_debugfs.c
> > @@ -39,6 +39,13 @@
> >  
> >  #if defined(CONFIG_DEBUG_FS)
> >  
> > +struct drm_debugfs_callback {
> > +	void (*init)(void *data);
> > +	void (*cleanup_cb)(void *data);
> > +	void *data;
> > +	struct list_head list;
> > +};
> > +
> >  /***************************************************
> >   * Initialization, etc.
> >   **************************************************/
> > @@ -67,6 +74,113 @@ static const struct file_operations drm_debugfs_fops =
> > {
> >  	.release = single_release,
> >  };
> >  
> > +/**
> > + * drm_debugfs_register_callback - Register a callback for initializing
> > + *                                 dynamic driver-agnostic debugfs files
> > + * @minor: device minor number
> > + * @init: The callback to invoke to perform the debugfs initialization
> > + * @cleanup_cb: The callback to invoke to cleanup any resources for the
> > + * callback
> > + * @data: Data to pass to @init and @cleanup_cb
> > + * @out: Where to store the pointer to the callback struct
> > + *
> > + * Register an initialization callback with debugfs. This callback can be
> > used
> > + * to creating debugfs nodes for DRM resources that might get created
> > before
> > + * the debugfs node for @minor has been created.
> > + *
> > + * When a callback is registered with this function before the debugfs
> > root
> > + * has been created, the callback's execution will be delayed until all
> > other
> > + * debugfs nodes (including those owned by the DRM device's driver) have
> > been
> > + * instantiated. If a callback is registered with this function after the
> > + * debugfs root has been created, @init and @cleanup_cb will be executed
> > + * immediately without creating a &struct drm_debugfs_callback.
> > + *
> > + * In the event that debugfs creation for the device fails; all
> > registered
> > + * debugfs callbacks will have their @cleanup_cb callbacks invoked
> > without
> > + * having their @init callbacks invoked. This is to ensure that no
> > resources
> > + * are leaked during initialization of debugfs, even if the
> > initialization
> > + * process fails. Likewise; any callbacks that are registered after DRM
> > has
> > + * failed to initialize it's debugfs files will have their @cleanup_cb
> > + * callbacks invoked immediately and all of their respective resources
> > + * destroyed.
> > + *
> > + * Implementations of @cleanup_cb should clean up all resources for the
> > + * callback, with the exception of freeing the memory for @out. Freeing
> > @out
> > + * will be handled by the DRM core automatically.
> > + *
> > + * Users of this function should take care to add a symmetric call to
> > + * @drm_debugfs_unregister_callback to handle destroying a registered
> > callback
> > + * in case the resources for the user of this function are destroyed
> > before
> > + * debugfs root is initialized.
> > + *
> > + */
> > +int
> > +drm_debugfs_register_callback(struct drm_minor *minor,
> > +			      void (*init)(void *),
> > +			      void (*cleanup_cb)(void *),
> > +			      void *data, struct drm_debugfs_callback **out)
> > +{
> > +	int ret = 0;
> > +
> > +	mutex_lock(&minor->debugfs_callback_lock);
> > +	if (minor->debugfs_callbacks_done) {
> > +		/* debugfs is already setup, so just handle the callback
> > +		 * immediately
> > +		 */
> > +		if (minor->debugfs_root)
> > +			(*init)(data);
> > +		(*cleanup_cb)(data);
> > +		goto out_unlock;
> > +	}
> > +
> > +	*out = kzalloc(sizeof(**out), GFP_KERNEL);
> > +	if (!*out) {
> > +		ret = -ENOMEM;
> > +		goto out_unlock;
> > +	}
> > +
> > +	(*out)->init = init;
> > +	(*out)->cleanup_cb = cleanup_cb;
> > +	(*out)->data = data;
> > +	list_add(&(*out)->list, &minor->debugfs_callback_list);
> > +
> > +out_unlock:
> > +	mutex_unlock(&minor->debugfs_callback_lock);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(drm_debugfs_register_callback);
> > +
> > +/**
> > + * drm_debugfs_unregister_callback - Unregister and release the resources
> > + *                                   associated with a debugfs init
> > callback
> > + * @minor: device minor number
> > + * @cb: A pointer to the &struct drm_debugfs_callback struct returned by
> > + * drm_debugfs_register_callback(). May be NULL
> > + *
> > + * Unregisters a &struct drm_debugfs_callback struct with debugfs and
> > destroys
> > + * all of it's associated resources. This includes a call to the
> > callback's
> > + * @cleanup_cb implementation.
> > + *
> > + * Once the debugfs root is initialized or has failed initialization, all
> > + * registered callbacks are automatically destroyed. If this function is
> > + * called after that point; it will automatically be a no-op.
> > + */
> > +void
> > +drm_debugfs_unregister_callback(struct drm_minor *minor,
> > +				struct drm_debugfs_callback *cb)
> > +{
> > +	mutex_lock(&minor->debugfs_callback_lock);
> > +	/* We don't have to do anything if we've already completed any
> > +	 * registered callbacks, as they will have already been destroyed
> > +	 */
> > +	if (!minor->debugfs_callbacks_done) {
> > +		cb->cleanup_cb(cb->data);
> > +		list_del(&cb->list);
> > +		kfree(cb);
> > +	}
> > +	mutex_unlock(&minor->debugfs_callback_lock);
> > +}
> > +EXPORT_SYMBOL(drm_debugfs_unregister_callback);
> >  
> >  /**
> >   * drm_debugfs_create_files - Initialize a given set of debugfs files for
> > DRM
> > @@ -126,12 +240,24 @@ int drm_debugfs_create_files(const struct
> > drm_info_list *files, int count,
> >  }
> >  EXPORT_SYMBOL(drm_debugfs_create_files);
> >  
> > +int drm_debugfs_alloc(struct drm_minor *minor)
> > +{
> > +	INIT_LIST_HEAD(&minor->debugfs_callback_list);
> > +	mutex_init(&minor->debugfs_callback_lock);
> > +	return 0;
> > +}
> > +
> >  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> >  		     struct dentry *root)
> >  {
> >  	struct drm_device *dev = minor->dev;
> > +	struct drm_debugfs_callback *pos, *tmp;
> >  	char name[64];
> > -	int ret;
> > +	int ret = 0;
> > +
> > +	/* Don't allow any more callbacks to be registered while we setup */
> > +	mutex_lock(&minor->debugfs_callback_lock);
> > +	minor->debugfs_callbacks_done = true;
> >  
> >  	INIT_LIST_HEAD(&minor->debugfs_list);
> >  	mutex_init(&minor->debugfs_lock);
> > @@ -139,7 +265,8 @@ int drm_debugfs_init(struct drm_minor *minor, int
> > minor_id,
> >  	minor->debugfs_root = debugfs_create_dir(name, root);
> >  	if (!minor->debugfs_root) {
> >  		DRM_ERROR("Cannot create /sys/kernel/debug/dri/%s\n", name);
> > -		return -1;
> > +		ret = -1;
> > +		goto out_unlock;
> >  	}
> >  
> >  	ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
> > @@ -148,14 +275,14 @@ int drm_debugfs_init(struct drm_minor *minor, int
> > minor_id,
> >  		debugfs_remove(minor->debugfs_root);
> >  		minor->debugfs_root = NULL;
> >  		DRM_ERROR("Failed to create core drm debugfs files\n");
> > -		return ret;
> > +		goto out_unlock;
> >  	}
> >  
> >  	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> >  		ret = drm_atomic_debugfs_init(minor);
> >  		if (ret) {
> >  			DRM_ERROR("Failed to create atomic debugfs files\n");
> > -			return ret;
> > +			goto out_unlock;
> >  		}
> >  	}
> >  
> > @@ -163,13 +290,13 @@ int drm_debugfs_init(struct drm_minor *minor, int
> > minor_id,
> >  		ret = drm_framebuffer_debugfs_init(minor);
> >  		if (ret) {
> >  			DRM_ERROR("Failed to create framebuffer debugfs
> > file\n");
> > -			return ret;
> > +			goto out_unlock;
> >  		}
> >  
> >  		ret = drm_client_debugfs_init(minor);
> >  		if (ret) {
> >  			DRM_ERROR("Failed to create client debugfs file\n");
> > -			return ret;
> > +			goto out_unlock;
> >  		}
> >  	}
> >  
> > @@ -178,10 +305,23 @@ int drm_debugfs_init(struct drm_minor *minor, int
> > minor_id,
> >  		if (ret) {
> >  			DRM_ERROR("DRM: Driver failed to initialize "
> >  				  "/sys/kernel/debug/dri.\n");
> > -			return ret;
> > +			goto out_unlock;
> >  		}
> >  	}
> > -	return 0;
> > +
> > +out_unlock:
> > +	/* Execute any delayed callbacks if we succeeded, or just clean them
> > +	 * up without running them if we failed
> > +	 */
> > +	list_for_each_entry_safe(pos, tmp, &minor->debugfs_callback_list,
> > +				 list) {
> > +		if (!ret)
> > +			pos->init(pos->data);
> > +		pos->cleanup_cb(pos->data);
> > +		kfree(pos);
> > +	}
> > +	mutex_unlock(&minor->debugfs_callback_lock);
> > +	return ret;
> >  }
> >  
> >  
> > @@ -223,14 +363,29 @@ static void drm_debugfs_remove_all_files(struct
> > drm_minor *minor)
> >  
> >  int drm_debugfs_cleanup(struct drm_minor *minor)
> >  {
> > +	struct drm_debugfs_callback *pos, *tmp;
> > +
> > +	mutex_lock(&minor->debugfs_callback_lock);
> > +	if (!minor->debugfs_callbacks_done) {
> > +		list_for_each_entry_safe(pos, tmp,
> > +					 &minor->debugfs_callback_list,
> > +					 list) {
> > +			pos->cleanup_cb(pos->data);
> > +			kfree(pos);
> > +		}
> > +	}
> > +	minor->debugfs_callbacks_done = true;
> > +
> >  	if (!minor->debugfs_root)
> > -		return 0;
> > +		goto out;
> >  
> >  	drm_debugfs_remove_all_files(minor);
> >  
> >  	debugfs_remove_recursive(minor->debugfs_root);
> >  	minor->debugfs_root = NULL;
> >  
> > +out:
> > +	mutex_unlock(&minor->debugfs_callback_lock);
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index ea4941da9b27..7041b3137229 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -118,6 +118,9 @@ static int drm_minor_alloc(struct drm_device *dev,
> > unsigned int type)
> >  
> >  	minor->type = type;
> >  	minor->dev = dev;
> > +	r = drm_debugfs_alloc(minor);
> > +	if (r)
> > +		goto err_free;
> >  
> >  	idr_preload(GFP_KERNEL);
> >  	spin_lock_irqsave(&drm_minor_lock, flags);
> > diff --git a/drivers/gpu/drm/drm_internal.h
> > b/drivers/gpu/drm/drm_internal.h
> > index 40179c5fc6b8..d6394246967d 100644
> > --- a/drivers/gpu/drm/drm_internal.h
> > +++ b/drivers/gpu/drm/drm_internal.h
> > @@ -118,6 +118,7 @@ void drm_gem_print_info(struct drm_printer *p,
> > unsigned int indent,
> >  
> >  /* drm_debugfs.c drm_debugfs_crc.c */
> >  #if defined(CONFIG_DEBUG_FS)
> > +int drm_debugfs_alloc(struct drm_minor *minor);
> >  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> >  		     struct dentry *root);
> >  int drm_debugfs_cleanup(struct drm_minor *minor);
> > @@ -127,6 +128,10 @@ int drm_debugfs_crtc_add(struct drm_crtc *crtc);
> >  void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
> >  int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
> >  #else
> > +static inline int drm_debugfs_alloc(struct drm_minor *minor)
> > +{
> > +	return 0;
> > +}
> >  static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> >  				   struct dentry *root)
> >  {
> > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> > index ac0f75df1ac9..6ac45d96fcd1 100644
> > --- a/include/drm/drm_debugfs.h
> > +++ b/include/drm/drm_debugfs.h
> > @@ -77,12 +77,23 @@ struct drm_info_node {
> >  	struct dentry *dent;
> >  };
> >  
> > +struct drm_debugfs_callback;
> > +
> >  #if defined(CONFIG_DEBUG_FS)
> >  int drm_debugfs_create_files(const struct drm_info_list *files,
> >  			     int count, struct dentry *root,
> >  			     struct drm_minor *minor);
> >  int drm_debugfs_remove_files(const struct drm_info_list *files,
> >  			     int count, struct drm_minor *minor);
> > +
> > +int drm_debugfs_register_callback(struct drm_minor *minor,
> > +				  void (*init)(void *),
> > +				  void (*cleanup_cb)(void *),
> > +				  void *data,
> > +				  struct drm_debugfs_callback **out);
> > +void drm_debugfs_unregister_callback(struct drm_minor *minor,
> > +				     struct drm_debugfs_callback *cb);
> > +
> >  #else
> >  static inline int drm_debugfs_create_files(const struct drm_info_list
> > *files,
> >  					   int count, struct dentry *root,
> > @@ -96,6 +107,22 @@ static inline int drm_debugfs_remove_files(const
> > struct drm_info_list *files,
> >  {
> >  	return 0;
> >  }
> > +
> > +static inline int
> > +drm_debugfs_register_callback(struct drm_minor *minor,
> > +			      void (*init)(void *),
> > +			      void (*cleanup_cb)(void *),
> > +			      void *data,
> > +			      struct drm_debugfs_callback **out)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline void
> > +drm_debugfs_unregister_callback(struct drm_minor *minor,
> > +				struct drm_debugfs_callback *cb)
> > +{
> > +}
> >  #endif
> >  
> >  #endif /* _DRM_DEBUGFS_H_ */
> > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> > index 26485acc51d7..180052b51712 100644
> > --- a/include/drm/drm_file.h
> > +++ b/include/drm/drm_file.h
> > @@ -74,6 +74,10 @@ struct drm_minor {
> >  
> >  	struct dentry *debugfs_root;
> >  
> > +	bool debugfs_callbacks_done;
> > +	struct list_head debugfs_callback_list;
> > +	struct mutex debugfs_callback_lock;
> > +
> >  	struct list_head debugfs_list;
> >  	struct mutex debugfs_lock; /* Protects debugfs_list. */
> >  };
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
-- 
Cheers,
	Lyude Paul

Re: [PATCH 1/4] drm/debugfs: Add support for dynamic debugfs initialization

[Daniel Vetter]

On Fri, Aug 31, 2018 at 07:31:10PM -0400, Lyude Paul wrote:
> On Fri, 2018-08-31 at 10:53 +0200, Daniel Vetter wrote:
> > On Mon, Aug 27, 2018 at 08:36:24PM -0400, Lyude Paul wrote:
> > > Currently all debugfs related initialization for the DRM core happens in
> > > drm_debugfs_init(), which is called when registering the minor device.
> > > While this works fine for features such as atomic modesetting and GEM,
> > > this doesn't work at all for resources like DP MST topology managers
> > > which can potentially be created both before and after the minor
> > > device has been registered.
> > > 
> > > So, in order to add driver-wide debugfs files for MST we'll need to be
> > > able to handle debugfs initialization for such resources. We do so by
> > > introducing drm_debugfs_callback_register() and
> > > drm_debugfs_callback_unregister(). These functions allow driver-agnostic
> > > parts of DRM to add additional debugfs initialization callbacks at any
> > > point during a DRM driver's lifetime.
> > > 
> > > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Daniel Stone <daniel@fooishbar.org>
> > 
> > So the debugfs lifetime rules are indeed silly and broken. I'm not sure
> > this is what we want to do though:
> > 
> > - Thanks to Noralf's cleanup you don't need a debugfs cleanup callback
> >   anymore really, it will auto-clean-up on device unregistration.
> This is true, but the cleanup there is more-so to handle the theoretical case
> that a driver uninitializes an MST topology mgr before the rest of debugfs is
> torn down.
> > 
> > - For stuff tied to connectors we have the connector->register/unregister
> >   callbacks. Imo connector-related debugfs stuff, like for mst, should be
> >   put there.
> 
> Since this would tie the lifetime of the debugfs files we make to the lifetime
> of the connector, as it should be, we'd be able to pull off a much nicer
> debugfs hierarchy:
> 
> /sys/kernel/debug/dri/0
>    DP-1
>       edid_override
>       dp_mst
>          status
>    DP-2
>       edid_override
>       dp_mst
>          status
>    DP-3
>       edid_override
>       dp_mst
>          status
> 
> The only thing I don't like about that approach though is that now we've given
> up on the idea of these debugfs nodes being added to drivers using MST
> automatically, since drivers wanting this would have to add calls into
> late_register/early_unregister.

Yup, that's part of the fallout of insisting on the core/helper split. Mst
is helper, ->late_register is core.

What we could do is combine the ->late_register callback with some of your
ideas here, so that at least the ->early_unregister callback isn't
necessary. So still explicit call to set up the mst connector files, but
the cleanup would happen automatically (using a per-connector list of
files and the fs structure you laid out above).

> Since I think this would probably be useful for more then just connectors,
> what if we added some sort of similar dynamic initialization system that could
> be used per-DRM object? That way drivers would still get the debugfs files for
> free, and we'd get a nice hierarchy and a sane way for DRM helpers to add
> additional debugfs files to drivers for free. Assumably, such a system would
> be added in addition to a device-wide dynamic init system like the one this
> patch adds.

I wouldn't start out with adding it everywhere, just for connectors.
Should be easy to refactor later on if there's a need. And to be used from
a ->late_register callback.

And I really like the per-connector debugfs files.

> > - debugfs_init is a dead idea, as you point out it's fundamentally racy.
> > 
> > - Dropping the silly notion that we have to duplicate all debugfs entries
> >   per-minor would be really sweet (last time I checked there's not a
> >   single debugfs file that's actually different per minor).
> 
> Yeah I'm down for this as well, I've never once seen anything actually use the
> minor debugfs files. Plus, we're supposed to be able to do whatever we want in
> debugfs anyway! So I don't see the harm in just gutting the duplicate minor
> debugfs stuff entirely

There's epic amounts of scripts and libraries (e.g. igt) hard-coding the
dri/0/ thing. But I think we can at least convert all the others into a
symlink to the main drm minor number.
-Daniel

> > 
> > Cheers, Daniel
> > 
> > > ---
> > >  drivers/gpu/drm/drm_debugfs.c  | 173 +++++++++++++++++++++++++++++++--
> > >  drivers/gpu/drm/drm_drv.c      |   3 +
> > >  drivers/gpu/drm/drm_internal.h |   5 +
> > >  include/drm/drm_debugfs.h      |  27 +++++
> > >  include/drm/drm_file.h         |   4 +
> > >  5 files changed, 203 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> > > index 6f28fe58f169..a53a454b167f 100644
> > > --- a/drivers/gpu/drm/drm_debugfs.c
> > > +++ b/drivers/gpu/drm/drm_debugfs.c
> > > @@ -39,6 +39,13 @@
> > >  
> > >  #if defined(CONFIG_DEBUG_FS)
> > >  
> > > +struct drm_debugfs_callback {
> > > +	void (*init)(void *data);
> > > +	void (*cleanup_cb)(void *data);
> > > +	void *data;
> > > +	struct list_head list;
> > > +};
> > > +
> > >  /***************************************************
> > >   * Initialization, etc.
> > >   **************************************************/
> > > @@ -67,6 +74,113 @@ static const struct file_operations drm_debugfs_fops =
> > > {
> > >  	.release = single_release,
> > >  };
> > >  
> > > +/**
> > > + * drm_debugfs_register_callback - Register a callback for initializing
> > > + *                                 dynamic driver-agnostic debugfs files
> > > + * @minor: device minor number
> > > + * @init: The callback to invoke to perform the debugfs initialization
> > > + * @cleanup_cb: The callback to invoke to cleanup any resources for the
> > > + * callback
> > > + * @data: Data to pass to @init and @cleanup_cb
> > > + * @out: Where to store the pointer to the callback struct
> > > + *
> > > + * Register an initialization callback with debugfs. This callback can be
> > > used
> > > + * to creating debugfs nodes for DRM resources that might get created
> > > before
> > > + * the debugfs node for @minor has been created.
> > > + *
> > > + * When a callback is registered with this function before the debugfs
> > > root
> > > + * has been created, the callback's execution will be delayed until all
> > > other
> > > + * debugfs nodes (including those owned by the DRM device's driver) have
> > > been
> > > + * instantiated. If a callback is registered with this function after the
> > > + * debugfs root has been created, @init and @cleanup_cb will be executed
> > > + * immediately without creating a &struct drm_debugfs_callback.
> > > + *
> > > + * In the event that debugfs creation for the device fails; all
> > > registered
> > > + * debugfs callbacks will have their @cleanup_cb callbacks invoked
> > > without
> > > + * having their @init callbacks invoked. This is to ensure that no
> > > resources
> > > + * are leaked during initialization of debugfs, even if the
> > > initialization
> > > + * process fails. Likewise; any callbacks that are registered after DRM
> > > has
> > > + * failed to initialize it's debugfs files will have their @cleanup_cb
> > > + * callbacks invoked immediately and all of their respective resources
> > > + * destroyed.
> > > + *
> > > + * Implementations of @cleanup_cb should clean up all resources for the
> > > + * callback, with the exception of freeing the memory for @out. Freeing
> > > @out
> > > + * will be handled by the DRM core automatically.
> > > + *
> > > + * Users of this function should take care to add a symmetric call to
> > > + * @drm_debugfs_unregister_callback to handle destroying a registered
> > > callback
> > > + * in case the resources for the user of this function are destroyed
> > > before
> > > + * debugfs root is initialized.
> > > + *
> > > + */
> > > +int
> > > +drm_debugfs_register_callback(struct drm_minor *minor,
> > > +			      void (*init)(void *),
> > > +			      void (*cleanup_cb)(void *),
> > > +			      void *data, struct drm_debugfs_callback **out)
> > > +{
> > > +	int ret = 0;
> > > +
> > > +	mutex_lock(&minor->debugfs_callback_lock);
> > > +	if (minor->debugfs_callbacks_done) {
> > > +		/* debugfs is already setup, so just handle the callback
> > > +		 * immediately
> > > +		 */
> > > +		if (minor->debugfs_root)
> > > +			(*init)(data);
> > > +		(*cleanup_cb)(data);
> > > +		goto out_unlock;
> > > +	}
> > > +
> > > +	*out = kzalloc(sizeof(**out), GFP_KERNEL);
> > > +	if (!*out) {
> > > +		ret = -ENOMEM;
> > > +		goto out_unlock;
> > > +	}
> > > +
> > > +	(*out)->init = init;
> > > +	(*out)->cleanup_cb = cleanup_cb;
> > > +	(*out)->data = data;
> > > +	list_add(&(*out)->list, &minor->debugfs_callback_list);
> > > +
> > > +out_unlock:
> > > +	mutex_unlock(&minor->debugfs_callback_lock);
> > > +	return ret;
> > > +}
> > > +EXPORT_SYMBOL(drm_debugfs_register_callback);
> > > +
> > > +/**
> > > + * drm_debugfs_unregister_callback - Unregister and release the resources
> > > + *                                   associated with a debugfs init
> > > callback
> > > + * @minor: device minor number
> > > + * @cb: A pointer to the &struct drm_debugfs_callback struct returned by
> > > + * drm_debugfs_register_callback(). May be NULL
> > > + *
> > > + * Unregisters a &struct drm_debugfs_callback struct with debugfs and
> > > destroys
> > > + * all of it's associated resources. This includes a call to the
> > > callback's
> > > + * @cleanup_cb implementation.
> > > + *
> > > + * Once the debugfs root is initialized or has failed initialization, all
> > > + * registered callbacks are automatically destroyed. If this function is
> > > + * called after that point; it will automatically be a no-op.
> > > + */
> > > +void
> > > +drm_debugfs_unregister_callback(struct drm_minor *minor,
> > > +				struct drm_debugfs_callback *cb)
> > > +{
> > > +	mutex_lock(&minor->debugfs_callback_lock);
> > > +	/* We don't have to do anything if we've already completed any
> > > +	 * registered callbacks, as they will have already been destroyed
> > > +	 */
> > > +	if (!minor->debugfs_callbacks_done) {
> > > +		cb->cleanup_cb(cb->data);
> > > +		list_del(&cb->list);
> > > +		kfree(cb);
> > > +	}
> > > +	mutex_unlock(&minor->debugfs_callback_lock);
> > > +}
> > > +EXPORT_SYMBOL(drm_debugfs_unregister_callback);
> > >  
> > >  /**
> > >   * drm_debugfs_create_files - Initialize a given set of debugfs files for
> > > DRM
> > > @@ -126,12 +240,24 @@ int drm_debugfs_create_files(const struct
> > > drm_info_list *files, int count,
> > >  }
> > >  EXPORT_SYMBOL(drm_debugfs_create_files);
> > >  
> > > +int drm_debugfs_alloc(struct drm_minor *minor)
> > > +{
> > > +	INIT_LIST_HEAD(&minor->debugfs_callback_list);
> > > +	mutex_init(&minor->debugfs_callback_lock);
> > > +	return 0;
> > > +}
> > > +
> > >  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> > >  		     struct dentry *root)
> > >  {
> > >  	struct drm_device *dev = minor->dev;
> > > +	struct drm_debugfs_callback *pos, *tmp;
> > >  	char name[64];
> > > -	int ret;
> > > +	int ret = 0;
> > > +
> > > +	/* Don't allow any more callbacks to be registered while we setup */
> > > +	mutex_lock(&minor->debugfs_callback_lock);
> > > +	minor->debugfs_callbacks_done = true;
> > >  
> > >  	INIT_LIST_HEAD(&minor->debugfs_list);
> > >  	mutex_init(&minor->debugfs_lock);
> > > @@ -139,7 +265,8 @@ int drm_debugfs_init(struct drm_minor *minor, int
> > > minor_id,
> > >  	minor->debugfs_root = debugfs_create_dir(name, root);
> > >  	if (!minor->debugfs_root) {
> > >  		DRM_ERROR("Cannot create /sys/kernel/debug/dri/%s\n", name);
> > > -		return -1;
> > > +		ret = -1;
> > > +		goto out_unlock;
> > >  	}
> > >  
> > >  	ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
> > > @@ -148,14 +275,14 @@ int drm_debugfs_init(struct drm_minor *minor, int
> > > minor_id,
> > >  		debugfs_remove(minor->debugfs_root);
> > >  		minor->debugfs_root = NULL;
> > >  		DRM_ERROR("Failed to create core drm debugfs files\n");
> > > -		return ret;
> > > +		goto out_unlock;
> > >  	}
> > >  
> > >  	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> > >  		ret = drm_atomic_debugfs_init(minor);
> > >  		if (ret) {
> > >  			DRM_ERROR("Failed to create atomic debugfs files\n");
> > > -			return ret;
> > > +			goto out_unlock;
> > >  		}
> > >  	}
> > >  
> > > @@ -163,13 +290,13 @@ int drm_debugfs_init(struct drm_minor *minor, int
> > > minor_id,
> > >  		ret = drm_framebuffer_debugfs_init(minor);
> > >  		if (ret) {
> > >  			DRM_ERROR("Failed to create framebuffer debugfs
> > > file\n");
> > > -			return ret;
> > > +			goto out_unlock;
> > >  		}
> > >  
> > >  		ret = drm_client_debugfs_init(minor);
> > >  		if (ret) {
> > >  			DRM_ERROR("Failed to create client debugfs file\n");
> > > -			return ret;
> > > +			goto out_unlock;
> > >  		}
> > >  	}
> > >  
> > > @@ -178,10 +305,23 @@ int drm_debugfs_init(struct drm_minor *minor, int
> > > minor_id,
> > >  		if (ret) {
> > >  			DRM_ERROR("DRM: Driver failed to initialize "
> > >  				  "/sys/kernel/debug/dri.\n");
> > > -			return ret;
> > > +			goto out_unlock;
> > >  		}
> > >  	}
> > > -	return 0;
> > > +
> > > +out_unlock:
> > > +	/* Execute any delayed callbacks if we succeeded, or just clean them
> > > +	 * up without running them if we failed
> > > +	 */
> > > +	list_for_each_entry_safe(pos, tmp, &minor->debugfs_callback_list,
> > > +				 list) {
> > > +		if (!ret)
> > > +			pos->init(pos->data);
> > > +		pos->cleanup_cb(pos->data);
> > > +		kfree(pos);
> > > +	}
> > > +	mutex_unlock(&minor->debugfs_callback_lock);
> > > +	return ret;
> > >  }
> > >  
> > >  
> > > @@ -223,14 +363,29 @@ static void drm_debugfs_remove_all_files(struct
> > > drm_minor *minor)
> > >  
> > >  int drm_debugfs_cleanup(struct drm_minor *minor)
> > >  {
> > > +	struct drm_debugfs_callback *pos, *tmp;
> > > +
> > > +	mutex_lock(&minor->debugfs_callback_lock);
> > > +	if (!minor->debugfs_callbacks_done) {
> > > +		list_for_each_entry_safe(pos, tmp,
> > > +					 &minor->debugfs_callback_list,
> > > +					 list) {
> > > +			pos->cleanup_cb(pos->data);
> > > +			kfree(pos);
> > > +		}
> > > +	}
> > > +	minor->debugfs_callbacks_done = true;
> > > +
> > >  	if (!minor->debugfs_root)
> > > -		return 0;
> > > +		goto out;
> > >  
> > >  	drm_debugfs_remove_all_files(minor);
> > >  
> > >  	debugfs_remove_recursive(minor->debugfs_root);
> > >  	minor->debugfs_root = NULL;
> > >  
> > > +out:
> > > +	mutex_unlock(&minor->debugfs_callback_lock);
> > >  	return 0;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > index ea4941da9b27..7041b3137229 100644
> > > --- a/drivers/gpu/drm/drm_drv.c
> > > +++ b/drivers/gpu/drm/drm_drv.c
> > > @@ -118,6 +118,9 @@ static int drm_minor_alloc(struct drm_device *dev,
> > > unsigned int type)
> > >  
> > >  	minor->type = type;
> > >  	minor->dev = dev;
> > > +	r = drm_debugfs_alloc(minor);
> > > +	if (r)
> > > +		goto err_free;
> > >  
> > >  	idr_preload(GFP_KERNEL);
> > >  	spin_lock_irqsave(&drm_minor_lock, flags);
> > > diff --git a/drivers/gpu/drm/drm_internal.h
> > > b/drivers/gpu/drm/drm_internal.h
> > > index 40179c5fc6b8..d6394246967d 100644
> > > --- a/drivers/gpu/drm/drm_internal.h
> > > +++ b/drivers/gpu/drm/drm_internal.h
> > > @@ -118,6 +118,7 @@ void drm_gem_print_info(struct drm_printer *p,
> > > unsigned int indent,
> > >  
> > >  /* drm_debugfs.c drm_debugfs_crc.c */
> > >  #if defined(CONFIG_DEBUG_FS)
> > > +int drm_debugfs_alloc(struct drm_minor *minor);
> > >  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> > >  		     struct dentry *root);
> > >  int drm_debugfs_cleanup(struct drm_minor *minor);
> > > @@ -127,6 +128,10 @@ int drm_debugfs_crtc_add(struct drm_crtc *crtc);
> > >  void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
> > >  int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
> > >  #else
> > > +static inline int drm_debugfs_alloc(struct drm_minor *minor)
> > > +{
> > > +	return 0;
> > > +}
> > >  static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> > >  				   struct dentry *root)
> > >  {
> > > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> > > index ac0f75df1ac9..6ac45d96fcd1 100644
> > > --- a/include/drm/drm_debugfs.h
> > > +++ b/include/drm/drm_debugfs.h
> > > @@ -77,12 +77,23 @@ struct drm_info_node {
> > >  	struct dentry *dent;
> > >  };
> > >  
> > > +struct drm_debugfs_callback;
> > > +
> > >  #if defined(CONFIG_DEBUG_FS)
> > >  int drm_debugfs_create_files(const struct drm_info_list *files,
> > >  			     int count, struct dentry *root,
> > >  			     struct drm_minor *minor);
> > >  int drm_debugfs_remove_files(const struct drm_info_list *files,
> > >  			     int count, struct drm_minor *minor);
> > > +
> > > +int drm_debugfs_register_callback(struct drm_minor *minor,
> > > +				  void (*init)(void *),
> > > +				  void (*cleanup_cb)(void *),
> > > +				  void *data,
> > > +				  struct drm_debugfs_callback **out);
> > > +void drm_debugfs_unregister_callback(struct drm_minor *minor,
> > > +				     struct drm_debugfs_callback *cb);
> > > +
> > >  #else
> > >  static inline int drm_debugfs_create_files(const struct drm_info_list
> > > *files,
> > >  					   int count, struct dentry *root,
> > > @@ -96,6 +107,22 @@ static inline int drm_debugfs_remove_files(const
> > > struct drm_info_list *files,
> > >  {
> > >  	return 0;
> > >  }
> > > +
> > > +static inline int
> > > +drm_debugfs_register_callback(struct drm_minor *minor,
> > > +			      void (*init)(void *),
> > > +			      void (*cleanup_cb)(void *),
> > > +			      void *data,
> > > +			      struct drm_debugfs_callback **out)
> > > +{
> > > +	return 0;
> > > +}
> > > +
> > > +static inline void
> > > +drm_debugfs_unregister_callback(struct drm_minor *minor,
> > > +				struct drm_debugfs_callback *cb)
> > > +{
> > > +}
> > >  #endif
> > >  
> > >  #endif /* _DRM_DEBUGFS_H_ */
> > > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> > > index 26485acc51d7..180052b51712 100644
> > > --- a/include/drm/drm_file.h
> > > +++ b/include/drm/drm_file.h
> > > @@ -74,6 +74,10 @@ struct drm_minor {
> > >  
> > >  	struct dentry *debugfs_root;
> > >  
> > > +	bool debugfs_callbacks_done;
> > > +	struct list_head debugfs_callback_list;
> > > +	struct mutex debugfs_callback_lock;
> > > +
> > >  	struct list_head debugfs_list;
> > >  	struct mutex debugfs_lock; /* Protects debugfs_list. */
> > >  };
> > > -- 
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > 
> -- 
> Cheers,
> 	Lyude Paul
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch