On Wed, Dec 03, 2025 at 12:31:28PM +0200, Jouni Högander wrote:
> Currently we are storing only one set of granularity information for panels
> supporting both PSR and Panel Replay. As panel is informing own
> granularities for PSR and Panel Replay they could be different. Let's use
> own granularities for PSR and Panel Replay instead of having only one set
> for both. This is done by having intel_connector::psr_caps and
> panel_replay_caps both containing granularity information.
>
> Also remove complexity of sharing granularity read between PSR and Panel
> Replay.
>
> v2:
> - use __le16 for two byte values in dpcd
> - use sizeof instead of hardcoded size in reading dpcd
> - drop unnecessarily passing intel_dp pointer
>
> Signed-off-by: Jouni Högander <[email protected]>
> Reviewed-by: Imre Deak <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 4 +-
> drivers/gpu/drm/i915/display/intel_psr.c | 147 +++++++++++------------
> drivers/gpu/drm/i915/display/intel_psr.h | 2 +-
> 3 files changed, 72 insertions(+), 81 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7df0e5e13688d..dcceb0ae2a56d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4562,7 +4562,7 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp, struct
> intel_connector *connector
> * This has to be called after intel_dp->edp_dpcd is filled, PSR checks
> * for SET_POWER_CAPABLE bit in intel_dp->edp_dpcd[1]
> */
> - intel_psr_init_dpcd(intel_dp);
> + intel_psr_init_dpcd(intel_dp, connector);
>
> intel_edp_set_sink_rates(intel_dp);
> intel_dp_set_max_sink_lane_count(intel_dp);
> @@ -6074,7 +6074,7 @@ intel_dp_detect(struct drm_connector *_connector,
> connector->base.epoch_counter++;
>
> if (!intel_dp_is_edp(intel_dp))
> - intel_psr_init_dpcd(intel_dp);
> + intel_psr_init_dpcd(intel_dp, connector);
>
> intel_dp_detect_dsc_caps(intel_dp, connector);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 15ef3b6caad6e..5f8df67f9993e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -494,82 +494,37 @@ static u8 intel_dp_get_sink_sync_latency(struct
> intel_dp *intel_dp)
> return val;
> }
>
> -static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp)
> -{
> - u8 su_capability = 0;
> -
> - if (intel_dp->psr.sink_panel_replay_su_support) {
> - if (drm_dp_dpcd_read_byte(&intel_dp->aux,
> - DP_PANEL_REPLAY_CAP_CAPABILITY,
> - &su_capability) < 0)
> - return 0;
> - } else {
> - su_capability = intel_dp->psr_dpcd[1];
> - }
> -
> - return su_capability;
> -}
> -
> -static unsigned int
> -intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp)
> -{
> - return intel_dp->psr.sink_panel_replay_su_support ?
> - DP_PANEL_REPLAY_CAP_X_GRANULARITY :
> - DP_PSR2_SU_X_GRANULARITY;
> -}
> -
> -static unsigned int
> -intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp)
> -{
> - return intel_dp->psr.sink_panel_replay_su_support ?
> - DP_PANEL_REPLAY_CAP_Y_GRANULARITY :
> - DP_PSR2_SU_Y_GRANULARITY;
> -}
> -
> -/*
> - * Note: Bits related to granularity are same in panel replay and psr
> - * registers. Rely on PSR definitions on these "common" bits.
> - */
> -static void intel_dp_get_su_granularity(struct intel_dp *intel_dp)
> +static void _psr_compute_su_granularity(struct intel_dp *intel_dp,
> + struct intel_connector *connector)
> {
> struct intel_display *display = to_intel_display(intel_dp);
> ssize_t r;
> - u16 w;
> + __le16 w;
> u8 y;
>
> - /*
> - * TODO: Do we need to take into account panel supporting both PSR and
> - * Panel replay?
> - */
> -
> /*
> * If sink don't have specific granularity requirements set legacy
> * ones.
> */
> - if (!(intel_dp_get_su_capability(intel_dp) &
> - DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> + if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED)) {
> /* As PSR2 HW sends full lines, we do not care about x
> granularity */
> w = 4;
This needs now a cpu_to_le16() or a separate variable.
> y = 4;
> goto exit;
> }
>
> - r = drm_dp_dpcd_read(&intel_dp->aux,
> - intel_dp_get_su_x_granularity_offset(intel_dp),
> - &w, 2);
> - if (r != 2)
> + r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_X_GRANULARITY, &w,
> sizeof(w));
> + if (r != sizeof(w))
> drm_dbg_kms(display->drm,
> "Unable to read selective update x granularity\n");
> /*
> * Spec says that if the value read is 0 the default granularity should
> * be used instead.
> */
> - if (r != 2 || w == 0)
> + if (r != sizeof(w) || w == 0)
> w = 4;
This also needs a conversion/separate variable as above.
The patch otherwise looks ok to me.
>
> - r = drm_dp_dpcd_read(&intel_dp->aux,
> - intel_dp_get_su_y_granularity_offset(intel_dp),
> - &y, 1);
> + r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_Y_GRANULARITY, &y, 1);
> if (r != 1) {
> drm_dbg_kms(display->drm,
> "Unable to read selective update y granularity\n");
> @@ -579,8 +534,8 @@ static void intel_dp_get_su_granularity(struct intel_dp
> *intel_dp)
> y = 1;
>
> exit:
> - intel_dp->psr.su_w_granularity = w;
> - intel_dp->psr.su_y_granularity = y;
> + connector->dp.psr_caps.su_w_granularity = le16_to_cpu(w);
> + connector->dp.psr_caps.su_y_granularity = y;
> }
>
> static enum intel_panel_replay_dsc_support
> @@ -621,7 +576,32 @@ static const char *panel_replay_dsc_support_str(enum
> intel_panel_replay_dsc_supp
> };
> }
>
> -static void _panel_replay_init_dpcd(struct intel_dp *intel_dp)
> +static void _panel_replay_compute_su_granularity(struct intel_dp *intel_dp,
> + struct intel_connector
> *connector)
> +{
> + u16 w;
> + u8 y;
> +
> + if
> (!(intel_dp->pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)] &
> + DP_PANEL_REPLAY_SU_GRANULARITY_REQUIRED)) {
> + w = 4;
> + y = 4;
> + goto exit;
> + }
> +
> + /*
> + * Spec says that if the value read is 0 the default granularity should
> + * be used instead.
> + */
> + w = le16_to_cpu(*(__le16
> *)&intel_dp->pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_X_GRANULARITY)])
> ? : 4;
> + y =
> intel_dp->pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_Y_GRANULARITY)] ? :
> 1;
> +
> +exit:
> + connector->dp.panel_replay_caps.su_w_granularity = w;
> + connector->dp.panel_replay_caps.su_y_granularity = y;
> +}
> +
> +static void _panel_replay_init_dpcd(struct intel_dp *intel_dp, struct
> intel_connector *connector)
> {
> struct intel_display *display = to_intel_display(intel_dp);
> int ret;
> @@ -657,9 +637,12 @@ static void _panel_replay_init_dpcd(struct intel_dp
> *intel_dp)
> intel_dp->psr.sink_panel_replay_support = true;
>
> if (intel_dp->pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_SUPPORT)]
> &
> - DP_PANEL_REPLAY_SU_SUPPORT)
> + DP_PANEL_REPLAY_SU_SUPPORT) {
> intel_dp->psr.sink_panel_replay_su_support = true;
>
> + _panel_replay_compute_su_granularity(intel_dp, connector);
> + }
> +
> intel_dp->psr.sink_panel_replay_dsc_support =
> compute_pr_dsc_support(intel_dp);
>
> drm_dbg_kms(display->drm,
> @@ -669,7 +652,7 @@ static void _panel_replay_init_dpcd(struct intel_dp
> *intel_dp)
>
> panel_replay_dsc_support_str(intel_dp->psr.sink_panel_replay_dsc_support));
> }
>
> -static void _psr_init_dpcd(struct intel_dp *intel_dp)
> +static void _psr_init_dpcd(struct intel_dp *intel_dp, struct intel_connector
> *connector)
> {
> struct intel_display *display = to_intel_display(intel_dp);
> int ret;
> @@ -722,17 +705,16 @@ static void _psr_init_dpcd(struct intel_dp *intel_dp)
> drm_dbg_kms(display->drm, "PSR2 %ssupported\n",
> intel_dp->psr.sink_psr2_support ? "" : "not ");
> }
> +
> + if (intel_dp->psr.sink_psr2_support)
> + _psr_compute_su_granularity(intel_dp, connector);
> }
>
> -void intel_psr_init_dpcd(struct intel_dp *intel_dp)
> +void intel_psr_init_dpcd(struct intel_dp *intel_dp, struct intel_connector
> *connector)
> {
> - _psr_init_dpcd(intel_dp);
> -
> - _panel_replay_init_dpcd(intel_dp);
> + _psr_init_dpcd(intel_dp, connector);
>
> - if (intel_dp->psr.sink_psr2_support ||
> - intel_dp->psr.sink_panel_replay_su_support)
> - intel_dp_get_su_granularity(intel_dp);
> + _panel_replay_init_dpcd(intel_dp, connector);
> }
>
> static void hsw_psr_setup_aux(struct intel_dp *intel_dp)
> @@ -1304,25 +1286,32 @@ static bool intel_psr2_sel_fetch_config_valid(struct
> intel_dp *intel_dp,
> return crtc_state->enable_psr2_sel_fetch = true;
> }
>
> -static bool psr2_granularity_check(struct intel_dp *intel_dp,
> - struct intel_crtc_state *crtc_state)
> +static bool psr2_granularity_check(struct intel_crtc_state *crtc_state,
> + struct intel_connector *connector)
> {
> + struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct intel_display *display = to_intel_display(intel_dp);
> const struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
> const int crtc_hdisplay = crtc_state->hw.adjusted_mode.crtc_hdisplay;
> const int crtc_vdisplay = crtc_state->hw.adjusted_mode.crtc_vdisplay;
> u16 y_granularity = 0;
> + u16 sink_y_granularity = crtc_state->has_panel_replay ?
> + connector->dp.panel_replay_caps.su_y_granularity :
> + connector->dp.psr_caps.su_y_granularity;
> + u16 sink_w_granularity = crtc_state->has_panel_replay ?
> + connector->dp.panel_replay_caps.su_w_granularity :
> + connector->dp.psr_caps.su_w_granularity;
>
> /* PSR2 HW only send full lines so we only need to validate the width */
> - if (crtc_hdisplay % intel_dp->psr.su_w_granularity)
> + if (crtc_hdisplay % sink_w_granularity)
> return false;
>
> - if (crtc_vdisplay % intel_dp->psr.su_y_granularity)
> + if (crtc_vdisplay % sink_y_granularity)
> return false;
>
> /* HW tracking is only aligned to 4 lines */
> if (!crtc_state->enable_psr2_sel_fetch)
> - return intel_dp->psr.su_y_granularity == 4;
> + return sink_y_granularity == 4;
>
> /*
> * adl_p and mtl platforms have 1 line granularity.
> @@ -1330,11 +1319,11 @@ static bool psr2_granularity_check(struct intel_dp
> *intel_dp,
> * to match sink requirement if multiple of 4.
> */
> if (display->platform.alderlake_p || DISPLAY_VER(display) >= 14)
> - y_granularity = intel_dp->psr.su_y_granularity;
> - else if (intel_dp->psr.su_y_granularity <= 2)
> + y_granularity = sink_y_granularity;
> + else if (sink_y_granularity <= 2)
> y_granularity = 4;
> - else if ((intel_dp->psr.su_y_granularity % 4) == 0)
> - y_granularity = intel_dp->psr.su_y_granularity;
> + else if ((sink_y_granularity % 4) == 0)
> + y_granularity = sink_y_granularity;
>
> if (y_granularity == 0 || crtc_vdisplay % y_granularity)
> return false;
> @@ -1621,9 +1610,11 @@ static bool intel_psr2_config_valid(struct intel_dp
> *intel_dp,
> return true;
> }
>
> -static bool intel_sel_update_config_valid(struct intel_dp *intel_dp,
> - struct intel_crtc_state *crtc_state)
> +static bool intel_sel_update_config_valid(struct intel_crtc_state
> *crtc_state,
> + struct drm_connector_state
> *conn_state)
> {
> + struct intel_connector *connector =
> to_intel_connector(conn_state->connector);
> + struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct intel_display *display = to_intel_display(intel_dp);
>
> if (HAS_PSR2_SEL_FETCH(display) &&
> @@ -1671,7 +1662,7 @@ static bool intel_sel_update_config_valid(struct
> intel_dp *intel_dp,
> goto unsupported;
> }
>
> - if (!psr2_granularity_check(intel_dp, crtc_state)) {
> + if (!psr2_granularity_check(crtc_state, connector)) {
> drm_dbg_kms(display->drm,
> "Selective update not enabled, SU granularity not
> compatible\n");
> goto unsupported;
> @@ -1866,7 +1857,7 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> if (!crtc_state->has_psr)
> return;
>
> - crtc_state->has_sel_update = intel_sel_update_config_valid(intel_dp,
> crtc_state);
> + crtc_state->has_sel_update = intel_sel_update_config_valid(crtc_state,
> conn_state);
> }
>
> void intel_psr_get_config(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index 024ee4c309852..b41dc4d44ff29 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -28,7 +28,7 @@ struct intel_plane_state;
> bool intel_encoder_can_psr(struct intel_encoder *encoder);
> bool intel_psr_needs_aux_io_power(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state);
> -void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> +void intel_psr_init_dpcd(struct intel_dp *intel_dp, struct intel_connector
> *connector);
> void intel_psr_panel_replay_enable_sink(struct intel_dp *intel_dp);
> void intel_psr_pre_plane_update(struct intel_atomic_state *state,
> struct intel_crtc *crtc);
> --
> 2.43.0
>
