2023-11-29 20:32:30

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v1] 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.

Signed-off-by: Kuogee Hsieh <[email protected]>
---
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++) {
+ if (!rm->dsc_blks[i]) /* end of dsc list */
+ break;

- /* 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;
+
+ /*
+ * 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


2023-11-30 03:57:44

by Dmitry Baryshkov

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

On Wed, 29 Nov 2023 at 22:31, 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.

Is this applicable to old DSC 1.1 encoders?

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

2023-11-30 14:25:46

by kernel test robot

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

Hi Kuogee,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7-rc3 next-20231130]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Kuogee-Hsieh/drm-msm-dpu-improve-DSC-allocation/20231130-064646
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/1701289898-12235-1-git-send-email-quic_khsieh%40quicinc.com
patch subject: [PATCH v1] drm/msm/dpu: improve DSC allocation
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20231130/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231130/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c: In function '_dpu_rm_reserve_dsc':
>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c:537:38: warning: assignment to 'uint32_t' {aka 'unsigned int'} from 'void *' makes integer from pointer without a cast [-Wint-conversion]
537 | pp_to_enc_id[pp_idx] = NULL;
| ^


vim +537 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c

463
464 static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
465 struct dpu_global_state *global_state,
466 struct drm_encoder *enc,
467 const struct msm_display_topology *top)
468 {
469 int num_dsc = 0;
470 int i, pp_idx;
471 bool pair = false;
472 int dsc_idx[DSC_MAX - DSC_0];
473 uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0];
474 int pp_max = PINGPONG_MAX - PINGPONG_0;
475
476 if (!top->num_dsc || !top->num_intf)
477 return 0;
478
479 /*
480 * Truth:
481 * 1) every layer mixer only connects to one pingpong
482 * 2) no pingpong split -- two layer mixers shared one pingpong
483 * 3) each DSC engine contains two dsc encoders
484 * -- index(0,1), index (2,3),... etc
485 * 4) dsc pair can only happens with same DSC engine except 4 dsc
486 * merge mode application (8k) which need two DSC engines
487 * 5) odd pingpong connect to odd dsc
488 * 6) even pingpong connect even dsc
489 */
490
491 /* num_dsc should be either 1, 2 or 4 */
492 if (top->num_dsc > top->num_intf) /* merge mode */
493 pair = true;
494
495 /* fill working copy with pingpong list */
496 memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id, sizeof(pp_to_enc_id));
497
498 for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
499 if (!rm->dsc_blks[i]) /* end of dsc list */
500 break;
501
502 if (global_state->dsc_to_enc_id[i]) { /* used */
503 /* consective dsc index to be paired */
504 if (pair && num_dsc) { /* already start pairing, re start */
505 num_dsc = 0;
506 /* fill working copy with pingpong list */
507 memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id,
508 sizeof(pp_to_enc_id));
509 }
510 continue;
511 }
512
513 /* odd index can not become start of pairing */
514 if (pair && (i & 0x01) && !num_dsc)
515 continue;
516
517 /*
518 * find the pingpong index which had been reserved
519 * previously at layer mixer allocation
520 */
521 for (pp_idx = 0; pp_idx < pp_max; pp_idx++) {
522 if (pp_to_enc_id[pp_idx] == enc->base.id)
523 break;
524 }
525
526 /*
527 * dsc even index must map to pingpong even index
528 * dsc odd index must map to pingpong odd index
529 */
530 if ((i & 0x01) != (pp_idx & 0x01))
531 continue;
532
533 /*
534 * delete pp_idx so that it can not be found at next search
535 * in the case of pairing
536 */
> 537 pp_to_enc_id[pp_idx] = NULL;
538
539 dsc_idx[num_dsc++] = i;
540 if (num_dsc >= top->num_dsc)
541 break;
542 }
543
544 if (num_dsc < top->num_dsc) {
545 DPU_ERROR("DSC allocation failed num_dsc=%d required=%d\n",
546 num_dsc, top->num_dsc );
547 return -ENAVAIL;
548 }
549
550 /* reserve dsc */
551 for (i = 0; i < top->num_dsc; i++) {
552 int j;
553
554 j = dsc_idx[i];
555 global_state->dsc_to_enc_id[j] = enc->base.id;
556 }
557
558 return 0;
559 }
560

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-11-30 15:19:48

