Re: [v1] drm/msm/disp/dpu1: fix warn stack reported during dpu resume

From: Dmitry Baryshkov
Date: Wed Mar 31 2021 - 22:08:33 EST


On 01/04/2021 01:47, Rob Clark wrote:
On Wed, Mar 31, 2021 at 9:03 AM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:

On 31/03/2021 14:27, Kalyan Thota wrote:
WARN_ON was introduced by the below commit to catch runtime resumes
that are getting triggered before icc path was set.

"drm/msm/disp/dpu1: icc path needs to be set before dpu runtime resume"

For the targets where the bw scaling is not enabled, this WARN_ON is
a false alarm. Fix the WARN condition appropriately.

Should we change all DPU targets to use bw scaling to the mdp from the
mdss nodes? The limitation to sc7180 looks artificial.

yes, we should, this keeps biting us on 845

Done, https://lore.kernel.org/linux-arm-msm/20210401020533.3956787-2-dmitry.baryshkov@xxxxxxxxxx/



Reported-by: Steev Klimaszewski <steev@xxxxxxxx>

Please add Fixes: tag as well

Signed-off-by: Kalyan Thota <kalyan_t@xxxxxxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 +++++---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 9 +++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 11 ++++++-----
3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index cab387f..0071a4d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -294,6 +294,9 @@ static int dpu_kms_parse_data_bus_icc_path(struct dpu_kms *dpu_kms)
struct icc_path *path1;
struct drm_device *dev = dpu_kms->dev;

+ if (!dpu_supports_bw_scaling(dev))
+ return 0;
+
path0 = of_icc_get(dev->dev, "mdp0-mem");
path1 = of_icc_get(dev->dev, "mdp1-mem");

@@ -934,8 +937,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
DPU_DEBUG("REG_DMA is not defined");
}

- if (of_device_is_compatible(dev->dev->of_node, "qcom,sc7180-mdss"))
- dpu_kms_parse_data_bus_icc_path(dpu_kms);
+ dpu_kms_parse_data_bus_icc_path(dpu_kms);

pm_runtime_get_sync(&dpu_kms->pdev->dev);

@@ -1198,7 +1200,7 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)

ddev = dpu_kms->dev;

- WARN_ON(!(dpu_kms->num_paths));
+ WARN_ON((dpu_supports_bw_scaling(ddev) && !dpu_kms->num_paths));
/* Min vote of BW is required before turning on AXI clk */
for (i = 0; i < dpu_kms->num_paths; i++)
icc_set_bw(dpu_kms->path[i], 0, Bps_to_icc(MIN_IB_BW));
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index d6717d6..f7bcc0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -154,6 +154,15 @@ struct vsync_info {

#define to_dpu_global_state(x) container_of(x, struct dpu_global_state, base)

+/**
+ * dpu_supports_bw_scaling: returns true for drivers that support bw scaling.
+ * @dev: Pointer to drm_device structure
+ */
+static inline int dpu_supports_bw_scaling(struct drm_device *dev)
+{
+ return of_device_is_compatible(dev->dev->of_node, "qcom,sc7180-mdss");
+}
+
/* Global private object state for tracking resources that are shared across
* multiple kms objects (planes/crtcs/etc).
*/
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index cd40788..8cd712c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -41,6 +41,9 @@ static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev,
struct icc_path *path0 = of_icc_get(dev->dev, "mdp0-mem");
struct icc_path *path1 = of_icc_get(dev->dev, "mdp1-mem");

+ if (dpu_supports_bw_scaling(dev))
+ return 0;
+
if (IS_ERR_OR_NULL(path0))
return PTR_ERR_OR_ZERO(path0);

@@ -276,11 +279,9 @@ int dpu_mdss_init(struct drm_device *dev)

DRM_DEBUG("mapped mdss address space @%pK\n", dpu_mdss->mmio);

- if (!of_device_is_compatible(dev->dev->of_node, "qcom,sc7180-mdss")) {
- ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss);
- if (ret)
- return ret;
- }
+ ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss);
+ if (ret)
+ return ret;

mp = &dpu_mdss->mp;
ret = msm_dss_parse_clock(pdev, mp);



--
With best wishes
Dmitry


--
With best wishes
Dmitry