Hi,
On Tue, May 31, 2022 at 2:29 PM Abhinav Kumar quic_abhinavk@quicinc.com wrote:
@@ -136,6 +178,13 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss) { int ret;
/** Several components have AXI clocks that can only be turned on if* the interconnect is enabled (non-zero bandwidth). Let's make sure* that the interconnects are at least at a minimum amount.*/msm_mdss_icc_request_bw(msm_mdss, MIN_IB_BW);This patch does two things :
- Move the min icc vote from the dpu_runtime_resume() path to the
msm_mdss_enable() so that while resuming, we add the min vote to the parent device. We do need a min vote before turning on the AXI clk as per this comment mentioned in c33b7c0389e1 (drm/msm/dpu: add support for clk and bw scaling for display)
@@ -1101,8 +1129,15 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev) struct drm_encoder *encoder; struct drm_device *ddev; struct dss_module_power *mp = &dpu_kms->mp;
int i; ddev = dpu_kms->dev;/* 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,dpu_kms->catalog->perf.min_dram_ib);So I understand and I am fine with this part.
- Add a min vote in msm_mdss_init().
This is the part I am not able to fully follow.
If we only need to add the min vote before voting for the clocks, adding it in mdss_mdss_enable() should be enough.
Do we need this part of adding the min vote to the msm_mdss_init()?
If so, why?
Ah, good question. Mostly I was matching how things were done before commit a670ff578f1f ("drm/msm/dpu: always use mdp device to scale bandwidth"). Before that commit we used to put the min vote in the init path.
...but you're right, I don't see any good reason for it. I'll send a v2 removing that line from msm_mdss_init().
-Doug