| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1faa4d88-7acf-1895-f93b-59fd20d6de06@linaro.org>
Date: Tue, 27 Oct 2020 11:32:36 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Rob Clark <robdclark@...il.com>,
Kalyan Thota <kalyan_t@...eaurora.org>
Cc: dri-devel <dri-devel@...ts.freedesktop.org>,
linux-arm-msm <linux-arm-msm@...r.kernel.org>,
freedreno <freedreno@...ts.freedesktop.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Sean Paul <seanpaul@...omium.org>,
"Kristian H. Kristensen" <hoegsberg@...omium.org>,
Douglas Anderson <dianders@...omium.org>,
Krishna Manikandan <mkrishn@...eaurora.org>,
Jeykumar Sankaran <jsanka@...eaurora.org>,
Raviteja Tamatam <travitej@...eaurora.org>,
nganji@...eaurora.org
Subject: Re: [PATCH 3/3] drm/msm/dpu: add support for clk and bw scaling for
display
Hello,
On 04/08/2020 18:40, Rob Clark wrote:
> On Thu, Jul 16, 2020 at 4:36 AM Kalyan Thota <kalyan_t@...eaurora.org> wrote:
>>
>> This change adds support to scale src clk and bandwidth as
>> per composition requirements.
>>
>> Interconnect registration for bw has been moved to mdp
>> device node from mdss to facilitate the scaling.
>>
>> Changes in v1:
>> - Address armv7 compilation issues with the patch (Rob)
>>
>> Signed-off-by: Kalyan Thota <kalyan_t@...eaurora.org>
Kalyan, back in July you promised to provide a followup patchset,
removing code duplication. It's close to November now. Are there any
plans for the followup or is a forgotten topic?
>
> Reviewed-by: Rob Clark <robdclark@...omium.org>
>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 109 +++++++++++++++++++++----
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +
>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 37 ++++++++-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 +
>> drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 9 +-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 84 +++++++++++++++++++
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 +
>> 8 files changed, 233 insertions(+), 23 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 7c230f7..e52bc44 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
>> @@ -29,6 +29,74 @@ enum dpu_perf_mode {
>> DPU_PERF_MODE_MAX
>> };
>>
>> +/**
>> + * @_dpu_core_perf_calc_bw() - to calculate BW per crtc
>> + * @kms - pointer to the dpu_kms
>> + * @crtc - pointer to a crtc
>> + * Return: returns aggregated BW for all planes in crtc.
>> + */
>> +static u64 _dpu_core_perf_calc_bw(struct dpu_kms *kms,
>> + struct drm_crtc *crtc)
>> +{
>> + struct drm_plane *plane;
>> + struct dpu_plane_state *pstate;
>> + u64 crtc_plane_bw = 0;
>> + u32 bw_factor;
>> +
>> + drm_atomic_crtc_for_each_plane(plane, crtc) {
>> + pstate = to_dpu_plane_state(plane->state);
>> + if (!pstate)
>> + continue;
>> +
>> + crtc_plane_bw += pstate->plane_fetch_bw;
>> + }
>> +
>> + bw_factor = kms->catalog->perf.bw_inefficiency_factor;
>> + if (bw_factor) {
>> + crtc_plane_bw *= bw_factor;
>> + do_div(crtc_plane_bw, 100);
>> + }
>> +
>> + return crtc_plane_bw;
>> +}
>> +
>> +/**
>> + * _dpu_core_perf_calc_clk() - to calculate clock per crtc
>> + * @kms - pointer to the dpu_kms
>> + * @crtc - pointer to a crtc
>> + * @state - pointer to a crtc state
>> + * Return: returns max clk for all planes in crtc.
>> + */
>> +static u64 _dpu_core_perf_calc_clk(struct dpu_kms *kms,
>> + struct drm_crtc *crtc, struct drm_crtc_state *state)
>> +{
>> + struct drm_plane *plane;
>> + struct dpu_plane_state *pstate;
>> + struct drm_display_mode *mode;
>> + u64 crtc_clk;
>> + u32 clk_factor;
>> +
>> + mode = &state->adjusted_mode;
>> +
>> + crtc_clk = mode->vtotal * mode->hdisplay * drm_mode_vrefresh(mode);
>> +
>> + drm_atomic_crtc_for_each_plane(plane, crtc) {
>> + pstate = to_dpu_plane_state(plane->state);
>> + if (!pstate)
>> + continue;
>> +
>> + crtc_clk = max(pstate->plane_clk, crtc_clk);
>> + }
>> +
>> + clk_factor = kms->catalog->perf.clk_inefficiency_factor;
>> + if (clk_factor) {
>> + crtc_clk *= clk_factor;
>> + do_div(crtc_clk, 100);
>> + }
>> +
>> + return crtc_clk;
>> +}
>> +
>> static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
>> {
>> struct msm_drm_private *priv;
>> @@ -51,12 +119,7 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
>> dpu_cstate = to_dpu_crtc_state(state);
>> memset(perf, 0, sizeof(struct dpu_core_perf_params));
>>
>> - if (!dpu_cstate->bw_control) {
>> - perf->bw_ctl = kms->catalog->perf.max_bw_high *
>> - 1000ULL;
>> - perf->max_per_pipe_ib = perf->bw_ctl;
>> - perf->core_clk_rate = kms->perf.max_core_clk_rate;
>> - } else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
>> + if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
>> perf->bw_ctl = 0;
>> perf->max_per_pipe_ib = 0;
>> perf->core_clk_rate = 0;
Now bw_control is unused and can be removed alltogether.
>> @@ -64,6 +127,10 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
>> perf->bw_ctl = kms->perf.fix_core_ab_vote;
>> perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote;
>> perf->core_clk_rate = kms->perf.fix_core_clk_rate;
>> + } else {
>> + perf->bw_ctl = _dpu_core_perf_calc_bw(kms, crtc);
>> + perf->max_per_pipe_ib = kms->catalog->perf.min_dram_ib;
>> + perf->core_clk_rate = _dpu_core_perf_calc_clk(kms, crtc, state);
>> }
>>
>> DPU_DEBUG(
>> @@ -115,11 +182,7 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>> DPU_DEBUG("crtc:%d bw:%llu ctrl:%d\n",
>> tmp_crtc->base.id, tmp_cstate->new_perf.bw_ctl,
>> tmp_cstate->bw_control);
>> - /*
>> - * For bw check only use the bw if the
>> - * atomic property has been already set
>> - */
>> - if (tmp_cstate->bw_control)
>> +
>> bw_sum_of_intfs += tmp_cstate->new_perf.bw_ctl;
Just a nitpick: indent is wrong.
>> }
>>
>> @@ -131,9 +194,7 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>>
>> DPU_DEBUG("final threshold bw limit = %d\n", threshold);
>>
>> - if (!dpu_cstate->bw_control) {
>> - DPU_DEBUG("bypass bandwidth check\n");
>> - } else if (!threshold) {
>> + if (!threshold) {
>> DPU_ERROR("no bandwidth limits specified\n");
>> return -E2BIG;
>> } else if (bw > threshold) {
>> @@ -154,7 +215,11 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
>> = dpu_crtc_get_client_type(crtc);
>> struct drm_crtc *tmp_crtc;
>> struct dpu_crtc_state *dpu_cstate;
>> - int ret = 0;
>> + int i, ret = 0;
>> + u64 avg_bw;
>> +
>> + if (!kms->num_paths)
>> + return -EINVAL;
This broke bandwidth setting for everybody except sc7180, since
_dpu_core_perf_crtc_update_bus will be still called for them, and
returning -EINVAL here prevents dpu_core_perf_crtc_update() from setting
clock rate. Returning 0 here fixes the issue.
>>
>> drm_for_each_crtc(tmp_crtc, crtc->dev) {
>> if (tmp_crtc->enabled &&
>> @@ -165,10 +230,20 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms,
--
With best wishes
Dmitry
Powered by blists - more mailing lists