On Mon, Sep 22, 2025 at 04:46:37PM +0300, Imre Deak wrote:
> Handle the DSC pixel throughput quirk, limiting the compressed link-bpp
> value for Synaptics Panamera branch devices, working around a
> blank/unstable output issue observed on docking stations containing
> these branch devices, when using a mode with a high pixel clock and a
> high compressed link-bpp value.
>
> For now use the same mode clock limit for RGB/YUV444 and YUV422/420
> output modes. This may result in limiting the link-bpp value for a
> YUV422/420 output mode already at a lower than required mode clock.
>
> v2: Apply the quirk only when DSC is enabled.
>
> Reported-by: Vidya Srinivas <vidya.srini...@intel.com>
> Reported-by: Swati Sharma <swati2.sha...@intel.com>
> Signed-off-by: Imre Deak <imre.d...@intel.com>
> ---
> .../drm/i915/display/intel_display_types.h | 1 +
> drivers/gpu/drm/i915/display/intel_dp.c | 58 +++++++++++++++++++
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 73bdafae604f..aff63bf0f229 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -551,6 +551,7 @@ struct intel_connector {
> u8 fec_capability;
>
> u8 dsc_hblank_expansion_quirk:1;
> + u8 dsc_throughput_quirk:1;
> u8 dsc_decompression_enabled:1;
>
> struct {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index dd082d2fcc96..5e7aea560910 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2509,6 +2509,56 @@ int intel_dp_dsc_compute_config(struct intel_dp
> *intel_dp,
> return 0;
> }
>
> +static void
> +adjust_limits_for_dsc_throughput_quirk(const struct intel_connector
> *connector,
> + const struct intel_crtc_state
> *crtc_state,
> + struct link_config_limits *limits)
> +{
> + struct intel_display *display = to_intel_display(connector);
> + const struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + const struct drm_display_mode *adjusted_mode =
> + &crtc_state->hw.adjusted_mode;
> + int max_bpp_x16;
> +
> + if (!connector->dp.dsc_throughput_quirk)
> + return;
> +
> + /*
> + * Synaptics Panamera branch devices have a problem decompressing a
> + * stream with a compressed link-bpp higher than 12, if the pixel
> + * clock is higher than ~50 % of the maximum overall throughput
> + * reported by the branch device. Work around this by limiting the
> + * maximum link bpp for such pixel clocks.
> + *
> + * TODO: Use the throughput value specific to the actual RGB/YUV
> + * format of the output, after determining the pixel clock limit for
> + * YUV modes. For now use the smaller of the throughput values, which
> + * may result in limiting the link-bpp value already at a lower than
> + * required mode clock in case of native YUV422/420 output formats.
> + * The RGB/YUV444 throughput value should be always either equal or
> + * smaller than the YUV422/420 value, but let's not depend on this
> + * assumption.
> + */
> + if (adjusted_mode->crtc_clock <
> + min(connector->dp.dsc_branch_caps.overall_throughput.rgb_yuv444,
> + connector->dp.dsc_branch_caps.overall_throughput.yuv422_420) /
> 2)
> + return;
> +
> + max_bpp_x16 = clamp(fxp_q4_from_int(12),
> + limits->link.min_bpp_x16, limits->link.max_bpp_x16);
> +
> + if (max_bpp_x16 >= limits->link.max_bpp_x16)
> + return;
> +
> + drm_dbg_kms(display->drm,
> + "[CRTC:%d:%s][CONNECTOR:%d:%s] Decreasing link max bpp to "
> FXP_Q4_FMT " due to DSC throughput quirk\n",
> + crtc->base.base.id, crtc->base.name,
> + connector->base.base.id, connector->base.name,
> + FXP_Q4_ARGS(max_bpp_x16));
> +
> + limits->link.max_bpp_x16 = max_bpp_x16;
> +}
> +
> /*
> * Calculate the output link min, max bpp values in limits based on the pipe
> bpp
> * range, crtc_state and dsc mode. Return true on success.
> @@ -2558,6 +2608,9 @@ intel_dp_compute_config_link_bpp_limits(struct intel_dp
> *intel_dp,
>
> limits->link.max_bpp_x16 = max_link_bpp_x16;
>
> + if (dsc)
> + adjust_limits_for_dsc_throughput_quirk(connector, crtc_state,
> limits);
Would feel cleaner if we did this in the actual dsc==true branch above.
Maybe the actual thing that determines that max bpp should be some kind
of pure function that returns the max acceptable bpp, and then the
caller can deal with it.
Or at least move the 'limits->link.max_bpp_x16 = max_link_bpp_x16' into both
!dsc and dsc branches. That way we don't have this strange looking drop out
from the if statement just to do the single assignment, and then going back in.
> +
> drm_dbg_kms(display->drm,
> "[ENCODER:%d:%s][CRTC:%d:%s] DP link limits: pixel clock %d
> kHz DSC %s max lanes %d max rate %d max pipe_bpp %d max link_bpp " FXP_Q4_FMT
> "\n",
> encoder->base.base.id, encoder->base.name,
> @@ -4272,6 +4325,7 @@ void intel_dp_get_dsc_sink_cap(u8 dpcd_rev,
> connector->dp.fec_capability = 0;
>
> memset(&connector->dp.dsc_branch_caps, 0,
> sizeof(connector->dp.dsc_branch_caps));
> + connector->dp.dsc_throughput_quirk = false;
>
> if (dpcd_rev < DP_DPCD_REV_14)
> return;
> @@ -4292,6 +4346,10 @@ void intel_dp_get_dsc_sink_cap(u8 dpcd_rev,
> return;
>
> init_dsc_overall_throughput_limits(connector, is_branch);
> +
> + if (drm_dp_has_quirk(desc, DP_DPCD_QUIRK_DSC_THROUGHPUT_BPP_LIMIT) &&
> + desc->ident.hw_rev == 0x10)
Rather annoying to have part of the detection here and part in the
quirk list. Can we extend the quirk framework to support hw revisions
as well?
Could be a followup.
> + connector->dp.dsc_throughput_quirk = true;
> }
>
> static void intel_edp_get_dsc_sink_cap(u8 edp_dpcd_rev, struct
> intel_connector *connector)
> --
> 2.49.1
--
Ville Syrjälä
Intel
