On Wed, 2025-10-08 at 12:41 +0300, Imre Deak wrote:
> Prevent enabling panel replay if the sink doesn't support this due to
> DSC being enabled.
>
> Panel replay has two modes, updating full frames or only selected
> regions of the frame. If the sink doesn't support PR in full frame
> update mode with DSC prevent PR completely if DSC is enabled. If the
> sink doesn't support PR only in the selective update mode while DSC
> is
> enabled, it will still support PR in the full frame update mode, so
> only
> prevent selective updates in this case.
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14869
> Signed-off-by: Imre Deak <[email protected]>
> ---
> .../drm/i915/display/intel_display_types.h | 9 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 2 +
> drivers/gpu/drm/i915/display/intel_psr.c | 93
> ++++++++++++++++++-
> 3 files changed, 99 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index ca5a87772e93a..e0c5abab3b380 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -955,6 +955,12 @@ struct intel_csc_matrix {
> u16 postoff[3];
> };
>
> +enum intel_panel_replay_dsc_support {
> + INTEL_DP_PANEL_REPLAY_DSC_NOT_SUPPORTED,
> + INTEL_DP_PANEL_REPLAY_DSC_FULL_FRAME_ONLY,
> + INTEL_DP_PANEL_REPLAY_DSC_SELECTIVE_UPDATE,
> +};
> +
How about relying on pr_dpcd in _panel_replay_compute_config and
intel_sel_update_config_valid with some helpers?
> struct intel_crtc_state {
> /*
> * uapi (drm) state. This is the software state shown to
> userspace.
> @@ -1133,6 +1139,8 @@ struct intel_crtc_state {
> bool has_panel_replay;
> bool wm_level_disabled;
> bool pkg_c_latency_used;
> + /* Only used for state verification. */
> + enum intel_panel_replay_dsc_support
> panel_replay_dsc_support;
> u32 dc3co_exitline;
> u16 su_y_granularity;
> u8 active_non_psr_pipes;
> @@ -1691,6 +1699,7 @@ struct intel_psr {
> bool source_panel_replay_support;
> bool sink_panel_replay_support;
> bool sink_panel_replay_su_support;
> + enum intel_panel_replay_dsc_support
> sink_panel_replay_dsc_support;
> bool panel_replay_enabled;
> u32 dc3co_exitline;
> u32 dc3co_exit_delay;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8a2fc1fcb5d04..116d7720f427b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6043,6 +6043,8 @@ intel_dp_detect(struct drm_connector
> *_connector,
> memset(connector->dp.dsc_dpcd, 0, sizeof(connector-
> >dp.dsc_dpcd));
> intel_dp->psr.sink_panel_replay_support = false;
> intel_dp->psr.sink_panel_replay_su_support = false;
> + intel_dp->psr.sink_panel_replay_dsc_support =
> + INTEL_DP_PANEL_REPLAY_DSC_NOT_SUPPORTED;
>
> intel_dp_mst_disconnect(intel_dp);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index f7115969b4c5e..080a86f8dc80b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -29,6 +29,7 @@
> #include <drm/drm_vblank.h>
>
> #include "i915_reg.h"
> +#include "i915_utils.h"
> #include "intel_alpm.h"
> #include "intel_atomic.h"
> #include "intel_crtc.h"
> @@ -50,6 +51,7 @@
> #include "intel_snps_phy.h"
> #include "intel_step.h"
> #include "intel_vblank.h"
> +#include "intel_vdsc.h"
> #include "intel_vrr.h"
> #include "skl_universal_plane.h"
>
> @@ -580,6 +582,44 @@ static void intel_dp_get_su_granularity(struct
> intel_dp *intel_dp)
> intel_dp->psr.su_y_granularity = y;
> }
>
> +static enum intel_panel_replay_dsc_support
> +compute_pr_dsc_support(struct intel_dp *intel_dp)
> +{
> + u8 pr_dsc_mode;
> + u8 val;
> +
> + val = intel_dp-
> >pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)];
> + pr_dsc_mode =
> REG_FIELD_GET8(DP_PANEL_REPLAY_DSC_DECODE_CAPABILITY_IN_PR_MASK,
> val);
> +
> + switch (pr_dsc_mode) {
> + case DP_DSC_DECODE_CAPABILITY_IN_PR_FULL_FRAME_ONLY:
> + return INTEL_DP_PANEL_REPLAY_DSC_FULL_FRAME_ONLY;
> + case DP_DSC_DECODE_CAPABILITY_IN_PR_SUPPORTED:
> + return INTEL_DP_PANEL_REPLAY_DSC_SELECTIVE_UPDATE;
> + default:
> + MISSING_CASE(pr_dsc_mode);
> + fallthrough;
> + case DP_DSC_DECODE_CAPABILITY_IN_PR_NOT_SUPPORTED:
> + case DP_DSC_DECODE_CAPABILITY_IN_PR_RESERVED:
> + return INTEL_DP_PANEL_REPLAY_DSC_NOT_SUPPORTED;
> + }
> +}
> +
> +static const char *panel_replay_dsc_support_str(enum
> intel_panel_replay_dsc_support dsc_support)
> +{
> + switch (dsc_support) {
> + case INTEL_DP_PANEL_REPLAY_DSC_NOT_SUPPORTED:
> + return "not supported";
> + case INTEL_DP_PANEL_REPLAY_DSC_FULL_FRAME_ONLY:
> + return "full frame only";
> + case INTEL_DP_PANEL_REPLAY_DSC_SELECTIVE_UPDATE:
> + return "selective update";
> + default:
> + MISSING_CASE(dsc_support);
> + return "n/a";
> + };
> +}
> +
> static void _panel_replay_init_dpcd(struct intel_dp *intel_dp)
> {
> struct intel_display *display = to_intel_display(intel_dp);
> @@ -615,10 +655,13 @@ static void _panel_replay_init_dpcd(struct
> intel_dp *intel_dp)
> DP_PANEL_REPLAY_SU_SUPPORT)
> intel_dp->psr.sink_panel_replay_su_support = true;
>
> + intel_dp->psr.sink_panel_replay_dsc_support =
> compute_pr_dsc_support(intel_dp);
> +
> drm_dbg_kms(display->drm,
> - "Panel replay %sis supported by panel\n",
> + "Panel replay %sis supported by panel (in DSC
> mode: %s)\n",
> intel_dp->psr.sink_panel_replay_su_support ?
> - "selective_update " : "");
> + "selective_update " : "",
> + panel_replay_dsc_support_str(intel_dp-
> >psr.sink_panel_replay_dsc_support));
> }
>
> static void _psr_init_dpcd(struct intel_dp *intel_dp)
> @@ -1535,9 +1578,21 @@ static bool
> intel_sel_update_config_valid(struct intel_dp *intel_dp,
> goto unsupported;
> }
>
> - if (crtc_state->has_panel_replay && (DISPLAY_VER(display) <
> 14 ||
> - !intel_dp-
> >psr.sink_panel_replay_su_support))
> - goto unsupported;
> + if (crtc_state->has_panel_replay) {
> + if (DISPLAY_VER(display) < 14)
> + goto unsupported;
> +
> + if (!intel_dp->psr.sink_panel_replay_su_support)
> + goto unsupported;
> +
> + if (intel_dsc_is_enabled_on_link(crtc_state) &&
> + intel_dp->psr.sink_panel_replay_dsc_support !=
> + INTEL_DP_PANEL_REPLAY_DSC_SELECTIVE_UPDATE) {
> + drm_dbg_kms(display->drm,
> + "Selective update with PR not
> enabled because it's not
"Panel Replay" used everywhere else.
> supported with DSC\n");
> + goto unsupported;
> + }
> + }
>
> if (crtc_state->crc_enabled) {
> drm_dbg_kms(display->drm,
> @@ -1614,6 +1669,14 @@ _panel_replay_compute_config(struct intel_dp
> *intel_dp,
> return false;
> }
>
> + if (intel_dsc_is_enabled_on_link(crtc_state) &&
> + intel_dp->psr.sink_panel_replay_dsc_support ==
> + INTEL_DP_PANEL_REPLAY_DSC_NOT_SUPPORTED) {
> + drm_dbg_kms(display->drm,
> + "Panel Replay not enabled because it's
> not supported with DSC\n");
> + return false;
> + }
> +
> if (!intel_dp_is_edp(intel_dp))
> return true;
>
> @@ -1694,6 +1757,8 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
> return;
> }
>
> + /* Only used for state verification. */
> + crtc_state->panel_replay_dsc_support = intel_dp-
> >psr.sink_panel_replay_dsc_support;
> crtc_state->has_panel_replay =
> _panel_replay_compute_config(intel_dp,
>
> crtc_state,
>
> conn_state);
> @@ -2951,6 +3016,20 @@ void intel_psr_pre_plane_update(struct
> intel_atomic_state *state,
> }
> }
>
> +static void
> +verify_panel_replay_dsc_state(const struct intel_crtc_state
> *crtc_state)
> +{
> + struct intel_display *display =
> to_intel_display(crtc_state);
> +
> + if (!crtc_state->has_panel_replay)
> + return;
> +
> + drm_WARN_ON(display->drm,
> + intel_dsc_is_enabled_on_link(crtc_state) &&
> + crtc_state->panel_replay_dsc_support ==
> + INTEL_DP_PANEL_REPLAY_DSC_NOT_SUPPORTED);
> +}
> +
> void intel_psr_post_plane_update(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> @@ -2962,6 +3041,8 @@ void intel_psr_post_plane_update(struct
> intel_atomic_state *state,
> if (!crtc_state->has_psr)
> return;
>
> + verify_panel_replay_dsc_state(crtc_state);
I'm not convinced adding this verify is valuable. There are so many
restrictions taken into account in _compute_configs and I don't see why
DSC would be somehow different. Maybe our IGT tests should just take
this into account?
Also support parameter you added into crtc_state can be dropped if you
leave this out.
> +
> for_each_intel_encoder_mask_with_psr(state->base.dev,
> encoder,
> crtc_state-
> >uapi.encoder_mask) {
> struct intel_dp *intel_dp =
> enc_to_intel_dp(encoder);
> @@ -3991,6 +4072,8 @@ static void intel_psr_sink_capability(struct
> intel_dp *intel_dp,
> seq_printf(m, ", Panel Replay = %s", str_yes_no(psr-
> >sink_panel_replay_support));
> seq_printf(m, ", Panel Replay Selective Update = %s",
> str_yes_no(psr->sink_panel_replay_su_support));
> + seq_printf(m, ", PR DSC support = %s",
"Panel Replay" used everywhere else.
BR,
Jouni Högander
> + panel_replay_dsc_support_str(psr-
> >sink_panel_replay_dsc_support));
> if (intel_dp-
> >pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_SUPPORT)] &
> DP_PANEL_REPLAY_EARLY_TRANSPORT_SUPPORT)
> seq_printf(m, " (Early Transport)");
