Re: [PATCH 10/13] drm/i915: fix the CFB size check

["Zanoni, Paulo R" <paulo.r.zanoni@intel.com>]

Em Ter, 2015-11-10 às 14:04 +0100, Maarten Lankhorst escreveu:
> Op 10-11-15 om 13:20 schreef Zanoni, Paulo R:
> > Em Ter, 2015-11-10 às 11:22 +0100, Maarten Lankhorst escreveu:
> > > Op 04-11-15 om 20:10 schreef Paulo Zanoni:
> > > > In function find_compression_threshold() we try to over-
> > > > allocate
> > > > CFB
> > > > space in order to reudce reallocations and fragmentation, and
> > > > we're
> > > > not considering that at the CFB size check. Consider it.
> > > > 
> > > > There is also a longer-term plan to kill
> > > > dev_priv->fbc.uncompressed_size, but this will come later.
> > > > 
> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_fbc.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > > > b/drivers/gpu/drm/i915/intel_fbc.c
> > > > index dee99c9..e99aacc 100644
> > > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > > @@ -719,7 +719,8 @@ static int intel_fbc_setup_cfb(struct
> > > > intel_crtc *crtc)
> > > >  	size = intel_fbc_calculate_cfb_size(crtc);
> > > >  	cpp = drm_format_plane_cpp(fb->pixel_format, 0);
> > > >  
> > > > -	if (size <= dev_priv->fbc.uncompressed_size)
> > > > +	if (dev_priv->fbc.compressed_fb.allocated &&
> > > > +	    size <= dev_priv->fbc.compressed_fb.size *
> > > > dev_priv-
> > > > > fbc.threshold)
> > > >  		return 0;
> > > >  
> > > >  	/* Release any current block */
> > > Should i8xx_fbc_enable be changed too then?
> > As far as I understand, no. We're just reserving a bigger buffer in
> > case we need it later, but the size used by the hardware is still
> > the
> > same. But I'm not 100% sure the i8xx code is actually correct since
> > I
> > didn't dig deep into the ancient scrolls. By not touching i8xx
> > we're
> > also avoiding a possible new regression.
> > 
> The original check was for size <= uncompressed_size,
> new is threshold * compressed.

But threshold is always 1 for i8xx, so the difference of
uncompressed_size vs compressed_fb.size is really just the
overallocation, and the hardware doesn't even need to know we're
overallocating.

> 
> I think i8xx_fbc_enable might have to do the same when calculating
> cfb_pitch
> for this patch to work as intended.

Why?

Please notice I'm trying to keep the i8xx behavior the same instead of
risking introducing a new bug I can't test.

> 
> ~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx