Re: [PATCH] drm/i915/psr: ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR not valid for eDP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915/psr: Incomplete bypass of DP-specific PR bits for eDP
--

commit ec05ee2ea4b73ad11b56cabf344de75a2dc66b32
Author: Jouni Högander <jouni.hogander@intel.com>

drm/i915/psr: ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR not valid for eDP

This commit stops using the ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR bit for
eDP panels when computing AS SDP disablement. The eDP 1.5 specification
states that this bit and the DSC_DECODE_NOT_SUPPORTED_IN_PR bit are
DP-specific and not used in eDP.

> EDP1.5 specification says:
> 
> "Although defined for the PR function in DP Standard, the
> ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR and DSC_DECODE_NOT_SUPPORTED_IN_PR
> bits (DPCD 000B1h[3, 2], respectively) are DP-specific and are therefore
> not used in eDP."
> 
> Stop using SYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR on eDP.

[Severity: Medium]
Since the DSC decode capability bits are not used in eDP either, does
compute_pr_dsc_support() need to be updated to ignore them? Currently, it
reads the DP_PANEL_REPLAY_CAP_CAPABILITY register and extracts the DSC mode
which includes these DP-specific bits:

drivers/gpu/drm/i915/display/intel_psr.c:
compute_pr_dsc_support() {
	val = connector->dp.panel_replay_caps.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);
	...
}

Also, does intel_psr_pr_async_video_timing_supported() need the same eDP
bypass that is being added in this patch? It still unconditionally checks
the DP_PANEL_REPLAY_ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR bit:

drivers/gpu/drm/i915/display/intel_psr.c:
intel_psr_pr_async_video_timing_supported() {
	...
	u8 pr_cap = dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)];

	return (pr_support & DP_PANEL_REPLAY_SUPPORT) &&
		!(pr_cap & DP_PANEL_REPLAY_ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR);
}

Could this lead to incorrect capability detection on eDP panels if these
unused bits contain non-zero garbage values?

> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 872e253db1786..338a8dab75945 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1694,8 +1694,11 @@ static inline bool compute_link_off_after_as_sdp_when_pr_active(struct intel_con
>  
>  static inline bool compute_disable_as_sdp_when_pr_active(struct intel_connector *connector)
>  {
> -	return !(connector->dp.panel_replay_caps.dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)] &
> -		 DP_PANEL_REPLAY_ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR);
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +
> +	return intel_dp_is_edp(intel_dp) ? false :
> +		!(connector->dp.panel_replay_caps.dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)] &
> +		  DP_PANEL_REPLAY_ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR);
>  }
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007083443.647649-1-jouni.hogander@intel.com?part=1