by kernel test robot

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

Hi Kuogee,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7-rc3 next-20231130]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Kuogee-Hsieh/drm-msm-dpu-improve-DSC-allocation/20231130-064646
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/1701289898-12235-1-git-send-email-quic_khsieh%40quicinc.com
patch subject: [PATCH v1] drm/msm/dpu: improve DSC allocation
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20231130/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231130/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c:537:24: error: incompatible pointer to integer conversion assigning to 'uint32_t' (aka 'unsigned int') from 'void *' [-Wint-conversion]
537 | pp_to_enc_id[pp_idx] = NULL;
| ^ ~~~~
1 error generated.


vim +537 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c

463
464 static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
465 struct dpu_global_state *global_state,
466 struct drm_encoder *enc,
467 const struct msm_display_topology *top)
468 {
469 int num_dsc = 0;
470 int i, pp_idx;
471 bool pair = false;
472 int dsc_idx[DSC_MAX - DSC_0];
473 uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0];
474 int pp_max = PINGPONG_MAX - PINGPONG_0;
475
476 if (!top->num_dsc || !top->num_intf)
477 return 0;
478
479 /*
480 * Truth:
481 * 1) every layer mixer only connects to one pingpong
482 * 2) no pingpong split -- two layer mixers shared one pingpong
483 * 3) each DSC engine contains two dsc encoders
484 * -- index(0,1), index (2,3),... etc
485 * 4) dsc pair can only happens with same DSC engine except 4 dsc
486 * merge mode application (8k) which need two DSC engines
487 * 5) odd pingpong connect to odd dsc
488 * 6) even pingpong connect even dsc
489 */
490
491 /* num_dsc should be either 1, 2 or 4 */
492 if (top->num_dsc > top->num_intf) /* merge mode */
493 pair = true;
494
495 /* fill working copy with pingpong list */
496 memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id, sizeof(pp_to_enc_id));
497
498 for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
499 if (!rm->dsc_blks[i]) /* end of dsc list */
500 break;
501
502 if (global_state->dsc_to_enc_id[i]) { /* used */
503 /* consective dsc index to be paired */
504 if (pair && num_dsc) { /* already start pairing, re start */
505 num_dsc = 0;
506 /* fill working copy with pingpong list */
507 memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id,
508 sizeof(pp_to_enc_id));
509 }
510 continue;
511 }
512
513 /* odd index can not become start of pairing */
514 if (pair && (i & 0x01) && !num_dsc)
515 continue;
516
517 /*
518 * find the pingpong index which had been reserved
519 * previously at layer mixer allocation
520 */
521 for (pp_idx = 0; pp_idx < pp_max; pp_idx++) {
522 if (pp_to_enc_id[pp_idx] == enc->base.id)
523 break;
524 }
525
526 /*
527 * dsc even index must map to pingpong even index
528 * dsc odd index must map to pingpong odd index
529 */
530 if ((i & 0x01) != (pp_idx & 0x01))
531 continue;
532
533 /*
534 * delete pp_idx so that it can not be found at next search
535 * in the case of pairing
536 */
> 537 pp_to_enc_id[pp_idx] = NULL;
538
539 dsc_idx[num_dsc++] = i;
540 if (num_dsc >= top->num_dsc)
541 break;
542 }
543
544 if (num_dsc < top->num_dsc) {
545 DPU_ERROR("DSC allocation failed num_dsc=%d required=%d\n",
546 num_dsc, top->num_dsc );
547 return -ENAVAIL;
548 }
549
550 /* reserve dsc */
551 for (i = 0; i < top->num_dsc; i++) {
552 int j;
553
554 j = dsc_idx[i];
555 global_state->dsc_to_enc_id[j] = enc->base.id;
556 }
557
558 return 0;
559 }
560

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-11-30 15:45:07

by kernel test robot

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

Hi Kuogee,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7-rc3 next-20231130]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Kuogee-Hsieh/drm-msm-dpu-improve-DSC-allocation/20231130-064646
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/1701289898-12235-1-git-send-email-quic_khsieh%40quicinc.com
patch subject: [PATCH v1] drm/msm/dpu: improve DSC allocation
config: arm-defconfig (https://download.01.org/0day-ci/archive/20231130/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231130/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c:537:24: warning: incompatible pointer to integer conversion assigning to 'uint32_t' (aka 'unsigned int') from 'void *' [-Wint-conversion]
pp_to_enc_id[pp_idx] = NULL;
^ ~~~~
1 warning generated.


vim +537 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c

463
464 static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
465 struct dpu_global_state *global_state,
466 struct drm_encoder *enc,
467 const struct msm_display_topology *top)
468 {
469 int num_dsc = 0;
470 int i, pp_idx;
471 bool pair = false;
472 int dsc_idx[DSC_MAX - DSC_0];
473 uint32_t pp_to_enc_id[PINGPONG_MAX - PINGPONG_0];
474 int pp_max = PINGPONG_MAX - PINGPONG_0;
475
476 if (!top->num_dsc || !top->num_intf)
477 return 0;
478
479 /*
480 * Truth:
481 * 1) every layer mixer only connects to one pingpong
482 * 2) no pingpong split -- two layer mixers shared one pingpong
483 * 3) each DSC engine contains two dsc encoders
484 * -- index(0,1), index (2,3),... etc
485 * 4) dsc pair can only happens with same DSC engine except 4 dsc
486 * merge mode application (8k) which need two DSC engines
487 * 5) odd pingpong connect to odd dsc
488 * 6) even pingpong connect even dsc
489 */
490
491 /* num_dsc should be either 1, 2 or 4 */
492 if (top->num_dsc > top->num_intf) /* merge mode */
493 pair = true;
494
495 /* fill working copy with pingpong list */
496 memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id, sizeof(pp_to_enc_id));
497
498 for (i = 0; i < ARRAY_SIZE(rm->dsc_blks); i++) {
499 if (!rm->dsc_blks[i]) /* end of dsc list */
500 break;
501
502 if (global_state->dsc_to_enc_id[i]) { /* used */
503 /* consective dsc index to be paired */
504 if (pair && num_dsc) { /* already start pairing, re start */
505 num_dsc = 0;
506 /* fill working copy with pingpong list */
507 memcpy(pp_to_enc_id, global_state->pingpong_to_enc_id,
508 sizeof(pp_to_enc_id));
509 }
510 continue;
511 }
512
513 /* odd index can not become start of pairing */
514 if (pair && (i & 0x01) && !num_dsc)
515 continue;
516
517 /*
518 * find the pingpong index which had been reserved
519 * previously at layer mixer allocation
520 */
521 for (pp_idx = 0; pp_idx < pp_max; pp_idx++) {
522 if (pp_to_enc_id[pp_idx] == enc->base.id)
523 break;
524 }
525
526 /*
527 * dsc even index must map to pingpong even index
528 * dsc odd index must map to pingpong odd index
529 */
530 if ((i & 0x01) != (pp_idx & 0x01))
531 continue;
532
533 /*
534 * delete pp_idx so that it can not be found at next search
535 * in the case of pairing
536 */
> 537 pp_to_enc_id[pp_idx] = NULL;
538
539 dsc_idx[num_dsc++] = i;
540 if (num_dsc >= top->num_dsc)
541 break;
542 }
543
544 if (num_dsc < top->num_dsc) {
545 DPU_ERROR("DSC allocation failed num_dsc=%d required=%d\n",
546 num_dsc, top->num_dsc );
547 return -ENAVAIL;
548 }
549
550 /* reserve dsc */
551 for (i = 0; i < top->num_dsc; i++) {
552 int j;
553
554 j = dsc_idx[i];
555 global_state->dsc_to_enc_id[j] = enc->base.id;
556 }
557
558 return 0;
559 }
560

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-04 16:37:45

by Kuogee Hsieh

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


On 11/29/2023 7:57 PM, Dmitry Baryshkov wrote:
> On Wed, 29 Nov 2023 at 22:31, 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.
> Is this applicable to old DSC 1.1 encoders?
yes, this algorithm should work with V1 too
>
>> Signed-off-by: Kuogee Hsieh <[email protected]>
>> ---
>> 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
>>
>

2023-12-04 16:58:51

by Dmitry Baryshkov

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

On Mon, 4 Dec 2023 at 18:37, Kuogee Hsieh <[email protected]> wrote:
>
>
> On 11/29/2023 7:57 PM, Dmitry Baryshkov wrote:
> > On Wed, 29 Nov 2023 at 22:31, 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.
> > 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 <[email protected]>
> >> ---
> >> 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