diff mbox

drm/i915: Skip fence installation for objects with rotated views

Message ID 1442369112-21987-1-git-send-email-vivek.kasireddy@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kasireddy, Vivek Sept. 16, 2015, 2:05 a.m. UTC
While pinning a fb object to the display plane, only install a fence
if the object is using a normal view. This corresponds with the
behavior found in i915_gem_object_do_pin() where the fencability
criteria is determined only for objects with normal views.

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Chris Wilson Sept. 16, 2015, 8:03 a.m. UTC | #1
On Tue, Sep 15, 2015 at 07:05:12PM -0700, Vivek Kasireddy wrote:
> While pinning a fb object to the display plane, only install a fence
> if the object is using a normal view. This corresponds with the
> behavior found in i915_gem_object_do_pin() where the fencability
> criteria is determined only for objects with normal views.
> 
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 52fb3f2..8b3e943 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2357,7 +2357,8 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>  	 * framebuffer compression.  For simplicity, we always install
>  	 * a fence as the cost is not that onerous.
>  	 */
> -	ret = i915_gem_object_get_fence(obj);
> +	ret = view.type == I915_GGTT_VIEW_NORMAL ?
> +				i915_gem_object_get_fence(obj) : 0;

ret = 0;
if (vma->map_and_fenceable)
	ret = i915_gem_object_get_fence(obj);

is how I wrote it in my patch. One day that will become
i915_vma_get_fence().
-Chris
Kasireddy, Vivek Sept. 16, 2015, 5:36 p.m. UTC | #2
On Wed, 16 Sep 2015 09:03:33 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, Sep 15, 2015 at 07:05:12PM -0700, Vivek Kasireddy wrote:
> > While pinning a fb object to the display plane, only install a fence
> > if the object is using a normal view. This corresponds with the
> > behavior found in i915_gem_object_do_pin() where the fencability
> > criteria is determined only for objects with normal views.
> > 
> > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c index 52fb3f2..8b3e943 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2357,7 +2357,8 @@ intel_pin_and_fence_fb_obj(struct drm_plane
> > *plane,
> >  	 * framebuffer compression.  For simplicity, we always
> > install
> >  	 * a fence as the cost is not that onerous.
> >  	 */
> > -	ret = i915_gem_object_get_fence(obj);
> > +	ret = view.type == I915_GGTT_VIEW_NORMAL ?
> > +				i915_gem_object_get_fence(obj) : 0;
> 
> ret = 0;
> if (vma->map_and_fenceable)
> 	ret = i915_gem_object_get_fence(obj);
> 
> is how I wrote it in my patch. One day that will become
> i915_vma_get_fence().
> -Chris
Hi Chris,

Looks like your solution to this problem -- and potentially others --
is better and more comprehensive. When do you plan on sending your
patch out to the mailing list?

Thanks,
Vivek

>
Tvrtko Ursulin Sept. 17, 2015, 10:25 a.m. UTC | #3
On 09/16/2015 06:36 PM, Vivek Kasireddy wrote:
> On Wed, 16 Sep 2015 09:03:33 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
>> On Tue, Sep 15, 2015 at 07:05:12PM -0700, Vivek Kasireddy wrote:
>>> While pinning a fb object to the display plane, only install a fence
>>> if the object is using a normal view. This corresponds with the
>>> behavior found in i915_gem_object_do_pin() where the fencability
>>> criteria is determined only for objects with normal views.
>>>
>>> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_display.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>> b/drivers/gpu/drm/i915/intel_display.c index 52fb3f2..8b3e943 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -2357,7 +2357,8 @@ intel_pin_and_fence_fb_obj(struct drm_plane
>>> *plane,
>>>   	 * framebuffer compression.  For simplicity, we always
>>> install
>>>   	 * a fence as the cost is not that onerous.
>>>   	 */
>>> -	ret = i915_gem_object_get_fence(obj);
>>> +	ret = view.type == I915_GGTT_VIEW_NORMAL ?
>>> +				i915_gem_object_get_fence(obj) : 0;
>>
>> ret = 0;
>> if (vma->map_and_fenceable)
>> 	ret = i915_gem_object_get_fence(obj);
>>
>> is how I wrote it in my patch. One day that will become
>> i915_vma_get_fence().
>> -Chris
> Hi Chris,
>
> Looks like your solution to this problem -- and potentially others --
> is better and more comprehensive. When do you plan on sending your
> patch out to the mailing list?

For a quick fix maybe just respin this patch to use 
obj->map_and_fenceable criteria as Chris suggested?

You should also add a test case for this into igt/kms_addfb_basic.

Regards,

Tvrtko
Kasireddy, Vivek Sept. 19, 2015, 1:56 a.m. UTC | #4
On Thu, 17 Sep 2015 11:25:18 +0100
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:

> 
> On 09/16/2015 06:36 PM, Vivek Kasireddy wrote:
> > On Wed, 16 Sep 2015 09:03:33 +0100
> > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> >> On Tue, Sep 15, 2015 at 07:05:12PM -0700, Vivek Kasireddy wrote:
> >>> While pinning a fb object to the display plane, only install a
> >>> fence if the object is using a normal view. This corresponds with
> >>> the behavior found in i915_gem_object_do_pin() where the
> >>> fencability criteria is determined only for objects with normal
> >>> views.
> >>>
> >>> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> >>> ---
> >>>   drivers/gpu/drm/i915/intel_display.c | 3 ++-
> >>>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >>> b/drivers/gpu/drm/i915/intel_display.c index 52fb3f2..8b3e943
> >>> 100644 --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -2357,7 +2357,8 @@ intel_pin_and_fence_fb_obj(struct drm_plane
> >>> *plane,
> >>>   	 * framebuffer compression.  For simplicity, we always
> >>> install
> >>>   	 * a fence as the cost is not that onerous.
> >>>   	 */
> >>> -	ret = i915_gem_object_get_fence(obj);
> >>> +	ret = view.type == I915_GGTT_VIEW_NORMAL ?
> >>> +				i915_gem_object_get_fence(obj) :
> >>> 0;
> >>
> >> ret = 0;
> >> if (vma->map_and_fenceable)
> >> 	ret = i915_gem_object_get_fence(obj);
> >>
> >> is how I wrote it in my patch. One day that will become
> >> i915_vma_get_fence().
> >> -Chris
> > Hi Chris,
> >
> > Looks like your solution to this problem -- and potentially others
> > -- is better and more comprehensive. When do you plan on sending
> > your patch out to the mailing list?
> 
> For a quick fix maybe just respin this patch to use 
> obj->map_and_fenceable criteria as Chris suggested?
> 
> You should also add a test case for this into igt/kms_addfb_basic.
Hi Tvrtko,
I'll send out a patch soon that will serve as as stop-gap measure until
Chris revamps that part of the code. As far as the igt test case is
concerned, I'll do that in the next few days.


Thanks,
Vivek

> 
> Regards,
> 
> Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 52fb3f2..8b3e943 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2357,7 +2357,8 @@  intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 	 * framebuffer compression.  For simplicity, we always install
 	 * a fence as the cost is not that onerous.
 	 */
-	ret = i915_gem_object_get_fence(obj);
+	ret = view.type == I915_GGTT_VIEW_NORMAL ?
+				i915_gem_object_get_fence(obj) : 0;
 	if (ret == -EDEADLK) {
 		/*
 		 * -EDEADLK means there are no free fences