[PATCH] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping

[PATCH] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping

[Chris Wilson]

A long time ago (before 3.14) we relied on a permanent pinning of the
ifbdev to lock the fb in place inside the GGTT. However, the
introduction of stealing the BIOS framebuffer and reusing its address in
the GGTT for the fbdev has muddied waters and we use an inherited fb.
However, the inherited fb is only pinned whilst it is active and we no
longer have an explicit pin for the info->system_base mmapping used by
the fbdev. The result is that after some aperture pressure the fbdev may
be evicted, but we continue to write the fbcon into the same GGTT
address - overwriting anything else that may be put into that offset.
The effect is most pronounced across suspend/resume as
intel_fbdev_set_suspend() does a full clear over the whole scanout.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: "Goel, Akash" <akash.goel@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_fbdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 96476d7d7ed2..082f2938ec97 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -215,6 +215,16 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	obj = intel_fb->obj;
 	size = obj->base.size;
 
+	/* The fb constructor will have already pinned us (or inherited a
+	 * GGTT region from the BIOS) suitable for a scanout, so
+	 * this should just be a no-op and increment the pin count for the
+	 * fbdev mmapping. It does have a useful side-effect of validating
+	 * the pin for fbdev's use via a GGTT mmapping.
+	 */
+	ret = i915_gem_object_ggtt_pin(obj, NULL, 0, PIN_MAPPABLE);
+	if (ret)
+		goto out_unlock;
+
 	info = drm_fb_helper_alloc_fbi(helper);
 	if (IS_ERR(info)) {
 		ret = PTR_ERR(info);
@@ -274,6 +284,9 @@ static int intelfb_create(struct drm_fb_helper *helper,
 out_destroy_fbi:
 	drm_fb_helper_release_fbi(helper);
 out_unpin:
+	/* Once for info->screen_base mmaping... */
+	i915_gem_object_ggtt_unpin(obj);
+	/* ...and once for the intel_fb */
 	i915_gem_object_ggtt_unpin(obj);
 	drm_gem_object_unreference(&obj->base);
 out_unlock:
@@ -514,6 +527,8 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
 static void intel_fbdev_destroy(struct drm_device *dev,
 				struct intel_fbdev *ifbdev)
 {
+	/* Release the pinning for the info->screen_base mmaping. */
+	i915_gem_object_ggtt_unpin(ifbdev->fb->obj);
 
 	drm_fb_helper_unregister_fbi(&ifbdev->helper);
 	drm_fb_helper_release_fbi(&ifbdev->helper);
-- 
2.5.0

[PATCH] drm/i915: Set the map-and-fenceable flag for preallocated objects

[Chris Wilson]

As we mark the preallocated objects as bound, we should also flag them
correctly as being map-and-fenceable (if appropriate!) so that latter
users do not get confused and try and rebind the pinned vma in order to
get a map-and-fenceable binding.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: "Goel, Akash" <akash.goel@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_gem.c     | 43 +++++++++++++++++++++----------------
 drivers/gpu/drm/i915/i915_gem_gtt.c |  1 +
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 55611d81ec6c..ec731e6db126 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2798,6 +2798,7 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
 
 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
 		  u32 flags);
+void __i915_vma_set_map_and_fenceable(struct i915_vma *vma);
 int __must_check i915_vma_unbind(struct i915_vma *vma);
 int i915_gem_object_put_pages(struct drm_i915_gem_object *obj);
 void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 407b6b3576ae..39571e67f9a5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3980,6 +3980,29 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags)
 	return false;
 }
 
+void __i915_vma_set_map_and_fenceable(struct i915_vma *vma)
+{
+	struct drm_i915_gem_object *obj = vma->obj;
+	bool mappable, fenceable;
+	u32 fence_size, fence_alignment;
+
+	fence_size = i915_gem_get_gtt_size(obj->base.dev,
+					   obj->base.size,
+					   obj->tiling_mode);
+	fence_alignment = i915_gem_get_gtt_alignment(obj->base.dev,
+						     obj->base.size,
+						     obj->tiling_mode,
+						     true);
+
+	fenceable = (vma->node.size == fence_size &&
+		     (vma->node.start & (fence_alignment - 1)) == 0);
+
+	mappable = (vma->node.start + fence_size <=
+		    to_i915(obj->base.dev)->gtt.mappable_end);
+
+	obj->map_and_fenceable = mappable && fenceable;
+}
+
 static int
 i915_gem_object_do_pin(struct drm_i915_gem_object *obj,
 		       struct i915_address_space *vm,
@@ -4047,25 +4070,7 @@ i915_gem_object_do_pin(struct drm_i915_gem_object *obj,
 
 	if (ggtt_view && ggtt_view->type == I915_GGTT_VIEW_NORMAL &&
 	    (bound ^ vma->bound) & GLOBAL_BIND) {
-		bool mappable, fenceable;
-		u32 fence_size, fence_alignment;
-
-		fence_size = i915_gem_get_gtt_size(obj->base.dev,
-						   obj->base.size,
-						   obj->tiling_mode);
-		fence_alignment = i915_gem_get_gtt_alignment(obj->base.dev,
-							     obj->base.size,
-							     obj->tiling_mode,
-							     true);
-
-		fenceable = (vma->node.size == fence_size &&
-			     (vma->node.start & (fence_alignment - 1)) == 0);
-
-		mappable = (vma->node.start + fence_size <=
-			    dev_priv->gtt.mappable_end);
-
-		obj->map_and_fenceable = mappable && fenceable;
-
+		__i915_vma_set_map_and_fenceable(vma);
 		WARN_ON(flags & PIN_MAPPABLE && !obj->map_and_fenceable);
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4a76807143b1..112d84c32257 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2586,6 +2586,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
 			return ret;
 		}
 		vma->bound |= GLOBAL_BIND;
+		__i915_vma_set_map_and_fenceable(vma);
 	}
 
 	/* Clear any non-preallocated blocks */
-- 
2.5.0

Re: [PATCH] drm/i915: Set the map-and-fenceable flag for preallocated objects

[Daniel Vetter]

On Wed, Aug 26, 2015 at 12:55:57PM +0100, Chris Wilson wrote:
> As we mark the preallocated objects as bound, we should also flag them
> correctly as being map-and-fenceable (if appropriate!) so that latter
> users do not get confused and try and rebind the pinned vma in order to
> get a map-and-fenceable binding.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: "Goel, Akash" <akash.goel@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: stable@vger.kernel.org

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Jani, can you please pick up both? And some bugzilla references for either
would be great too - Chris?

Oh and does patch 1 fix the execlist resume troubles? Execlist having
bigger contexts might be enough explanations for the apparent regression.

And can we igt patch 1 somehow? E.g. with memory pressure plus doing an
mmap on the legacy fbdev ...
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/i915_gem.c     | 43 +++++++++++++++++++++----------------
>  drivers/gpu/drm/i915/i915_gem_gtt.c |  1 +
>  3 files changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 55611d81ec6c..ec731e6db126 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2798,6 +2798,7 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
>  
>  int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
>  		  u32 flags);
> +void __i915_vma_set_map_and_fenceable(struct i915_vma *vma);
>  int __must_check i915_vma_unbind(struct i915_vma *vma);
>  int i915_gem_object_put_pages(struct drm_i915_gem_object *obj);
>  void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 407b6b3576ae..39571e67f9a5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3980,6 +3980,29 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags)
>  	return false;
>  }
>  
> +void __i915_vma_set_map_and_fenceable(struct i915_vma *vma)
> +{
> +	struct drm_i915_gem_object *obj = vma->obj;
> +	bool mappable, fenceable;
> +	u32 fence_size, fence_alignment;
> +
> +	fence_size = i915_gem_get_gtt_size(obj->base.dev,
> +					   obj->base.size,
> +					   obj->tiling_mode);
> +	fence_alignment = i915_gem_get_gtt_alignment(obj->base.dev,
> +						     obj->base.size,
> +						     obj->tiling_mode,
> +						     true);
> +
> +	fenceable = (vma->node.size == fence_size &&
> +		     (vma->node.start & (fence_alignment - 1)) == 0);
> +
> +	mappable = (vma->node.start + fence_size <=
> +		    to_i915(obj->base.dev)->gtt.mappable_end);
> +
> +	obj->map_and_fenceable = mappable && fenceable;
> +}
> +
>  static int
>  i915_gem_object_do_pin(struct drm_i915_gem_object *obj,
>  		       struct i915_address_space *vm,
> @@ -4047,25 +4070,7 @@ i915_gem_object_do_pin(struct drm_i915_gem_object *obj,
>  
>  	if (ggtt_view && ggtt_view->type == I915_GGTT_VIEW_NORMAL &&
>  	    (bound ^ vma->bound) & GLOBAL_BIND) {
> -		bool mappable, fenceable;
> -		u32 fence_size, fence_alignment;
> -
> -		fence_size = i915_gem_get_gtt_size(obj->base.dev,
> -						   obj->base.size,
> -						   obj->tiling_mode);
> -		fence_alignment = i915_gem_get_gtt_alignment(obj->base.dev,
> -							     obj->base.size,
> -							     obj->tiling_mode,
> -							     true);
> -
> -		fenceable = (vma->node.size == fence_size &&
> -			     (vma->node.start & (fence_alignment - 1)) == 0);
> -
> -		mappable = (vma->node.start + fence_size <=
> -			    dev_priv->gtt.mappable_end);
> -
> -		obj->map_and_fenceable = mappable && fenceable;
> -
> +		__i915_vma_set_map_and_fenceable(vma);
>  		WARN_ON(flags & PIN_MAPPABLE && !obj->map_and_fenceable);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4a76807143b1..112d84c32257 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2586,6 +2586,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>  			return ret;
>  		}
>  		vma->bound |= GLOBAL_BIND;
> +		__i915_vma_set_map_and_fenceable(vma);
>  	}
>  
>  	/* Clear any non-preallocated blocks */
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/i915: Set the map-and-fenceable flag for preallocated objects

[Daniel Vetter]

On Wed, Aug 26, 2015 at 03:06:59PM +0200, Daniel Vetter wrote:
> On Wed, Aug 26, 2015 at 12:55:57PM +0100, Chris Wilson wrote:
> > As we mark the preallocated objects as bound, we should also flag them
> > correctly as being map-and-fenceable (if appropriate!) so that latter
> > users do not get confused and try and rebind the pinned vma in order to
> > get a map-and-fenceable binding.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: "Goel, Akash" <akash.goel@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Cc: stable@vger.kernel.org
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Jani, can you please pick up both? And some bugzilla references for either
> would be great too - Chris?
> 
> Oh and does patch 1 fix the execlist resume troubles? Execlist having
> bigger contexts might be enough explanations for the apparent regression.
> 
> And can we igt patch 1 somehow? E.g. with memory pressure plus doing an
> mmap on the legacy fbdev ...

Actually add Jani ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/i915: Set the map-and-fenceable flag for preallocated objects

[Chris Wilson]

On Wed, Aug 26, 2015 at 03:06:59PM +0200, Daniel Vetter wrote:
> On Wed, Aug 26, 2015 at 12:55:57PM +0100, Chris Wilson wrote:
> > As we mark the preallocated objects as bound, we should also flag them
> > correctly as being map-and-fenceable (if appropriate!) so that latter
> > users do not get confused and try and rebind the pinned vma in order to
> > get a map-and-fenceable binding.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: "Goel, Akash" <akash.goel@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Cc: stable@vger.kernel.org
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Jani, can you please pick up both? And some bugzilla references for either
> would be great too - Chris?

There's a few candidate "overwrite of address 0" bugs, but no way to
really tell if that is the fbcon or userspace without trial and error.
 
> Oh and does patch 1 fix the execlist resume troubles? Execlist having
> bigger contexts might be enough explanations for the apparent regression.

Hmm, plausible.
 
> And can we igt patch 1 somehow? E.g. with memory pressure plus doing an
> mmap on the legacy fbdev ...

Look at i915_gem_framebuffer for the fbcon gtt address before/after
aperture thrashing. Would also need to switch to a user framebuffer to
drop the scanout pinning.

Or just assert that it is still pinned following a modeset away from
fbcon.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH] drm/i915: Set the map-and-fenceable flag for preallocated objects

[Jani Nikula]

On Wed, 26 Aug 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Aug 26, 2015 at 12:55:57PM +0100, Chris Wilson wrote:
>> As we mark the preallocated objects as bound, we should also flag them
>> correctly as being map-and-fenceable (if appropriate!) so that latter
>> users do not get confused and try and rebind the pinned vma in order to
>> get a map-and-fenceable binding.
>> 
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: "Goel, Akash" <akash.goel@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
>> Cc: stable@vger.kernel.org
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Jani, can you please pick up both? And some bugzilla references for either
> would be great too - Chris?

Both pushed to drm-intel-next-fixes. Thanks for the patches and review.

BR,
Jani.

>
> Oh and does patch 1 fix the execlist resume troubles? Execlist having
> bigger contexts might be enough explanations for the apparent regression.
>
> And can we igt patch 1 somehow? E.g. with memory pressure plus doing an
> mmap on the legacy fbdev ...
> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>>  drivers/gpu/drm/i915/i915_gem.c     | 43 +++++++++++++++++++++----------------
>>  drivers/gpu/drm/i915/i915_gem_gtt.c |  1 +
>>  3 files changed, 26 insertions(+), 19 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 55611d81ec6c..ec731e6db126 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2798,6 +2798,7 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
>>  
>>  int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
>>  		  u32 flags);
>> +void __i915_vma_set_map_and_fenceable(struct i915_vma *vma);
>>  int __must_check i915_vma_unbind(struct i915_vma *vma);
>>  int i915_gem_object_put_pages(struct drm_i915_gem_object *obj);
>>  void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 407b6b3576ae..39571e67f9a5 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -3980,6 +3980,29 @@ i915_vma_misplaced(struct i915_vma *vma, uint32_t alignment, uint64_t flags)
>>  	return false;
>>  }
>>  
>> +void __i915_vma_set_map_and_fenceable(struct i915_vma *vma)
>> +{
>> +	struct drm_i915_gem_object *obj = vma->obj;
>> +	bool mappable, fenceable;
>> +	u32 fence_size, fence_alignment;
>> +
>> +	fence_size = i915_gem_get_gtt_size(obj->base.dev,
>> +					   obj->base.size,
>> +					   obj->tiling_mode);
>> +	fence_alignment = i915_gem_get_gtt_alignment(obj->base.dev,
>> +						     obj->base.size,
>> +						     obj->tiling_mode,
>> +						     true);
>> +
>> +	fenceable = (vma->node.size == fence_size &&
>> +		     (vma->node.start & (fence_alignment - 1)) == 0);
>> +
>> +	mappable = (vma->node.start + fence_size <=
>> +		    to_i915(obj->base.dev)->gtt.mappable_end);
>> +
>> +	obj->map_and_fenceable = mappable && fenceable;
>> +}
>> +
>>  static int
>>  i915_gem_object_do_pin(struct drm_i915_gem_object *obj,
>>  		       struct i915_address_space *vm,
>> @@ -4047,25 +4070,7 @@ i915_gem_object_do_pin(struct drm_i915_gem_object *obj,
>>  
>>  	if (ggtt_view && ggtt_view->type == I915_GGTT_VIEW_NORMAL &&
>>  	    (bound ^ vma->bound) & GLOBAL_BIND) {
>> -		bool mappable, fenceable;
>> -		u32 fence_size, fence_alignment;
>> -
>> -		fence_size = i915_gem_get_gtt_size(obj->base.dev,
>> -						   obj->base.size,
>> -						   obj->tiling_mode);
>> -		fence_alignment = i915_gem_get_gtt_alignment(obj->base.dev,
>> -							     obj->base.size,
>> -							     obj->tiling_mode,
>> -							     true);
>> -
>> -		fenceable = (vma->node.size == fence_size &&
>> -			     (vma->node.start & (fence_alignment - 1)) == 0);
>> -
>> -		mappable = (vma->node.start + fence_size <=
>> -			    dev_priv->gtt.mappable_end);
>> -
>> -		obj->map_and_fenceable = mappable && fenceable;
>> -
>> +		__i915_vma_set_map_and_fenceable(vma);
>>  		WARN_ON(flags & PIN_MAPPABLE && !obj->map_and_fenceable);
>>  	}
>>  
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 4a76807143b1..112d84c32257 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -2586,6 +2586,7 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>>  			return ret;
>>  		}
>>  		vma->bound |= GLOBAL_BIND;
>> +		__i915_vma_set_map_and_fenceable(vma);
>>  	}
>>  
>>  	/* Clear any non-preallocated blocks */
>> -- 
>> 2.5.0
>> 
>
> -- 
> 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

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [Intel-gfx] [PATCH] drm/i915: Set the map-and-fenceable flag for preallocated objects

