Re: [PATCH v6 1/9] drm/i915/vrr: Return from PSR2 compute config in case of CMRR enabled

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 7/24/2026 3:52 PM, Golani, Mitulkumar Ajitkumar wrote:
> 
> 
>> -----Original Message-----
>> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>
>> Sent: 23 July 2026 21:45
>> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>;
>> intel-gfx@lists.freedesktop.org
>> Cc: intel-xe@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
>> Subject: Re: [PATCH v6 1/9] drm/i915/vrr: Return from PSR2 compute config in
>> case of CMRR enabled
>>
>>
>>
>> On 7/22/2026 10:09 AM, Mitul Golani wrote:
>>> CMRR is mutually exclusive to PSR2, do not enable PSR2 when CMRR is
>>> enabled.
>>>
>>> --v2:
>>> - Restrict selective update config check (Ankit)
>>> - Commit message change
>>>
>>> --v3:
>>> - Commit changes
>>>
>>> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_psr.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>>> b/drivers/gpu/drm/i915/display/intel_psr.c
>>> index 40e3d7095996..6a90ded5d1fa 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>>> @@ -1587,6 +1587,12 @@ static bool intel_sel_update_config_valid(struct
>> intel_crtc_state *crtc_state,
>>>    	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>>    	struct intel_display *display = to_intel_display(intel_dp);
>>>
>>> +	if (crtc_state->cmrr.enable) {
>>> +		drm_dbg_kms(display->drm,
>>> +			    "Selective update cannot be enabled when CMRR is
>> enabled\n");
>>> +		goto unsupported;
>>> +	}
>>
>> This also disables panel replay, please make sure that this is desired.
>> If so, add it in commit message.
> 
> Thanks for the review,
> 
> The check is placed in intel_sel_update_config_valid(), whose return value only feeds has_sel_update. has_panel_replay is computed independently beforehand, so Panel Replay is not disabled, only Selective Update (PSR2 and Panel Replay Selective Update) is.
> 
> *  has_psr (alone):					PSR1
>   *  has_psr + has_sel_update:				PSR2
>   *  has_psr + has_panel_replay:				Panel Replay
>   *  has_psr + has_panel_replay + has_sel_update:	Panel Replay Selective Update
> 
> 
> Per the mode table in intel_psr.c at start, disabling has_sel_update when CMRR is enabled turns off case 2 (PSR2) and case 4 (Panel Replay SU), while PSR1 and Panel Replay remain functional.
> This is the desired behaviour since CMRR is incompatible with selective update.

Are we sure?

Bspec only ever calls out PSR2 as incompatible. It never calls out 
selective update.

> 
> Although this is explicitly called out as part of code comment at start of this file.
> 
> Regards,
> Mitul
> 
>>
>>> +
>>>    	if (HAS_PSR2_SEL_FETCH(display) &&
>>>    	    !intel_psr2_sel_fetch_config_valid(intel_dp, crtc_state) &&
>>>    	    !HAS_PSR_HW_TRACKING(display)) {
>