Re: [PATCH] drm/msm/dp: cleanup debugfs handling

Thu, 19 Oct 2023 05:33:13 -0700 



On 10/19/2023 3:44 AM, Dmitry Baryshkov wrote:

Currently there are two subdirs for DP debugfs files, e.g. DP-1, created
by the drm core for the connector, and the msm_dp-DP-1, created by the
DP driver itself. Merge those two, so that there are no extraneous
connector-related subdirs.

Signed-off-by: Dmitry Baryshkov <dmitry.barysh...@linaro.org>
---




One concern with this is, we are migrating from one debugfs per 
dp_display to one debugfs per bridge.


Today we create one bridge per dp_display so its fine.

With MST, I am unsure if there will be changes needed.

But, we will figure that out once we add that support,

Hence,

Reviewed-by: Abhinav Kumar <quic_abhin...@quicinc.com>

Overall, nice cleanup!


  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c |  8 ---
  drivers/gpu/drm/msm/dp/dp_debug.c       | 69 ++++++-------------------
  drivers/gpu/drm/msm/dp/dp_debug.h       | 23 +++------
  drivers/gpu/drm/msm/dp/dp_display.c     |  5 +-
  drivers/gpu/drm/msm/dp/dp_display.h     |  1 +
  drivers/gpu/drm/msm/dp/dp_drm.c         | 16 ++++++
  drivers/gpu/drm/msm/msm_drv.h           |  6 ---
  7 files changed, 42 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index fe7267b3bff5..f15cf7592212 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -275,8 +275,6 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct 
drm_minor *minor)
        void *p = dpu_hw_util_get_log_mask_ptr();
        struct dentry *entry;
        struct drm_device *dev;
-       struct msm_drm_private *priv;
-       int i;

  
  	if (!p)

                return -EINVAL;
@@ -286,7 +284,6 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct 
drm_minor *minor)
                return 0;

  
  	dev = dpu_kms->dev;

-       priv = dev->dev_private;

  
  	entry = debugfs_create_dir("debug", minor->debugfs_root);
  
@@ -297,11 +294,6 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)

        dpu_debugfs_core_irq_init(dpu_kms, entry);
        dpu_debugfs_sspp_init(dpu_kms, entry);

  
-	for (i = 0; i < ARRAY_SIZE(priv->dp); i++) {

-               if (priv->dp[i])
-                       msm_dp_debugfs_init(priv->dp[i], minor);
-       }
-
        return dpu_core_perf_debugfs_init(dpu_kms, entry);
  }
  #endif
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c 
b/drivers/gpu/drm/msm/dp/dp_debug.c
index 3bba901afe33..6c281dc095b9 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -19,13 +19,9 @@
  #define DEBUG_NAME "msm_dp"

  
  struct dp_debug_private {

-       struct dentry *root;
-
        struct dp_link *link;
        struct dp_panel *panel;
        struct drm_connector *connector;
-       struct device *dev;
-       struct drm_device *drm_dev;

  
  	struct dp_debug dp_debug;

  };
@@ -204,35 +200,33 @@ static const struct file_operations test_active_fops = {
        .write = dp_test_active_write
  };

  
-static void dp_debug_init(struct dp_debug *dp_debug, struct drm_minor *minor)

+static void dp_debug_init(struct dp_debug *dp_debug, struct dentry *root, bool 
is_edp)
  {
-       char path[64];
        struct dp_debug_private *debug = container_of(dp_debug,
                        struct dp_debug_private, dp_debug);

  
-	snprintf(path, sizeof(path), "msm_dp-%s", debug->connector->name);

-
-       debug->root = debugfs_create_dir(path, minor->debugfs_root);
-
-       debugfs_create_file("dp_debug", 0444, debug->root,
+       debugfs_create_file("dp_debug", 0444, root,
                        debug, &dp_debug_fops);

  
-	debugfs_create_file("msm_dp_test_active", 0444,

-                       debug->root,
-                       debug, &test_active_fops);
+       if (!is_edp) {
+               debugfs_create_file("msm_dp_test_active", 0444,
+                                   root,
+                                   debug, &test_active_fops);

  
-	debugfs_create_file("msm_dp_test_data", 0444,

-                       debug->root,
-                       debug, &dp_test_data_fops);
+               debugfs_create_file("msm_dp_test_data", 0444,
+                                   root,
+                                   debug, &dp_test_data_fops);

  
-	debugfs_create_file("msm_dp_test_type", 0444,

-                       debug->root,
-                       debug, &dp_test_type_fops);
+               debugfs_create_file("msm_dp_test_type", 0444,
+                                   root,
+                                   debug, &dp_test_type_fops);
+       }
  }

  
  struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,

                struct dp_link *link,
-               struct drm_connector *connector, struct drm_minor *minor)
+               struct drm_connector *connector,
+               struct dentry *root, bool is_edp)
  {
        struct dp_debug_private *debug;
        struct dp_debug *dp_debug;
@@ -253,46 +247,15 @@ struct dp_debug *dp_debug_get(struct device *dev, struct 
dp_panel *panel,
        debug->dp_debug.debug_en = false;
        debug->link = link;
        debug->panel = panel;
-       debug->dev = dev;
-       debug->drm_dev = minor->dev;
-       debug->connector = connector;

  
  	dp_debug = &debug->dp_debug;

        dp_debug->vdisplay = 0;
        dp_debug->hdisplay = 0;
        dp_debug->vrefresh = 0;

  
-	dp_debug_init(dp_debug, minor);

+       dp_debug_init(dp_debug, root, is_edp);

  
  	return dp_debug;

   error:
        return ERR_PTR(rc);
  }
-
-static int dp_debug_deinit(struct dp_debug *dp_debug)
-{
-       struct dp_debug_private *debug;
-
-       if (!dp_debug)
-               return -EINVAL;
-
-       debug = container_of(dp_debug, struct dp_debug_private, dp_debug);
-
-       debugfs_remove_recursive(debug->root);
-
-       return 0;
-}
-
-void dp_debug_put(struct dp_debug *dp_debug)
-{
-       struct dp_debug_private *debug;
-
-       if (!dp_debug)
-               return;
-
-       debug = container_of(dp_debug, struct dp_debug_private, dp_debug);
-
-       dp_debug_deinit(dp_debug);
-
-       devm_kfree(debug->dev, debug);
-}
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.h 
b/drivers/gpu/drm/msm/dp/dp_debug.h
index 124227873d58..9b3b2e702f65 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.h
+++ b/drivers/gpu/drm/msm/dp/dp_debug.h
@@ -34,7 +34,8 @@ struct dp_debug {
   * @panel: instance of panel module
   * @link: instance of link module
   * @connector: double pointer to display connector
- * @minor: pointer to drm minor number after device registration
+ * @root: connector's debugfs root
+ * @is_edp: set for eDP connectors / panels
   * return: pointer to allocated debug module data
   *
   * This function sets up the debug module and provides a way
@@ -43,31 +44,21 @@ struct dp_debug {
  struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
                struct dp_link *link,
                struct drm_connector *connector,
-               struct drm_minor *minor);
-
-/**
- * dp_debug_put()
- *
- * Cleans up dp_debug instance
- *
- * @dp_debug: instance of dp_debug
- */
-void dp_debug_put(struct dp_debug *dp_debug);
+               struct dentry *root,
+               bool is_edp);

  
  #else
  
  static inline

  struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
                struct dp_link *link,
-               struct drm_connector *connector, struct drm_minor *minor)
+               struct drm_connector *connector,
+               struct dentry *root,
+               bool is_edp)
  {
        return ERR_PTR(-EINVAL);
  }

  
-static inline void dp_debug_put(struct dp_debug *dp_debug)

-{
-}
-
  #endif /* defined(CONFIG_DEBUG_FS) */

  
  #endif /* _DP_DEBUG_H_ */

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index e329e03e068d..2ac9d61501c7 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -715,7 +715,6 @@ static int dp_irq_hpd_handle(struct dp_display_private *dp, 
u32 data)

  
  static void dp_display_deinit_sub_modules(struct dp_display_private *dp)

  {
-       dp_debug_put(dp->debug);
        dp_audio_put(dp->audio);
        dp_panel_put(dp->panel);
        dp_aux_put(dp->aux);
@@ -1451,7 +1450,7 @@ bool msm_dp_wide_bus_available(const struct msm_dp 
*dp_display)
        return dp->wide_bus_en;
  }

  
