Re: [PATCH 2/2] drm/i915/psr: Use DMC wakelock instead of DC state for VBI

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

On 29-04-2026 20:01, Gustavo Sousa wrote:
> Gustavo Sousa <gustavo.sousa@intel.com> writes:
>
>> Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>
>> writes:
>>
>>>  From display version 30+, use the DMC wakelock mechanism for
>>> preventing DC entry. Older platforms continue to use
>>> set_target_dc_state() to disable DC entry during active vblank.
>>>
>>> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/2296
>>> Fixes: acfc688bc51b ("drm/i915/dmc: Reduce wakelock hold time")
>>> Signed-off-by: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_psr.c | 26 +++++++++++++++---------
>>>   1 file changed, 16 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
>>> index 9acd47392192..d01b4013f33f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>>> @@ -4151,16 +4151,22 @@ void intel_psr_notify_vblank_enable_disable(struct intel_display *display,
>>>   		mutex_unlock(&intel_dp->psr.lock);
>>>   		break;
>>>   	}
>>> -
>>> -	/*
>>> -	 * NOTE: intel_display_power_set_target_dc_state is used
>>> -	 * only by PSR * code for DC3CO handling. DC3CO target
>>> -	 * state is currently disabled in * PSR code. If DC3CO
>>> -	 * is taken into use we need take that into account here
>>> -	 * as well.
>>> -	 */
>>> -	intel_display_power_set_target_dc_state(display, enable ? DC_STATE_DISABLE :
>>> -						DC_STATE_EN_UPTO_DC6);
>>> +	if (DISPLAY_VER(display) < 30) {
>> Drive-by comment:
>>
>> I think we should avoid using open-coded version check for testing
>> features.  It is usually better to use the HAS_SOME_FEATURE() macros
>> that we expose in intel_display_device.h.
>>
>> In this case though, it is better to use to expose and use
>> __intel_dmc_wl_supported (dropping the "__") instead of doing a display
>> version check here, since that is what will actually tell whether the
>> driver is using the wakelock or not.
>>
>> I even think we can even drop the HAS_DMC_WAKELOCK, since it is not used
>> outside of intel_dmc_wl.c.
> For this last part:
> https://lore.kernel.org/all/20260429-drop-has_dmc_wakelock-v1-1-62cb6fab1da0@intel.com

Thanks for the review. I agree that the open-coded version check is short-sighted here.
I will refactor the code to export and use intel_dmc_wl_supported().

> --
> Gustavo Sousa
>
>> --
>> Gustavo Sousa
>>
>>> +		/*
>>> +		 * NOTE: intel_display_power_set_target_dc_state is used
>>> +		 * only by PSR code for DC3CO handling. DC3CO target
>>> +		 * state is currently disabled in PSR code. If DC3CO
>>> +		 * is taken into use we need take that into account here
>>> +		 * as well.
>>> +		 */
>>> +		intel_display_power_set_target_dc_state(display, enable ? DC_STATE_DISABLE :
>>> +							DC_STATE_EN_UPTO_DC6);
>>> +	} else {
>>> +		if (enable)
>>> +			intel_dmc_wl_get_noreg(display);
>>> +		else
>>> +			intel_dmc_wl_put_noreg(display);
>>> +	}
>>>   }
>>>   
>>>   static void
>>> -- 
>>> 2.43.0