Re: [Intel-gfx] [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters

2023-05-16 Thread Kandpal, Suraj
> 
> The array of rc_parameters contains a mixture of parameters from DSC 1.1
> and DSC 1.2 standards. Split these tow configuration arrays in preparation to
> adding more configuration data.
> 

Hi ,
Needed to add some more comments apart from the previous ones already given

> Signed-off-by: Dmitry Baryshkov 
> ---
>  drivers/gpu/drm/display/drm_dsc_helper.c  | 127 ++
> drivers/gpu/drm/i915/display/intel_vdsc.c |  10 +-
>  include/drm/display/drm_dsc_helper.h  |   7 +-
>  3 files changed, 119 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index acb93d4116e0..35b39f3109c4 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -324,11 +324,81 @@ struct rc_parameters_data {
> 
>  #define DSC_BPP(bpp) ((bpp) << 4)
> 
> +static const struct rc_parameters_data rc_parameters_pre_scr[] = {
> + {
> + .bpp = DSC_BPP(8), .bpc = 8,
> + { 512, 12, 6144, 3, 12, 11, 11, {
> + { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12, 
> -12 },
> + { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 10,
> + { 512, 12, 6144, 7, 16, 15, 15, {
> + /*
> +  * DSC model/pre-SCR-cfg has 8 for
> range_max_qp[0], however
> +  * VESA DSC 1.1 Table E-5 sets it to 4.
> +  */
> + { 0, 4, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12, 
> -8 },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16, 
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 12,
> + { 512, 12, 6144, 11, 20, 19, 19, {
> + { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11, 
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 8,
> + { 341, 15, 2048, 3, 12, 11, 11, {
> + { 0, 2, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 },
> + { 5, 12, -12 }, { 5, 13, -12 }, { 7, 13, -12 }, { 13, 
> 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 10,
> + { 341, 15, 2048, 7, 16, 15, 15, {
> + { 0, 2, 2 }, { 2, 5, 0 }, { 3, 7, 0 }, { 4, 8, -2 },
> + { 6, 9, -4 }, { 7, 10, -6 }, { 7, 11, -8 }, { 7, 12, -8 
> },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16, 
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 12,
> + { 341, 15, 2048, 11, 20, 19, 19, {
> + { 0, 6, 2 }, { 4, 9, 0 }, { 7, 11, 0 }, { 8, 12, -2 },
> + { 10, 13, -4 }, { 11, 14, -6 }, { 11, 15, -8 }, { 11, 
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + { /* sentinel */ }
> +};
> +
>  /*
>   * Selected Rate Control Related Parameter Recommended Values
>   * from DSC_v1.11 spec & C Model release: DSC_model_20161212
>   */
> -static const struct rc_parameters_data rc_parameters[] = {
> +static const struct rc_parameters_data rc_parameters_1_2_444[] = {
>   {
>   .bpp = DSC_BPP(6), .bpc = 8,
>   { 768, 15, 6144, 3, 13, 11, 11, {
> @@ -388,22 +458,18 @@ static const struct rc_parameters_data
> rc_parameters[] = {
>   { 512, 12, 6144, 3, 12, 11, 11, {
>   { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
>   { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> - { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12, 
> -12 },
> - { 5, 13, -12 }, { 7, 13, -12 

Re: [Intel-gfx] [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters

2023-05-16 Thread Kandpal, Suraj
> Subject: [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR)
> parameters
> 
> The array of rc_parameters contains a mixture of parameters from DSC 1.1
> and DSC 1.2 standards. Split these tow configuration arrays in preparation to
> adding more configuration data.
> 
> Signed-off-by: Dmitry Baryshkov 
> ---
>  drivers/gpu/drm/display/drm_dsc_helper.c  | 127 ++
> drivers/gpu/drm/i915/display/intel_vdsc.c |  10 +-
>  include/drm/display/drm_dsc_helper.h  |   7 +-
>  3 files changed, 119 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index acb93d4116e0..35b39f3109c4 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -324,11 +324,81 @@ struct rc_parameters_data {
> 
>  #define DSC_BPP(bpp) ((bpp) << 4)
> 

Maybe  comment here mentioning the DSC version and the C Model
we follow would be useful

> +static const struct rc_parameters_data rc_parameters_pre_scr[] = {
> + {
> + .bpp = DSC_BPP(8), .bpc = 8,
> + { 512, 12, 6144, 3, 12, 11, 11, {
> + { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12, 
> -12 },
> + { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 10,
> + { 512, 12, 6144, 7, 16, 15, 15, {
> + /*
> +  * DSC model/pre-SCR-cfg has 8 for
> range_max_qp[0], however
> +  * VESA DSC 1.1 Table E-5 sets it to 4.
> +  */
> + { 0, 4, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
> + { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12, 
> -8 },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16, 
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(8), .bpc = 12,
> + { 512, 12, 6144, 11, 20, 19, 19, {
> + { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
> + { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11, 
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 8,
> + { 341, 15, 2048, 3, 12, 11, 11, {
> + { 0, 2, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
> + { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
> + { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 },
> + { 5, 12, -12 }, { 5, 13, -12 }, { 7, 13, -12 }, { 13, 
> 15, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 10,
> + { 341, 15, 2048, 7, 16, 15, 15, {
> + { 0, 2, 2 }, { 2, 5, 0 }, { 3, 7, 0 }, { 4, 8, -2 },
> + { 6, 9, -4 }, { 7, 10, -6 }, { 7, 11, -8 }, { 7, 12, -8 
> },
> + { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16, 
> -12 },
> + { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
> + }
> + }
> + },
> + {
> + .bpp = DSC_BPP(12), .bpc = 12,
> + { 341, 15, 2048, 11, 20, 19, 19, {
> + { 0, 6, 2 }, { 4, 9, 0 }, { 7, 11, 0 }, { 8, 12, -2 },
> + { 10, 13, -4 }, { 11, 14, -6 }, { 11, 15, -8 }, { 11, 
> 16, -8 },
> + { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
> + { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
> + { 21, 23, -12 }
> + }
> + }
> + },
> + { /* sentinel */ }
> +};
> +
>  /*
>   * Selected Rate Control Related Parameter Recommended Values
>   * from DSC_v1.11 spec & C Model release: DSC_model_20161212
>   */

The above comment shouldn't be above this function anymore since
This represent dsc_v1.2 I presume maybe move this comment above
and add a new comment for this function.

> -static const struct rc_parameters_data rc_parameters[] = {
> +static const struct rc_parameters_data rc_parameters_1_2_444[] = {
>   {
>   .bpp = DSC_BPP(6), .bpc = 8,
>   { 768, 15, 6144, 3, 13, 11, 11, {
> @@ -388,22 +458,18 @@ static const struct rc_parameters_data
> rc_parameters[] = {
>   { 512, 12, 6144, 3, 12, 11, 11, {
>   { 

[Intel-gfx] [PATCH v5 6/8] drm/display/dsc: split DSC 1.2 and DSC 1.1 (pre-SCR) parameters

2023-05-04 Thread Dmitry Baryshkov
The array of rc_parameters contains a mixture of parameters from DSC 1.1
and DSC 1.2 standards. Split these tow configuration arrays in
preparation to adding more configuration data.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/display/drm_dsc_helper.c  | 127 ++
 drivers/gpu/drm/i915/display/intel_vdsc.c |  10 +-
 include/drm/display/drm_dsc_helper.h  |   7 +-
 3 files changed, 119 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c 
b/drivers/gpu/drm/display/drm_dsc_helper.c
index acb93d4116e0..35b39f3109c4 100644
--- a/drivers/gpu/drm/display/drm_dsc_helper.c
+++ b/drivers/gpu/drm/display/drm_dsc_helper.c
@@ -324,11 +324,81 @@ struct rc_parameters_data {
 
 #define DSC_BPP(bpp)   ((bpp) << 4)
 
+static const struct rc_parameters_data rc_parameters_pre_scr[] = {
+   {
+   .bpp = DSC_BPP(8), .bpc = 8,
+   { 512, 12, 6144, 3, 12, 11, 11, {
+   { 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
+   { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
+   { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12, 
-12 },
+   { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
+   }
+   }
+   },
+   {
+   .bpp = DSC_BPP(8), .bpc = 10,
+   { 512, 12, 6144, 7, 16, 15, 15, {
+   /*
+* DSC model/pre-SCR-cfg has 8 for range_max_qp[0], 
however
+* VESA DSC 1.1 Table E-5 sets it to 4.
+*/
+   { 0, 4, 2 }, { 4, 8, 0 }, { 5, 9, 0 }, { 5, 10, -2 },
+   { 7, 11, -4 }, { 7, 11, -6 }, { 7, 11, -8 }, { 7, 12, 
-8 },
+   { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16, 
-12 },
+   { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
+   }
+   }
+   },
+   {
+   .bpp = DSC_BPP(8), .bpc = 12,
+   { 512, 12, 6144, 11, 20, 19, 19, {
+   { 0, 12, 2 }, { 4, 12, 0 }, { 9, 13, 0 }, { 9, 14, -2 },
+   { 11, 15, -4 }, { 11, 15, -6 }, { 11, 15, -8 }, { 11, 
16, -8 },
+   { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
+   { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
+   { 21, 23, -12 }
+   }
+   }
+   },
+   {
+   .bpp = DSC_BPP(12), .bpc = 8,
+   { 341, 15, 2048, 3, 12, 11, 11, {
+   { 0, 2, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
+   { 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
+   { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 },
+   { 5, 12, -12 }, { 5, 13, -12 }, { 7, 13, -12 }, { 13, 
15, -12 }
+   }
+   }
+   },
+   {
+   .bpp = DSC_BPP(12), .bpc = 10,
+   { 341, 15, 2048, 7, 16, 15, 15, {
+   { 0, 2, 2 }, { 2, 5, 0 }, { 3, 7, 0 }, { 4, 8, -2 },
+   { 6, 9, -4 }, { 7, 10, -6 }, { 7, 11, -8 }, { 7, 12, -8 
},
+   { 7, 13, -8 }, { 7, 14, -10 }, { 9, 15, -10 }, { 9, 16, 
-12 },
+   { 9, 17, -12 }, { 11, 17, -12 }, { 17, 19, -12 }
+   }
+   }
+   },
+   {
+   .bpp = DSC_BPP(12), .bpc = 12,
+   { 341, 15, 2048, 11, 20, 19, 19, {
+   { 0, 6, 2 }, { 4, 9, 0 }, { 7, 11, 0 }, { 8, 12, -2 },
+   { 10, 13, -4 }, { 11, 14, -6 }, { 11, 15, -8 }, { 11, 
16, -8 },
+   { 11, 17, -8 }, { 11, 18, -10 }, { 13, 19, -10 },
+   { 13, 20, -12 }, { 13, 21, -12 }, { 15, 21, -12 },
+   { 21, 23, -12 }
+   }
+   }
+   },
+   { /* sentinel */ }
+};
+
 /*
  * Selected Rate Control Related Parameter Recommended Values
  * from DSC_v1.11 spec & C Model release: DSC_model_20161212
  */
-static const struct rc_parameters_data rc_parameters[] = {
+static const struct rc_parameters_data rc_parameters_1_2_444[] = {
{
.bpp = DSC_BPP(6), .bpc = 8,
{ 768, 15, 6144, 3, 13, 11, 11, {
@@ -388,22 +458,18 @@ static const struct rc_parameters_data rc_parameters[] = {
{ 512, 12, 6144, 3, 12, 11, 11, {
{ 0, 4, 2 }, { 0, 4, 0 }, { 1, 5, 0 }, { 1, 6, -2 },
{ 3, 7, -4 }, { 3, 7, -6 }, { 3, 7, -8 }, { 3, 8, -8 },
-   { 3, 9, -8 }, { 3, 10, -10 }, { 5, 11, -10 }, { 5, 12, 
-12 },
-   { 5, 13, -12 }, { 7, 13, -12 }, { 13, 15, -12 }
+   { 3, 9, -8 }, { 3, 10, -10 }, { 5, 10, -10 }, { 5, 11, 
-12 },
+   { 5, 11, -12 }, { 9, 12, -12 },