diff mbox

drm/i915: Check lane sharing between pipes B & C using atomic state

Message ID 1427693592-12941-1-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira March 30, 2015, 5:33 a.m. UTC 
Makes that code atomic ready.

v2: Acquire crtc_state for the "other" pipe only when needed. (Daniel)

v3: Really only acquire the other state if necessary. (Daniel)
---
 drivers/gpu/drm/i915/intel_display.c | 69 ++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 27 deletions(-)

Comments

Daniel Vetter March 30, 2015, 7:17 a.m. UTC | #1 
On Mon, Mar 30, 2015 at 08:33:12AM +0300, Ander Conselvan de Oliveira wrote:
> Makes that code atomic ready.
> 
> v2: Acquire crtc_state for the "other" pipe only when needed. (Daniel)
> 
> v3: Really only acquire the other state if necessary. (Daniel)

Missing sob added and queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c | 69 ++++++++++++++++++++++--------------
>  1 file changed, 42 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 75955fe..cfe34c3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5667,65 +5667,80 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
>  	return encoder->get_hw_state(encoder, &pipe);
>  }
>  
> -static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
> +static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_crtc *crtc =
> -		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> -
> -	if (crtc->base.state->enable &&
> -	    crtc->config->has_pch_encoder)
> -		return crtc->config->fdi_lanes;
> +	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
> +		return crtc_state->fdi_lanes;
>  
>  	return 0;
>  }
>  
> -static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
> +static int ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  				     struct intel_crtc_state *pipe_config)
>  {
> +	struct drm_atomic_state *state = pipe_config->base.state;
> +	struct intel_crtc *other_crtc;
> +	struct intel_crtc_state *other_crtc_state;
> +
>  	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
>  		      pipe_name(pipe), pipe_config->fdi_lanes);
>  	if (pipe_config->fdi_lanes > 4) {
>  		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
>  			      pipe_name(pipe), pipe_config->fdi_lanes);
> -		return false;
> +		return -EINVAL;
>  	}
>  
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		if (pipe_config->fdi_lanes > 2) {
>  			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
>  				      pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		} else {
> -			return true;
> +			return 0;
>  		}
>  	}
>  
>  	if (INTEL_INFO(dev)->num_pipes == 2)
> -		return true;
> +		return 0;
>  
>  	/* Ivybridge 3 pipe is really complicated */
>  	switch (pipe) {
>  	case PIPE_A:
> -		return true;
> +		return 0;
>  	case PIPE_B:
> -		if (pipe_config->fdi_lanes > 2 &&
> -		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
> +		if (pipe_config->fdi_lanes <= 2)
> +			return 0;
> +
> +		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
> +		other_crtc_state =
> +			intel_atomic_get_crtc_state(state, other_crtc);
> +		if (IS_ERR(other_crtc_state))
> +			return PTR_ERR(other_crtc_state);
> +
> +		if (pipe_required_fdi_lanes(other_crtc_state) > 0) {
>  			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		}
> -		return true;
> +		return 0;
>  	case PIPE_C:
>  		if (pipe_config->fdi_lanes > 2) {
>  			DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i lanes\n",
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		}
> -		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
> +
> +		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
> +		other_crtc_state =
> +			intel_atomic_get_crtc_state(state, other_crtc);
> +		if (IS_ERR(other_crtc_state))
> +			return PTR_ERR(other_crtc_state);
> +
> +		if (pipe_required_fdi_lanes(other_crtc_state) > 2) {
>  			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
> -			return false;
> +			return -EINVAL;
>  		}
> -		return true;
> +		return 0;
>  	default:
>  		BUG();
>  	}
> @@ -5737,8 +5752,8 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> -	int lane, link_bw, fdi_dotclock;
> -	bool setup_ok, needs_recompute = false;
> +	int lane, link_bw, fdi_dotclock, ret;
> +	bool needs_recompute = false;
>  
>  retry:
>  	/* FDI is a binary signal running at ~2.7GHz, encoding
> @@ -5760,9 +5775,9 @@ retry:
>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
>  			       link_bw, &pipe_config->fdi_m_n);
>  
> -	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> -					    intel_crtc->pipe, pipe_config);
> -	if (!setup_ok && pipe_config->pipe_bpp > 6*3) {
> +	ret = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> +				       intel_crtc->pipe, pipe_config);
> +	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
>  		pipe_config->pipe_bpp -= 2*3;
>  		DRM_DEBUG_KMS("fdi link bw constraint, reducing pipe bpp to %i\n",
>  			      pipe_config->pipe_bpp);
> @@ -5775,7 +5790,7 @@ retry:
>  	if (needs_recompute)
>  		return RETRY;
>  
> -	return setup_ok ? 0 : -EINVAL;
> +	return ret;
>  }
>  
>  static void hsw_compute_ips_config(struct intel_crtc *crtc,
> -- 
> 2.1.0
>
Shuang He March 30, 2015, 7:58 a.m. UTC | #2 
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6087
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              270/270              268/270
ILK                                  303/303              303/303
SNB                                  304/304              304/304
IVB                                  337/337              337/337
BYT                                  287/287              287/287
HSW                                  361/361              361/361
BDW                                  309/309              309/309
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(1)PASS(2)      CRASH(1)PASS(1)
*PNV  igt@gem_tiled_pread_pwrite      PASS(3)      FAIL(1)PASS(1)
Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 75955fe..cfe34c3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5667,65 +5667,80 @@  bool intel_connector_get_hw_state(struct intel_connector *connector)
 	return encoder->get_hw_state(encoder, &pipe);
 }
 
-static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
+static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
 {
-	struct intel_crtc *crtc =
-		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
-
-	if (crtc->base.state->enable &&
-	    crtc->config->has_pch_encoder)
-		return crtc->config->fdi_lanes;
+	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
+		return crtc_state->fdi_lanes;
 
 	return 0;
 }
 
-static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
+static int ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 				     struct intel_crtc_state *pipe_config)
 {
+	struct drm_atomic_state *state = pipe_config->base.state;
+	struct intel_crtc *other_crtc;
+	struct intel_crtc_state *other_crtc_state;
+
 	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
 		      pipe_name(pipe), pipe_config->fdi_lanes);
 	if (pipe_config->fdi_lanes > 4) {
 		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
 			      pipe_name(pipe), pipe_config->fdi_lanes);
-		return false;
+		return -EINVAL;
 	}
 
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		if (pipe_config->fdi_lanes > 2) {
 			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
 				      pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		} else {
-			return true;
+			return 0;
 		}
 	}
 
 	if (INTEL_INFO(dev)->num_pipes == 2)
-		return true;
+		return 0;
 
 	/* Ivybridge 3 pipe is really complicated */
 	switch (pipe) {
 	case PIPE_A:
-		return true;
+		return 0;
 	case PIPE_B:
-		if (pipe_config->fdi_lanes > 2 &&
-		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
+		if (pipe_config->fdi_lanes <= 2)
+			return 0;
+
+		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
+		other_crtc_state =
+			intel_atomic_get_crtc_state(state, other_crtc);
+		if (IS_ERR(other_crtc_state))
+			return PTR_ERR(other_crtc_state);
+
+		if (pipe_required_fdi_lanes(other_crtc_state) > 0) {
 			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
 				      pipe_name(pipe), pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		}
-		return true;
+		return 0;
 	case PIPE_C:
 		if (pipe_config->fdi_lanes > 2) {
 			DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i lanes\n",
 				      pipe_name(pipe), pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		}
-		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
+
+		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
+		other_crtc_state =
+			intel_atomic_get_crtc_state(state, other_crtc);
+		if (IS_ERR(other_crtc_state))
+			return PTR_ERR(other_crtc_state);
+
+		if (pipe_required_fdi_lanes(other_crtc_state) > 2) {
 			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
-			return false;
+			return -EINVAL;
 		}
-		return true;
+		return 0;
 	default:
 		BUG();
 	}
@@ -5737,8 +5752,8 @@  static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	int lane, link_bw, fdi_dotclock;
-	bool setup_ok, needs_recompute = false;
+	int lane, link_bw, fdi_dotclock, ret;
+	bool needs_recompute = false;
 
 retry:
 	/* FDI is a binary signal running at ~2.7GHz, encoding
@@ -5760,9 +5775,9 @@  retry:
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
 			       link_bw, &pipe_config->fdi_m_n);
 
-	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
-					    intel_crtc->pipe, pipe_config);
-	if (!setup_ok && pipe_config->pipe_bpp > 6*3) {
+	ret = ironlake_check_fdi_lanes(intel_crtc->base.dev,
+				       intel_crtc->pipe, pipe_config);
+	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
 		pipe_config->pipe_bpp -= 2*3;
 		DRM_DEBUG_KMS("fdi link bw constraint, reducing pipe bpp to %i\n",
 			      pipe_config->pipe_bpp);
@@ -5775,7 +5790,7 @@  retry:
 	if (needs_recompute)
 		return RETRY;
 
-	return setup_ok ? 0 : -EINVAL;
+	return ret;
 }
 
 static void hsw_compute_ips_config(struct intel_crtc *crtc,