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

[Conselvan De Oliveira, Ander]

On Thu, 2015-03-19 at 20:58 +0000, Konduru, Chandra wrote:
> 
> > -----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?

The function pipe_required_fdi_lanes() above is changed in this patch to
use the crtc_state in the drm atomic state to determined FDI lane
availability. At this point we don't yet allow changes to multiple
pipes, so the mode set is rejected if the pipe that is not being mode
set uses too many lanes.

Ander

> 
> > +		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
> 

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.