Re: [v4 4/4] drm/i915/display: Gate periodic AS SDP skip frames behind a module parameter

[Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>]

On 9/1/2026 3:04 AM, Uma Shankar wrote:
> Periodic AS SDP (skip frames) drives a Panel Replay panel down toward its
> minimum refresh rate. It is a new, panel- and platform-sensitive behaviour,
> so keep it opt-in rather than enabling it unconditionally.
>
> Add a display module parameter, periodic_assdp_enable, shared by both the
> i915 and xe drivers via the intel_display_params infrastructure. It defaults
> to false (feature disabled); set it to true to enable periodic AS SDP.
>
> Gate the feature at its single choke point, intel_pr_as_sdp_skip_frames():
> returning a zero skip count when the parameter is off makes both the
> PR_ALPM_CTL programming (intel_alpm_configure_pr_as_sdp()) and the DC3co
> force-disable predicate (intel_alpm_pr_as_sdp_skip_frames_enabled()) a no-op,
> so AS SDP continues to be sent on every frame as before.
>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_alpm.c           | 5 +++++
>   drivers/gpu/drm/i915/display/intel_display_params.c | 6 ++++++
>   drivers/gpu/drm/i915/display/intel_display_params.h | 1 +
>   3 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index c784e77f610b..537cad13f0b4 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -392,11 +392,16 @@ static u32 get_pr_alpm_as_sdp_transmission_time(const struct intel_crtc_state *c
>    */
>   static u32 intel_pr_as_sdp_skip_frames(struct intel_dp *intel_dp)
>   {
> +	struct intel_display *display = to_intel_display(intel_dp);
>   	const struct drm_display_info *info =
>   		&intel_dp->attached_connector->base.display_info;
>   	int max_vrefresh = info->monitor_range.max_vfreq;
>   	int min_vrefresh = info->monitor_range.min_vfreq;
>   
> +	/* Off by default; gated by the periodic_assdp_enable module parameter. */
> +	if (!display->params.periodic_assdp_enable)
> +		return 0;
> +
>   	if (min_vrefresh <= 0 || max_vrefresh <= min_vrefresh)
>   		return 0;
>   
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c
> index 2aed110c5b09..a845d29610cf 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
> @@ -139,6 +139,12 @@ intel_display_param_named_unsafe(enable_dmc_wl, int, 0400,
>   	"(-1=use per-chip default, 0=disabled, 1=enabled, 2=match any register, 3=always locked) "
>   	"Default: -1");
>   
> +intel_display_param_named_unsafe(periodic_assdp_enable, bool, 0600,
> +	"Enable periodic Adaptive-Sync SDP (AS SDP skip frames) for Panel Replay "
> +	"to drive the panel down to its minimum refresh rate "
> +	"(0=disabled, 1=enabled) "
> +	"Default: 0");
> +
>   __maybe_unused
>   static void _param_print_bool(struct drm_printer *p, const char *driver_name,
>   			      const char *name, bool val)
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
> index b95ecf728daa..4d912aab4811 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.h
> @@ -50,6 +50,7 @@ struct drm_printer;
>   	param(bool, psr_safest_params, false, 0400) \
>   	param(bool, enable_psr2_sel_fetch, true, 0400) \
>   	param(int, enable_dmc_wl, -1, 0400) \
> +	param(bool, periodic_assdp_enable, false, 0600) \
>   
>   #define MEMBER(T, member, ...) T member;
>   struct intel_display_params {

Wondering if we can rename the parameter to|enable_periodic_assdp| to match the existing params.
Also, I assume the|0600| write permission is intentional to allow runtime enable/disable for testing.

Other than this, LGTM.
Reviewed-by: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>