Re: [Intel-gfx] [PATCH] drm/i915: disable non-sequential pfits on ivb/hsw

[Daniel Vetter]

On Wed, Jan 13, 2016 at 05:13:31PM +0100, Daniel Vetter wrote:
> On Wed, Jan 13, 2016 at 02:33:47PM +0000, Chris Bainbridge wrote:
> > The existing code assumes a sequential mapping of panel fitters to pipes
> > (pfit0-pipeA, pfit1-pipeB, pfit2-pipeC), but boot firmware can
> > arbitrarily assign any pipe to a pfit on IVB hardware e.g. Macbook UEFI
> > uses pfit 0 and pipe C for eDP1 when the firmware boots in a non-16:10
> > resolution (the last-used resolution is stored in NVRAM by OS X so the
> > firmware can immediately restore it at boot). When this happens, the
> > display will appear letterboxed due to incorrect aspect ratio and
> > attempting to switch to alternative resolutions will fail. Fix this by
> > disabling any panel fitters which have been non-sequentially assigned at
> > boot time.
> > 
> > Link: https://bugs.freedesktop.org/show_bug.cgi?id=93523
> > Signed-off-by: Chris Bainbridge <chris.bainbridge@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 32cf97346978..9e588139a2dd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9170,6 +9170,24 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc,
> 
> get_config should never touch hw state, only read it out. The right place
> to put fixup code is in sanitize_crtc. What we need in get_config would be
> a check to make sure pfit is assigned to our pipe (and not take over the
> state if so).

maybe even throw a new sanitize_pfit function in for clarity, since the
problem is that pfit is _not_ associated with the crtc at a hw level.
-Daniel

> 
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	uint32_t tmp;
> > +	int pipe;
> > +
> > +	/*
> > +	 * PF_CTL assumes panel fitter 0 is on pipe A, panel fitter 1 is on
> > +	 * pipe B, and panel fitter 2 is on pipe C, but firmware can init IVB
> > +	 * panel fitters to any arbitrary pipe (Macbook UEFI uses pfit 0 for
> > +	 * pipe C), so find and disable any other mappings.
> > +	 */
> > +	for (pipe = 0; pipe < INTEL_INFO(dev)->num_pipes; pipe++) {
> > +		tmp = I915_READ(PF_CTL(pipe));
> > +		if (IS_GEN7(dev) && (tmp & PF_ENABLE) &&
> > +		    PF_PIPE_SEL_IVB(pipe) != (tmp & PF_PIPE_SEL_MASK_IVB)) {
> > +			DRM_DEBUG_KMS("disabling initial panel fitter\n");
> > +			I915_WRITE(PF_CTL(pipe), 0);
> > +			I915_WRITE(PF_WIN_POS(pipe), 0);
> > +			I915_WRITE(PF_WIN_SZ(pipe), 0);
> > +		}
> > +	}
> >  
> >  	tmp = I915_READ(PF_CTL(crtc->pipe));
> >  
> > @@ -9177,14 +9195,6 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc,
> >  		pipe_config->pch_pfit.enabled = true;
> >  		pipe_config->pch_pfit.pos = I915_READ(PF_WIN_POS(crtc->pipe));
> >  		pipe_config->pch_pfit.size = I915_READ(PF_WIN_SZ(crtc->pipe));
> > -
> > -		/* We currently do not free assignements of panel fitters on
> > -		 * ivb/hsw (since we don't use the higher upscaling modes which
> > -		 * differentiates them) so just WARN about this case for now. */
> > -		if (IS_GEN7(dev)) {
> > -			WARN_ON((tmp & PF_PIPE_SEL_MASK_IVB) !=
> > -				PF_PIPE_SEL_IVB(crtc->pipe));
> > -		}
> >  	}
> >  }
> >  
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch