| 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: <CAA8EJprgES1tA1E=17Bnrz8n7mX1COpkx=w-vhWge4ywZ1E0ZQ@mail.gmail.com>
Date: Tue, 5 Dec 2023 02:23:48 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Kuogee Hsieh <quic_khsieh@...cinc.com>
Cc: dri-devel@...ts.freedesktop.org, robdclark@...il.com,
sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
vkoul@...nel.org, daniel@...ll.ch, airlied@...il.com,
agross@...nel.org, andersson@...nel.org, quic_abhinavk@...cinc.com,
quic_jesszhan@...cinc.com, quic_sbillaka@...cinc.com,
marijn.suijten@...ainline.org, freedreno@...ts.freedesktop.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/msm/dpu: improve DSC allocation
On Tue, 5 Dec 2023 at 01:55, Kuogee Hsieh <quic_khsieh@...cinc.com> wrote:
>
> A DCE (Display Compression Engine) contains two DSC hard slice
> encoders. Each DCE start with even DSC encoder index followed by
> an odd DSC encoder index. Each encoder can work independently.
> But Only two DSC encoders from same DCE can be paired to work
> together to support merge mode. In addition, the DSC with even
> index have to mapping to even pingpong index and DSC with odd
> index have to mapping to odd pingpong index at its data path.
> This patch improve DSC allocation mechanism with consideration
> of above factors.
>
> Changes in V2:
> -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and
> _dpu_rm_reserve_dsc_pair()
Please don't send the new iteration of the patch if the discussion is ongoing.
Quoting v1 review:
Are the limitations (odd:odd, allocation in pairs, etc) applicable to
v1.1 encoders?
I assume that at least 'allocate two consecutive DSC for DSC merge' is
not applicable, since there are no separate DCE units.
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 171 ++++++++++++++++++++++++++++++---
> 1 file changed, 156 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 17ecf23..dafe1ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -470,33 +470,174 @@ static int _dpu_rm_reserve_ctls(
> return 0;
> }
>
> -static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
> +static int _dpu_rm_reserve_dsc_single(struct dpu_rm *rm,
> struct dpu_global_state *global_state,
> - struct drm_encoder *enc,
> + uint32_t enc_id,
> const struct msm_display_topology *top)
> {
> - int num_dsc = top->num_dsc;
> - int i;
> + int num_dsc = 0;
> + int i, pp_idx;
> + int dsc_idx[DSC_MAX - DSC_0];
> + uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0];
> + int pp_max = PINGPONG_MAX - PINGPONG_0;
> +
> + for (i = 0; i < DSC_MAX - DSC_0; i++)
> + dsc_idx[i] = 0;
> +
> + /* fill working copy with pingpong list */
> + memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id, sizeof(pp_to_enc_id));
> +
> + for (i = 0; i < ARRAY_SIZE(rm->dsc_blks) && num_dsc >= top->num_dsc; i++) {
> + if (!rm->dsc_blks[i])
> + continue;
>
> - /* check if DSC required are allocated or not */
> - for (i = 0; i < num_dsc; i++) {
> - if (!rm->dsc_blks[i]) {
> - DPU_ERROR("DSC %d does not exist\n", i);
> - return -EIO;
> + if (global_state->dsc_to_enc_id[i]) /* used */
> + continue;
> +
> + /*
> + * find the pingpong index which had been reserved
> + * previously at layer mixer allocation
> + */
> + for (pp_idx = 0; pp_idx < pp_max; pp_idx++) {
> + if (pp_to_enc_id[pp_idx] == enc_id)
> + break;
> }
>
> - if (global_state->dsc_to_enc_id[i]) {
> - DPU_ERROR("DSC %d is already allocated\n", i);
> - return -EIO;
> + /*
> + * dsc even index must map to pingpong even index
> + * dsc odd index must map to pingpong odd index
> + */
> + if ((i & 0x01) != (pp_idx & 0x01))
> + continue;
> +
> + dsc_idx[num_dsc++] = i + 1; /* found, start from 1 */
> + }
> +
> + if (num_dsc < top->num_dsc) {
> + DPU_ERROR("DSC allocation failed num_dsc=%d required=%d\n",
> + num_dsc, top->num_dsc);
> + return -ENAVAIL;
> + }
> +
> + /* reserve dsc */
> + for (i = 0; i < top->num_dsc; i++) {
> + int j;
> +
> + j = dsc_idx[i];
> + if (j)
> + global_state->dsc_to_enc_id[j-1] = enc_id;
> + }
> +
> + return 0;
> +}
> +
> +static int _dpu_rm_reserve_dsc_pair(struct dpu_rm *rm,
> + struct dpu_global_state *global_state,
> + uint32_t enc_id,
> + const struct msm_display_topology *top)
> +{
> + int num_dsc = 0;
> + int i, pp_idx;
> + int dsc_idx[DSC_MAX - DSC_0];
> + uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0];
> + int pp_max = PINGPONG_MAX - PINGPONG_0;
> +
> + for (i = 0; i < DSC_MAX - DSC_0; i++)
> + dsc_idx[i] = 0;
> +
> + /* fill working copy with pingpong list */
> + memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id, sizeof(pp_to_enc_id));
> +
> + for (i = 0; i < ARRAY_SIZE(rm->dsc_blks) && num_dsc >= top->num_dsc; i++) {
> + if (!rm->dsc_blks[i])
> + continue;
> +
> + if (global_state->dsc_to_enc_id[i]) { /* used */
> + /* consective dsc index to be paired */
> + if (num_dsc) { /* already start pairing, re start search */
> + num_dsc = 0;
> + /* fill working copy with pingpong list */
> + memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id,
> + sizeof(pp_to_enc_id));
> + }
> + continue;
> + }
> +
> + /* odd index can not become start of pairing */
> + if (i & 0x01 && !num_dsc)
> + continue;
> +
> + /*
> + * find the pingpong index which had been reserved
> + * previously at layer mixer allocation
> + */
> + for (pp_idx = 0; pp_idx < pp_max; pp_idx++) {
> + if (pp_to_enc_id[pp_idx] == enc_id)
> + break;
> }
> +
> + /*
> + * dsc even index must map to pingpong even index
> + * dsc odd index must map to pingpong odd index
> + */
> + if ((i & 0x01) != (pp_idx & 0x01))
> + continue;
> +
> + /*
> + * delete pp_idx so that next pp_idx can be paired with
> + * next dsc_idx
> + */
> + pp_to_enc_id[pp_idx] = 0xffff;
> +
> + dsc_idx[num_dsc++] = i + 1; /* found, start from 1 */
> }
>
> - for (i = 0; i < num_dsc; i++)
> - global_state->dsc_to_enc_id[i] = enc->base.id;
> + if (num_dsc < top->num_dsc) {
> + DPU_ERROR("DSC allocation failed num_dsc=%d required=%d\n",
> + num_dsc, top->num_dsc);
> + return -ENAVAIL;
> + }
> +
> + /* reserve dsc */
> + for (i = 0; i < top->num_dsc; i++) {
> + int j;
> +
> + j = dsc_idx[i];
> + if (j)
> + global_state->dsc_to_enc_id[j-1] = enc_id;
> + }
>
> return 0;
> }
>
> +static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
> + struct dpu_global_state *global_state,
> + uint32_t enc_id,
> + const struct msm_display_topology *top)
> +{
> + if (!top->num_dsc || !top->num_intf)
> + return 0;
> +
> + /*
> + * Truth:
> + * 1) every layer mixer only connects to one pingpong
> + * 2) no pingpong split -- which is two layer mixers shared one pingpong
> + * 3) each DSC engine contains two dsc encoders
> + * -- index(0,1), index (2,3),... etc
Does this apply to v1.1 encoders?
> + * 4) dsc pair can only happens with same DSC engine
> + * 5) odd pingpong connect to odd dsc
> + * 6) even pingpong connect to even dsc
> + * 7) pair: encoder +--> pp_idx_0 --> dsc_idx_0
> + +--> pp_idx_1 --> dsc_idx_1
> + */
> +
> + /* num_dsc should be either 1, 2 or 4 */
> + if (top->num_dsc > top->num_intf) /* merge mode */
> + return _dpu_rm_reserve_dsc_pair(rm, global_state, enc_id, top);
> + else
> + return _dpu_rm_reserve_dsc_single(rm, global_state, enc_id, top);
> +}
> +
> static int _dpu_rm_make_reservation(
> struct dpu_rm *rm,
> struct dpu_global_state *global_state,
> @@ -518,7 +659,7 @@ static int _dpu_rm_make_reservation(
> return ret;
> }
>
> - ret = _dpu_rm_reserve_dsc(rm, global_state, enc, &reqs->topology);
> + ret = _dpu_rm_reserve_dsc(rm, global_state, enc->base.id, &reqs->topology);
> if (ret)
> return ret;
>
> --
> 2.7.4
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists