diff mbox series

[6/9] drm/i915/dsc/mtl: Add support for fractional bpp

Message ID 20230822121033.597447-7-ankit.k.nautiyal@intel.com (mailing list archive)
State New, archived
Headers show
Series Add DSC fractional bpp support | expand

Commit Message

Nautiyal, Ankit K Aug. 22, 2023, 12:10 p.m. UTC
From: Vandita Kulkarni <vandita.kulkarni@intel.com>

Consider the fractional bpp while reading the qp values.

v2: Use helpers for fractional, integral bits of bits_per_pixel.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 .../gpu/drm/i915/display/intel_qp_tables.c    |  3 ---
 drivers/gpu/drm/i915/display/intel_vdsc.c     | 23 +++++++++++++++----
 2 files changed, 18 insertions(+), 8 deletions(-)

Comments

Suraj Kandpal Aug. 26, 2023, 6:18 p.m. UTC | #1
> Subject: [Intel-gfx] [PATCH 6/9] drm/i915/dsc/mtl: Add support for fractional
> bpp
> 
> From: Vandita Kulkarni <vandita.kulkarni@intel.com>
> 
> Consider the fractional bpp while reading the qp values.
> 
> v2: Use helpers for fractional, integral bits of bits_per_pixel.
> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_qp_tables.c    |  3 ---
>  drivers/gpu/drm/i915/display/intel_vdsc.c     | 23 +++++++++++++++----
>  2 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_qp_tables.c
> b/drivers/gpu/drm/i915/display/intel_qp_tables.c
> index 543cdc46aa1d..600c815e37e4 100644
> --- a/drivers/gpu/drm/i915/display/intel_qp_tables.c
> +++ b/drivers/gpu/drm/i915/display/intel_qp_tables.c
> @@ -34,9 +34,6 @@
>   * These qp tables are as per the C model
>   * and it has the rows pointing to bpps which increment
>   * in steps of 0.5
> - * We do not support fractional bpps as of today,
> - * hence we would skip the fractional bpps during
> - * our references for qp calclulations.
>   */
>  static const u8
> rc_range_minqp444_8bpc[DSC_NUM_BUF_RANGES][RC_RANGE_QP444_8BPC
> _MAX_NUM_BPP] = {
>  	{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, diff --git
> a/drivers/gpu/drm/i915/display/intel_vdsc.c
> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 2dc6ea82c024..4bd570fb0ab2 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -77,8 +77,9 @@ intel_vdsc_set_min_max_qp(struct drm_dsc_config
> *vdsc_cfg, int buf,  static void  calculate_rc_params(struct drm_dsc_config
> *vdsc_cfg)  {
> +	int fractional_bits = dsc_fractional_compressed_bpp(vdsc_cfg-
> >bits_per_pixel);
> +	int bpp = dsc_integral_compressed_bpp(vdsc_cfg->bits_per_pixel);
>  	int bpc = vdsc_cfg->bits_per_component;
> -	int bpp = vdsc_cfg->bits_per_pixel >> 4;
>  	int qp_bpc_modifier = (bpc - 8) * 2;
>  	int uncompressed_bpg_rate;
>  	int first_line_bpg_offset;
> @@ -148,7 +149,13 @@ calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
>  		static const s8 ofs_und8[] = {
>  			10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12, -12
>  		};
> -
> +	/*
> +	 * For 420 format since bits_per_pixel (bpp) is set to target bpp * 2,
> +	 * QP table values for target bpp 4.0 to 4.4375 (rounded to 4.0) are
> +	 * actually for bpp 8 to 8.875 (rounded to 4.0 * 2 i.e 8).
> +	 * Similarly values for target bpp 4.5 to 4.8375 (rounded to 4.5)
> +	 * are for bpp 9 to 9.875 (rounded to 4.5 * 2 i.e 9), and so on.
> +	 */
>  		bpp_i  = bpp - 8;
>  		for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) {
>  			u8 range_bpg_offset;
> @@ -191,7 +198,14 @@ calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
>  			10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12, -12
>  		};
> 
> -		bpp_i  = (2 * (bpp - 6));
> +		/*
> +		 * QP table rows have values in increment of 0.5.
> +		 * So 6.0 bpp to 6.4375 will have index 0, 6.5 to 6.9375 will
> have index 1,
> +		 * and so on.
> +		 * 0.5 represented as 0x8 in U6.4 format.
> +		 */
> +		bpp_i  = ((bpp - 6) + (fractional_bits < 0x8 ? 0 : 1));

Can we have a the 0x8 as a #define rather than a direct comparison to 0x8

Also isn't what was previously present doing the same thing
Sure fractional bits weren't taken into consideration but they would still fall in the same
Index. Anyways the bpp taken is the integer part so I thing no change is required to the formula.

> +
>  		for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) {
>  			u8 range_bpg_offset;
> 
> @@ -279,8 +293,7 @@ int intel_dsc_compute_params(struct intel_crtc_state
> *pipe_config)
>  	/* Gen 11 does not support VBR */
>  	vdsc_cfg->vbr_enable = false;
> 
> -	/* Gen 11 only supports integral values of bpp */
> -	vdsc_cfg->bits_per_pixel = compressed_bpp << 4;
> +	vdsc_cfg->bits_per_pixel = pipe_config->dsc.compressed_bpp;
> 
>  	/*
>  	 * According to DSC 1.2 specs in Section 4.1 if native_420 is set
> --
> 2.40.1
Suraj Kandpal Aug. 28, 2023, 2:16 a.m. UTC | #2
> Subject: RE: [Intel-gfx] [PATCH 6/9] drm/i915/dsc/mtl: Add support for fractional
> bpp
> 
> > Subject: [Intel-gfx] [PATCH 6/9] drm/i915/dsc/mtl: Add support for
> > fractional bpp
> >
> > From: Vandita Kulkarni <vandita.kulkarni@intel.com>
> >
> > Consider the fractional bpp while reading the qp values.
> >
> > v2: Use helpers for fractional, integral bits of bits_per_pixel.
> >
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > ---
> >  .../gpu/drm/i915/display/intel_qp_tables.c    |  3 ---
> >  drivers/gpu/drm/i915/display/intel_vdsc.c     | 23 +++++++++++++++----
> >  2 files changed, 18 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_qp_tables.c
> > b/drivers/gpu/drm/i915/display/intel_qp_tables.c
> > index 543cdc46aa1d..600c815e37e4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_qp_tables.c
> > +++ b/drivers/gpu/drm/i915/display/intel_qp_tables.c
> > @@ -34,9 +34,6 @@
> >   * These qp tables are as per the C model
> >   * and it has the rows pointing to bpps which increment
> >   * in steps of 0.5
> > - * We do not support fractional bpps as of today,
> > - * hence we would skip the fractional bpps during
> > - * our references for qp calclulations.
> >   */
> >  static const u8
> >
> rc_range_minqp444_8bpc[DSC_NUM_BUF_RANGES][RC_RANGE_QP444_8BPC
> > _MAX_NUM_BPP] = {
> >  	{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > 0, diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index 2dc6ea82c024..4bd570fb0ab2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -77,8 +77,9 @@ intel_vdsc_set_min_max_qp(struct drm_dsc_config
> > *vdsc_cfg, int buf,  static void  calculate_rc_params(struct
> > drm_dsc_config
> > *vdsc_cfg)  {
> > +	int fractional_bits = dsc_fractional_compressed_bpp(vdsc_cfg-
> > >bits_per_pixel);
> > +	int bpp = dsc_integral_compressed_bpp(vdsc_cfg->bits_per_pixel);
> >  	int bpc = vdsc_cfg->bits_per_component;
> > -	int bpp = vdsc_cfg->bits_per_pixel >> 4;
> >  	int qp_bpc_modifier = (bpc - 8) * 2;
> >  	int uncompressed_bpg_rate;
> >  	int first_line_bpg_offset;
> > @@ -148,7 +149,13 @@ calculate_rc_params(struct drm_dsc_config
> *vdsc_cfg)
> >  		static const s8 ofs_und8[] = {
> >  			10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12, -12
> >  		};
> > -
> > +	/*
> > +	 * For 420 format since bits_per_pixel (bpp) is set to target bpp * 2,
> > +	 * QP table values for target bpp 4.0 to 4.4375 (rounded to 4.0) are
> > +	 * actually for bpp 8 to 8.875 (rounded to 4.0 * 2 i.e 8).
> > +	 * Similarly values for target bpp 4.5 to 4.8375 (rounded to 4.5)
> > +	 * are for bpp 9 to 9.875 (rounded to 4.5 * 2 i.e 9), and so on.
> > +	 */
> >  		bpp_i  = bpp - 8;
> >  		for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) {
> >  			u8 range_bpg_offset;
> > @@ -191,7 +198,14 @@ calculate_rc_params(struct drm_dsc_config
> *vdsc_cfg)
> >  			10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12, -12
> >  		};
> >
> > -		bpp_i  = (2 * (bpp - 6));
> > +		/*
> > +		 * QP table rows have values in increment of 0.5.
> > +		 * So 6.0 bpp to 6.4375 will have index 0, 6.5 to 6.9375 will
> > have index 1,
> > +		 * and so on.
> > +		 * 0.5 represented as 0x8 in U6.4 format.
> > +		 */
> > +		bpp_i  = ((bpp - 6) + (fractional_bits < 0x8 ? 0 : 1));
> 
> Can we have a the 0x8 as a #define rather than a direct comparison to 0x8
> 

Also maybe we can have a function/macro which takes in the integral and fractional
part and spits out the closest integer
something like DIV_ROUND_CLOSEST but that won't directly work with this case
but I do think we can repurpose it for our use case.

> Also isn't what was previously present doing the same thing Sure fractional bits
> weren't taken into consideration but they would still fall in the same Index.
> Anyways the bpp taken is the integer part so I thing no change is required to
> the formula.

Also scratch this comment I had another looks and seems logical to me what we are doing.

Regards,
Suraj Kandpal

> 
> > +
> >  		for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) {
> >  			u8 range_bpg_offset;
> >
> > @@ -279,8 +293,7 @@ int intel_dsc_compute_params(struct
> > intel_crtc_state
> > *pipe_config)
> >  	/* Gen 11 does not support VBR */
> >  	vdsc_cfg->vbr_enable = false;
> >
> > -	/* Gen 11 only supports integral values of bpp */
> > -	vdsc_cfg->bits_per_pixel = compressed_bpp << 4;
> > +	vdsc_cfg->bits_per_pixel = pipe_config->dsc.compressed_bpp;
> >
> >  	/*
> >  	 * According to DSC 1.2 specs in Section 4.1 if native_420 is set
> > --
> > 2.40.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_qp_tables.c b/drivers/gpu/drm/i915/display/intel_qp_tables.c
index 543cdc46aa1d..600c815e37e4 100644
--- a/drivers/gpu/drm/i915/display/intel_qp_tables.c
+++ b/drivers/gpu/drm/i915/display/intel_qp_tables.c
@@ -34,9 +34,6 @@ 
  * These qp tables are as per the C model
  * and it has the rows pointing to bpps which increment
  * in steps of 0.5
- * We do not support fractional bpps as of today,
- * hence we would skip the fractional bpps during
- * our references for qp calclulations.
  */
 static const u8 rc_range_minqp444_8bpc[DSC_NUM_BUF_RANGES][RC_RANGE_QP444_8BPC_MAX_NUM_BPP] = {
 	{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 2dc6ea82c024..4bd570fb0ab2 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -77,8 +77,9 @@  intel_vdsc_set_min_max_qp(struct drm_dsc_config *vdsc_cfg, int buf,
 static void
 calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
 {
+	int fractional_bits = dsc_fractional_compressed_bpp(vdsc_cfg->bits_per_pixel);
+	int bpp = dsc_integral_compressed_bpp(vdsc_cfg->bits_per_pixel);
 	int bpc = vdsc_cfg->bits_per_component;
-	int bpp = vdsc_cfg->bits_per_pixel >> 4;
 	int qp_bpc_modifier = (bpc - 8) * 2;
 	int uncompressed_bpg_rate;
 	int first_line_bpg_offset;
@@ -148,7 +149,13 @@  calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
 		static const s8 ofs_und8[] = {
 			10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12, -12
 		};
-
+	/*
+	 * For 420 format since bits_per_pixel (bpp) is set to target bpp * 2,
+	 * QP table values for target bpp 4.0 to 4.4375 (rounded to 4.0) are
+	 * actually for bpp 8 to 8.875 (rounded to 4.0 * 2 i.e 8).
+	 * Similarly values for target bpp 4.5 to 4.8375 (rounded to 4.5)
+	 * are for bpp 9 to 9.875 (rounded to 4.5 * 2 i.e 9), and so on.
+	 */
 		bpp_i  = bpp - 8;
 		for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) {
 			u8 range_bpg_offset;
@@ -191,7 +198,14 @@  calculate_rc_params(struct drm_dsc_config *vdsc_cfg)
 			10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12, -12
 		};
 
-		bpp_i  = (2 * (bpp - 6));
+		/*
+		 * QP table rows have values in increment of 0.5.
+		 * So 6.0 bpp to 6.4375 will have index 0, 6.5 to 6.9375 will have index 1,
+		 * and so on.
+		 * 0.5 represented as 0x8 in U6.4 format.
+		 */
+		bpp_i  = ((bpp - 6) + (fractional_bits < 0x8 ? 0 : 1));
+
 		for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) {
 			u8 range_bpg_offset;
 
@@ -279,8 +293,7 @@  int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
 	/* Gen 11 does not support VBR */
 	vdsc_cfg->vbr_enable = false;
 
-	/* Gen 11 only supports integral values of bpp */
-	vdsc_cfg->bits_per_pixel = compressed_bpp << 4;
+	vdsc_cfg->bits_per_pixel = pipe_config->dsc.compressed_bpp;
 
 	/*
 	 * According to DSC 1.2 specs in Section 4.1 if native_420 is set