Re: [PATCH] drm/msm/dp: cleanup debugfs handling
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
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
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
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