[Jesse Barnes]

On 08/27/2015 01:36 AM, Jani Nikula wrote:
> On Wed, 26 Aug 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Aug 26, 2015 at 12:55:57PM +0100, Chris Wilson wrote:
>>> As we mark the preallocated objects as bound, we should also flag them
>>> correctly as being map-and-fenceable (if appropriate!) so that latter
>>> users do not get confused and try and rebind the pinned vma in order to
>>> get a map-and-fenceable binding.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: "Goel, Akash" <akash.goel@intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
>>> Cc: stable@vger.kernel.org
>>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Jani, can you please pick up both? And some bugzilla references for either
>> would be great too - Chris?
> 
> Both pushed to drm-intel-next-fixes. Thanks for the patches and review.

This one breaks my HSW.  I hit the warn in 


int
i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
			 const struct i915_ggtt_view *view,
			 uint32_t alignment,
			 uint64_t flags)
{
	if (WARN_ONCE(!view, "no view specified"))
		return -EINVAL;

	return i915_gem_object_do_pin(obj, i915_obj_to_ggtt(obj), view,
				      alignment, flags | PIN_GLOBAL);
}

and the fb console doesn't come up.  Is this a merge error somehow?  I don't see how it could have worked... maybe w/o fbdev enabled or something?

Thanks,
Jesse

Re: [Intel-gfx] [PATCH] drm/i915: Set the map-and-fenceable flag for preallocated objects

[Chris Wilson]

On Thu, Aug 27, 2015 at 09:19:18AM -0700, Jesse Barnes wrote:
> On 08/27/2015 01:36 AM, Jani Nikula wrote:
> > On Wed, 26 Aug 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> On Wed, Aug 26, 2015 at 12:55:57PM +0100, Chris Wilson wrote:
> >>> As we mark the preallocated objects as bound, we should also flag them
> >>> correctly as being map-and-fenceable (if appropriate!) so that latter
> >>> users do not get confused and try and rebind the pinned vma in order to
> >>> get a map-and-fenceable binding.
> >>>
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Cc: "Goel, Akash" <akash.goel@intel.com>
> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> >>> Cc: stable@vger.kernel.org
> >>
> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>
> >> Jani, can you please pick up both? And some bugzilla references for either
> >> would be great too - Chris?
> > 
> > Both pushed to drm-intel-next-fixes. Thanks for the patches and review.
> 
> This one breaks my HSW.  I hit the warn in 
> 
> 
> int
> i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
> 			 const struct i915_ggtt_view *view,
> 			 uint32_t alignment,
> 			 uint64_t flags)
> {
> 	if (WARN_ONCE(!view, "no view specified"))
> 		return -EINVAL;
> 
> 	return i915_gem_object_do_pin(obj, i915_obj_to_ggtt(obj), view,
> 				      alignment, flags | PIN_GLOBAL);
> }
> 
> and the fb console doesn't come up.  Is this a merge error somehow?  I don't see how it could have worked... maybe w/o fbdev enabled or something?

No, it was just written against a different pin interface.

Use i915_gem_obj_ggtt_pin() instead.

Exactly.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH] drm/i915: Set the map-and-fenceable flag for preallocated objects

[Jani Nikula]

On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Aug 27, 2015 at 09:19:18AM -0700, Jesse Barnes wrote:
>> On 08/27/2015 01:36 AM, Jani Nikula wrote:
>> > On Wed, 26 Aug 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >> On Wed, Aug 26, 2015 at 12:55:57PM +0100, Chris Wilson wrote:
>> >>> As we mark the preallocated objects as bound, we should also flag them
>> >>> correctly as being map-and-fenceable (if appropriate!) so that latter
>> >>> users do not get confused and try and rebind the pinned vma in order to
>> >>> get a map-and-fenceable binding.
>> >>>
>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> >>> Cc: "Goel, Akash" <akash.goel@intel.com>
>> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> >>> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
>> >>> Cc: stable@vger.kernel.org
>> >>
>> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> >>
>> >> Jani, can you please pick up both? And some bugzilla references for either
>> >> would be great too - Chris?
>> > 
>> > Both pushed to drm-intel-next-fixes. Thanks for the patches and review.
>> 
>> This one breaks my HSW.  I hit the warn in 
>> 
>> 
>> int
>> i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
>> 			 const struct i915_ggtt_view *view,
>> 			 uint32_t alignment,
>> 			 uint64_t flags)
>> {
>> 	if (WARN_ONCE(!view, "no view specified"))
>> 		return -EINVAL;
>> 
>> 	return i915_gem_object_do_pin(obj, i915_obj_to_ggtt(obj), view,
>> 				      alignment, flags | PIN_GLOBAL);
>> }
>> 
>> and the fb console doesn't come up.  Is this a merge error somehow?  I don't see how it could have worked... maybe w/o fbdev enabled or something?
>
> No, it was just written against a different pin interface.
>
> Use i915_gem_obj_ggtt_pin() instead.
>
> Exactly.

Both patches dropped from drm-intel-next-fixes.

BR,
Jani.


> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Jani Nikula, Intel Open Source Technology Center