2024-03-26 15:08:54

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 1/6] drm/msm/dp: Drop unused dp_debug struct

From: Bjorn Andersson <[email protected]>

The members of struct dp_debug are no longer used, so the only purpose
of this struct is as a type of the return value of dp_debug_get(), to
signal success/error.

Drop the struct in favor of signalling the result of initialization
using an int.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/gpu/drm/msm/dp/dp_debug.c | 38 ++++++++++---------------------------
drivers/gpu/drm/msm/dp/dp_debug.h | 38 +++++++++++--------------------------
drivers/gpu/drm/msm/dp/dp_display.c | 10 ++--------
3 files changed, 23 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c b/drivers/gpu/drm/msm/dp/dp_debug.c
index eca5a02f9003..a631cbe0e599 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -21,8 +21,6 @@ struct dp_debug_private {
struct dp_link *link;
struct dp_panel *panel;
struct drm_connector *connector;
-
- struct dp_debug dp_debug;
};

static int dp_debug_show(struct seq_file *seq, void *p)
@@ -199,11 +197,8 @@ static const struct file_operations test_active_fops = {
.write = dp_test_active_write
};

-static void dp_debug_init(struct dp_debug *dp_debug, struct dentry *root, bool is_edp)
+static void dp_debug_init(struct dp_debug_private *debug, struct dentry *root, bool is_edp)
{
- struct dp_debug_private *debug = container_of(dp_debug,
- struct dp_debug_private, dp_debug);
-
debugfs_create_file("dp_debug", 0444, root,
debug, &dp_debug_fops);

@@ -222,39 +217,26 @@ static void dp_debug_init(struct dp_debug *dp_debug, struct dentry *root, bool i
}
}

-struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
- struct dp_link *link,
- struct drm_connector *connector,
- struct dentry *root, bool is_edp)
+int dp_debug_get(struct device *dev, struct dp_panel *panel,
+ struct dp_link *link,
+ struct drm_connector *connector,
+ struct dentry *root, bool is_edp)
{
struct dp_debug_private *debug;
- struct dp_debug *dp_debug;
- int rc;

if (!dev || !panel || !link) {
DRM_ERROR("invalid input\n");
- rc = -EINVAL;
- goto error;
+ return -EINVAL;
}

debug = devm_kzalloc(dev, sizeof(*debug), GFP_KERNEL);
- if (!debug) {
- rc = -ENOMEM;
- goto error;
- }
+ if (!debug)
+ return -ENOMEM;

- debug->dp_debug.debug_en = false;
debug->link = link;
debug->panel = panel;

- dp_debug = &debug->dp_debug;
- dp_debug->vdisplay = 0;
- dp_debug->hdisplay = 0;
- dp_debug->vrefresh = 0;
-
- dp_debug_init(dp_debug, root, is_edp);
+ dp_debug_init(debug, root, is_edp);

- return dp_debug;
- error:
- return ERR_PTR(rc);
+ return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.h b/drivers/gpu/drm/msm/dp/dp_debug.h
index 9b3b2e702f65..c57200751c9f 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.h
+++ b/drivers/gpu/drm/msm/dp/dp_debug.h
@@ -9,22 +9,6 @@
#include "dp_panel.h"
#include "dp_link.h"

-/**
- * struct dp_debug
- * @debug_en: specifies whether debug mode enabled
- * @vdisplay: used to filter out vdisplay value
- * @hdisplay: used to filter out hdisplay value
- * @vrefresh: used to filter out vrefresh value
- * @tpg_state: specifies whether tpg feature is enabled
- */
-struct dp_debug {
- bool debug_en;
- int aspect_ratio;
- int vdisplay;
- int hdisplay;
- int vrefresh;
-};
-
#if defined(CONFIG_DEBUG_FS)

/**
@@ -41,22 +25,22 @@ struct dp_debug {
* This function sets up the debug module and provides a way
* for debugfs input to be communicated with existing modules
*/
-struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
- struct dp_link *link,
- struct drm_connector *connector,
- struct dentry *root,
- bool is_edp);
+int dp_debug_get(struct device *dev, struct dp_panel *panel,
+ struct dp_link *link,
+ struct drm_connector *connector,
+ 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 dentry *root,
- bool is_edp)
+int dp_debug_get(struct device *dev, struct dp_panel *panel,
+ struct dp_link *link,
+ struct drm_connector *connector,
+ struct dentry *root,
+ bool is_edp)
{
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}

