Re: [PATCH V3] drn/i915/edp: Only use alternate fixed mode when requested

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 30 Apr 2018, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> In commit dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP
> if available."), the patch was always selecting the alternate refresh rate
> even though user space was asking for the higher rate. This patch confirms
> the alt mode setup time meets requirements and only uses the alt mode if
> psr is enable for the platform.

I'm confused.

If the panel supports two refresh rates, why would PSR have to be a
requirement for selecting one of them? If we now have bugs about only
being able to select one, won't we then get bugs about only being able
to select the other?

I don't think this should be tied to PSR.

BR,
Jani.

>
> V2: use clock instead of vrefresh for compare.
> V3: Confirm PSR is enabled and alt mode setup time is valid
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105469
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |  4 +++-
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_psr.c | 35 +++++++++++++++++++++++------------
>  3 files changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 83da50b..59b8a5c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1864,7 +1864,9 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
>  			intel_connector->panel.alt_fixed_mode;
>  		struct drm_display_mode *req_mode = &pipe_config->base.mode;
>  
> -		if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
> +		if ((!intel_edp_compare_alt_mode(req_mode, panel_mode) &&
> +		    intel_psr_setup_time_valid(intel_dp, req_mode)) ||
> +		    (!i915_modparams.enable_psr))
>  			panel_mode = intel_connector->panel.fixed_mode;
>  
>  		drm_mode_debug_printmodeline(panel_mode);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 11a1932..e69ce10 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1908,6 +1908,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>  void intel_psr_init(struct drm_i915_private *dev_priv);
>  void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
>  				   unsigned frontbuffer_bits);
> +bool intel_psr_setup_time_valid(struct intel_dp *intel_dp,
> +				    const struct drm_display_mode *mode);
>  void intel_psr_compute_config(struct intel_dp *intel_dp,
>  			      struct intel_crtc_state *crtc_state);
>  void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 6233a32..9cc4109 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -578,6 +578,27 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>  	return true;
>  }
>  
> +bool intel_psr_setup_time_valid(struct intel_dp *intel_dp, const struct drm_display_mode *mode)
> +{
> +	int psr_setup_time = 0;
> +
> +	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> +	if (psr_setup_time < 0) {
> +		DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time (0x%02x)\n",
> +			      intel_dp->psr_dpcd[1]);
> +		return false;
> +	}
> +
> +	if (intel_usecs_to_scanlines(mode, psr_setup_time) >
> +	    mode->crtc_vtotal - mode->crtc_vdisplay - 1) {
> +		DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us) too long\n",
> +			      psr_setup_time);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  void intel_psr_compute_config(struct intel_dp *intel_dp,
>  			      struct intel_crtc_state *crtc_state)
>  {
> @@ -585,7 +606,6 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  	struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->base.adjusted_mode;
> -	int psr_setup_time;
>  
>  	if (!CAN_PSR(dev_priv))
>  		return;
> @@ -626,17 +646,8 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  		return;
>  	}
>  
> -	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> -	if (psr_setup_time < 0) {
> -		DRM_DEBUG_KMS("PSR condition failed: Invalid PSR setup time (0x%02x)\n",
> -			      intel_dp->psr_dpcd[1]);
> -		return;
> -	}
> -
> -	if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
> -	    adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
> -		DRM_DEBUG_KMS("PSR condition failed: PSR setup time (%d us) too long\n",
> -			      psr_setup_time);
> +	if (!intel_psr_setup_time_valid(intel_dp, adjusted_mode)) {
> +		DRM_DEBUG_KMS("PSR Setup invalid\n");
>  		return;
>  	}

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx