On Wed, Oct 21, 2015 at 05:32:16PM +0000, Zanoni, Paulo R wrote: > 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@xxxxxxxxx> > > > --- > > > 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. Honestly, I would just squash this patch, or apply it after the moving the code to correctly reflect the split in work between update/enable. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx