[PATCH] drm/msm/dp: cleanup debugfs handling
Abhinav Kumar
quic_abhinavk at quicinc.com
Thu Oct 19 12:32:51 UTC 2023
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.baryshkov at 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_abhinavk at 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;
More information about the dri-devel
mailing list