Re: [PATCH 02/11] drm/i915/psr: Read both Panel Replay capability registers from DPCD

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 5/2/2025 2:28 PM, Jouni Högander wrote:
> There is a second Panel Replay capability register in DPCD. Read that as
> well for later use.
>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_display_types.h |  2 +-
>   drivers/gpu/drm/i915/display/intel_psr.c           | 14 +++++++-------
>   2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 7415564d058a2..356287309817e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1665,7 +1665,7 @@ struct intel_dp {
>   	bool use_max_params;
>   	u8 dpcd[DP_RECEIVER_CAP_SIZE];
>   	u8 psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> -	u8 pr_dpcd;
> +	u8 pr_dpcd[2];


As mentioned in my comments on the previous patch, defining a 
DP_PANEL_REPLAY_CAP_SIZE could be helpful here.

Also, using pr_dpcd[Some_PR_FEATURE_CAP - DP_PANEL_REPLAY_CAP_SUPPORT] 
for accessing specific Panel Replay features would allow us to avoid 
hardcoded indices.

Regards,

Ankit

>   	u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
>   	u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
>   	u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE];
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index ccd66bbc72f79..2d78d64b8db8d 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -608,7 +608,7 @@ static void _panel_replay_init_dpcd(struct intel_dp *intel_dp)
>   			return;
>   		}
>   
> -		if (!(intel_dp->pr_dpcd & DP_PANEL_REPLAY_EARLY_TRANSPORT_SUPPORT)) {
> +		if (!(intel_dp->pr_dpcd[0] & DP_PANEL_REPLAY_EARLY_TRANSPORT_SUPPORT)) {
>   			drm_dbg_kms(display->drm,
>   				    "Panel doesn't support early transport, eDP Panel Replay not possible\n");
>   			return;
> @@ -617,7 +617,7 @@ static void _panel_replay_init_dpcd(struct intel_dp *intel_dp)
>   
>   	intel_dp->psr.sink_panel_replay_support = true;
>   
> -	if (intel_dp->pr_dpcd & DP_PANEL_REPLAY_SU_SUPPORT)
> +	if (intel_dp->pr_dpcd[0] & DP_PANEL_REPLAY_SU_SUPPORT)
>   		intel_dp->psr.sink_panel_replay_su_support = true;
>   
>   	drm_dbg_kms(display->drm,
> @@ -676,10 +676,10 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>   {
>   	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
>   			 sizeof(intel_dp->psr_dpcd));
> -	drm_dp_dpcd_readb(&intel_dp->aux, DP_PANEL_REPLAY_CAP,
> -			  &intel_dp->pr_dpcd);
> +	drm_dp_dpcd_read(&intel_dp->aux, DP_PANEL_REPLAY_CAP,
> +			 &intel_dp->pr_dpcd, sizeof(intel_dp->pr_dpcd));
>   
> -	if (intel_dp->pr_dpcd & DP_PANEL_REPLAY_SUPPORT)
> +	if (intel_dp->pr_dpcd[0] & DP_PANEL_REPLAY_SUPPORT)
>   		_panel_replay_init_dpcd(intel_dp);
>   
>   	if (intel_dp->psr_dpcd[0])
> @@ -736,7 +736,7 @@ static bool psr2_su_region_et_valid(struct intel_dp *intel_dp, bool panel_replay
>   		return false;
>   
>   	return panel_replay ?
> -		intel_dp->pr_dpcd & DP_PANEL_REPLAY_EARLY_TRANSPORT_SUPPORT :
> +		intel_dp->pr_dpcd[0] & DP_PANEL_REPLAY_EARLY_TRANSPORT_SUPPORT :
>   		intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_ET_SUPPORTED &&
>   		psr2_su_region_et_global_enabled(intel_dp);
>   }
> @@ -3909,7 +3909,7 @@ 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));
> -	if (intel_dp->pr_dpcd & DP_PANEL_REPLAY_EARLY_TRANSPORT_SUPPORT)
> +	if (intel_dp->pr_dpcd[0] & DP_PANEL_REPLAY_EARLY_TRANSPORT_SUPPORT)
>   		seq_printf(m, " (Early Transport)");
>   	seq_printf(m, "\n");
>   }