From: Imre Deak <imre.deak@intel.com>
To: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Cc: <intel-gfx@lists.freedesktop.org>, <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v3] drm/i915/vdsc: Account for DSC slice overhead in intel_vdsc_min_cdclk()
Date: Tue, 23 Dec 2025 20:45:16 +0200 [thread overview]
Message-ID: <aUrjPJaxbvhNhUv5@ideak-desk> (raw)
In-Reply-To: <20251223150826.2591182-1-ankit.k.nautiyal@intel.com>
On Tue, Dec 23, 2025 at 08:38:26PM +0530, Ankit Nautiyal wrote:
> When DSC is enabled on a pipe, the pipe pixel rate input to the
> CDCLK frequency and pipe joining calculation needs an adjustment to
> account for compression overhead "bubbles" added at each horizontal
> slice boundary.
>
> Account for this overhead while computing min cdclk required for DSC.
>
> v2:
> - Get rid of the scaling factor and return unchanged pixel-rate
> instead of 0.
> v3:
> - Use mul_u32_u32() for the bubble-adjusted pixel rate to avoid 64x64
> multiplication and drop redundant casts in DIV_ROUND_UP_ULL(). (Imre)
>
> Bspec:68912
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_vdsc.c | 35 ++++++++++++++++++++---
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index ad5fe841e4b3..5493082f30a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -1050,15 +1050,40 @@ void intel_vdsc_state_dump(struct drm_printer *p, int indent,
> drm_dsc_dump_config(p, indent, &crtc_state->dsc.config);
> }
>
> +static
> +int intel_dsc_get_pixel_rate_with_dsc_bubbles(struct intel_display *display,
> + int pixel_rate, int htotal,
> + int dsc_horizontal_slices)
> +{
> + int dsc_slice_bubbles;
> + u64 num;
> +
> + if (drm_WARN_ON(display->drm, !htotal))
> + return pixel_rate;
> +
> + dsc_slice_bubbles = 14 * dsc_horizontal_slices;
> + num = mul_u32_u32(pixel_rate, (htotal + dsc_slice_bubbles));
> +
> + return DIV_ROUND_UP_ULL(num, htotal);
> +}
> +
> int intel_vdsc_min_cdclk(const struct intel_crtc_state *crtc_state)
> {
> struct intel_display *display = to_intel_display(crtc_state);
> int num_vdsc_instances = intel_dsc_get_num_vdsc_instances(crtc_state);
> + int htotal = crtc_state->hw.adjusted_mode.crtc_htotal;
> + int dsc_slices = crtc_state->dsc.slice_count;
> + int pixel_rate;
> int min_cdclk;
>
> if (!crtc_state->dsc.compression_enable)
> return 0;
>
> + pixel_rate = intel_dsc_get_pixel_rate_with_dsc_bubbles(display,
> + crtc_state->pixel_rate,
> + htotal,
> + dsc_slices);
> +
> /*
> * When we decide to use only one VDSC engine, since
> * each VDSC operates with 1 ppc throughput, pixel clock
> @@ -1066,7 +1091,7 @@ int intel_vdsc_min_cdclk(const struct intel_crtc_state *crtc_state)
> * If there 2 VDSC engines, then pixel clock can't be higher than
> * VDSC clock(cdclk) * 2 and so on.
> */
> - min_cdclk = DIV_ROUND_UP(crtc_state->pixel_rate, num_vdsc_instances);
> + min_cdclk = DIV_ROUND_UP(pixel_rate, num_vdsc_instances);
>
> if (crtc_state->joiner_pipes) {
> int pixel_clock = intel_dp_mode_to_fec_clock(crtc_state->hw.adjusted_mode.clock);
> @@ -1084,9 +1109,11 @@ int intel_vdsc_min_cdclk(const struct intel_crtc_state *crtc_state)
> * => CDCLK >= compressed_bpp * Pixel clock / 2 * Bigjoiner Interface bits
> */
> int bigjoiner_interface_bits = DISPLAY_VER(display) >= 14 ? 36 : 24;
> - int min_cdclk_bj =
> - (fxp_q4_to_int_roundup(crtc_state->dsc.compressed_bpp_x16) *
> - pixel_clock) / (2 * bigjoiner_interface_bits);
> + int adjusted_pixel_rate =
> + intel_dsc_get_pixel_rate_with_dsc_bubbles(display, pixel_clock,
> + htotal, dsc_slices);
> + int min_cdclk_bj = (fxp_q4_to_int_roundup(crtc_state->dsc.compressed_bpp_x16) *
> + adjusted_pixel_rate) / (2 * bigjoiner_interface_bits);
The patch looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>
Fwiw: I suppose when calculating min_cdclk_bj instead of
fxp_q4_to_int_roundup() the x16 adjustment could be in the divider for
more precision and the division should round up not down. However neither
of these are related to your changes, they can be revised later.
>
> min_cdclk = max(min_cdclk, min_cdclk_bj);
> }
> --
> 2.45.2
>
next prev parent reply other threads:[~2025-12-23 18:45 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-23 15:08 [PATCH v3] drm/i915/vdsc: Account for DSC slice overhead in intel_vdsc_min_cdclk() Ankit Nautiyal
2025-12-23 15:27 ` ✓ CI.KUnit: success for drm/i915/vdsc: Account for DSC slice overhead in intel_vdsc_min_cdclk() (rev2) Patchwork
2025-12-23 16:18 ` ✓ Xe.CI.BAT: " Patchwork
2025-12-23 18:45 ` Imre Deak [this message]
2025-12-29 5:30 ` [PATCH v3] drm/i915/vdsc: Account for DSC slice overhead in intel_vdsc_min_cdclk() Nautiyal, Ankit K
2025-12-24 1:10 ` ✗ Xe.CI.Full: failure for drm/i915/vdsc: Account for DSC slice overhead in intel_vdsc_min_cdclk() (rev2) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aUrjPJaxbvhNhUv5@ideak-desk \
--to=imre.deak@intel.com \
--cc=ankit.k.nautiyal@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox