Re: [PATCH 18/19] drm/i915/dp: Suspend/resume DP tunnels

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Jan 23, 2024 at 12:28:49PM +0200, Imre Deak wrote:
> Suspend and resume DP tunnels during system suspend/resume, disabling
> the BW allocation mode during suspend, re-enabling it after resume. This
> reflects the link's BW management component (Thunderbolt CM) disabling
> BWA during suspend. Before any BW requests the driver must read the
> sink's DPRX capabilities (since the BW manager requires this
> information, so snoops for it on AUX), so ensure this read takes place.

Isn't that going to screw up the age old problem of .compute_config()
potentially failing during the resume modeset if we no longer have
the same amount of bandwidth available as we had before suspend?
So far we've been getting away with this exactly by not updating 
the dpcd stuff before the modeset during resume.

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8ebfb039000f6..bc138a54f8d7b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -36,6 +36,7 @@
>  #include <asm/byteorder.h>
>  
>  #include <drm/display/drm_dp_helper.h>
> +#include <drm/display/drm_dp_tunnel.h>
>  #include <drm/display/drm_dsc_helper.h>
>  #include <drm/display/drm_hdmi_helper.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -3320,18 +3321,21 @@ void intel_dp_sync_state(struct intel_encoder *encoder,
>  			 const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -
> -	if (!crtc_state)
> -		return;
> +	bool dpcd_updated = false;
>  
>  	/*
>  	 * Don't clobber DPCD if it's been already read out during output
>  	 * setup (eDP) or detect.
>  	 */
> -	if (intel_dp->dpcd[DP_DPCD_REV] == 0)
> +	if (crtc_state && intel_dp->dpcd[DP_DPCD_REV] == 0) {
>  		intel_dp_get_dpcd(intel_dp);
> +		dpcd_updated = true;
> +	}
>  
> -	intel_dp_reset_max_link_params(intel_dp);
> +	intel_dp_tunnel_resume(intel_dp, dpcd_updated);
> +
> +	if (crtc_state)
> +		intel_dp_reset_max_link_params(intel_dp);
>  }
>  
>  bool intel_dp_initial_fastset_check(struct intel_encoder *encoder,
> @@ -5973,6 +5977,8 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
>  	struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
>  
>  	intel_pps_vdd_off_sync(intel_dp);
> +
> +	intel_dp_tunnel_suspend(intel_dp);
>  }
>  
>  void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
> -- 
> 2.39.2

-- 
Ville Syrjälä
Intel