Re: [DPU PATCH v2 10/12] drm/msm/dpu: use runtime_pm calls in dpu_dbg

On Fri, May 11, 2018 at 08:19:36PM +0530, Rajesh Yadav wrote:

Currently, msm_drv was creating dpu_power_handle client which was used by dpu_dbg module to enable power resources before register debug dumping.

Now since, the mdss core power resource handling is implemented via runtime_pm and same has been removed from dpu_power_handle. Remove dpu_power_handle dependency from msm_drv and use pm_runtime_get/put_sync calls from dpu_dbg module on dpu_mdss top level device for core, ahb clock and power resource management (for register access).

Changes in v2:

• resolved conflict in dpu_core_perf_init
• dropped (Reviewed-by: Sean Paul) due to above change

Reviewed-by: Sean Paul seanpaul@chromium.org

Signed-off-by: Rajesh Yadav ryadav@codeaurora.org

drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 7 ------- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 4 ---- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +-- drivers/gpu/drm/msm/dpu_dbg.c | 18 +++++++----------- drivers/gpu/drm/msm/dpu_dbg.h | 13 ++----------- drivers/gpu/drm/msm/msm_drv.c | 27 +-------------------------- drivers/gpu/drm/msm/msm_drv.h | 1 - 7 files changed, 11 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c index 5b79077..2cf3fca 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c @@ -673,18 +673,11 @@ int dpu_core_perf_init(struct dpu_core_perf *perf, struct drm_device *dev, struct dpu_mdss_cfg *catalog, struct dpu_power_handle *phandle,

• struct dpu_power_client *pclient,
  

  struct dss_clk *core_clk)

{

• if (!pclient) {

• DPU_ERROR("invalid parameters\n");
  

• return -EINVAL;
  

• }

• perf->dev = dev; perf->catalog = catalog; perf->phandle = phandle;

• perf->pclient = pclient; perf->core_clk = core_clk;

