On 1/15/2025 12:27 AM, Dmitry Baryshkov wrote:
On Tue, Jan 14, 2025 at 01:18:26PM -0800, Abhinav Kumar wrote:
On 1/14/2025 3:10 AM, Dmitry Baryshkov wrote:
On Mon, Jan 13, 2025 at 07:38:16PM -0800, Abhinav Kumar wrote:
On 1/5/2025 7:07 PM, Dmitry Baryshkov wrote:
Move perf mode handling for the bandwidth to
_dpu_core_perf_crtc_update_bus() rather than overriding per-CRTC data
and then aggregating known values.
Note, this changes the fix_core_ab_vote. Previously it would be
multiplied per the CRTC number, now it will be used directly for
interconnect voting. This better reflects user requirements in the case
of different resolutions being set on different CRTCs: instead of using
the same bandwidth for each CRTC (which is incorrect) user can now
calculate overall bandwidth required by all outputs and use that value.
There are two things this change is doing:
1) Dropping the core_clk_rate setting because its already handled inside
_dpu_core_perf_get_core_clk_rate() and hence dpu_core_perf_crtc_update()
will still work.
and
2) Then this part of moving the ab/ib setting to
_dpu_core_perf_crtc_update_bus().
Can we split this into two changes so that its clear that dropping
core_clk_rate setting in this change will not cause an issue.
Ack
Actually I think this is incorrect.
If the user puts in an incorrect value beyond the bounds, earlier the code
will reject that by failing the in _dpu_core_perf_calc_crtc().
This function doesn't perform any validation nor returns an error code.
Probably you've meant some other function.
Sorry, let me explain a little more to complete the flow I am seeing.
_dpu_core_perf_calc_crtc() gets called by dpu_core_perf_crtc_check().
That one checks against erroneous values.
if (!threshold) {
DPU_ERROR("no bandwidth limits specified\n");
return -E2BIG;
} else if (bw > threshold) {
DPU_ERROR("exceeds bandwidth: %ukb > %ukb\n", bw,
threshold);
return -E2BIG;
}
Now, if we move it to _dpu_core_perf_crtc_update_bus(), this is beyond the
check phase so incorrect values cannot be rejected.
So we will still need to preserve overriding the values in
_dpu_core_perf_calc_crtc().
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 40 +++++++++++++--------------
1 file changed, 19 insertions(+), 21 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 70f43e8359caee2082f2ca9944a17a6a20aa3d49..7ff3405c6867556a8dc776783b91f1da6c86ef3f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -118,22 +118,9 @@ static void _dpu_core_perf_calc_crtc(const struct dpu_core_perf *core_perf,
return;
}
- memset(perf, 0, sizeof(struct dpu_core_perf_params));
-
- if (core_perf->perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
- perf->bw_ctl = 0;
- perf->max_per_pipe_ib = 0;
- perf->core_clk_rate = 0;
- } else if (core_perf->perf_tune.mode == DPU_PERF_MODE_FIXED) {
- perf->bw_ctl = core_perf->fix_core_ab_vote * 1000ULL;
- perf->max_per_pipe_ib = core_perf->fix_core_ib_vote;
- perf->core_clk_rate = core_perf->fix_core_clk_rate;
- } else {
- perf->bw_ctl = _dpu_core_perf_calc_bw(perf_cfg, crtc);
- perf->max_per_pipe_ib = perf_cfg->min_dram_ib;
- perf->core_clk_rate = _dpu_core_perf_calc_clk(perf_cfg, crtc, state);
- }
-
+ perf->bw_ctl = _dpu_core_perf_calc_bw(perf_cfg, crtc);
+ perf->max_per_pipe_ib = perf_cfg->min_dram_ib;
+ perf->core_clk_rate = _dpu_core_perf_calc_clk(perf_cfg, crtc, state);
DRM_DEBUG_ATOMIC(
"crtc=%d clk_rate=%llu core_ib=%u core_ab=%u\n",
crtc->base.id, perf->core_clk_rate,
@@ -222,18 +209,29 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
{
struct dpu_core_perf_params perf = { 0 };
int i, ret = 0;
- u64 avg_bw;
+ u32 avg_bw;
+ u32 peak_bw;
if (!kms->num_paths)
return 0;
- dpu_core_perf_aggregate(crtc->dev, dpu_crtc_get_client_type(crtc), &perf);
+ if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
+ avg_bw = 0;
+ peak_bw = 0;
+ } else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
+ avg_bw = kms->perf.fix_core_ab_vote;
+ peak_bw = kms->perf.fix_core_ib_vote;
+ } else {
+ dpu_core_perf_aggregate(crtc->dev, dpu_crtc_get_client_type(crtc), &perf);
+
+ avg_bw = div_u64(perf.bw_ctl, 1000); /*Bps_to_icc*/
+ peak_bw = perf.max_per_pipe_ib;
+ }
- avg_bw = perf.bw_ctl;
- do_div(avg_bw, (kms->num_paths * 1000)); /*Bps_to_icc*/
+ avg_bw /= kms->num_paths;
for (i = 0; i < kms->num_paths; i++)
- icc_set_bw(kms->path[i], avg_bw, perf.max_per_pipe_ib);
+ icc_set_bw(kms->path[i], avg_bw, peak_bw);
return ret;
}
