| 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 |
> 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
> 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 --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