[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1086300a-2c8e-f897-a0d7-84d36276a6b6@quicinc.com>
Date: Mon, 4 Dec 2023 08:37:11 -0800
From: Kuogee Hsieh <quic_khsieh@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
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 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
>
>> 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
>>
>
Powered by blists - more mailing lists