On Tue, 5 Dec 2023 at 01:55, Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> 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@xxxxxxxxxxx> > --- > 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