#endif /* defined(CONFIG_DEBUG_FS) */
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index c4cb82af5c2f..a9187be95166 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -93,7 +93,6 @@ struct dp_display_private {
struct dp_link *link;
struct dp_panel *panel;
struct dp_ctrl *ctrl;
- struct dp_debug *debug;

struct dp_display_mode dp_mode;
struct msm_dp dp_display;
@@ -1431,14 +1430,9 @@ void dp_display_debugfs_init(struct msm_dp *dp_display, struct dentry *root, boo
dp = container_of(dp_display, struct dp_display_private, dp_display);
dev = &dp->dp_display.pdev->dev;

- dp->debug = dp_debug_get(dev, dp->panel,
- dp->link, dp->dp_display.connector,
- root, is_edp);
- if (IS_ERR(dp->debug)) {
- rc = PTR_ERR(dp->debug);
+ rc = dp_debug_get(dev, dp->panel, dp->link, dp->dp_display.connector, root, is_edp);
+ if (rc)
DRM_ERROR("failed to initialize debug, rc = %d\n", rc);
- dp->debug = NULL;
- }
}

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

--
2.43.0



2024-03-26 17:29:23

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 1/6] drm/msm/dp: Drop unused dp_debug struct

On Tue, 26 Mar 2024 at 17:06, Bjorn Andersson <[email protected]> wrote:
>
> From: Bjorn Andersson <[email protected]>
>
> The members of struct dp_debug are no longer used, so the only purpose
> of this struct is as a type of the return value of dp_debug_get(), to
> signal success/error.
>
> Drop the struct in favor of signalling the result of initialization
> using an int.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/gpu/drm/msm/dp/dp_debug.c | 38 ++++++++++---------------------------
> drivers/gpu/drm/msm/dp/dp_debug.h | 38 +++++++++++--------------------------
> drivers/gpu/drm/msm/dp/dp_display.c | 10 ++--------
> 3 files changed, 23 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c b/drivers/gpu/drm/msm/dp/dp_debug.c
> index eca5a02f9003..a631cbe0e599 100644
> --- a/drivers/gpu/drm/msm/dp/dp_debug.c
> +++ b/drivers/gpu/drm/msm/dp/dp_debug.c
> @@ -21,8 +21,6 @@ struct dp_debug_private {
> struct dp_link *link;
> struct dp_panel *panel;
> struct drm_connector *connector;
> -
> - struct dp_debug dp_debug;
> };
>
> static int dp_debug_show(struct seq_file *seq, void *p)
> @@ -199,11 +197,8 @@ static const struct file_operations test_active_fops = {
> .write = dp_test_active_write
> };
>
> -static void dp_debug_init(struct dp_debug *dp_debug, struct dentry *root, bool is_edp)
> +static void dp_debug_init(struct dp_debug_private *debug, struct dentry *root, bool is_edp)
> {
> - struct dp_debug_private *debug = container_of(dp_debug,
> - struct dp_debug_private, dp_debug);
> -
> debugfs_create_file("dp_debug", 0444, root,
> debug, &dp_debug_fops);
>
> @@ -222,39 +217,26 @@ static void dp_debug_init(struct dp_debug *dp_debug, struct dentry *root, bool i
> }
> }
>
> -struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
> - struct dp_link *link,
> - struct drm_connector *connector,
> - struct dentry *root, bool is_edp)
> +int dp_debug_get(struct device *dev, struct dp_panel *panel,
> + struct dp_link *link,
> + struct drm_connector *connector,
> + struct dentry *root, bool is_edp)
> {
> struct dp_debug_private *debug;
> - struct dp_debug *dp_debug;
> - int rc;
>
> if (!dev || !panel || !link) {
> DRM_ERROR("invalid input\n");
> - rc = -EINVAL;
> - goto error;
> + return -EINVAL;
> }
>
> debug = devm_kzalloc(dev, sizeof(*debug), GFP_KERNEL);
> - if (!debug) {
> - rc = -ENOMEM;
> - goto error;
> - }
> + if (!debug)
> + return -ENOMEM;
>
> - debug->dp_debug.debug_en = false;
> debug->link = link;
> debug->panel = panel;
>
> - dp_debug = &debug->dp_debug;
> - dp_debug->vdisplay = 0;
> - dp_debug->hdisplay = 0;
> - dp_debug->vrefresh = 0;
> -
> - dp_debug_init(dp_debug, root, is_edp);
> + dp_debug_init(debug, root, is_edp);
>
> - return dp_debug;
> - error:
> - return ERR_PTR(rc);
> + return 0;

Since there is nothing more to get, could you please move the
devm_kzalloc to dp_debug_init and call it directly from dp_display.c?

> }
> diff --git a/drivers/gpu/drm/msm/dp/dp_debug.h b/drivers/gpu/drm/msm/dp/dp_debug.h
> index 9b3b2e702f65..c57200751c9f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_debug.h
> +++ b/drivers/gpu/drm/msm/dp/dp_debug.h
> @@ -9,22 +9,6 @@
> #include "dp_panel.h"
> #include "dp_link.h"
>
> -/**
> - * struct dp_debug
> - * @debug_en: specifies whether debug mode enabled
> - * @vdisplay: used to filter out vdisplay value
> - * @hdisplay: used to filter out hdisplay value
> - * @vrefresh: used to filter out vrefresh value
> - * @tpg_state: specifies whether tpg feature is enabled
> - */
> -struct dp_debug {
> - bool debug_en;
> - int aspect_ratio;
> - int vdisplay;
> - int hdisplay;
> - int vrefresh;
> -};
> -
> #if defined(CONFIG_DEBUG_FS)
>
> /**
> @@ -41,22 +25,22 @@ struct dp_debug {
> * This function sets up the debug module and provides a way
> * for debugfs input to be communicated with existing modules
> */
> -struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel,
> - struct dp_link *link,
> - struct drm_connector *connector,
> - struct dentry *root,
> - bool is_edp);
> +int dp_debug_get(struct device *dev, struct dp_panel *panel,
> + struct dp_link *link,
> + struct drm_connector *connector,
> + 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 dentry *root,
> - bool is_edp)
> +int dp_debug_get(struct device *dev, struct dp_panel *panel,
> + struct dp_link *link,
> + struct drm_connector *connector,
> + struct dentry *root,
> + bool is_edp)
> {
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
> }
>
> #endif /* defined(CONFIG_DEBUG_FS) */
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index c4cb82af5c2f..a9187be95166 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -93,7 +93,6 @@ struct dp_display_private {
> struct dp_link *link;
> struct dp_panel *panel;
> struct dp_ctrl *ctrl;
> - struct dp_debug *debug;
>
> struct dp_display_mode dp_mode;
> struct msm_dp dp_display;
> @@ -1431,14 +1430,9 @@ void dp_display_debugfs_init(struct msm_dp *dp_display, struct dentry *root, boo
> dp = container_of(dp_display, struct dp_display_private, dp_display);
> dev = &dp->dp_display.pdev->dev;
>
> - dp->debug = dp_debug_get(dev, dp->panel,
> - dp->link, dp->dp_display.connector,
> - root, is_edp);
> - if (IS_ERR(dp->debug)) {
> - rc = PTR_ERR(dp->debug);
> + rc = dp_debug_get(dev, dp->panel, dp->link, dp->dp_display.connector, root, is_edp);
> + if (rc)
> DRM_ERROR("failed to initialize debug, rc = %d\n", rc);
> - dp->debug = NULL;
> - }
> }
>
> int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>
> --
> 2.43.0
>


--
With best wishes
Dmitry