Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-12-02 Thread Chris Wilson 
On Tue, Oct 27, 2015 at 05:21:37PM +0530, akash goel wrote:
> On Tue, Oct 6, 2015 at 4:23 PM, Chris Wilson  wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 19dd6b05ee1d..c35c9dc526e7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -603,6 +603,8 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> > flags |= BATCH_OFFSET_BIAS | PIN_OFFSET_BIAS;
> > if ((flags & PIN_MAPPABLE) == 0)
> > flags |= PIN_HIGH;
> > +   if (entry->flags & EXEC_OBJECT_PINNED)
> > +   flags |= entry->offset | PIN_OFFSET_FIXED;
> > }
> >
> > ret = i915_gem_object_pin(obj, vma->vm,
> > @@ -679,6 +681,10 @@ eb_vma_misplaced(struct i915_vma *vma)
> > if (vma->node.size < entry->pad_to_size)
> > return true;
> >
> > +   if (entry->flags & EXEC_OBJECT_PINNED &&
> > +   vma->node.start != entry->offset)
> > +   return true;
> > +
> > if (entry->flags & __EXEC_OBJECT_NEEDS_BIAS &&
> > vma->node.start < BATCH_OFFSET_BIAS)
> > return true;
> 
> 
> I think would be better to add the following change here.
> 
> - if ((entry->flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) == 0 &&
> + if (!(entry->flags &
> +(EXEC_OBJECT_SUPPORTS_48B_ADDRESS | EXEC_OBJECT_PINNED)) &&
>  (vma->node.start + vma->node.size - 1) >> 32)
>   return true;
> 
> This way, User will not have to necessarily pass the 48B_ADDRESS flag
> also along with the OBJECT_PINNED flag, if the offset is > 4 GB.
> The OBJECT_PINNED flag will take precedence over 48B_ADDRESS flag.

No, let's not start having flags mean multiple things if we can avoid
it as that makes the ABI harder to extend in future.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-11-06 Thread Kristian Høgsberg 
On Fri, Nov 6, 2015 at 5:38 AM, Chris Wilson  wrote:
> On Thu, Nov 05, 2015 at 10:17:56AM -0800, Jesse Barnes wrote:
>> On 11/05/2015 09:51 AM, Kristian Høgsberg wrote:
>> > On Tue, Oct 6, 2015 at 3:53 AM, Chris Wilson  
>> > wrote:
>> >> Userspace can pass in an offset that it presumes the object is located
>> >> at. The kernel will then do its utmost to fit the object into that
>> >> location. The assumption is that userspace is handling its own object
>> >> locations (for example along with full-ppgtt) and that the kernel will
>> >> rarely have to make space for the user's requests.
>> >
>> > I know the commit message isn't documentation, but the phrase "do its
>> > utmost" makes me uncomfortable. I'd like to be explicit about what
>> > might make it fail (should only be pinned fbs in case of aliased ppgtt
>> > or userspace errors such as overlapping placements), or conversely,
>> > spell out when the flag can be expected to work (full ppgtt).
>>
>> Ooh yeah that would be good to add to the execbuf man page with the
>> softpin additions.  Oh wait, we don't have a man page for execbuf?
>> Someone should write one!
>
> How about:
>
> This extends the DRM_I915_GEM_EXECBUFFER2 ioctl to do the following:
> * if the user supplies a virtual address via the execobject->offset
>   *and* sets the EXEC_OBJECT_PINNED flag in execobject->flags, then
>   that object is placed at that offset in the address space selected
>   by the context specifier in execbuffer.
> * the location must be aligned to the GTT page size, 4096 bytes
> * as the object is placed exactly as specified, it may be used in this 
> batch
>   without relocations pointing to it
>
> It may fail to do so if:
> * EINVAL is returned if the object does not have a 4096 byte aligned
>   address
> * the object conflicts with another pinned object (either pinned by
>   hardware in that address space, e.g. scanouts in the aliasing ppgtt)
>   or within the same batch.
>   EBUSY is returned if the location is pinned by hardware
>   EINVAL is returned if the location is already in use by the batch
> * EINVAL is returned if the object conflicts with its own alignment (as 
> meets
>   the hardware requirements) or if the placement of the object does not 
> fit
>   within the address space
>
> All other execbuffer errors apply.

Yes, that looks better.

Thanks,
Kristian

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-11-06 Thread Jesse Barnes 
On 11/06/2015 05:38 AM, Chris Wilson wrote:
> On Thu, Nov 05, 2015 at 10:17:56AM -0800, Jesse Barnes wrote:
>> On 11/05/2015 09:51 AM, Kristian Høgsberg wrote:
>>> On Tue, Oct 6, 2015 at 3:53 AM, Chris Wilson  
>>> wrote:
 Userspace can pass in an offset that it presumes the object is located
 at. The kernel will then do its utmost to fit the object into that
 location. The assumption is that userspace is handling its own object
 locations (for example along with full-ppgtt) and that the kernel will
 rarely have to make space for the user's requests.
>>>
>>> I know the commit message isn't documentation, but the phrase "do its
>>> utmost" makes me uncomfortable. I'd like to be explicit about what
>>> might make it fail (should only be pinned fbs in case of aliased ppgtt
>>> or userspace errors such as overlapping placements), or conversely,
>>> spell out when the flag can be expected to work (full ppgtt).
>>
>> Ooh yeah that would be good to add to the execbuf man page with the
>> softpin additions.  Oh wait, we don't have a man page for execbuf?
>> Someone should write one!
> 
> How about:
> 
> This extends the DRM_I915_GEM_EXECBUFFER2 ioctl to do the following:
> * if the user supplies a virtual address via the execobject->offset
>   *and* sets the EXEC_OBJECT_PINNED flag in execobject->flags, then
>   that object is placed at that offset in the address space selected
>   by the context specifier in execbuffer.
> * the location must be aligned to the GTT page size, 4096 bytes
> * as the object is placed exactly as specified, it may be used in this 
> batch
>   without relocations pointing to it
> 
> It may fail to do so if:
> * EINVAL is returned if the object does not have a 4096 byte aligned
>   address
> * the object conflicts with another pinned object (either pinned by
>   hardware in that address space, e.g. scanouts in the aliasing ppgtt)
>   or within the same batch.
>   EBUSY is returned if the location is pinned by hardware
>   EINVAL is returned if the location is already in use by the batch
> * EINVAL is returned if the object conflicts with its own alignment (as 
> meets
>   the hardware requirements) or if the placement of the object does not 
> fit
>   within the address space
> 
> All other execbuffer errors apply.

Looks great, now we just need an existing man page in which to integrate
this additional text.

Jesse

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-11-06 Thread Chris Wilson 
On Thu, Nov 05, 2015 at 10:17:56AM -0800, Jesse Barnes wrote:
> On 11/05/2015 09:51 AM, Kristian Høgsberg wrote:
> > On Tue, Oct 6, 2015 at 3:53 AM, Chris Wilson  
> > wrote:
> >> Userspace can pass in an offset that it presumes the object is located
> >> at. The kernel will then do its utmost to fit the object into that
> >> location. The assumption is that userspace is handling its own object
> >> locations (for example along with full-ppgtt) and that the kernel will
> >> rarely have to make space for the user's requests.
> > 
> > I know the commit message isn't documentation, but the phrase "do its
> > utmost" makes me uncomfortable. I'd like to be explicit about what
> > might make it fail (should only be pinned fbs in case of aliased ppgtt
> > or userspace errors such as overlapping placements), or conversely,
> > spell out when the flag can be expected to work (full ppgtt).
> 
> Ooh yeah that would be good to add to the execbuf man page with the
> softpin additions.  Oh wait, we don't have a man page for execbuf?
> Someone should write one!

How about:

This extends the DRM_I915_GEM_EXECBUFFER2 ioctl to do the following:
* if the user supplies a virtual address via the execobject->offset
  *and* sets the EXEC_OBJECT_PINNED flag in execobject->flags, then
  that object is placed at that offset in the address space selected
  by the context specifier in execbuffer.
* the location must be aligned to the GTT page size, 4096 bytes
* as the object is placed exactly as specified, it may be used in this batch
  without relocations pointing to it

It may fail to do so if:
* EINVAL is returned if the object does not have a 4096 byte aligned
  address
* the object conflicts with another pinned object (either pinned by
  hardware in that address space, e.g. scanouts in the aliasing ppgtt)
  or within the same batch.
  EBUSY is returned if the location is pinned by hardware
  EINVAL is returned if the location is already in use by the batch
* EINVAL is returned if the object conflicts with its own alignment (as 
meets
  the hardware requirements) or if the placement of the object does not fit
  within the address space

All other execbuffer errors apply.

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-11-05 Thread Daniel, Thomas 
> -Original Message-
> From: akash goel [mailto:akash.go...@gmail.com]
> Sent: Tuesday, October 27, 2015 11:52 AM
> To: Chris Wilson
> Cc: intel-gfx@lists.freedesktop.org; Goel, Akash; Daniel, Thomas
> Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for
> execbuffer
> 
> On Tue, Oct 6, 2015 at 4:23 PM, Chris Wilson <ch...@chris-wilson.co.uk> wrote:
> > Userspace can pass in an offset that it presumes the object is located
> > at. The kernel will then do its utmost to fit the object into that
> > location. The assumption is that userspace is handling its own object
> > locations (for example along with full-ppgtt) and that the kernel will
> > rarely have to make space for the user's requests.
> >
> > v2: Fix i915_gem_evict_range() (now evict_for_vma) to handle ordinary
> > and fixed objects within the same batch
> >
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Cc: "Daniel, Thomas" <thomas.dan...@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c|  3 ++
> >  drivers/gpu/drm/i915/i915_drv.h| 10 +++--
> >  drivers/gpu/drm/i915/i915_gem.c| 68 
> > +
> -
> >  drivers/gpu/drm/i915/i915_gem_evict.c  | 61
> +++
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  9 +++-
> >  drivers/gpu/drm/i915/i915_trace.h  | 23 ++
> >  include/uapi/drm/i915_drm.h|  4 +-
> >  7 files changed, 151 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c
> > index ab37d1121be8..cd79ef114b8e 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -170,6 +170,9 @@ static int i915_getparam(struct drm_device *dev, void
> *data,
> > case I915_PARAM_HAS_RESOURCE_STREAMER:
> > value = HAS_RESOURCE_STREAMER(dev);
> > break;
> > +   case I915_PARAM_HAS_EXEC_SOFTPIN:
> > +   value = 1;
> > +   break;
> > default:
> > DRM_DEBUG("Unknown parameter %d\n", param->param);
> > return -EINVAL;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> > index a0ce011a5dc0..7d351d991022 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2778,10 +2778,11 @@ void i915_gem_vma_destroy(struct i915_vma
> *vma);
> >  #define PIN_NONBLOCK   (1<<1)
> >  #define PIN_GLOBAL (1<<2)
> >  #define PIN_OFFSET_BIAS(1<<3)
> > -#define PIN_USER   (1<<4)
> > -#define PIN_UPDATE (1<<5)
> > -#define PIN_ZONE_4G(1<<6)
> > -#define PIN_HIGH   (1<<7)
> > +#define PIN_OFFSET_FIXED (1<<4)
> > +#define PIN_USER   (1<<5)
> > +#define PIN_UPDATE (1<<6)
> > +#define PIN_ZONE_4G(1<<7)
> > +#define PIN_HIGH   (1<<8)
> >  #define PIN_OFFSET_MASK (~4095)
> >  int __must_check
> >  i915_gem_object_pin(struct drm_i915_gem_object *obj,
> > @@ -3127,6 +3128,7 @@ int __must_check
> i915_gem_evict_something(struct drm_device *dev,
> >   unsigned long start,
> >   unsigned long end,
> >   unsigned flags);
> > +int __must_check i915_gem_evict_for_vma(struct i915_vma *vma, unsigned
> flags);
> >  int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
> >
> >  /* belongs in i915_gem_gtt.h */
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c
> > index 8fe3df0cdcb8..82efd6a6dee0 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3334,7 +3334,6 @@ i915_gem_object_bind_to_vm(struct
> drm_i915_gem_object *obj,
> > struct drm_device *dev = obj->base.dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > u64 start, end;
> > -   u32 search_flag, alloc_flag;
> > struct i915_vma *vma;
> > int ret;
> >
> > @@ -3409,30 +3408,53 @@ i915_gem_object_bind_to_vm(struct
> drm_i915_gem_object *obj,
> > if (IS_ERR(vma))
> > goto err_unpin;
> >
> > -   if (flags & PIN_HIGH) {
> > -   search_flag = DRM_MM_SEARCH_BELOW;
> > -

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-11-05 Thread Kristian Høgsberg 
On Tue, Oct 6, 2015 at 3:53 AM, Chris Wilson  wrote:
> Userspace can pass in an offset that it presumes the object is located
> at. The kernel will then do its utmost to fit the object into that
> location. The assumption is that userspace is handling its own object
> locations (for example along with full-ppgtt) and that the kernel will
> rarely have to make space for the user's requests.

I know the commit message isn't documentation, but the phrase "do its
utmost" makes me uncomfortable. I'd like to be explicit about what
might make it fail (should only be pinned fbs in case of aliased ppgtt
or userspace errors such as overlapping placements), or conversely,
spell out when the flag can be expected to work (full ppgtt).

Kristian

> v2: Fix i915_gem_evict_range() (now evict_for_vma) to handle ordinary
> and fixed objects within the same batch
>
> Signed-off-by: Chris Wilson 
> Cc: "Daniel, Thomas" 
> ---
>  drivers/gpu/drm/i915/i915_dma.c|  3 ++
>  drivers/gpu/drm/i915/i915_drv.h| 10 +++--
>  drivers/gpu/drm/i915/i915_gem.c| 68 
> +-
>  drivers/gpu/drm/i915/i915_gem_evict.c  | 61 +++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  9 +++-
>  drivers/gpu/drm/i915/i915_trace.h  | 23 ++
>  include/uapi/drm/i915_drm.h|  4 +-
>  7 files changed, 151 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index ab37d1121be8..cd79ef114b8e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -170,6 +170,9 @@ static int i915_getparam(struct drm_device *dev, void 
> *data,
> case I915_PARAM_HAS_RESOURCE_STREAMER:
> value = HAS_RESOURCE_STREAMER(dev);
> break;
> +   case I915_PARAM_HAS_EXEC_SOFTPIN:
> +   value = 1;
> +   break;
> default:
> DRM_DEBUG("Unknown parameter %d\n", param->param);
> return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a0ce011a5dc0..7d351d991022 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2778,10 +2778,11 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
>  #define PIN_NONBLOCK   (1<<1)
>  #define PIN_GLOBAL (1<<2)
>  #define PIN_OFFSET_BIAS(1<<3)
> -#define PIN_USER   (1<<4)
> -#define PIN_UPDATE (1<<5)
> -#define PIN_ZONE_4G(1<<6)
> -#define PIN_HIGH   (1<<7)
> +#define PIN_OFFSET_FIXED (1<<4)
> +#define PIN_USER   (1<<5)
> +#define PIN_UPDATE (1<<6)
> +#define PIN_ZONE_4G(1<<7)
> +#define PIN_HIGH   (1<<8)
>  #define PIN_OFFSET_MASK (~4095)
>  int __must_check
>  i915_gem_object_pin(struct drm_i915_gem_object *obj,
> @@ -3127,6 +3128,7 @@ int __must_check i915_gem_evict_something(struct 
> drm_device *dev,
>   unsigned long start,
>   unsigned long end,
>   unsigned flags);
> +int __must_check i915_gem_evict_for_vma(struct i915_vma *vma, unsigned 
> flags);
>  int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
>
>  /* belongs in i915_gem_gtt.h */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8fe3df0cdcb8..82efd6a6dee0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3334,7 +3334,6 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
> *obj,
> struct drm_device *dev = obj->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> u64 start, end;
> -   u32 search_flag, alloc_flag;
> struct i915_vma *vma;
> int ret;
>
> @@ -3409,30 +3408,53 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
> *obj,
> if (IS_ERR(vma))
> goto err_unpin;
>
> -   if (flags & PIN_HIGH) {
> -   search_flag = DRM_MM_SEARCH_BELOW;
> -   alloc_flag = DRM_MM_CREATE_TOP;
> +   if (flags & PIN_OFFSET_FIXED) {
> +   uint64_t offset = flags & PIN_OFFSET_MASK;
> +   if (offset & (alignment - 1) || offset + size > end) {
> +   vma = ERR_PTR(-EINVAL);
> +   goto err_free_vma;
> +   }
> +   vma->node.start = offset;
> +   vma->node.size = size;
> +   vma->node.color = obj->cache_level;
> +   ret = drm_mm_reserve_node(>mm, >node);
> +   if (ret) {
> +   ret = i915_gem_evict_for_vma(vma, flags);
> +   if (ret == 0)
> +   ret = drm_mm_reserve_node(>mm, 
> >node);
> +   }
> +   if (ret) {
> +   vma

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-10-27 Thread akash goel 
On Tue, Oct 6, 2015 at 4:23 PM, Chris Wilson  wrote:
> Userspace can pass in an offset that it presumes the object is located
> at. The kernel will then do its utmost to fit the object into that
> location. The assumption is that userspace is handling its own object
> locations (for example along with full-ppgtt) and that the kernel will
> rarely have to make space for the user's requests.
>
> v2: Fix i915_gem_evict_range() (now evict_for_vma) to handle ordinary
> and fixed objects within the same batch
>
> Signed-off-by: Chris Wilson 
> Cc: "Daniel, Thomas" 
> ---
>  drivers/gpu/drm/i915/i915_dma.c|  3 ++
>  drivers/gpu/drm/i915/i915_drv.h| 10 +++--
>  drivers/gpu/drm/i915/i915_gem.c| 68 
> +-
>  drivers/gpu/drm/i915/i915_gem_evict.c  | 61 +++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  9 +++-
>  drivers/gpu/drm/i915/i915_trace.h  | 23 ++
>  include/uapi/drm/i915_drm.h|  4 +-
>  7 files changed, 151 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index ab37d1121be8..cd79ef114b8e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -170,6 +170,9 @@ static int i915_getparam(struct drm_device *dev, void 
> *data,
> case I915_PARAM_HAS_RESOURCE_STREAMER:
> value = HAS_RESOURCE_STREAMER(dev);
> break;
> +   case I915_PARAM_HAS_EXEC_SOFTPIN:
> +   value = 1;
> +   break;
> default:
> DRM_DEBUG("Unknown parameter %d\n", param->param);
> return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a0ce011a5dc0..7d351d991022 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2778,10 +2778,11 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
>  #define PIN_NONBLOCK   (1<<1)
>  #define PIN_GLOBAL (1<<2)
>  #define PIN_OFFSET_BIAS(1<<3)
> -#define PIN_USER   (1<<4)
> -#define PIN_UPDATE (1<<5)
> -#define PIN_ZONE_4G(1<<6)
> -#define PIN_HIGH   (1<<7)
> +#define PIN_OFFSET_FIXED (1<<4)
> +#define PIN_USER   (1<<5)
> +#define PIN_UPDATE (1<<6)
> +#define PIN_ZONE_4G(1<<7)
> +#define PIN_HIGH   (1<<8)
>  #define PIN_OFFSET_MASK (~4095)
>  int __must_check
>  i915_gem_object_pin(struct drm_i915_gem_object *obj,
> @@ -3127,6 +3128,7 @@ int __must_check i915_gem_evict_something(struct 
> drm_device *dev,
>   unsigned long start,
>   unsigned long end,
>   unsigned flags);
> +int __must_check i915_gem_evict_for_vma(struct i915_vma *vma, unsigned 
> flags);
>  int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
>
>  /* belongs in i915_gem_gtt.h */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8fe3df0cdcb8..82efd6a6dee0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3334,7 +3334,6 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
> *obj,
> struct drm_device *dev = obj->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> u64 start, end;
> -   u32 search_flag, alloc_flag;
> struct i915_vma *vma;
> int ret;
>
> @@ -3409,30 +3408,53 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
> *obj,
> if (IS_ERR(vma))
> goto err_unpin;
>
> -   if (flags & PIN_HIGH) {
> -   search_flag = DRM_MM_SEARCH_BELOW;
> -   alloc_flag = DRM_MM_CREATE_TOP;
> +   if (flags & PIN_OFFSET_FIXED) {
> +   uint64_t offset = flags & PIN_OFFSET_MASK;
> +   if (offset & (alignment - 1) || offset + size > end) {
> +   vma = ERR_PTR(-EINVAL);
> +   goto err_free_vma;
> +   }
> +   vma->node.start = offset;
> +   vma->node.size = size;
> +   vma->node.color = obj->cache_level;
> +   ret = drm_mm_reserve_node(>mm, >node);
> +   if (ret) {
> +   ret = i915_gem_evict_for_vma(vma, flags);
> +   if (ret == 0)
> +   ret = drm_mm_reserve_node(>mm, 
> >node);
> +   }
> +   if (ret) {
> +   vma = ERR_PTR(ret);
> +   goto err_free_vma;
> +   }
> } else {
> -   search_flag = DRM_MM_SEARCH_DEFAULT;
> -   alloc_flag = DRM_MM_CREATE_DEFAULT;
> -   }
> +   u32 search_flag, alloc_flag;
> +
> +   if (flags & PIN_HIGH) {
> +

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-10-22 Thread Yang, Rong R 


> -Original Message-
> From: Daniel, Thomas
> Sent: Wednesday, October 21, 2015 23:11
> To: Daniel Vetter
> Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Belgaumkar, Vinay; Yang,
> Rong R
> Subject: RE: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for
> execbuffer
> 
> > -Original Message-
> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of
> > Daniel Vetter
> > Sent: Wednesday, October 21, 2015 4:08 PM
> > To: Daniel, Thomas
> > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API
> > for execbuffer
> >
> > On Tue, Oct 06, 2015 at 01:59:12PM +, Daniel, Thomas wrote:
> > > > -Original Message-
> > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > > > Sent: Tuesday, October 6, 2015 11:53 AM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: Chris Wilson; Daniel, Thomas
> > > > Subject: [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer
> > > >
> > > > Userspace can pass in an offset that it presumes the object is
> > > > located at. The kernel will then do its utmost to fit the object
> > > > into that location. The assumption is that userspace is handling
> > > > its own object locations (for example along with full-ppgtt) and
> > > > that the kernel will rarely have to make space for the user's requests.
> > > >
> > > > v2: Fix i915_gem_evict_range() (now evict_for_vma) to handle
> > > > ordinary and fixed objects within the same batch
> > > >
> > > > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > > > Cc: "Daniel, Thomas" <thomas.dan...@intel.com>
> > >
> > > This didn't apply cleanly to my tree pulled today (after patches 1
> > > and 2 of this
> > series).
> > > Are you going to post a rebase?
> >
> > It's a really trivial conflict in the uapi flag allocation. Happens
> > all the time with interface extensions.
> >
> > What I'm looking for here is the userspace for this new interface. And
> > the testcases.
> Hm I thought the beignet guys had already posted.
> Vinay has written i-g-t for this

Beignet svm patch haven't post, because the beignet's svm patch only work on 
i386 linux now, the x86_64 svm depends on 48bits pointer support in Beignet 
compiler's backend.
If the i386 svm patch is worthy for this patch, I will send it out.
 
> 
> > -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

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-10-21 Thread Daniel, Thomas 
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, October 21, 2015 4:08 PM
> To: Daniel, Thomas
> Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for
> execbuffer
> 
> On Tue, Oct 06, 2015 at 01:59:12PM +, Daniel, Thomas wrote:
> > > -Original Message-
> > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > > Sent: Tuesday, October 6, 2015 11:53 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Chris Wilson; Daniel, Thomas
> > > Subject: [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer
> > >
> > > Userspace can pass in an offset that it presumes the object is located
> > > at. The kernel will then do its utmost to fit the object into that
> > > location. The assumption is that userspace is handling its own object
> > > locations (for example along with full-ppgtt) and that the kernel will
> > > rarely have to make space for the user's requests.
> > >
> > > v2: Fix i915_gem_evict_range() (now evict_for_vma) to handle ordinary
> > > and fixed objects within the same batch
> > >
> > > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > > Cc: "Daniel, Thomas" <thomas.dan...@intel.com>
> >
> > This didn't apply cleanly to my tree pulled today (after patches 1 and 2 of 
> > this
> series).
> > Are you going to post a rebase?
> 
> It's a really trivial conflict in the uapi flag allocation. Happens all
> the time with interface extensions.
> 
> What I'm looking for here is the userspace for this new interface. And the
> testcases.
Hm I thought the beignet guys had already posted.
Vinay has written i-g-t for this

> -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

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-10-21 Thread Daniel Vetter 
On Tue, Oct 06, 2015 at 01:59:12PM +, Daniel, Thomas wrote:
> > -Original Message-
> > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > Sent: Tuesday, October 6, 2015 11:53 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Chris Wilson; Daniel, Thomas
> > Subject: [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer
> > 
> > Userspace can pass in an offset that it presumes the object is located
> > at. The kernel will then do its utmost to fit the object into that
> > location. The assumption is that userspace is handling its own object
> > locations (for example along with full-ppgtt) and that the kernel will
> > rarely have to make space for the user's requests.
> > 
> > v2: Fix i915_gem_evict_range() (now evict_for_vma) to handle ordinary
> > and fixed objects within the same batch
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: "Daniel, Thomas" 
> 
> This didn't apply cleanly to my tree pulled today (after patches 1 and 2 of 
> this series).
> Are you going to post a rebase?

It's a really trivial conflict in the uapi flag allocation. Happens all
the time with interface extensions.

What I'm looking for here is the userspace for this new interface. And the
testcases.
-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

[Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-10-06 Thread Chris Wilson 
Userspace can pass in an offset that it presumes the object is located
at. The kernel will then do its utmost to fit the object into that
location. The assumption is that userspace is handling its own object
locations (for example along with full-ppgtt) and that the kernel will
rarely have to make space for the user's requests.

v2: Fix i915_gem_evict_range() (now evict_for_vma) to handle ordinary
and fixed objects within the same batch

Signed-off-by: Chris Wilson 
Cc: "Daniel, Thomas" 
---
 drivers/gpu/drm/i915/i915_dma.c|  3 ++
 drivers/gpu/drm/i915/i915_drv.h| 10 +++--
 drivers/gpu/drm/i915/i915_gem.c| 68 +-
 drivers/gpu/drm/i915/i915_gem_evict.c  | 61 +++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  9 +++-
 drivers/gpu/drm/i915/i915_trace.h  | 23 ++
 include/uapi/drm/i915_drm.h|  4 +-
 7 files changed, 151 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ab37d1121be8..cd79ef114b8e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -170,6 +170,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_RESOURCE_STREAMER:
value = HAS_RESOURCE_STREAMER(dev);
break;
+   case I915_PARAM_HAS_EXEC_SOFTPIN:
+   value = 1;
+   break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0ce011a5dc0..7d351d991022 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2778,10 +2778,11 @@ void i915_gem_vma_destroy(struct i915_vma *vma);
 #define PIN_NONBLOCK   (1<<1)
 #define PIN_GLOBAL (1<<2)
 #define PIN_OFFSET_BIAS(1<<3)
-#define PIN_USER   (1<<4)
-#define PIN_UPDATE (1<<5)
-#define PIN_ZONE_4G(1<<6)
-#define PIN_HIGH   (1<<7)
+#define PIN_OFFSET_FIXED (1<<4)
+#define PIN_USER   (1<<5)
+#define PIN_UPDATE (1<<6)
+#define PIN_ZONE_4G(1<<7)
+#define PIN_HIGH   (1<<8)
 #define PIN_OFFSET_MASK (~4095)
 int __must_check
 i915_gem_object_pin(struct drm_i915_gem_object *obj,
@@ -3127,6 +3128,7 @@ int __must_check i915_gem_evict_something(struct 
drm_device *dev,
  unsigned long start,
  unsigned long end,
  unsigned flags);
+int __must_check i915_gem_evict_for_vma(struct i915_vma *vma, unsigned flags);
 int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
 
 /* belongs in i915_gem_gtt.h */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8fe3df0cdcb8..82efd6a6dee0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3334,7 +3334,6 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
*obj,
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
u64 start, end;
-   u32 search_flag, alloc_flag;
struct i915_vma *vma;
int ret;
 
@@ -3409,30 +3408,53 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
*obj,
if (IS_ERR(vma))
goto err_unpin;
 
-   if (flags & PIN_HIGH) {
-   search_flag = DRM_MM_SEARCH_BELOW;
-   alloc_flag = DRM_MM_CREATE_TOP;
+   if (flags & PIN_OFFSET_FIXED) {
+   uint64_t offset = flags & PIN_OFFSET_MASK;
+   if (offset & (alignment - 1) || offset + size > end) {
+   vma = ERR_PTR(-EINVAL);
+   goto err_free_vma;
+   }
+   vma->node.start = offset;
+   vma->node.size = size;
+   vma->node.color = obj->cache_level;
+   ret = drm_mm_reserve_node(>mm, >node);
+   if (ret) {
+   ret = i915_gem_evict_for_vma(vma, flags);
+   if (ret == 0)
+   ret = drm_mm_reserve_node(>mm, >node);
+   }
+   if (ret) {
+   vma = ERR_PTR(ret);
+   goto err_free_vma;
+   }
} else {
-   search_flag = DRM_MM_SEARCH_DEFAULT;
-   alloc_flag = DRM_MM_CREATE_DEFAULT;
-   }
+   u32 search_flag, alloc_flag;
+
+   if (flags & PIN_HIGH) {
+   search_flag = DRM_MM_SEARCH_BELOW;
+   alloc_flag = DRM_MM_CREATE_TOP;
+   } else {
+   search_flag = DRM_MM_SEARCH_DEFAULT;
+   alloc_flag = DRM_MM_CREATE_DEFAULT;
+   }
 
 search_free:
-   ret =

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2015-10-06 Thread Daniel, Thomas 
> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Tuesday, October 6, 2015 11:53 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson; Daniel, Thomas
> Subject: [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer
> 
> Userspace can pass in an offset that it presumes the object is located
> at. The kernel will then do its utmost to fit the object into that
> location. The assumption is that userspace is handling its own object
> locations (for example along with full-ppgtt) and that the kernel will
> rarely have to make space for the user's requests.
> 
> v2: Fix i915_gem_evict_range() (now evict_for_vma) to handle ordinary
> and fixed objects within the same batch
> 
> Signed-off-by: Chris Wilson 
> Cc: "Daniel, Thomas" 

This didn't apply cleanly to my tree pulled today (after patches 1 and 2 of 
this series).
Are you going to post a rebase?

Cheers,
Thomas.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

2014-05-15 Thread Chris Wilson 
Userspace can pass in an offset that it presumes the object is located
at. The kernel will then do its utmost to fit the object into that
location. The assumption is that userspace is handling its own object
locations (for example along with full-ppgtt) and that the kernel will
rarely have to make space for the user's requests.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_drv.h|  3 ++
 drivers/gpu/drm/i915/i915_gem.c| 50 
 drivers/gpu/drm/i915/i915_gem_evict.c  | 52 ++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  9 --
 include/uapi/drm/i915_drm.h|  3 +-
 5 files changed, 100 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 64a1b2340e3a..04fe312df8e4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2406,6 +2406,9 @@ int __must_check i915_gem_evict_something(struct 
drm_device *dev,
  unsigned long start,
  unsigned long end,
  unsigned flags);
+int __must_check
+i915_gem_evict_range(struct drm_device *dev, struct i915_address_space *vm,
+unsigned long start, unsigned long end);
 int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
 int i915_gem_evict_everything(struct drm_device *dev);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 79244b911125..ba4f266b5f2e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3313,22 +3313,43 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object 
*obj,
if (IS_ERR(vma))
goto err_unpin;
 
+   if (flags  PIN_OFFSET_FIXED) {
+   uint64_t offset = flags  PIN_OFFSET_MASK;
+   if (alignment  offset  (alignment - 1)) {
+   vma = ERR_PTR(-EINVAL);
+   goto err_free_vma;
+   }
+   vma-node.start = offset;
+   vma-node.size = size;
+   vma-node.color = obj-cache_level;
+   ret = drm_mm_reserve_node(vm-mm, vma-node);
+   if (ret) {
+   ret = i915_gem_evict_range(dev, vm, start, end);
+   if (ret == 0)
+   ret = drm_mm_reserve_node(vm-mm, vma-node);
+   }
+   if (ret) {
+   vma = ERR_PTR(ret);
+   goto err_free_vma;
+   }
+   } else {
 search_free:
-   ret = drm_mm_insert_node_in_range_generic(vm-mm, vma-node,
- size, alignment,
- obj-cache_level,
- start, end,
- DRM_MM_SEARCH_DEFAULT,
- DRM_MM_CREATE_DEFAULT);
-   if (ret) {
-   ret = i915_gem_evict_something(dev, vm, size, alignment,
-  obj-cache_level,
-  start, end,
-  flags);
-   if (ret == 0)
-   goto search_free;
+   ret = drm_mm_insert_node_in_range_generic(vm-mm, vma-node,
+ size, alignment,
+ obj-cache_level,
+ start, end,
+ DRM_MM_SEARCH_DEFAULT,
+ 
DRM_MM_CREATE_DEFAULT);
+   if (ret) {
+   ret = i915_gem_evict_something(dev, vm, size, alignment,
+  obj-cache_level,
+  start, end,
+  flags);
+   if (ret == 0)
+   goto search_free;
 
-   goto err_free_vma;
+   goto err_free_vma;
+   }
}
if (WARN_ON(!i915_gem_valid_gtt_space(dev, vma-node,
  obj-cache_level))) {
@@ -3911,6 +3932,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
if ((alignment 
 vma-node.start  (alignment - 1)) ||
(flags  PIN_MAPPABLE  !obj-map_and_fenceable) ||
+   (flags  PIN_OFFSET_FIXED  vma-node.start != (flags  
PIN_OFFSET_MASK)) ||
(flags  PIN_OFFSET_BIAS  vma-node.start  (flags  
PIN_OFFSET_MASK))) {