Re: [Intel-gfx] [PATCH] drm/i915: Sanitize the PPT fdi lane bifurcate state on ivb

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Oct 22, 2013 at 02:37:53PM +0200, Daniel Vetter wrote:
> We expect this bit to be always set when possible, but some BIOSes are
> lazy and don't do this. The result is a pile of WARNs and unhappy fdi
> link training code ...
> 
> v2: It's actually the inverse: The BIOS sets this bit when it's not
> strictly needed. This should be cleaned up in the
> global_modeset_resources callback, but we've failed to look at the
> active bit. Which means this won't fire (and so clean up BIOS state)
> when enabling pipe B or C for the first time.
> 
> v3: Wrap lines.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70507
> Tested-by: Jan-Michael Brummer <jan.brummer@tabos.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cfe9e709..3569db6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2421,9 +2421,10 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
>  			   FDI_FE_ERRC_ENABLE);
>  }
>  
> -static bool pipe_has_enabled_pch(struct intel_crtc *intel_crtc)
> +static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
>  {
> -	return intel_crtc->base.enabled && intel_crtc->config.has_pch_encoder;
> +	return crtc->base.enabled && crtc->active &&
> +		crtc->config.has_pch_encoder;

I'm thinking the crtc->base.enabled check is actually pointless.
AFAICS we should never get here with crtc->base.enabled==false and
crtc->active==true.

We anyway re-enable the bifurcation bit when we do the mode_set.
Actually that in itself could be a maybe a bug. We'd turn off the
bifurcation bit when both pipes B and C are disabled based on
prepare_pipes, but we only do the mode_set based on modeset_pipes.
But currently we are saved by the fact that those two bitmasks are
the same.

Another potential bug I found is that we always set the bifurcation
bit in mode_set, even if we're not using FDI. So if we have eg.
pipe B enabled w/ FDI using 4 lanes, and then we enable pipe C w/ EDP,
we'd still flip the bifurcation bit in pipe C's mode_set and destroy pipe
B's output. The fix would be to call ivybridge_update_fdi_bc_bifurcation()
only when has_pch_encoder==true.

>  }

>  
>  static void ivb_modeset_global_resources(struct drm_device *dev)
> -- 
> 1.8.4.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC