diff mbox

drm/i915: Simplify the way BC bifurcation state consistency is kept

Message ID 1426073743-30724-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 11, 2015, 11:35 a.m. UTC
Remove the global modeset resource function that would disable the
bifurcation bit, and instead enable/disable it when enabling the pch
transcoder. The mode set consistency check should prevent us from
disabling the bit if pipe C is enabled so the change should be safe.

Note that this doens't affect the logic that prevents the bit being
set while a pipe is active, since the patch retains the behavior of
only chaging the bit if necessary. Because of the checks during mode
set, the first change would necessarily happen with both pipes B and
C disabled, and any subsequent write would be skipped.

v2: Only change the bit during pch trancoder enable. (Ville)
---
 drivers/gpu/drm/i915/intel_display.c | 46 ++++++++----------------------------
 1 file changed, 10 insertions(+), 36 deletions(-)

Comments

Ander Conselvan de Oliveira March 11, 2015, 11:37 a.m. UTC | #1
On Wed, 2015-03-11 at 13:35 +0200, Ander Conselvan de Oliveira wrote:
> Remove the global modeset resource function that would disable the

> bifurcation bit, and instead enable/disable it when enabling the pch

> transcoder. The mode set consistency check should prevent us from

> disabling the bit if pipe C is enabled so the change should be safe.

> 

> Note that this doens't affect the logic that prevents the bit being

> set while a pipe is active, since the patch retains the behavior of

> only chaging the bit if necessary. Because of the checks during mode

> set, the first change would necessarily happen with both pipes B and

> C disabled, and any subsequent write would be skipped.

> 

> v2: Only change the bit during pch trancoder enable. (Ville)


Oops, I forgot the sob line.

Signed-off-by: Ander Conselvan de Oliveira

<ander.conselvan.de.oliveira@intel.com>

> ---

>  drivers/gpu/drm/i915/intel_display.c | 46 ++++++++----------------------------

>  1 file changed, 10 insertions(+), 36 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c

> index 4008bf4..bfbd829 100644

> --- a/drivers/gpu/drm/i915/intel_display.c

> +++ b/drivers/gpu/drm/i915/intel_display.c

> @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc)

>  	return crtc->base.state->enable && crtc->config->has_pch_encoder;

>  }

>  

> -static void ivb_modeset_global_resources(struct drm_device *dev)

> -{

> -	struct drm_i915_private *dev_priv = dev->dev_private;

> -	struct intel_crtc *pipe_B_crtc =

> -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);

> -	struct intel_crtc *pipe_C_crtc =

> -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);

> -	uint32_t temp;

> -

> -	/*

> -	 * When everything is off disable fdi C so that we could enable fdi B

> -	 * with all lanes. Note that we don't care about enabled pipes without

> -	 * an enabled pch encoder.

> -	 */

> -	if (!pipe_has_enabled_pch(pipe_B_crtc) &&

> -	    !pipe_has_enabled_pch(pipe_C_crtc)) {

> -		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);

> -		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);

> -

> -		temp = I915_READ(SOUTH_CHICKEN1);

> -		temp &= ~FDI_BC_BIFURCATION_SELECT;

> -		DRM_DEBUG_KMS("disabling fdi C rx\n");

> -		I915_WRITE(SOUTH_CHICKEN1, temp);

> -	}

> -}

> -

>  /* The FDI link training functions for ILK/Ibexpeak. */

>  static void ironlake_fdi_link_train(struct drm_crtc *crtc)

>  {

> @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,

>  		   I915_READ(VSYNCSHIFT(cpu_transcoder)));

>  }

>  

> -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)

> +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)

>  {

>  	struct drm_i915_private *dev_priv = dev->dev_private;

>  	uint32_t temp;

>  

>  	temp = I915_READ(SOUTH_CHICKEN1);

> -	if (temp & FDI_BC_BIFURCATION_SELECT)

> +	if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable)

>  		return;

>  

>  	WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);

>  	WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);

>  

> -	temp |= FDI_BC_BIFURCATION_SELECT;

> -	DRM_DEBUG_KMS("enabling fdi C rx\n");

> +	temp &= ~FDI_BC_BIFURCATION_SELECT;

> +	if (enable)

> +		temp |= FDI_BC_BIFURCATION_SELECT;

