On Tue, Dec 23, 2025 at 11:05:59AM +0530, Nautiyal, Ankit K wrote:
>
> On 12/22/2025 6:26 PM, Imre Deak wrote:
> > On Mon, Dec 22, 2025 at 10:14:43AM +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.
> > >
> > > Bspec:68912
> > > Signed-off-by: Ankit Nautiyal <[email protected]>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_vdsc.c | 45 +++++++++++++++++++++--
> > > 1 file changed, 41 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..b91cd009be9d 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > @@ -1050,15 +1050,52 @@ 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(int pixel_rate, int htotal,
> > > + int dsc_horizontal_slices)
> > > +{
> > > + int dsc_slice_bubbles;
> > > + u64 num;
> > > +
> > > + if (!htotal)
> > Should this also warn as !slice_width below?
>
> Yeah can add add warn here..
>
> > > + return pixel_rate;
> > > +
> > > + dsc_slice_bubbles = 14 * dsc_horizontal_slices;
> > > + num = (u64)pixel_rate * (u64)(htotal + dsc_slice_bubbles);
> > Better to use mul_u32_u32() to avoid the casts and 64-bit x 64-bit
> > multiplication.
>
> Ok sure will use mul_u32_u32 to avoid casts here.
>
> > > +
> > > + return (int)DIV_ROUND_UP_ULL(num, (u64)htotal);
> > Both casts are ensured by the compiler already, so no need for doing
> > them explicitly.
>
> Got it.
>
> > > +}
> > > +
> > > +static
> > > +int pixel_rate_with_dsc_bubbles(const struct intel_crtc_state
> > > *crtc_state, int pixel_rate)
> > > +{
> > > + struct intel_display *display = to_intel_display(crtc_state);
> > > + const struct drm_display_mode *adjusted_mode =
> > > &crtc_state->hw.adjusted_mode;
> > > + const struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
> > > + int dsc_horizontal_slices;
> > > +
> > > + if (drm_WARN_ON(display->drm, !vdsc_cfg->slice_width))
> > > + return pixel_rate;
> > > +
> > > + dsc_horizontal_slices = vdsc_cfg->pic_width / vdsc_cfg->slice_width;
> > The above looks to be the same as crtc_state->dsc.slice_count, aka the
> > slices per scanline, could you use that instead?
>
> Apparently we are not filling dsc.slice_count. We are filling
> dsc.slice_width and the pic_width.
dsc.slice_count is computed for the given encoder and dsc.slice_width is
computed based on that (as vdsc->pic_width / crtc_state->dsc.slice_count).
> This parameter seems to be unused, perhaps can be dropped?
It's slice_count what is computed for a particular encoder and the rest
of DSC parameters are only derived from slice_count the same way for all
encoders.
> Regards,
> Ankit
>
> > > +
> > > + return intel_dsc_get_pixel_rate_with_dsc_bubbles(pixel_rate,
> > > +
> > > adjusted_mode->crtc_htotal,
> > > + dsc_horizontal_slices);
> > > +}
> > > +
> > > 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 pixel_rate;
> > > int min_cdclk;
> > > if (!crtc_state->dsc.compression_enable)
> > > return 0;
> > > + pixel_rate = pixel_rate_with_dsc_bubbles(crtc_state,
> > > crtc_state->pixel_rate);
> > > +
> > > /*
> > > * When we decide to use only one VDSC engine, since
> > > * each VDSC operates with 1 ppc throughput, pixel clock
> > > @@ -1066,7 +1103,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 +1121,9 @@ 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 =
> > > pixel_rate_with_dsc_bubbles(crtc_state, pixel_clock);
> > > + int min_cdclk_bj =
> > > (fxp_q4_to_int_roundup(crtc_state->dsc.compressed_bpp_x16) *
> > > + adjusted_pixel_rate) / (2 *
> > > bigjoiner_interface_bits);
> > > min_cdclk = max(min_cdclk, min_cdclk_bj);
> > > }
> > > --
> > > 2.45.2
> > >
