| Message ID | 1701289898-12235-1-git-send-email-quic_khsieh@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] drm/msm/dpu: improve DSC allocation | expand |
On Wed, 29 Nov 2023 at 22:31, Kuogee Hsieh <quic_khsieh@quicinc.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? > > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.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 >
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/202311302230.t6X5rroJ-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231130/202311302230.t6X5rroJ-lkp@intel.com/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 <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202311302230.t6X5rroJ-lkp@intel.com/ 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
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/202311302309.NPRFDAWm-lkp@intel.com/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/202311302309.NPRFDAWm-lkp@intel.com/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 <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202311302309.NPRFDAWm-lkp@intel.com/ 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
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/202311302343.3lXKkLxR-lkp@intel.com/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/202311302343.3lXKkLxR-lkp@intel.com/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 <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202311302343.3lXKkLxR-lkp@intel.com/ 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
On 11/29/2023 7:57 PM, Dmitry Baryshkov wrote: > On Wed, 29 Nov 2023 at 22:31, Kuogee Hsieh <quic_khsieh@quicinc.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 > >> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.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 >> >
On Mon, 4 Dec 2023 at 18:37, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote: > > > On 11/29/2023 7:57 PM, Dmitry Baryshkov wrote: > > On Wed, 29 Nov 2023 at 22:31, Kuogee Hsieh <quic_khsieh@quicinc.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@quicinc.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 > >> > >
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; }
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 <quic_khsieh@quicinc.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 94 +++++++++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 12 deletions(-)