  perf->max_core_clk_rate = core_clk->max_rate;

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h index 9c1a719..cde48df 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h @@ -53,7 +53,6 @@ struct dpu_core_perf_tune {

• @debugfs_root: top level debug folder
• @catalog: Pointer to catalog configuration
• @phandle: Pointer to power handler

• • @pclient: Pointer to power client
  • @core_clk: Pointer to core clock structure
  • @core_clk_rate: current core clock rate
  • @max_core_clk_rate: maximum allowable core clock rate

@@ -68,7 +67,6 @@ struct dpu_core_perf { struct dentry *debugfs_root; struct dpu_mdss_cfg *catalog; struct dpu_power_handle *phandle;

• struct dpu_power_client *pclient; struct dss_clk *core_clk; u64 core_clk_rate; u64 max_core_clk_rate;

@@ -115,14 +113,12 @@ void dpu_core_perf_crtc_update(struct drm_crtc *crtc,

• @dev: Pointer to drm device
• @catalog: Pointer to catalog
• @phandle: Pointer to power handle

• • @pclient: Pointer to power client
  • @core_clk: pointer to core clock
  */

int dpu_core_perf_init(struct dpu_core_perf *perf, struct drm_device *dev, struct dpu_mdss_cfg *catalog, struct dpu_power_handle *phandle,

• struct dpu_power_client *pclient,
  

  struct dss_clk *core_clk);

/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 349bda5..9c3b220 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1721,8 +1721,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms) #endif

rc = dpu_core_perf_init(&dpu_kms->perf, dev, dpu_kms->catalog,

• 	&priv->phandle, priv->pclient,
  

• 	_dpu_kms_get_clk(dpu_kms, "core_clk"));
  

• 	&priv->phandle, _dpu_kms_get_clk(dpu_kms, "core_clk"));
  

  if (rc) { DPU_ERROR("failed to init perf %d\n", rc); goto perf_err;

diff --git a/drivers/gpu/drm/msm/dpu_dbg.c b/drivers/gpu/drm/msm/dpu_dbg.c index 4a39b82..27538bc 100644 --- a/drivers/gpu/drm/msm/dpu_dbg.c +++ b/drivers/gpu/drm/msm/dpu_dbg.c @@ -20,6 +20,7 @@ #include <linux/dma-buf.h> #include <linux/slab.h> #include <linux/list_sort.h> +#include <linux/pm_runtime.h>

#include "dpu_dbg.h" #include "disp/dpu1/dpu_hw_catalog.h" @@ -167,7 +168,6 @@ struct dpu_dbg_vbif_debug_bus {

• @evtlog: event log instance
• @reg_base_list: list of register dumping regions
• @dev: device pointer

• • @power_ctrl: callback structure for enabling power for reading hw registers
  • @req_dump_blks: list of blocks requested for dumping
  • @panic_on_err: whether to kernel panic after triggering dump via debugfs
  • @dump_work: work struct for deferring register dump work to separate thread

@@ -182,7 +182,6 @@ struct dpu_dbg_vbif_debug_bus { struct dpu_dbg_evtlog *evtlog; struct list_head reg_base_list; struct device *dev;

• struct dpu_dbg_power_ctrl power_ctrl;

  struct dpu_dbg_reg_base *req_dump_blks[DPU_DBG_BASE_MAX];

@@ -2008,12 +2007,10 @@ static void _dpu_debug_bus_ppb1_dump(void __iomem *mem_base, */ static inline void _dpu_dbg_enable_power(int enable) {

• if (!dpu_dbg_base.power_ctrl.enable_fn)

• return;
  

• dpu_dbg_base.power_ctrl.enable_fn(

• 	dpu_dbg_base.power_ctrl.handle,
  

• 	dpu_dbg_base.power_ctrl.client,
  

• 	enable);
  

• if (enable)

• pm_runtime_get_sync(dpu_dbg_base.dev);
  

• else

• pm_runtime_put_sync(dpu_dbg_base.dev);
  

}

/** @@ -3099,16 +3096,15 @@ void dpu_dbg_init_dbg_buses(u32 hwversion) } }

-int dpu_dbg_init(struct device *dev, struct dpu_dbg_power_ctrl *power_ctrl) +int dpu_dbg_init(struct device *dev) {

• if (!dev || !power_ctrl) {

• if (!dev) { pr_err("invalid params\n"); return -EINVAL; }

  INIT_LIST_HEAD(&dpu_dbg_base.reg_base_list); dpu_dbg_base.dev = dev;

• dpu_dbg_base.power_ctrl = *power_ctrl;

  dpu_dbg_base.evtlog = dpu_evtlog_init(); if (IS_ERR_OR_NULL(dpu_dbg_base.evtlog))

diff --git a/drivers/gpu/drm/msm/dpu_dbg.h b/drivers/gpu/drm/msm/dpu_dbg.h index e79b5aa..283dbbc 100644 --- a/drivers/gpu/drm/msm/dpu_dbg.h +++ b/drivers/gpu/drm/msm/dpu_dbg.h @@ -71,12 +71,6 @@ enum dpu_dbg_dump_flag { #define DPU_EVTLOG_BUF_MAX 512 #define DPU_EVTLOG_BUF_ALIGN 32

-struct dpu_dbg_power_ctrl {

• void *handle;
• void *client;
• int (*enable_fn)(void *handle, void *client, bool enable);

-};

struct dpu_dbg_evtlog_log { s64 time; const char *name; @@ -211,11 +205,9 @@ ssize_t dpu_evtlog_dump_to_buffer(struct dpu_dbg_evtlog *evtlog, /**

• dpu_dbg_init - initialize global dpu debug facilities: evtlog, regdump
• @dev: device handle

• • @power_ctrl: power control callback structure for enabling clocks

• • 	during register dumping
    

  • Returns: 0 or -ERROR
  */

-int dpu_dbg_init(struct device *dev, struct dpu_dbg_power_ctrl *power_ctrl); +int dpu_dbg_init(struct device *dev);

/**

• dpu_dbg_debugfs_register - register entries at the given debugfs dir

@@ -359,8 +351,7 @@ static inline void dpu_dbg_init_dbg_buses(u32 hwversion) { }

-static inline int dpu_dbg_init(struct device *dev,

• struct dpu_dbg_power_ctrl *power_ctrl)
  

+static inline int dpu_dbg_init(struct device *dev) { return 0; } diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 5470529..5c267cd 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -340,7 +340,6 @@ static int msm_drm_uninit(struct device *dev) component_unbind_all(dev, ddev);

#ifdef CONFIG_DRM_MSM_DPU

• dpu_power_client_destroy(&priv->phandle, priv->pclient); dpu_power_resource_deinit(pdev, &priv->phandle); dpu_dbg_destroy();

#endif @@ -464,13 +463,6 @@ static int msm_component_bind_all(struct device *dev, } #endif

-#ifdef CONFIG_DRM_MSM_DPU -static int msm_power_enable_wrapper(void *handle, void *client, bool enable) -{

• return dpu_power_resource_enable(handle, client, enable);

-} -#endif

static int msm_drm_init(struct device *dev, struct drm_driver *drv) { struct platform_device *pdev = to_platform_device(dev); @@ -479,10 +471,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) struct msm_kms *kms; struct msm_mdss *mdss;

-#ifdef CONFIG_DRM_MSM_DPU

• struct dpu_dbg_power_ctrl dbg_power_ctrl = { 0 };

-#endif

• int ret, i; struct sched_param param;

@@ -537,18 +525,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) goto power_init_fail; }

• priv->pclient = dpu_power_client_create(&priv->phandle, "dpu");
• if (IS_ERR_OR_NULL(priv->pclient)) {

• pr_err("dpu power client create failed\n");
  

• ret = -EINVAL;
  

• goto power_client_fail;
  

• }
• dbg_power_ctrl.handle = &priv->phandle;
• dbg_power_ctrl.client = priv->pclient;
• dbg_power_ctrl.enable_fn = msm_power_enable_wrapper;
• ret = dpu_dbg_init(&pdev->dev, &dbg_power_ctrl);

• ret = dpu_dbg_init(&pdev->dev); if (ret) { dev_err(dev, "failed to init dpu dbg: %d\n", ret); goto dbg_init_fail;

@@ -756,8 +733,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) #ifdef CONFIG_DRM_MSM_DPU dpu_dbg_destroy(); dbg_init_fail:

• dpu_power_client_destroy(&priv->phandle, priv->pclient);

-power_client_fail: dpu_power_resource_deinit(pdev, &priv->phandle); power_init_fail: #endif diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 22a3096..f9ae96f 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -376,7 +376,6 @@ struct msm_drm_private { struct msm_kms *kms;

struct dpu_power_handle phandle;

• struct dpu_power_client *pclient;

  /* subordinate devices, if present: */ struct platform_device *gpu_pdev;

-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project