[Intel-gfx] [PATCH 16/19] drm/i915: Check lane sharing between pipes B & C using atomic state

[Konduru, Chandra]

> -----Original Message-----
> From: Conselvan De Oliveira, Ander
> Sent: Friday, March 13, 2015 2:49 AM
> To: intel-gfx at lists.freedesktop.org
> Cc: Konduru, Chandra; Conselvan De Oliveira, Ander
> Subject: [PATCH 16/19] drm/i915: Check lane sharing between pipes B & C using
> atomic state
> 
> Makes that code atomic ready.
> 
> Signed-off-by: Ander Conselvan de Oliveira
> <ander.conselvan.de.oliveira at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 49 ++++++++++++++++++++++++++++++-
> -----
>  1 file changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e720a48..8c97186 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5537,13 +5537,20 @@ 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 drm_atomic_state *state,
> +				   enum pipe pipe)
>  {
>  	struct intel_crtc *crtc =
> -		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> +		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, pipe));
> +	struct intel_crtc_state *crtc_state;
> +
> +	crtc_state = intel_atomic_get_crtc_state(state, crtc);
> +	if (WARN_ON(IS_ERR(crtc_state))) {
> +		/* Cause modeset to fail due to excess lanes. */
> +		return 5;
> +	}
> 
> -	if (crtc->base.state->enable &&
> -	    crtc->config->has_pch_encoder)
> +	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
>  		return crtc->config->fdi_lanes;
> 
>  	return 0;
> @@ -5552,6 +5559,8 @@ static int pipe_required_fdi_lanes(struct drm_device
> *dev, enum pipe pipe)  static bool 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;
> +
>  	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) {
> @@ -5579,7 +5588,7 @@ static bool ironlake_check_fdi_lanes(struct
> drm_device *dev, enum pipe pipe,
>  		return true;
>  	case PIPE_B:
>  		if (pipe_config->fdi_lanes > 2 &&
> -		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
> +		    pipe_required_fdi_lanes(state, PIPE_C) > 0) {
>  			DRM_DEBUG_KMS("invalid shared fdi lane config on
> pipe %c: %i lanes\n",
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
>  			return false;
> @@ -5591,7 +5600,7 @@ static bool ironlake_check_fdi_lanes(struct
> drm_device *dev, enum pipe pipe,
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
>  			return false;
>  		}
> -		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
> +		if (pipe_required_fdi_lanes(state, PIPE_B) > 2) {
>  			DRM_DEBUG_KMS("fdi link B uses too many lanes to
> enable link C\n");
>  			return false;
>  		}
> @@ -5601,15 +5610,41 @@ static bool ironlake_check_fdi_lanes(struct
> drm_device *dev, enum pipe pipe,
>  	}
>  }
> 
> +static int add_pipe_b_c_to_state(struct drm_atomic_state *state) {
> +	struct intel_crtc *pipe_B =
> +		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, PIPE_B));
> +	struct intel_crtc *pipe_C =
> +		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, PIPE_C));
> +	struct intel_crtc_state *crtc_state;
> +
> +	crtc_state = intel_atomic_get_crtc_state(state, pipe_B);
> +	if (IS_ERR(crtc_state))
> +		return PTR_ERR(crtc_state);
> +
> +	crtc_state = intel_atomic_get_crtc_state(state, pipe_C);
> +	if (IS_ERR(crtc_state))
> +		return PTR_ERR(crtc_state);
> +
> +	return 0;
> +}
> +
>  #define RETRY 1
>  static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  				       struct intel_crtc_state *pipe_config)  {
>  	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;
> +	int lane, link_bw, fdi_dotclock, ret;
>  	bool setup_ok, needs_recompute = false;
> 
> +	if (IS_IVYBRIDGE(dev) &&
> +	    (intel_crtc->pipe == PIPE_B || intel_crtc->pipe == PIPE_C)) {
> +		ret = add_pipe_b_c_to_state(pipe_config->base.state);

In this scenario, crtc_states are created for both pipe B & C as an operation
on one can affect the other. I may be missing something here, 
but where is the other crtc_state being used: compute/check flow and/or 
commit flow?

> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  retry:
>  	/* FDI is a binary signal running at ~2.7GHz, encoding
>  	 * each output octet as 10 bits. The actual frequency
> --
> 2.1.0