diff mbox

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

Message ID 1442246742-21557-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com Sept. 14, 2015, 4:05 p.m. UTC
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(-)

Comments

Daniel Vetter Sept. 14, 2015, 5:16 p.m. UTC | #1
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.
-Daniel

>  
>  struct drm_i915_gem_object {
>  	struct drm_gem_object base;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 9553859..4d27243 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1123,7 +1123,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  
>  	intel_plane->pipe = pipe;
>  	intel_plane->plane = plane;
> -	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe);
> +	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
>  	intel_plane->check_plane = intel_check_sprite_plane;
>  	intel_plane->commit_plane = intel_commit_sprite_plane;
>  	possible_crtcs = (1 << pipe);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Paulo Zanoni Sept. 16, 2015, 3:46 p.m. UTC | #2
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...

> -Daniel
>
>>
>>  struct drm_i915_gem_object {
>>       struct drm_gem_object base;
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index 9553859..4d27243 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -1123,7 +1123,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>>
>>       intel_plane->pipe = pipe;
>>       intel_plane->plane = plane;
>> -     intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe);
>> +     intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
>>       intel_plane->check_plane = intel_check_sprite_plane;
>>       intel_plane->commit_plane = intel_commit_sprite_plane;
>>       possible_crtcs = (1 << pipe);
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> 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
Daniel Vetter Sept. 23, 2015, 8:21 a.m. UTC | #3
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
sagar.a.kamble@intel.com Sept. 23, 2015, 9:37 a.m. UTC | #4
On 9/23/2015 1:51 PM, Daniel Vetter wrote:
> 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?
Yes. Will start preparing.

Thanks
Sagar

>
> Thanks, Daniel
Daniel Vetter Sept. 23, 2015, 11:05 a.m. UTC | #5
On Wed, Sep 23, 2015 at 03:07:21PM +0530, Kamble, Sagar A wrote:
> 
> 
> On 9/23/2015 1:51 PM, Daniel Vetter wrote:
> >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?
> Yes. Will start preparing.

Thanks. Should be fairly simple to pull off, we simply need a platform
with 2 non-primary/overlay planes and set up the same fb for both. So
hopefully just a few lines of testcase.
-Daniel
diff mbox

Patch

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)))
 
 struct drm_i915_gem_object {
 	struct drm_gem_object base;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9553859..4d27243 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1123,7 +1123,7 @@  intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 
 	intel_plane->pipe = pipe;
 	intel_plane->plane = plane;
-	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe);
+	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
 	intel_plane->check_plane = intel_check_sprite_plane;
 	intel_plane->commit_plane = intel_commit_sprite_plane;
 	possible_crtcs = (1 << pipe);