Re: [Intel-gfx] [PATCH 22/26] drm/i915: Allow M/N change during fastset on bdw+

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

On Tue, 03 May 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On BDW+ M/N are double buffered and so we can easily reprogram them
> during a fastset. So for eDP panels that support seamless DRRS we
> can just change these without a full modeset.
>
> For earlier platforms we'd need to play tricks with M1/N1 vs.
> M2/N2 during the fastset to make sure we do the switch atomically.
> Not sure the added complexity is worth the hassle, so leave it
> alone for now.

I guess the commit message should mention this bumps the bandwidth
requirement for eDP to the native mode with the max refresh rate.

It's also not obvious to me this takes link training fallback into
account. AFAICT the modes get pruned, but the userspace switching to a
different mode does not help?

One nitpick inline below.

BR,
Jani.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 13 +++++++---
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 26 +++++++++++++++----
>  3 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 2acc5f3a2c0e..f30bdcdd4c84 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6174,7 +6174,8 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  	PIPE_CONF_CHECK_X(lane_lat_optim_mask);
>  
>  	if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) {
> -		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
> +		if (!fastset || !pipe_config->seamless_m_n)
> +			PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
>  	} else {
>  		PIPE_CONF_CHECK_M_N(dp_m_n);
>  		PIPE_CONF_CHECK_M_N(dp_m2_n2);
> @@ -6306,8 +6307,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  	if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5)
>  		PIPE_CONF_CHECK_I(pipe_bpp);
>  
> -	PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock);
> -	PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock);
> +	if (!fastset || !pipe_config->seamless_m_n) {
> +		PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock);
> +		PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock);
> +	}
>  	PIPE_CONF_CHECK_I(port_clock);
>  
>  	PIPE_CONF_CHECK_I(min_voltage_level);
> @@ -7890,6 +7893,10 @@ static void intel_pipe_fastset(const struct intel_crtc_state *old_crtc_state,
>  	if (DISPLAY_VER(dev_priv) >= 9 ||
>  	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
>  		hsw_set_linetime_wm(new_crtc_state);
> +
> +	if (new_crtc_state->seamless_m_n)
> +		intel_cpu_transcoder_set_m1_n1(crtc, new_crtc_state->cpu_transcoder,
> +					       &new_crtc_state->dp_m_n);
>  }
>  
>  static void commit_pipe_pre_planes(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 408152f9f46a..fb58893510a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1057,6 +1057,7 @@ struct intel_crtc_state {
>  	/* m2_n2 for eDP downclock */
>  	struct intel_link_m_n dp_m2_n2;
>  	bool has_drrs;
> +	bool seamless_m_n;
>  
>  	/* PSR is supported but might not be enabled due the lack of enabled planes */
>  	bool has_psr;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index e4a79c11fd25..9385178c7fd6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1270,21 +1270,33 @@ intel_dp_adjust_compliance_config(struct intel_dp *intel_dp,
>  	}
>  }
>  
> +static int intel_dp_mode_clock(const struct intel_crtc_state *crtc_state,
> +			       const struct drm_connector_state *conn_state)
> +{
> +	struct intel_connector *connector = to_intel_connector(conn_state->connector);
> +	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +
> +	/* FIXME a bit of a mess wrt clock vs. crtc_clock */
> +	if (intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS)
> +		return intel_panel_highest_mode(connector, adjusted_mode)->clock;
> +	else
> +		return adjusted_mode->crtc_clock;
> +}
> +
>  /* Optimize link config in order: max bpp, min clock, min lanes */
>  static int
>  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  				  struct intel_crtc_state *pipe_config,
> +				  const struct drm_connector_state *conn_state,
>  				  const struct link_config_limits *limits)
>  {
> -	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
> -	int bpp, i, lane_count;
> +	int bpp, i, lane_count, clock = intel_dp_mode_clock(pipe_config, conn_state);
>  	int mode_rate, link_rate, link_avail;
>  
>  	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
>  		int output_bpp = intel_dp_output_bpp(pipe_config->output_format, bpp);
>  
> -		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
> -						   output_bpp);
> +		mode_rate = intel_dp_link_required(clock, output_bpp);
>  
>  		for (i = 0; i < intel_dp->num_common_rates; i++) {
>  			link_rate = intel_dp_common_rate(intel_dp, i);
> @@ -1584,7 +1596,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	 * Optimize for slow and wide for everything, because there are some
>  	 * eDP 1.3 and 1.4 panels don't work well with fast and narrow.
>  	 */
> -	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
> +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, conn_state, &limits);
>  
>  	if (ret || joiner_needs_dsc || intel_dp->force_dsc_en) {
>  		drm_dbg_kms(&i915->drm, "Try DSC (fallback=%s, joiner=%s, force=%s)\n",
> @@ -1873,6 +1885,10 @@ intel_dp_drrs_compute_config(struct intel_connector *connector,
>  		intel_panel_downclock_mode(connector, &pipe_config->hw.adjusted_mode);
>  	int pixel_clock;
>  
> +	if ((DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915)) &&

Seems like this condition is duplicated in a few places.

> +	    intel_panel_drrs_type(connector) == DRRS_TYPE_SEAMLESS)
> +		pipe_config->seamless_m_n = true;
> +
>  	if (!can_enable_drrs(connector, pipe_config, downclock_mode)) {
>  		if (intel_cpu_transcoder_has_m2_n2(i915, pipe_config->cpu_transcoder))
>  			intel_zero_m_n(&pipe_config->dp_m2_n2);

-- 
Jani Nikula, Intel Open Source Graphics Center