| 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: <CAA8EJpq_zE5J584smV=Gido9G-spo7hPYZyf-7vr8ki+r9Az9g@mail.gmail.com>
Date: Mon, 4 Dec 2023 18:47:17 +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 v1] drm/msm/dpu: improve DSC allocation
On Mon, 4 Dec 2023 at 18:37, Kuogee Hsieh <quic_khsieh@...cinc.com> wrote:
>
>
> On 11/29/2023 7:57 PM, Dmitry Baryshkov wrote:
> > On Wed, 29 Nov 2023 at 22:31, 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.
> > Is this applicable to old DSC 1.1 encoders?
> yes, this algorithm should work with V1 too
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 | 94 +++++++++++++++++++++++++++++-----
> >> 1 file changed, 82 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> >> index f9215643..427d70d 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> >> @@ -466,24 +466,94 @@ static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
> >> struct drm_encoder *enc,
> >> const struct msm_display_topology *top)
> >> {
> >> - int num_dsc = top->num_dsc;
> >> - int i;
> >> + int num_dsc = 0;
> >> + int i, pp_idx;
> >> + bool pair = false;
> >> + int dsc_idx[DSC_MAX - DSC_0];
> >> + uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0];
> >> + int pp_max = PINGPONG_MAX - PINGPONG_0;
> >> +
> >> + if (!top->num_dsc || !top->num_intf)
> >> + return 0;
> >> +
> >> + /*
> >> + * Truth:
> >> + * 1) every layer mixer only connects to one pingpong
> >> + * 2) no pingpong split -- two layer mixers shared one pingpong
> >> + * 3) each DSC engine contains two dsc encoders
> >> + * -- index(0,1), index (2,3),... etc
> >> + * 4) dsc pair can only happens with same DSC engine except 4 dsc
> >> + * merge mode application (8k) which need two DSC engines
> >> + * 5) odd pingpong connect to odd dsc
> >> + * 6) even pingpong connect even dsc
> >> + */
> >> +
> >> + /* num_dsc should be either 1, 2 or 4 */
> >> + if (top->num_dsc > top->num_intf) /* merge mode */
> >> + pair = true;
> >> +
> >> + /* 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); i++) {
> > && num_dsc < top->num_dsc
> >
> >> + if (!rm->dsc_blks[i]) /* end of dsc list */
> >> + break;
> > I'd say, it's `continue' instead, let's just skip the index.
> >
> >> - /* 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 */
> >> + /* consective dsc index to be paired */
> >> + if (pair && num_dsc) { /* already start pairing, re start */
> >> + 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;
> >> }
> >>
> >> - if (global_state->dsc_to_enc_id[i]) {
> >> - DPU_ERROR("DSC %d is already allocated\n", i);
> >> - return -EIO;
> >> + /* odd index can not become start of pairing */
> >> + if (pair && (i & 0x01) && !num_dsc)
> >> + continue;
> > After looking at all conditions, can we have two different helpers?
> > One which allocates a single DSC and another one which allocates a
> > pair. For the pair you can skip odd indices at all and just check if
> > DSC_i and DSC_i+1 are free.
> >
> >> +
> >> + /*
> >> + * 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->base.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 it can not be found at next search
> >> + * in the case of pairing
> >> + */
> >> + pp_to_enc_id[pp_idx] = NULL;
> >> +
> >> + dsc_idx[num_dsc++] = i;
> >> + if (num_dsc >= top->num_dsc)
> >> + break;
> >> }
> >>
> >> - 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];
> >> + global_state->dsc_to_enc_id[j] = enc->base.id;
> >> + }
> >>
> >> return 0;
> >> }
> >> --
> >> 2.7.4
> >>
> >
--
With best wishes
Dmitry
Powered by blists - more mailing lists