-void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)

+void dp_display_debugfs_init(struct msm_dp *dp_display, struct dentry *root, 
bool is_edp)
  {
        struct dp_display_private *dp;
        struct device *dev;
@@ -1462,7 +1461,7 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, 
struct drm_minor *minor)

  
  	dp->debug = dp_debug_get(dev, dp->panel,

                                        dp->link, dp->dp_display.connector,
-                                       minor);
+                                       root, is_edp);
        if (IS_ERR(dp->debug)) {
                rc = PTR_ERR(dp->debug);
                DRM_ERROR("failed to initialize debug, rc = %d\n", rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h 
b/drivers/gpu/drm/msm/dp/dp_display.h
index f66cdbc35785..5e2fbd8318e9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -42,5 +42,6 @@ int dp_display_get_test_bpp(struct msm_dp *dp_display);
  void dp_display_signal_audio_start(struct msm_dp *dp_display);
  void dp_display_signal_audio_complete(struct msm_dp *dp_display);
  void dp_display_set_psr(struct msm_dp *dp, bool enter);
+void dp_display_debugfs_init(struct msm_dp *dp_display, struct dentry *dentry, 
bool is_edp);

  
  #endif /* _DP_DISPLAY_H_ */

diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 284ff7df058a..7e34715bdd82 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -90,6 +90,13 @@ static int dp_bridge_get_modes(struct drm_bridge *bridge, 
struct drm_connector *
        return rc;
  }

  
+static void dp_bridge_debugfs_init(struct drm_bridge *bridge, struct dentry *root)

+{
+       struct msm_dp *dp = to_dp_bridge(bridge)->dp_display;
+
+       dp_display_debugfs_init(dp, root, false);
+}
+
  static const struct drm_bridge_funcs dp_bridge_ops = {
        .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
        .atomic_destroy_state   = drm_atomic_helper_bridge_destroy_state,
@@ -105,6 +112,7 @@ static const struct drm_bridge_funcs dp_bridge_ops = {
        .hpd_enable   = dp_bridge_hpd_enable,
        .hpd_disable  = dp_bridge_hpd_disable,
        .hpd_notify   = dp_bridge_hpd_notify,
+       .debugfs_init = dp_bridge_debugfs_init,
  };

  
  static int edp_bridge_atomic_check(struct drm_bridge *drm_bridge,

@@ -260,6 +268,13 @@ static enum drm_mode_status edp_bridge_mode_valid(struct 
drm_bridge *bridge,
        return MODE_OK;
  }

  
+static void edp_bridge_debugfs_init(struct drm_bridge *bridge, struct dentry *root)

+{
+       struct msm_dp *dp = to_dp_bridge(bridge)->dp_display;
+
+       dp_display_debugfs_init(dp, root, true);
+}
+
  static const struct drm_bridge_funcs edp_bridge_ops = {
        .atomic_enable = edp_bridge_atomic_enable,
        .atomic_disable = edp_bridge_atomic_disable,
@@ -270,6 +285,7 @@ static const struct drm_bridge_funcs edp_bridge_ops = {
        .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
        .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
        .atomic_check = edp_bridge_atomic_check,
+       .debugfs_init = edp_bridge_debugfs_init,
  };

  
  int dp_bridge_init(struct msm_dp *dp_display, struct drm_device *dev,

diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index cd5bf658df66..628ef3e663ea 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -389,7 +389,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct 
drm_device *dev,
  void msm_dp_irq_postinstall(struct msm_dp *dp_display);
  void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp 
*dp_display);

  
-void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor);

  bool msm_dp_wide_bus_available(const struct msm_dp *dp_display);

  
  #else

@@ -415,11 +414,6 @@ static inline void msm_dp_snapshot(struct msm_disp_state 
*disp_state, struct msm
  {
  }

  
-static inline void msm_dp_debugfs_init(struct msm_dp *dp_display,

-               struct drm_minor *minor)
-{
-}
-
  static inline bool msm_dp_wide_bus_available(const struct msm_dp *dp_display)
  {
        return false;






















					Reply via email to