> +

> +	DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis");

>  	I915_WRITE(SOUTH_CHICKEN1, temp);

>  	POSTING_READ(SOUTH_CHICKEN1);

>  }

> @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)

>  static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)

>  {

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

> -	struct drm_i915_private *dev_priv = dev->dev_private;

>  

>  	switch (intel_crtc->pipe) {

>  	case PIPE_A:

>  		break;

>  	case PIPE_B:

>  		if (intel_crtc->config->fdi_lanes > 2)

> -			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);

> +			cpt_set_fdi_bc_bifurcation(dev, false);

>  		else

> -			cpt_enable_fdi_bc_bifurcation(dev);

> +			cpt_set_fdi_bc_bifurcation(dev, true);

>  

>  		break;

>  	case PIPE_C:

> -		cpt_enable_fdi_bc_bifurcation(dev);

> +		cpt_set_fdi_bc_bifurcation(dev, true);

>  

>  		break;

>  	default:

> @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev)

>  	} else if (IS_IVYBRIDGE(dev)) {

>  		/* FIXME: detect B0+ stepping and use auto training */

>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;

> -		dev_priv->display.modeset_global_resources =

> -			ivb_modeset_global_resources;

>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {

>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;

>  	} else if (IS_VALLEYVIEW(dev)) {


---------------------------------------------------------------------
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.
Ville Syrjala March 11, 2015, 12:24 p.m. UTC | #2
On Wed, Mar 11, 2015 at 01:35:43PM +0200, Ander Conselvan de Oliveira wrote:
> Remove the global modeset resource function that would disable the
> bifurcation bit, and instead enable/disable it when enabling the pch
> transcoder. The mode set consistency check should prevent us from
> disabling the bit if pipe C is enabled so the change should be safe.
> 
> Note that this doens't affect the logic that prevents the bit being
> set while a pipe is active, since the patch retains the behavior of
> only chaging the bit if necessary. Because of the checks during mode
> set, the first change would necessarily happen with both pipes B and
> C disabled, and any subsequent write would be skipped.
> 
> v2: Only change the bit during pch trancoder enable. (Ville)


Actually what I had inb mind was something like this:

pch_enable()
{
	if (pipe == B && fdi_lanes > 2)
		disable_bifurcation()
	...
}

pch_disable()
{
	...
	if (pipe == B && fdi_lanes > 2)
		enable_bifurcation()
}

So we only change it when enabling/disabling pipe B, never for pipe C.
Hence it remains enabled whenever pipe B doesn't need >2 FDI lanes.


> ---
>  drivers/gpu/drm/i915/intel_display.c | 46 ++++++++----------------------------
>  1 file changed, 10 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4008bf4..bfbd829 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
>  	return crtc->base.state->enable && crtc->config->has_pch_encoder;
>  }
>  
> -static void ivb_modeset_global_resources(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *pipe_B_crtc =
> -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> -	struct intel_crtc *pipe_C_crtc =
> -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
> -	uint32_t temp;
> -
> -	/*
> -	 * When everything is off disable fdi C so that we could enable fdi B
> -	 * with all lanes. Note that we don't care about enabled pipes without
> -	 * an enabled pch encoder.
> -	 */
> -	if (!pipe_has_enabled_pch(pipe_B_crtc) &&
> -	    !pipe_has_enabled_pch(pipe_C_crtc)) {
> -		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
> -		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
> -
> -		temp = I915_READ(SOUTH_CHICKEN1);
> -		temp &= ~FDI_BC_BIFURCATION_SELECT;
> -		DRM_DEBUG_KMS("disabling fdi C rx\n");
> -		I915_WRITE(SOUTH_CHICKEN1, temp);
> -	}
> -}
> -
>  /* The FDI link training functions for ILK/Ibexpeak. */
>  static void ironlake_fdi_link_train(struct drm_crtc *crtc)
>  {
> @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
>  		   I915_READ(VSYNCSHIFT(cpu_transcoder)));
>  }
>  
> -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
> +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t temp;
>  
>  	temp = I915_READ(SOUTH_CHICKEN1);
> -	if (temp & FDI_BC_BIFURCATION_SELECT)
> +	if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable)
>  		return;
>  
>  	WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
>  	WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
>  
> -	temp |= FDI_BC_BIFURCATION_SELECT;
> -	DRM_DEBUG_KMS("enabling fdi C rx\n");
> +	temp &= ~FDI_BC_BIFURCATION_SELECT;
> +	if (enable)
> +		temp |= FDI_BC_BIFURCATION_SELECT;
> +
> +	DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis");
>  	I915_WRITE(SOUTH_CHICKEN1, temp);
>  	POSTING_READ(SOUTH_CHICKEN1);
>  }
> @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
>  static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	switch (intel_crtc->pipe) {
>  	case PIPE_A:
>  		break;
>  	case PIPE_B:
>  		if (intel_crtc->config->fdi_lanes > 2)
> -			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
> +			cpt_set_fdi_bc_bifurcation(dev, false);
>  		else
> -			cpt_enable_fdi_bc_bifurcation(dev);
> +			cpt_set_fdi_bc_bifurcation(dev, true);
>  
>  		break;
>  	case PIPE_C:
> -		cpt_enable_fdi_bc_bifurcation(dev);
> +		cpt_set_fdi_bc_bifurcation(dev, true);
>  
>  		break;
>  	default:
> @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev)
>  	} else if (IS_IVYBRIDGE(dev)) {
>  		/* FIXME: detect B0+ stepping and use auto training */
>  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> -		dev_priv->display.modeset_global_resources =
> -			ivb_modeset_global_resources;
>  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
>  	} else if (IS_VALLEYVIEW(dev)) {
> -- 
> 2.1.0
Ville Syrjala March 11, 2015, 4:58 p.m. UTC | #3
On Wed, Mar 11, 2015 at 11:37:54AM +0000, Conselvan De Oliveira, Ander wrote:
> On Wed, 2015-03-11 at 13:35 +0200, Ander Conselvan de Oliveira wrote:
> > Remove the global modeset resource function that would disable the
> > bifurcation bit, and instead enable/disable it when enabling the pch
> > transcoder. The mode set consistency check should prevent us from
> > disabling the bit if pipe C is enabled so the change should be safe.
> > 
> > Note that this doens't affect the logic that prevents the bit being
> > set while a pipe is active, since the patch retains the behavior of
> > only chaging the bit if necessary. Because of the checks during mode
> > set, the first change would necessarily happen with both pipes B and
> > C disabled, and any subsequent write would be skipped.
> > 
> > v2: Only change the bit during pch trancoder enable. (Ville)
> 
> Oops, I forgot the sob line.
> 
> Signed-off-by: Ander Conselvan de Oliveira
> <ander.conselvan.de.oliveira@intel.com>


So I was staring at this stuff for a while and I believe it should be
fine. We don't keep the bifurcation state entirely consistent when
neither of the the pipes B/C are actually driving a PCH transcoder, but
that shouldn't really matter. If we want to make it consistent then I
suggest that we go with my earlier idea of only changing the state at
transcoder B with >2 lanes enable/disable, and otherwise keep it enabled
all the time. The slight complication there is the initial state we get
from the BIOS which might not match that, so we'd need to sanitize it
or something.

Anyway, I also posted a couple of patches on top that try to sort out
ironlake_check_fdi_lanes() [1]. With those and this one I think things
should work even better than before.

So for this patch:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

[1] http://lists.freedesktop.org/archives/intel-gfx/2015-March/061828.html

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 46 ++++++++----------------------------
> >  1 file changed, 10 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 4008bf4..bfbd829 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
> >  	return crtc->base.state->enable && crtc->config->has_pch_encoder;
> >  }
> >  
> > -static void ivb_modeset_global_resources(struct drm_device *dev)
> > -{
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct intel_crtc *pipe_B_crtc =
> > -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> > -	struct intel_crtc *pipe_C_crtc =
> > -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
> > -	uint32_t temp;
> > -
> > -	/*
> > -	 * When everything is off disable fdi C so that we could enable fdi B
> > -	 * with all lanes. Note that we don't care about enabled pipes without
> > -	 * an enabled pch encoder.
> > -	 */
> > -	if (!pipe_has_enabled_pch(pipe_B_crtc) &&
> > -	    !pipe_has_enabled_pch(pipe_C_crtc)) {
> > -		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
> > -		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
> > -
> > -		temp = I915_READ(SOUTH_CHICKEN1);
> > -		temp &= ~FDI_BC_BIFURCATION_SELECT;
> > -		DRM_DEBUG_KMS("disabling fdi C rx\n");
> > -		I915_WRITE(SOUTH_CHICKEN1, temp);
> > -	}
> > -}
> > -
> >  /* The FDI link training functions for ILK/Ibexpeak. */
> >  static void ironlake_fdi_link_train(struct drm_crtc *crtc)
> >  {
> > @@ -3834,20 +3808,23 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
> >  		   I915_READ(VSYNCSHIFT(cpu_transcoder)));
> >  }
> >  
> > -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
> > +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	uint32_t temp;
> >  
> >  	temp = I915_READ(SOUTH_CHICKEN1);
> > -	if (temp & FDI_BC_BIFURCATION_SELECT)
> > +	if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable)
> >  		return;
> >  
> >  	WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
> >  	WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
> >  
> > -	temp |= FDI_BC_BIFURCATION_SELECT;
> > -	DRM_DEBUG_KMS("enabling fdi C rx\n");
> > +	temp &= ~FDI_BC_BIFURCATION_SELECT;
> > +	if (enable)
> > +		temp |= FDI_BC_BIFURCATION_SELECT;
> > +
> > +	DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis");
> >  	I915_WRITE(SOUTH_CHICKEN1, temp);
> >  	POSTING_READ(SOUTH_CHICKEN1);
> >  }
> > @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
> >  static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
> >  {
> >  	struct drm_device *dev = intel_crtc->base.dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> >  	switch (intel_crtc->pipe) {
> >  	case PIPE_A:
> >  		break;
> >  	case PIPE_B:
> >  		if (intel_crtc->config->fdi_lanes > 2)
> > -			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
> > +			cpt_set_fdi_bc_bifurcation(dev, false);
> >  		else
> > -			cpt_enable_fdi_bc_bifurcation(dev);
> > +			cpt_set_fdi_bc_bifurcation(dev, true);
> >  
> >  		break;
> >  	case PIPE_C:
> > -		cpt_enable_fdi_bc_bifurcation(dev);
> > +		cpt_set_fdi_bc_bifurcation(dev, true);
> >  
> >  		break;
> >  	default:
> > @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev)
> >  	} else if (IS_IVYBRIDGE(dev)) {
> >  		/* FIXME: detect B0+ stepping and use auto training */
> >  		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
> > -		dev_priv->display.modeset_global_resources =
> > -			ivb_modeset_global_resources;
> >  	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> >  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
> >  	} else if (IS_VALLEYVIEW(dev)) {
>
Shuang He March 11, 2015, 8:12 p.m. UTC | #4
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5933
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -3              281/281              278/281
ILK                                  308/308              308/308
SNB                 -1              284/284              283/284
IVB                                  375/375              375/375
BYT                                  294/294              294/294
HSW                                  384/384              384/384
BDW                                  315/315              315/315
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt_gem_userptr_blits_coherency-sync      CRASH(3)PASS(2)      CRASH(1)PASS(1)
 PNV  igt_gen3_render_tiledy_blits      FAIL(3)PASS(1)      FAIL(1)PASS(1)
*PNV  igt_gem_fence_thrash_bo-write-verify-threaded-none      PASS(4)      CRASH(1)PASS(1)
*SNB  igt_gem_exec_params_sol-reset-not-gen7      PASS(2)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
Daniel Vetter March 11, 2015, 8:42 p.m. UTC | #5
On Wed, Mar 11, 2015 at 06:58:12PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 11, 2015 at 11:37:54AM +0000, Conselvan De Oliveira, Ander wrote:
> > On Wed, 2015-03-11 at 13:35 +0200, Ander Conselvan de Oliveira wrote:
> > > Remove the global modeset resource function that would disable the
> > > bifurcation bit, and instead enable/disable it when enabling the pch
> > > transcoder. The mode set consistency check should prevent us from
> > > disabling the bit if pipe C is enabled so the change should be safe.
> > > 
> > > Note that this doens't affect the logic that prevents the bit being
> > > set while a pipe is active, since the patch retains the behavior of
> > > only chaging the bit if necessary. Because of the checks during mode
> > > set, the first change would necessarily happen with both pipes B and
> > > C disabled, and any subsequent write would be skipped.
> > > 
> > > v2: Only change the bit during pch trancoder enable. (Ville)
> > 
> > Oops, I forgot the sob line.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira
> > <ander.conselvan.de.oliveira@intel.com>
> 
> 
> So I was staring at this stuff for a while and I believe it should be
> fine. We don't keep the bifurcation state entirely consistent when
> neither of the the pipes B/C are actually driving a PCH transcoder, but
> that shouldn't really matter. If we want to make it consistent then I
> suggest that we go with my earlier idea of only changing the state at
> transcoder B with >2 lanes enable/disable, and otherwise keep it enabled
> all the time. The slight complication there is the initial state we get
> from the BIOS which might not match that, so we'd need to sanitize it
> or something.
> 
> Anyway, I also posted a couple of patches on top that try to sort out
> ironlake_check_fdi_lanes() [1]. With those and this one I think things
> should work even better than before.
> 
> So for this patch:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
>-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4008bf4..bfbd829 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3153,32 +3153,6 @@  static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
 	return crtc->base.state->enable && crtc->config->has_pch_encoder;
 }
 
