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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux