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

2023-12-03 Thread Dmitry Baryshkov


On Thu, 19 Oct 2023 13:44:19 +0300, 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.
> 
> 

Applied, thanks!

[1/1] drm/msm/dp: cleanup debugfs handling
  https://gitlab.freedesktop.org/lumag/msm/-/commit/ab8420418c2e

Best regards,
-- 
Dmitry Baryshkov 


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

2023-10-19 Thread Dmitry Baryshkov
On Thu, 19 Oct 2023 at 15:33, Abhinav Kumar  wrote:
>
>
>
> 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 
> > ---
>
>
> 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.

For MST the add_connector callback creates a new connector with its
own implementation of  drm_connector_funcs. So if necessary we can
create debugfs files for this new connector.

> But, we will figure that out once we add that support,
>
> Hence,
>
> Reviewed-by: Abhinav Kumar 

-- 
With best wishes
Dmitry


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

2023-10-19 Thread Abhinav Kumar




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 
---



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 

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

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

2023-10-19 Thread Dmitry Baryshkov
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 
---
 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_d