Commit 998d2cd3 authored by Imre Deak's avatar Imre Deak
Browse files

drm/i915/fdi: Improve FDI BW sharing between pipe B and C 



At the moment modesetting pipe C on IVB will fail if pipe B uses 4 FDI
lanes. Make the BW sharing more dynamic by trying to reduce pipe B's
link bpp in this case, until pipe B uses only up to 2 FDI lanes.

For this instead of the encoder compute config retry loop - which
reduced link bpp only for the encoder's pipe - reduce the maximum link
bpp for pipe B/C as required after all CRTC states are computed and
recompute the CRTC states with the new bpp limit.

Atm, all FDI encoder's compute config function returns an error if a BW
constrain prevents increasing the pipe bpp value. The corresponding
crtc_state->bw_constrained check can be replaced with checking
crtc_state->max_link_bpp_x16, add TODO comments for this. SDVO is an
exception where this case is only handled in the outer config retry
loop, failing the modeset with a WARN, add a FIXME comment to handle
this in the encoder code similarly to other encoders.

v2:
- Don't assume that a CRTC is already in the atomic state, while
  reducing its link bpp.
- Add DocBook description to intel_fdi_atomic_check_link().
v3:
- Enable BW management for FDI links in a separate patch. (Ville)
v4: (Ville)
- Fail the SDVO encoder config computation if it doesn't support the
  link bpp limit.
- Add TODO: comments about checking link_bpp_x16 instead of
  bw_constrained.
v5:
- Replace link bpp limit check with a FIXME: comment in
  intel_sdvo_compute_config(). (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
[Amended commit message wrt. changes in v5]
Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230921195159.2646027-11-imre.deak@intel.com
parent 8ca0b875

drivers/gpu/drm/i915/display/g4x_hdmi.c

+5 −1
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 
#include "intel_display_types.h"

#include "intel_dp_aux.h"

#include "intel_dpio_phy.h"

#include "intel_fdi.h"

#include "intel_fifo_underrun.h"

#include "intel_hdmi.h"

#include "intel_hotplug.h"
@@ -133,8 +134,11 @@ static int g4x_hdmi_compute_config(struct intel_encoder *encoder, 
	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);

	struct drm_i915_private *i915 = to_i915(encoder->base.dev);



	if (HAS_PCH_SPLIT(i915))

	if (HAS_PCH_SPLIT(i915)) {

		crtc_state->has_pch_encoder = true;

		if (!intel_fdi_compute_pipe_bpp(crtc_state))

			return -EINVAL;

	}



	if (IS_G4X(i915))

		crtc_state->has_hdmi_sink = g4x_compute_has_hdmi_sink(state, crtc);

drivers/gpu/drm/i915/display/intel_crt.c

+7 −0
Original line number Diff line number Diff line
@@ -413,6 +413,9 @@ static int pch_crt_compute_config(struct intel_encoder *encoder, 
		return -EINVAL;



	pipe_config->has_pch_encoder = true;

	if (!intel_fdi_compute_pipe_bpp(pipe_config))

		return -EINVAL;



	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;



	return 0;
@@ -435,10 +438,14 @@ static int hsw_crt_compute_config(struct intel_encoder *encoder, 
		return -EINVAL;



	pipe_config->has_pch_encoder = true;

	if (!intel_fdi_compute_pipe_bpp(pipe_config))

		return -EINVAL;



	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;



	/* LPT FDI RX only supports 8bpc. */

	if (HAS_PCH_LPT(dev_priv)) {

		/* TODO: Check crtc_state->max_link_bpp_x16 instead of bw_constrained */

		if (pipe_config->bw_constrained && pipe_config->pipe_bpp < 24) {

			drm_dbg_kms(&dev_priv->drm,

				    "LPT only supports 24bpp\n");

drivers/gpu/drm/i915/display/intel_display.c

+1 −13
Original line number Diff line number Diff line
@@ -4655,7 +4655,6 @@ intel_modeset_pipe_config(struct intel_atomic_state *state, 
	struct drm_connector_state *connector_state;

	int pipe_src_w, pipe_src_h;

	int base_bpp, ret, i;

	bool retry = true;



	crtc_state->cpu_transcoder = (enum transcoder) crtc->pipe;
@@ -4685,6 +4684,7 @@ intel_modeset_pipe_config(struct intel_atomic_state *state, 
			    "[CRTC:%d:%s] Link bpp limited to " BPP_X16_FMT "\n",

			    crtc->base.base.id, crtc->base.name,

			    BPP_X16_ARGS(crtc_state->max_link_bpp_x16));

		crtc_state->bw_constrained = true;

	}



	base_bpp = crtc_state->pipe_bpp;
@@ -4728,7 +4728,6 @@ intel_modeset_pipe_config(struct intel_atomic_state *state, 
			crtc_state->output_types |= BIT(encoder->type);

	}



encoder_retry:

	/* Ensure the port clock defaults are reset when retrying. */

	crtc_state->port_clock = 0;

	crtc_state->pixel_multiplier = 1;
@@ -4768,17 +4767,6 @@ intel_modeset_pipe_config(struct intel_atomic_state *state, 
	ret = intel_crtc_compute_config(state, crtc);

	if (ret == -EDEADLK)

		return ret;

	if (ret == -EAGAIN) {

		if (drm_WARN(&i915->drm, !retry,

			     "[CRTC:%d:%s] loop in pipe configuration computation\n",

			     crtc->base.base.id, crtc->base.name))

			return -EINVAL;



		drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] bw constrained, retrying\n",

			    crtc->base.base.id, crtc->base.name);

		retry = false;

		goto encoder_retry;

	}

	if (ret < 0) {

		drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] config failure: %d\n",

			    crtc->base.base.id, crtc->base.name, ret);

drivers/gpu/drm/i915/display/intel_dp.c

+2 −1
Original line number Diff line number Diff line
@@ -2219,7 +2219,8 @@ intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp, 
	const struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;

	int max_link_bpp_x16;



	max_link_bpp_x16 = to_bpp_x16(limits->pipe.max_bpp);

	max_link_bpp_x16 = min(crtc_state->max_link_bpp_x16,

			       to_bpp_x16(limits->pipe.max_bpp));



	if (!dsc) {

		max_link_bpp_x16 = rounddown(max_link_bpp_x16, to_bpp_x16(2 * 3));

drivers/gpu/drm/i915/display/intel_fdi.c

+95 −18
Original line number Diff line number Diff line
@@ -13,6 +13,7 @@ 
#include "intel_display_types.h"

#include "intel_fdi.h"

#include "intel_fdi_regs.h"

#include "intel_link_bw.h"



struct intel_fdi_funcs {

	void (*fdi_link_train)(struct intel_crtc *crtc,
@@ -129,13 +130,16 @@ static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state) 
}



static int ilk_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,

			       struct intel_crtc_state *pipe_config)

			       struct intel_crtc_state *pipe_config,

			       enum pipe *pipe_to_reduce)

{

	struct drm_i915_private *dev_priv = to_i915(dev);

	struct drm_atomic_state *state = pipe_config->uapi.state;

	struct intel_crtc *other_crtc;

	struct intel_crtc_state *other_crtc_state;



	*pipe_to_reduce = pipe;



	drm_dbg_kms(&dev_priv->drm,

		    "checking fdi config on pipe %c, lanes %i\n",

		    pipe_name(pipe), pipe_config->fdi_lanes);
@@ -198,6 +202,9 @@ static int ilk_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, 
		if (pipe_required_fdi_lanes(other_crtc_state) > 2) {

			drm_dbg_kms(&dev_priv->drm,

				    "fdi link B uses too many lanes to enable link C\n");



			*pipe_to_reduce = PIPE_B;



			return -EINVAL;

		}

		return 0;
@@ -232,16 +239,42 @@ int intel_fdi_link_freq(struct drm_i915_private *i915, 
		return i915->display.fdi.pll_freq;

}



/**

 * intel_fdi_compute_pipe_bpp - compute pipe bpp limited by max link bpp

 * @crtc_state: the crtc state

 *

 * Compute the pipe bpp limited by the CRTC's maximum link bpp. Encoders can

 * call this function during state computation in the simple case where the

 * link bpp will always match the pipe bpp. This is the case for all non-DP

 * encoders, while DP encoders will use a link bpp lower than pipe bpp in case

 * of DSC compression.

 *

 * Returns %true in case of success, %false if pipe bpp would need to be

 * reduced below its valid range.

 */

bool intel_fdi_compute_pipe_bpp(struct intel_crtc_state *crtc_state)

{

	int pipe_bpp = min(crtc_state->pipe_bpp,

			   to_bpp_int(crtc_state->max_link_bpp_x16));



	pipe_bpp = rounddown(pipe_bpp, 2 * 3);



	if (pipe_bpp < 6 * 3)

		return false;



	crtc_state->pipe_bpp = pipe_bpp;



	return true;

}



int ilk_fdi_compute_config(struct intel_crtc *crtc,

			   struct intel_crtc_state *pipe_config)

{

	struct drm_device *dev = crtc->base.dev;

	struct drm_i915_private *i915 = to_i915(dev);

	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;

	int lane, link_bw, fdi_dotclock, ret;

	bool needs_recompute = false;

	int lane, link_bw, fdi_dotclock;



retry:

	/* FDI is a binary signal running at ~2.7GHz, encoding

	 * each output octet as 10 bits. The actual frequency

	 * is stored as a divider into a 100MHz clock, and the
@@ -261,27 +294,71 @@ int ilk_fdi_compute_config(struct intel_crtc *crtc, 
	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,

			       link_bw, &pipe_config->fdi_m_n, false);



	ret = ilk_check_fdi_lanes(dev, crtc->pipe, pipe_config);

	if (ret == -EDEADLK)

	return 0;

}



static int intel_fdi_atomic_check_bw(struct intel_atomic_state *state,

				     struct intel_crtc *crtc,

				     struct intel_crtc_state *pipe_config,

				     struct intel_link_bw_limits *limits)

{

	struct drm_i915_private *i915 = to_i915(crtc->base.dev);

	enum pipe pipe_to_reduce;

	int ret;



	ret = ilk_check_fdi_lanes(&i915->drm, crtc->pipe, pipe_config,

				  &pipe_to_reduce);

	if (ret != -EINVAL)

		return ret;



	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {

		pipe_config->pipe_bpp -= 2*3;

		drm_dbg_kms(&i915->drm,

			    "fdi link bw constraint, reducing pipe bpp to %i\n",

			    pipe_config->pipe_bpp);

		needs_recompute = true;

		pipe_config->bw_constrained = true;

	ret = intel_link_bw_reduce_bpp(state, limits,

				       BIT(pipe_to_reduce),

				       "FDI link BW");



		goto retry;

	return ret ? : -EAGAIN;

}



	if (needs_recompute)

		return -EAGAIN;

/**

 * intel_fdi_atomic_check_link - check all modeset FDI link configuration

 * @state: intel atomic state

 * @limits: link BW limits

 *

 * Check the link configuration for all modeset FDI outputs. If the

 * configuration is invalid @limits will be updated if possible to

 * reduce the total BW, after which the configuration for all CRTCs in

 * @state must be recomputed with the updated @limits.

 *

 * Returns:

 *   - 0 if the confugration is valid

 *   - %-EAGAIN, if the configuration is invalid and @limits got updated

 *     with fallback values with which the configuration of all CRTCs

 *     in @state must be recomputed

 *   - Other negative error, if the configuration is invalid without a

 *     fallback possibility, or the check failed for another reason

 */

int intel_fdi_atomic_check_link(struct intel_atomic_state *state,

				struct intel_link_bw_limits *limits)

{

	struct intel_crtc *crtc;

	struct intel_crtc_state *crtc_state;

	int i;



	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {

		int ret;



		if (!crtc_state->has_pch_encoder ||

		    !intel_crtc_needs_modeset(crtc_state) ||

		    !crtc_state->hw.enable)

			continue;



		ret = intel_fdi_atomic_check_bw(state, crtc, crtc_state, limits);

		if (ret)

			return ret;

	}



	return 0;

}



static void cpt_set_fdi_bc_bifurcation(struct drm_i915_private *dev_priv, bool enable)

{

	u32 temp;