2023-12-04 23:56:17

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v2] drm/msm/dpu: improve DSC allocation

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()

Signed-off-by: Kuogee Hsieh <[email protected]>
---
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
+ * 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


2023-12-05 00:24:33

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v2] drm/msm/dpu: improve DSC allocation

On Tue, 5 Dec 2023 at 01:55, Kuogee Hsieh <[email protected]> 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 <[email protected]>
> ---
> 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