[v6] drm/msm/dpu: improve DSC allocation

  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@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 154 +++++++++++++++++++++++++++++----
 1 file changed, 139 insertions(+), 15 deletions(-)
  

Hi Dmitry,

Anymore comments from you?



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@quicinc.com>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 154 +++++++++++++++++++++++++++++----
>   1 file changed, 139 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 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,
> +				       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;
>   }
  

On Tue, 19 Dec 2023 at 18:18, Kuogee Hsieh <quic_khsieh@quicinc.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@quicinc.com>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 154 +++++++++++++++++++++++++++++----
> >   1 file changed, 139 insertions(+), 15 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.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.

> > +                                    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;
> >   }
  

On 12/19/2023 2:32 PM, Dmitry Baryshkov wrote:
> On Tue, 19 Dec 2023 at 18:18, Kuogee Hsieh <quic_khsieh@quicinc.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@quicinc.com>
>>> ---
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 154 +++++++++++++++++++++++++++++----
>>>    1 file changed, 139 insertions(+), 15 deletions(-)
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.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;
>>>    }
>
>
  

On Thu, 14 Dec 2023 10:56:12 -0800, 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.
> 
> [...]

Applied, thanks!

[1/1] drm/msm/dpu: improve DSC allocation
      https://gitlab.freedesktop.org/lumag/msm/-/commit/858ddb64f1ff

Best regards,