Re: [PATCH 07/18] drm/i915: fix the __intel_fbc_update() comments

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

Em Qua, 2015-10-21 às 13:37 +0100, Chris Wilson escreveu:
> On Tue, Oct 20, 2015 at 11:49:53AM -0200, Paulo Zanoni wrote:
> > Don't try to list in comments the cases where we should enable or
> > disable FBC: it varies a lot with the hardware generations and the
> > code should be the documentation. Also notice that there's already
> > a
> > huge gap between the comments and what's in the code.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_fbc.c | 26 ++------------------------
> >  1 file changed, 2 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > b/drivers/gpu/drm/i915/intel_fbc.c
> > index e856304..5fa4ce4 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -853,20 +853,8 @@ static bool
> > intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
> >   * __intel_fbc_update - enable/disable FBC as needed, unlocked
> >   * @dev_priv: i915 device instance
> >   *
> > - * Set up the framebuffer compression hardware at mode set
> > time.  We
> > - * enable it if possible:
> > - *   - plane A only (on pre-965)
> > - *   - no pixel mulitply/line duplication
> > - *   - no alpha buffer discard
> > - *   - no dual wide
> > - *   - framebuffer <= max_hdisplay in width, max_vdisplay in
> > height
> > - *
> > - * We can't assume that any compression will take place (worst
> > case),
> > - * so the compressed buffer has to be the same size as the
> > uncompressed
> > - * one.  It also must reside (along with the line length buffer)
> > in
> > - * stolen memory.
> > - *
> > - * We need to enable/disable FBC on a global basis.
> > + * This function completely reevaluates the status of FBC, then
> > enables,
> > + * disables or maintains it on the same state.
> 
> By the end, this comment is not strictly true ,right? Since you move
> some of the status checking into modeset enable paths. Could you
> refine
> the function comment?

I only move the status checking into modeset enable paths on patch 12.
The comment above is modified on patch 12 to reflect
activate/deactivate instead of enable/disable.

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