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

[Gustavo Sousa <gustavo.sousa@intel.com>]

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.

--
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