On Thu, Oct 09, 2025 at 06:56:13AM +0300, Jouni Hogander wrote:
> 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?
This cap depends on other conditions as well, besides the flag in
pr_dpcd[] so I think it should be determined once, already where all
those other conditions are checked as well, in
_panel_replay_init_dpcd(). The other sink panel replay/psr caps are also
decoded there, so it would be strange to not do this at the same place
for the panel replay DSC caps.
> > 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.
Okay, will change this.
> > 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.
The panel replay in DSC mode setting depends on the DSC state, computed
separately wrt. the PSR/panel replay state and also depends on the state
computed for other CRTCs than what is tracked by crtc_state. I would
like to verify at this point - after everything is computed - that all
these states are as expected.
> 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)");
>