-static void ivb_modeset_global_resources(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *pipe_B_crtc =
-		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
-	struct intel_crtc *pipe_C_crtc =
-		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
-	uint32_t temp;
-
-	/*
-	 * When everything is off disable fdi C so that we could enable fdi B
-	 * with all lanes. Note that we don't care about enabled pipes without
-	 * an enabled pch encoder.
-	 */
-	if (!pipe_has_enabled_pch(pipe_B_crtc) &&
-	    !pipe_has_enabled_pch(pipe_C_crtc)) {
-		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
-		WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
-
-		temp = I915_READ(SOUTH_CHICKEN1);
-		temp &= ~FDI_BC_BIFURCATION_SELECT;
-		DRM_DEBUG_KMS("disabling fdi C rx\n");
-		I915_WRITE(SOUTH_CHICKEN1, temp);
-	}
-}
-
 /* The FDI link training functions for ILK/Ibexpeak. */
 static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 {
@@ -3834,20 +3808,23 @@  static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
 		   I915_READ(VSYNCSHIFT(cpu_transcoder)));
 }
 
-static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
+static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t temp;
 
 	temp = I915_READ(SOUTH_CHICKEN1);
-	if (temp & FDI_BC_BIFURCATION_SELECT)
+	if (!!(temp & FDI_BC_BIFURCATION_SELECT) == enable)
 		return;
 
 	WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
 	WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
 
