Re: [PATCH v2 1/1] drm/i915: Fix fb object's frontbuffer-bits

From: Daniel Vetter <daniel@ffwll.ch>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2 1/1] drm/i915: Fix fb object's frontbuffer-bits
Date: Wed, 23 Sep 2015 10:21:48 +0200	[thread overview]
Message-ID: <20150923082147.GM3383@phenom.ffwll.local> (raw)
In-Reply-To: <CA+gsUGTEbU_DsoNkXVLkdpZtvcFs3EYConJkvcqM1Yvb81vS2Q@mail.gmail.com>

On Wed, Sep 16, 2015 at 12:46:24PM -0300, Paulo Zanoni wrote:
> 2015-09-14 14:16 GMT-03:00 Daniel Vetter <daniel@ffwll.ch>:
> > On Mon, Sep 14, 2015 at 09:35:42PM +0530, Sagar Arun Kamble wrote:
> >> Shared frontbuffer bits are causing warnings when same FB is displayed
> >> in another plane without clearing the bits from previous plane.
> >>
> >> v2: Removing coversion of fb bits to 64 bit as it is not needed for now. (Daniel)
> >>
> >> Change-Id: Ic2df80747f314b82afd22f8326297c57d1e652c6
> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_drv.h     | 17 ++++++++++-------
> >>  drivers/gpu/drm/i915/intel_sprite.c |  2 +-
> >>  2 files changed, 11 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 16e604e..892aa78 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -2014,25 +2014,28 @@ struct drm_i915_gem_object_ops {
> >>
> >>  /*
> >>   * Frontbuffer tracking bits. Set in obj->frontbuffer_bits while a gem bo is
> >> - * considered to be the frontbuffer for the given plane interface-vise. This
> >> + * considered to be the frontbuffer for the given plane interface-wise. This
> >>   * doesn't mean that the hw necessarily already scans it out, but that any
> >>   * rendering (by the cpu or gpu) will land in the frontbuffer eventually.
> >>   *
> >>   * We have one bit per pipe and per scanout plane type.
> >>   */
> >> -#define INTEL_FRONTBUFFER_BITS_PER_PIPE 4
> >> +#define INTEL_MAX_SPRITE_BITS_PER_PIPE 5
> >> +#define INTEL_FRONTBUFFER_BITS_PER_PIPE 8
> >>  #define INTEL_FRONTBUFFER_BITS \
> >>       (INTEL_FRONTBUFFER_BITS_PER_PIPE * I915_MAX_PIPES)
> >>  #define INTEL_FRONTBUFFER_PRIMARY(pipe) \
> >>       (1 << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> >>  #define INTEL_FRONTBUFFER_CURSOR(pipe) \
> >> -     (1 << (1 +(INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> >> -#define INTEL_FRONTBUFFER_SPRITE(pipe) \
> >> -     (1 << (2 +(INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> >> +     (1 << (1 + (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> >> +#define INTEL_FRONTBUFFER_SPRITE(pipe, plane) \
> >> +     (1 << (2 + plane + (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> >>  #define INTEL_FRONTBUFFER_OVERLAY(pipe) \
> >> -     (1 << (3 +(INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> >> +     (1 << (2 + INTEL_MAX_SPRITE_BITS_PER_PIPE + (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> >>  #define INTEL_FRONTBUFFER_ALL_MASK(pipe) \
> >> -     (0xf << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> >> +     (0xff << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> >> +#define INTEL_FRONTBUFFER_SPRITE_MASK(pipe) \
> >> +     (0x7C << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> >
> > Patch was a bit confusing to read since tons of spurious whitespace
> > change. Anyway looks good once applied except for
> > INTEL_FRONTBUFFER_SPRITE_MASK which is unused and hence I removed it
> > again.
> >
> > Queued for -next, thanks for the patch.
> 
> Maybe we could have an IGT test for this...

Excellent point. Sagar, can you please supply the missing igt for this
issue?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx