> Subject: RE: [PATCH 6/9] drm/i915/dsc/mtl: Add support for fractional
> bpp
>
> > Subject: [PATCH 6/9] drm/i915/dsc/mtl: Add support for
> > fractional bpp
> >
> > From: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> > ---
> > .../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
