| Message ID | 20230228113342.2051425-2-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: move DSC RC tables to drm_dsc_helper.c | expand |
On Tue, 28 Feb 2023, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > After cross-checking DSC models (20150914, 20161212, 20210623) change > values in rc_parameters tables to follow config files present inside > the DSC model. Handle two places, where i915 tables diverged from the > model, by patching the rc values in the code. > > Note: I left one case uncorrected, 8bpp/10bpc/range_max_qp[0], because > the table in the VESA DSC 1.1 sets it to 4. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/i915/display/intel_vdsc.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c > index 207b2a648d32..d080741fd0b3 100644 > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > @@ -86,7 +86,7 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { > } > }, > /* 6BPP/14BPC */ > - { 768, 15, 6144, 15, 25, 23, 27, { > + { 768, 15, 6144, 15, 25, 23, 23, { > { 0, 16, 0 }, { 7, 18, -2 }, { 15, 20, -2 }, { 16, 20, -4 }, > { 17, 21, -6 }, { 17, 21, -6 }, { 18, 21, -6 }, { 18, 22, -8 }, > { 19, 23, -8 }, { 20, 24, -10 }, { 21, 24, -10 }, > @@ -115,6 +115,10 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { > }, > /* 8BPP/10BPC */ > { 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 }, > @@ -132,7 +136,7 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { > }, > /* 8BPP/14BPC */ > { 512, 12, 6144, 15, 24, 23, 23, { > - { 0, 12, 0 }, { 5, 13, 0 }, { 11, 15, 0 }, { 12, 17, -2 }, > + { 0, 12, 2 }, { 5, 13, 0 }, { 11, 15, 0 }, { 12, 17, -2 }, > { 15, 19, -4 }, { 15, 19, -6 }, { 15, 19, -8 }, { 15, 20, -8 }, > { 15, 21, -8 }, { 15, 22, -10 }, { 17, 22, -10 }, > { 17, 23, -12 }, { 17, 23, -12 }, { 21, 24, -12 }, > @@ -529,6 +533,16 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config) > DSC_RANGE_BPG_OFFSET_MASK; > } > > + if (DISPLAY_VER(dev_priv) < 13) { > + if (compressed_bpp == 6 && > + vdsc_cfg->bits_per_component == 8) > + vdsc_cfg->rc_quant_incr_limit1 = 23; > + > + if (compressed_bpp == 8 && > + vdsc_cfg->bits_per_component == 14) > + vdsc_cfg->rc_range_params[0].range_bpg_offset = 0; > + } > + I wonder if we shouldn't just use the updated values... Maybe add a FIXME comment above the block to consider removing it? Reviewed-by: Jani Nikula <jani.nikula@intel.com> > /* > * BitsPerComponent value determines mux_word_size: > * When BitsPerComponent is less than or 10bpc, muxWordSize will be equal to
On 28/02/2023 17:56, Jani Nikula wrote: > On Tue, 28 Feb 2023, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: >> After cross-checking DSC models (20150914, 20161212, 20210623) change >> values in rc_parameters tables to follow config files present inside >> the DSC model. Handle two places, where i915 tables diverged from the >> model, by patching the rc values in the code. >> >> Note: I left one case uncorrected, 8bpp/10bpc/range_max_qp[0], because >> the table in the VESA DSC 1.1 sets it to 4. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/gpu/drm/i915/display/intel_vdsc.c | 18 ++++++++++++++++-- >> 1 file changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c >> index 207b2a648d32..d080741fd0b3 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c >> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c >> @@ -86,7 +86,7 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { >> } >> }, >> /* 6BPP/14BPC */ >> - { 768, 15, 6144, 15, 25, 23, 27, { >> + { 768, 15, 6144, 15, 25, 23, 23, { >> { 0, 16, 0 }, { 7, 18, -2 }, { 15, 20, -2 }, { 16, 20, -4 }, >> { 17, 21, -6 }, { 17, 21, -6 }, { 18, 21, -6 }, { 18, 22, -8 }, >> { 19, 23, -8 }, { 20, 24, -10 }, { 21, 24, -10 }, >> @@ -115,6 +115,10 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { >> }, >> /* 8BPP/10BPC */ >> { 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 }, >> @@ -132,7 +136,7 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { >> }, >> /* 8BPP/14BPC */ >> { 512, 12, 6144, 15, 24, 23, 23, { >> - { 0, 12, 0 }, { 5, 13, 0 }, { 11, 15, 0 }, { 12, 17, -2 }, >> + { 0, 12, 2 }, { 5, 13, 0 }, { 11, 15, 0 }, { 12, 17, -2 }, >> { 15, 19, -4 }, { 15, 19, -6 }, { 15, 19, -8 }, { 15, 20, -8 }, >> { 15, 21, -8 }, { 15, 22, -10 }, { 17, 22, -10 }, >> { 17, 23, -12 }, { 17, 23, -12 }, { 21, 24, -12 }, >> @@ -529,6 +533,16 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config) >> DSC_RANGE_BPG_OFFSET_MASK; >> } >> >> + if (DISPLAY_VER(dev_priv) < 13) { >> + if (compressed_bpp == 6 && >> + vdsc_cfg->bits_per_component == 8) >> + vdsc_cfg->rc_quant_incr_limit1 = 23; >> + >> + if (compressed_bpp == 8 && >> + vdsc_cfg->bits_per_component == 14) >> + vdsc_cfg->rc_range_params[0].range_bpg_offset = 0; >> + } >> + > > I wonder if we shouldn't just use the updated values... I also wondered about this, so I wanted to get a double check from somebody having better knowledge of this part, if it is a typo in the original patch or a typo in the cfg files. E.g. the pre_scr_cfg_files_for_reference/rc_10bpc_8bpp.cfg has 8 as RX_MAXQP[0], which (for me) looks like a typo in the CFG file itself, rather than being a typo in the driver. On the other hand, these two issues belong to the 'current' CFG files, so they, most probably, received more attention from anybody working with the standard and with the model. I can change this patch to become a fix for the tables (dropping the if clause), if you can confirm that these values are typos in the driver. > > Maybe add a FIXME comment above the block to consider removing it? > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > >> /* >> * BitsPerComponent value determines mux_word_size: >> * When BitsPerComponent is less than or 10bpc, muxWordSize will be equal to >
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c index 207b2a648d32..d080741fd0b3 100644 --- a/drivers/gpu/drm/i915/display/intel_vdsc.c +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c @@ -86,7 +86,7 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { } }, /* 6BPP/14BPC */ - { 768, 15, 6144, 15, 25, 23, 27, { + { 768, 15, 6144, 15, 25, 23, 23, { { 0, 16, 0 }, { 7, 18, -2 }, { 15, 20, -2 }, { 16, 20, -4 }, { 17, 21, -6 }, { 17, 21, -6 }, { 18, 21, -6 }, { 18, 22, -8 }, { 19, 23, -8 }, { 20, 24, -10 }, { 21, 24, -10 }, @@ -115,6 +115,10 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { }, /* 8BPP/10BPC */ { 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 }, @@ -132,7 +136,7 @@ static const struct rc_parameters rc_parameters[][MAX_COLUMN_INDEX] = { }, /* 8BPP/14BPC */ { 512, 12, 6144, 15, 24, 23, 23, { - { 0, 12, 0 }, { 5, 13, 0 }, { 11, 15, 0 }, { 12, 17, -2 }, + { 0, 12, 2 }, { 5, 13, 0 }, { 11, 15, 0 }, { 12, 17, -2 }, { 15, 19, -4 }, { 15, 19, -6 }, { 15, 19, -8 }, { 15, 20, -8 }, { 15, 21, -8 }, { 15, 22, -10 }, { 17, 22, -10 }, { 17, 23, -12 }, { 17, 23, -12 }, { 21, 24, -12 }, @@ -529,6 +533,16 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config) DSC_RANGE_BPG_OFFSET_MASK; } + if (DISPLAY_VER(dev_priv) < 13) { + if (compressed_bpp == 6 && + vdsc_cfg->bits_per_component == 8) + vdsc_cfg->rc_quant_incr_limit1 = 23; + + if (compressed_bpp == 8 && + vdsc_cfg->bits_per_component == 14) + vdsc_cfg->rc_range_params[0].range_bpg_offset = 0; + } + /* * BitsPerComponent value determines mux_word_size: * When BitsPerComponent is less than or 10bpc, muxWordSize will be equal to
After cross-checking DSC models (20150914, 20161212, 20210623) change values in rc_parameters tables to follow config files present inside the DSC model. Handle two places, where i915 tables diverged from the model, by patching the rc values in the code. Note: I left one case uncorrected, 8bpp/10bpc/range_max_qp[0], because the table in the VESA DSC 1.1 sets it to 4. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/i915/display/intel_vdsc.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)