On Thu, 02 Feb 2023, Suraj Kandpal <suraj.kandpal@xxxxxxxxx> wrote:
> According VDSC spec 1.2a Section 3.8 Options for Slice
> implies that 108 lines is an optimal slice height, but any
> size can be used as long as vertical active
> integer multiple and maximum vertical slice count requirements are met.
>
> Bspec: 49259
>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> Cc: Swati Sharma <swati2.sharma@xxxxxxxxx>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 62cbab7402e9..cb4fbcd935db 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1415,6 +1415,30 @@ static int intel_dp_sink_dsc_version_minor(struct intel_dp *intel_dp)
> DP_DSC_MINOR_SHIFT;
> }
>
> +static int intel_dp_get_slice_height(int vactive)
> +{
> + int slice_height;
> +
> + /*
> + * VDSC1.2a spec in Section 3.8 Options for Slices implies that
> + * 108 lines is an optimal slice height,
> + * but any size can be used as long as vertical active integer
> + * multiple and maximum vertical slice count requirements are met.
> + */
> + for (slice_height = 108; slice_height <= vactive; slice_height += 2)
> + if (vactive % slice_height == 0)
> + return slice_height;
I realize now that this loop will always find the slice_height as it
iterates to vactive inclusive. vactive % slice_height == 0 if
slice_height == vactive.
I did a bit of scripting on this. It seems pretty common to find a
slice_height < 200, especially for any of the more standard vactives I
checked, but maybe 8% go up to vactive/2, and 15% to vactive, for
arbitrary even vactives. But the ones that go big are typically fairly
obscure heights.
Maybe it's good enough.
> +
> + if (vactive % 8 == 0)
> + slice_height = 8;
> + else if (vactive % 4 == 0)
> + slice_height = 4;
> + else
> + slice_height = 2;
> +
> + return slice_height;
Sorry to say, given what I said above, this fallback appears to be
unnecessary. Maybe just return 2 here instead of the if ladder?
BR,
Jani.
> +}
> +
> static int intel_dp_dsc_compute_params(struct intel_encoder *encoder,
> struct intel_crtc_state *crtc_state)
> {
> @@ -1433,17 +1457,7 @@ static int intel_dp_dsc_compute_params(struct intel_encoder *encoder,
> vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST;
> vdsc_cfg->pic_height = crtc_state->hw.adjusted_mode.crtc_vdisplay;
>
> - /*
> - * Slice Height of 8 works for all currently available panels. So start
> - * with that if pic_height is an integral multiple of 8. Eventually add
> - * logic to try multiple slice heights.
> - */
> - if (vdsc_cfg->pic_height % 8 == 0)
> - vdsc_cfg->slice_height = 8;
> - else if (vdsc_cfg->pic_height % 4 == 0)
> - vdsc_cfg->slice_height = 4;
> - else
> - vdsc_cfg->slice_height = 2;
> + vdsc_cfg->slice_height = intel_dp_get_slice_height(vdsc_cfg->pic_height);
>
> ret = intel_dsc_compute_params(crtc_state);
> if (ret)
--
Jani Nikula, Intel Open Source Graphics Center