-	temp |= FDI_BC_BIFURCATION_SELECT;
-	DRM_DEBUG_KMS("enabling fdi C rx\n");
+	temp &= ~FDI_BC_BIFURCATION_SELECT;
+	if (enable)
+		temp |= FDI_BC_BIFURCATION_SELECT;
+
+	DRM_DEBUG_KMS("%sabling fdi C rx\n", enable ? "en" : "dis");
 	I915_WRITE(SOUTH_CHICKEN1, temp);
 	POSTING_READ(SOUTH_CHICKEN1);
 }
@@ -3855,20 +3832,19 @@  static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
 static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	switch (intel_crtc->pipe) {
 	case PIPE_A:
 		break;
 	case PIPE_B:
 		if (intel_crtc->config->fdi_lanes > 2)
-			WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT);
+			cpt_set_fdi_bc_bifurcation(dev, false);
 		else
-			cpt_enable_fdi_bc_bifurcation(dev);
+			cpt_set_fdi_bc_bifurcation(dev, true);
 
 		break;
 	case PIPE_C:
-		cpt_enable_fdi_bc_bifurcation(dev);
+		cpt_set_fdi_bc_bifurcation(dev, true);
 
 		break;
 	default:
@@ -13056,8 +13032,6 @@  static void intel_init_display(struct drm_device *dev)
 	} else if (IS_IVYBRIDGE(dev)) {
 		/* FIXME: detect B0+ stepping and use auto training */
 		dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
-		dev_priv->display.modeset_global_resources =
-			ivb_modeset_global_resources;
 	} else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
 	} else if (IS_VALLEYVIEW(dev)) {