Hi Stan
On Mon, 2023-03-06 at 10:04 +0200, Stanislav Lisovskiy wrote:
> We have currently the issue with some BPPs when using DSC.
> According the HW team the reason is that single VDSC engine instance,
> has some BW limitations which have to be accounted, so whenever
> we approach around 90% of the CDCLK, second VDSC engine have to be
> used. Also that always means using 2 slices, however in our current code
> amount of slices is calculated for some reason independently of
> whether we need to enable 2nd VDSC engine or not, thus leading to
> some logical issues, when according to pixel clock needs we need to enable
> 2nd VDSC engine, however as we calculated previously that we can use
> only single slice, we can't do that and fail.
> So we need to fix that, so that amount of VDSC engines enabled should depend
> on amount of slices and amount of slices should also depend on BW requirements.
> Lastly we didn't have BPP limitation for ADLP/MTL/DG2 implemented which says
> that DSC output BPP's can only be chosen within range of 8 to 27(BSpec 49259).
> This all applied together allows to fix existing FIFO underruns, which we
> have in many DSC tests.
>
> BSpec: 49259
> HSDES: 18027167222
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index aee93b0d810e..e3680ae95b83 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -687,6 +687,12 @@ u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp,
> u32 p
> /* From XE_LPD onwards we support from bpc upto uncompressed bpp-1 BPPs */
> if (DISPLAY_VER(i915) >= 13) {
> bits_per_pixel = min(bits_per_pixel, pipe_bpp - 1);
> +
> + /* According to BSpec, 27 is the max DSC output bpp */
> + bits_per_pixel = min(bits_per_pixel, (u32)27);
> +
> + /* According to BSpec, 8 is the min DSC output bpp */
> + bits_per_pixel = max(bits_per_pixel, (u32)8);
Just before this loop, there is check if if (bits_per_pixel < valid_dsc_bpp[0]) then it returns as
"unsupported BPP". The valid_dsc_bpp[0] is 6. Should this need to be updated to 8 for disp_ver >= 13
onward? Please check..
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@xxxxxxxxx>
> } else {
> /* Find the nearest match in the array of known BPPs from VESA */
> for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
> @@ -771,6 +777,9 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> min_slice_count = DIV_ROUND_UP(mode_clock,
> DP_DSC_MAX_ENC_THROUGHPUT_1);
>
> + if (mode_clock >= ((i915->display.cdclk.max_cdclk_freq * 85) / 100))
> + min_slice_count = max(min_slice_count, (u8)2);
> +
> max_slice_width = drm_dp_dsc_sink_max_slice_width(intel_dp->dsc_dpcd);
> if (max_slice_width < DP_DSC_MIN_SLICE_WIDTH_VALUE) {
> drm_dbg_kms(&i915->drm,
> @@ -1597,16 +1606,8 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> * is greater than the maximum Cdclock and if slice count is even
> * then we need to use 2 VDSC instances.
> */
> - if (adjusted_mode->crtc_clock > dev_priv->display.cdclk.max_cdclk_freq ||
> - pipe_config->bigjoiner_pipes) {
> - if (pipe_config->dsc.slice_count > 1) {
> - pipe_config->dsc.dsc_split = true;
> - } else {
> - drm_dbg_kms(&dev_priv->drm,
> - "Cannot split stream to use 2 VDSC instances\n");
> - return -EINVAL;
> - }
> - }
> + if (pipe_config->bigjoiner_pipes || pipe_config->dsc.slice_count > 1)
> + pipe_config->dsc.dsc_split = true;
>
> ret = intel_dp_dsc_compute_params(&dig_port->base, pipe_config);
> if (ret < 0) {
