| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <91628bd3-45ab-0e2b-2331-f3f1ea2232b4@quicinc.com>
Date: Wed, 20 Dec 2023 08:48:56 -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 v6] drm/msm/dpu: improve DSC allocation
On 12/19/2023 2:32 PM, Dmitry Baryshkov wrote:
> On Tue, 19 Dec 2023 at 18:18, Kuogee Hsieh <quic_khsieh@...cinc.com> wrote:
>> Hi Dmitry,
>>
>> Anymore comments from you?
> No, for some reason I missed this patch. Please excuse me.
>
>> On 12/14/2023 10:56 AM, Kuogee Hsieh wrote:
>>> At DSC V1.1 DCE (Display Compression Engine) contains a DSC encoder.
>>> However, at DSC V1.2 DCE consists of two DSC encoders, one has an odd
>>> index and another one has an even index. Each encoder can work
>>> independently. But only two DSC encoders from same DCE can be paired
>>> to work together to support DSC merge mode at DSC V1.2. For DSC V1.1
>>> two consecutive DSC encoders (start with even index) have to be paired
>>> to support DSC merge mode. In addition, the DSC with even index have
>>> to be mapped to even PINGPONG index and DSC with odd index have to be
>>> mapped to odd PINGPONG index at its data path in regardless of DSC
>>> V1.1 or V1.2. This patch improves DSC allocation mechanism with
>>> consideration of those factors.
>>>
>>> Changes in V6:
>>> -- rename _dpu_rm_reserve_dsc_single to _dpu_rm_dsc_alloc
>>> -- rename _dpu_rm_reserve_dsc_pair to _dpu_rm_dsc_alloc_pair
>>> -- pass global_state to _dpu_rm_pingpong_next_index()
>>> -- remove pp_max
>>> -- fix for loop condition check at _dpu_rm_dsc_alloc()
>>>
>>> Changes in V5:
>>> -- delete dsc_id[]
>>> -- update to global_state->dsc_to_enc_id[] directly
>>> -- replace ndx with idx
>>> -- fix indentation at function declaration
>>> -- only one for loop at _dpu_rm_reserve_dsc_single()
>>>
>>> Changes in V4:
>>> -- rework commit message
>>> -- use reserved_by_other()
>>> -- add _dpu_rm_pingpong_next_index()
>>> -- revise _dpu_rm_pingpong_dsc_check()
>>>
>>> Changes in V3:
>>> -- add dpu_rm_pingpong_dsc_check()
>>> -- for pair allocation use i += 2 at for loop
>>>
>>> Changes in V2:
>>> -- split _dpu_rm_reserve_dsc() into _dpu_rm_reserve_dsc_single() and
>>> _dpu_rm_reserve_dsc_pair()
>>>
>>> Fixes: f2803ee91a41 ("drm/msm/disp/dpu1: Add DSC support in RM")
>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 154 +++++++++++++++++++++++++++++----
>>> 1 file changed, 139 insertions(+), 15 deletions(-)
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>
> See below for minor nit.
>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>> index f9215643..0ce2a25 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>> @@ -461,29 +461,153 @@ static int _dpu_rm_reserve_ctls(
>>> return 0;
>>> }
>>>
>>> -static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
>>> - struct dpu_global_state *global_state,
>>> - struct drm_encoder *enc,
>>> - const struct msm_display_topology *top)
>>> +static int _dpu_rm_pingpong_next_index(struct dpu_global_state *global_state,
>>> + int start,
> I'd still prefer to see `enum dpu_pingpong` as a parameter here
> instead of just an index, but this is just my taste.
Can you please elaborate more details (pseudo code) for this?
i can add it at my next DP dsc patches.
>
>>> + uint32_t enc_id)
>>> {
>>> - int num_dsc = top->num_dsc;
>>> int i;
>>>
>>> - /* 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;
>>> + for (i = start; i < (PINGPONG_MAX - PINGPONG_0); i++) {
>>> + if (global_state->pingpong_to_enc_id[i] == enc_id)
>>> + return i;
>>> + }
>>> +
>>> + return -ENAVAIL;
>>> +}
>>> +
>>> +static int _dpu_rm_pingpong_dsc_check(int dsc_idx, int pp_idx)
>>> +{
>>> + /*
>>> + * DSC with even index must be used with the PINGPONG with even index
>>> + * DSC with odd index must be used with the PINGPONG with odd index
>>> + */
>>> + if ((dsc_idx & 0x01) != (pp_idx & 0x01))
>>> + return -ENAVAIL;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int _dpu_rm_dsc_alloc(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 pp_idx = 0;
>>> + int dsc_idx;
>>> + int ret;
>>> +
>>> + for (dsc_idx = 0; dsc_idx < ARRAY_SIZE(rm->dsc_blks) &&
>>> + num_dsc < top->num_dsc; dsc_idx++) {
>>> + if (!rm->dsc_blks[dsc_idx])
>>> + continue;
>>> +
>>> + if (reserved_by_other(global_state->dsc_to_enc_id, dsc_idx, enc_id))
>>> + continue;
>>> +
>>> + pp_idx = _dpu_rm_pingpong_next_index(global_state, pp_idx, enc_id);
>>> + if (pp_idx < 0)
>>> + return -ENAVAIL;
>>> +
>>> + ret = _dpu_rm_pingpong_dsc_check(dsc_idx, pp_idx);
>>> + if (ret)
>>> + return -ENAVAIL;
>>> +
>>> + global_state->dsc_to_enc_id[dsc_idx] = enc_id;
>>> + num_dsc++;
>>> + pp_idx++;
>>> + }
>>> +
>>> + if (num_dsc < top->num_dsc) {
>>> + DPU_ERROR("DSC allocation failed num_dsc=%d required=%d\n",
>>> + num_dsc, top->num_dsc);
>>> + return -ENAVAIL;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int _dpu_rm_dsc_alloc_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 dsc_idx, pp_idx = 0;
>>> + int ret;
>>> +
>>> + /* only start from even dsc index */
>>> + for (dsc_idx = 0; dsc_idx < ARRAY_SIZE(rm->dsc_blks) &&
>>> + num_dsc < top->num_dsc; dsc_idx += 2) {
>>> + if (!rm->dsc_blks[dsc_idx] ||
>>> + !rm->dsc_blks[dsc_idx + 1])
>>> + continue;
>>> +
>>> + /* consective dsc index to be paired */
>>> + if (reserved_by_other(global_state->dsc_to_enc_id, dsc_idx, enc_id) ||
>>> + reserved_by_other(global_state->dsc_to_enc_id, dsc_idx + 1, enc_id))
>>> + continue;
>>> +
>>> + pp_idx = _dpu_rm_pingpong_next_index(global_state, pp_idx, enc_id);
>>> + if (pp_idx < 0)
>>> + return -ENAVAIL;
>>> +
>>> + ret = _dpu_rm_pingpong_dsc_check(dsc_idx, pp_idx);
>>> + if (ret) {
>>> + pp_idx = 0;
>>> + continue;
>>> }
>>>
>>> - if (global_state->dsc_to_enc_id[i]) {
>>> - DPU_ERROR("DSC %d is already allocated\n", i);
>>> - return -EIO;
>>> + pp_idx = _dpu_rm_pingpong_next_index(global_state, pp_idx + 1, enc_id);
>>> + if (pp_idx < 0)
>>> + return -ENAVAIL;
>>> +
>>> + ret = _dpu_rm_pingpong_dsc_check(dsc_idx + 1, pp_idx);
>>> + if (ret) {
>>> + pp_idx = 0;
>>> + continue;
>>> }
>>> +
>>> + global_state->dsc_to_enc_id[dsc_idx] = enc_id;
>>> + global_state->dsc_to_enc_id[dsc_idx + 1] = enc_id;
>>> + num_dsc += 2;
>>> + pp_idx++; /* start for next pair */
>>> }
>>>
>>> - 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;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int _dpu_rm_reserve_dsc(struct dpu_rm *rm,
>>> + struct dpu_global_state *global_state,
>>> + struct drm_encoder *enc,
>>> + const struct msm_display_topology *top)
>>> +{
>>> + uint32_t enc_id = enc->base.id;
>>> +
>>> + if (!top->num_dsc || !top->num_intf)
>>> + return 0;
>>> +
>>> + /*
>>> + * Facts:
>>> + * 1) no pingpong split (two layer mixers shared one pingpong)
>>> + * 2) DSC pair starts from even index, such as index(0,1), (2,3), etc
>>> + * 3) even PINGPONG connects to even DSC
>>> + * 4) odd PINGPONG connects to odd DSC
>>> + * 5) 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_dsc_alloc_pair(rm, global_state, enc_id, top);
>>> + else
>>> + return _dpu_rm_dsc_alloc(rm, global_state, enc_id, top);
>>>
>>> return 0;
>>> }
>
>
Powered by blists - more mailing lists