Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory region and object backend with TTM

2022-06-15 Thread Ruhl, Michael J
Hi Adrian,

Sorry for the delayed response, this got buried in my inbox...

>-Original Message-
>From: Adrian Larumbe 
>Sent: Monday, June 6, 2022 4:20 PM
>To: Ruhl, Michael J 
>Cc: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory
>region and object backend with TTM
>
>On 01.06.2022 19:43, Ruhl, Michael J wrote:
>>>-Original Message-
>>>From: Adrian Larumbe 
>>>Sent: Friday, May 27, 2022 12:08 PM
>>>To: Ruhl, Michael J 
>>>Cc: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
>>>Subject: Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem
>memory
>>>region and object backend with TTM
>>>
>>>On 17.05.2022 21:39, Ruhl, Michael J wrote:
>>>> >-Original Message-
>>>> >From: Intel-gfx  On Behalf Of
>>>> >Adrian Larumbe
>>>> >Sent: Tuesday, May 17, 2022 4:45 PM
>>>> >To: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
>>>> >Cc: adrian.laru...@collabora.com
>>>> >Subject: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem
>memory
>>>> >region and object backend with TTM
>>>> >
>>>> >Remove shmem region and object backend code as they are now
>>>> >unnecessary.
>>>> >In the case of objects placed in SMEM and backed by kernel shmem
>files,
>>>the
>>>> >file pointer has to be retrieved from the ttm_tt structure, so change this
>>>> >as well. This means accessing an shmem-backed BO's file pointer
>requires
>>>> >getting its pages beforehand, unlike in the old shmem backend.
>>>> >
>>>> >Expand TTM BO creation by handling devices with no LLC so that their
>>>> >caching and coherency properties are set accordingly.
>>>> >
>>>> >Adapt phys backend to put pages of original shmem object in a 'TTM
>way',
>>>> >also making sure its pages are put when a TTM shmem object has no
>struct
>>>> >page.
>>>> >
>>>> >Signed-off-by: Adrian Larumbe 
>>>> >---
>>>> > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c   |  12 +-
>>>> > drivers/gpu/drm/i915/gem/i915_gem_mman.c |  32 +-
>>>> > drivers/gpu/drm/i915/gem/i915_gem_object.h   |   4 +-
>>>> > drivers/gpu/drm/i915/gem/i915_gem_phys.c |  32 +-
>>>> > drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 390 +--
>>>> > drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 267 -
>>>> > drivers/gpu/drm/i915/gem/i915_gem_ttm.h  |   3 +
>>>> > drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |   9 +-
>>>> > drivers/gpu/drm/i915/gt/shmem_utils.c|  64 ++-
>>>> > drivers/gpu/drm/i915/intel_memory_region.c   |   7 +-
>>>> > 10 files changed, 398 insertions(+), 422 deletions(-)
>>>> >
>>>> >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>>>> >b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>>>> >index f5062d0c6333..de04ce4210b3 100644
>>>> >--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>>>> >+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>>>> >@@ -12,6 +12,7 @@
>>>> > #include 
>>>> >
>>>> > #include "gem/i915_gem_dmabuf.h"
>>>> >+#include "gem/i915_gem_ttm.h"
>>>> > #include "i915_drv.h"
>>>> > #include "i915_gem_object.h"
>>>> > #include "i915_scatterlist.h"
>>>> >@@ -94,22 +95,25 @@ static int i915_gem_dmabuf_mmap(struct
>dma_buf
>>>> >*dma_buf, struct vm_area_struct *
>>>> > {
>>>> >  struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
>>>> >  struct drm_i915_private *i915 = to_i915(obj->base.dev);
>>>> >+ struct file *filp = i915_gem_ttm_get_filep(obj);
>>>> >  int ret;
>>>> >
>>>> >+ GEM_BUG_ON(obj->base.import_attach != NULL);
>>>> >+
>>>> >  if (obj->base.size < vma->vm_end - vma->vm_start)
>>>> >  return -EINVAL;
>>>> >
>>>> >  if (HAS_LMEM(i915))
>>>> >  return drm_gem_prime_mmap(&obj->base, vma);
>>>>
>>>> Since all of the devices that will have device memory will be true for
>>>HAS_LMEM,
>&

Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory region and object backend with TTM

2022-06-06 Thread Adrian Larumbe
On 01.06.2022 19:43, Ruhl, Michael J wrote:
>>-Original Message-
>>From: Adrian Larumbe 
>>Sent: Friday, May 27, 2022 12:08 PM
>>To: Ruhl, Michael J 
>>Cc: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
>>Subject: Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory
>>region and object backend with TTM
>>
>>On 17.05.2022 21:39, Ruhl, Michael J wrote:
>>> >-Original Message-
>>> >From: Intel-gfx  On Behalf Of
>>> >Adrian Larumbe
>>> >Sent: Tuesday, May 17, 2022 4:45 PM
>>> >To: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
>>> >Cc: adrian.laru...@collabora.com
>>> >Subject: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory
>>> >region and object backend with TTM
>>> >
>>> >Remove shmem region and object backend code as they are now
>>> >unnecessary.
>>> >In the case of objects placed in SMEM and backed by kernel shmem files,
>>the
>>> >file pointer has to be retrieved from the ttm_tt structure, so change this
>>> >as well. This means accessing an shmem-backed BO's file pointer requires
>>> >getting its pages beforehand, unlike in the old shmem backend.
>>> >
>>> >Expand TTM BO creation by handling devices with no LLC so that their
>>> >caching and coherency properties are set accordingly.
>>> >
>>> >Adapt phys backend to put pages of original shmem object in a 'TTM way',
>>> >also making sure its pages are put when a TTM shmem object has no struct
>>> >page.
>>> >
>>> >Signed-off-by: Adrian Larumbe 
>>> >---
>>> > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c   |  12 +-
>>> > drivers/gpu/drm/i915/gem/i915_gem_mman.c |  32 +-
>>> > drivers/gpu/drm/i915/gem/i915_gem_object.h   |   4 +-
>>> > drivers/gpu/drm/i915/gem/i915_gem_phys.c |  32 +-
>>> > drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 390 +--
>>> > drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 267 -
>>> > drivers/gpu/drm/i915/gem/i915_gem_ttm.h  |   3 +
>>> > drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |   9 +-
>>> > drivers/gpu/drm/i915/gt/shmem_utils.c|  64 ++-
>>> > drivers/gpu/drm/i915/intel_memory_region.c   |   7 +-
>>> > 10 files changed, 398 insertions(+), 422 deletions(-)
>>> >
>>> >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>>> >b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>>> >index f5062d0c6333..de04ce4210b3 100644
>>> >--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>>> >+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>>> >@@ -12,6 +12,7 @@
>>> > #include 
>>> >
>>> > #include "gem/i915_gem_dmabuf.h"
>>> >+#include "gem/i915_gem_ttm.h"
>>> > #include "i915_drv.h"
>>> > #include "i915_gem_object.h"
>>> > #include "i915_scatterlist.h"
>>> >@@ -94,22 +95,25 @@ static int i915_gem_dmabuf_mmap(struct dma_buf
>>> >*dma_buf, struct vm_area_struct *
>>> > {
>>> >   struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
>>> >   struct drm_i915_private *i915 = to_i915(obj->base.dev);
>>> >+  struct file *filp = i915_gem_ttm_get_filep(obj);
>>> >   int ret;
>>> >
>>> >+  GEM_BUG_ON(obj->base.import_attach != NULL);
>>> >+
>>> >   if (obj->base.size < vma->vm_end - vma->vm_start)
>>> >   return -EINVAL;
>>> >
>>> >   if (HAS_LMEM(i915))
>>> >   return drm_gem_prime_mmap(&obj->base, vma);
>>>
>>> Since all of the devices that will have device memory will be true for
>>HAS_LMEM,
>>> won't your code path always go to drm_gem_prime_mmap()?
>>
>>This makes me wonder, how was mapping of a dmabuf BO working before, in
>>the case
>>of DG2 and DG1, when an object is smem-bound, and therefore backed by
>>shmem?
>
>LMEM is a new thing.  DG2 and DG1 have it available, but the initial patch
>sets did not support access via dma-buf.
>
>With the TTM being used for the LMEM objects, I think that the drm_gem code
>was able to be used.
>
>>
>>I guess in this case it might be more sensible to control for the case that 
>>it's
>>an lmem-only object on a discrete platform as follows:
>>
>>static int i915

Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory region and object backend with TTM

2022-06-01 Thread Ruhl, Michael J
>-Original Message-
>From: Adrian Larumbe 
>Sent: Friday, May 27, 2022 12:08 PM
>To: Ruhl, Michael J 
>Cc: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory
>region and object backend with TTM
>
>On 17.05.2022 21:39, Ruhl, Michael J wrote:
>> >-Original Message-
>> >From: Intel-gfx  On Behalf Of
>> >Adrian Larumbe
>> >Sent: Tuesday, May 17, 2022 4:45 PM
>> >To: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
>> >Cc: adrian.laru...@collabora.com
>> >Subject: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory
>> >region and object backend with TTM
>> >
>> >Remove shmem region and object backend code as they are now
>> >unnecessary.
>> >In the case of objects placed in SMEM and backed by kernel shmem files,
>the
>> >file pointer has to be retrieved from the ttm_tt structure, so change this
>> >as well. This means accessing an shmem-backed BO's file pointer requires
>> >getting its pages beforehand, unlike in the old shmem backend.
>> >
>> >Expand TTM BO creation by handling devices with no LLC so that their
>> >caching and coherency properties are set accordingly.
>> >
>> >Adapt phys backend to put pages of original shmem object in a 'TTM way',
>> >also making sure its pages are put when a TTM shmem object has no struct
>> >page.
>> >
>> >Signed-off-by: Adrian Larumbe 
>> >---
>> > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c   |  12 +-
>> > drivers/gpu/drm/i915/gem/i915_gem_mman.c |  32 +-
>> > drivers/gpu/drm/i915/gem/i915_gem_object.h   |   4 +-
>> > drivers/gpu/drm/i915/gem/i915_gem_phys.c |  32 +-
>> > drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 390 +--
>> > drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 267 -
>> > drivers/gpu/drm/i915/gem/i915_gem_ttm.h  |   3 +
>> > drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |   9 +-
>> > drivers/gpu/drm/i915/gt/shmem_utils.c|  64 ++-
>> > drivers/gpu/drm/i915/intel_memory_region.c   |   7 +-
>> > 10 files changed, 398 insertions(+), 422 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> >b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> >index f5062d0c6333..de04ce4210b3 100644
>> >--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> >+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> >@@ -12,6 +12,7 @@
>> > #include 
>> >
>> > #include "gem/i915_gem_dmabuf.h"
>> >+#include "gem/i915_gem_ttm.h"
>> > #include "i915_drv.h"
>> > #include "i915_gem_object.h"
>> > #include "i915_scatterlist.h"
>> >@@ -94,22 +95,25 @@ static int i915_gem_dmabuf_mmap(struct dma_buf
>> >*dma_buf, struct vm_area_struct *
>> > {
>> >struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
>> >struct drm_i915_private *i915 = to_i915(obj->base.dev);
>> >+   struct file *filp = i915_gem_ttm_get_filep(obj);
>> >int ret;
>> >
>> >+   GEM_BUG_ON(obj->base.import_attach != NULL);
>> >+
>> >if (obj->base.size < vma->vm_end - vma->vm_start)
>> >return -EINVAL;
>> >
>> >if (HAS_LMEM(i915))
>> >return drm_gem_prime_mmap(&obj->base, vma);
>>
>> Since all of the devices that will have device memory will be true for
>HAS_LMEM,
>> won't your code path always go to drm_gem_prime_mmap()?
>
>This makes me wonder, how was mapping of a dmabuf BO working before, in
>the case
>of DG2 and DG1, when an object is smem-bound, and therefore backed by
>shmem?

LMEM is a new thing.  DG2 and DG1 have it available, but the initial patch
sets did not support access via dma-buf.

With the TTM being used for the LMEM objects, I think that the drm_gem code
was able to be used.

>
>I guess in this case it might be more sensible to control for the case that 
>it's
>an lmem-only object on a discrete platform as follows:
>
>static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct
>vm_area_struct *vma)
>{
>   struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
>   struct drm_i915_private *i915 = to_i915(obj->base.dev);
>   struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
>   struct file *filp = i915_gem_ttm_get_filep(obj);
>   int ret;
>
>   if (obj->base.size < vma->vm_end

Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory region and object backend with TTM

2022-05-27 Thread Adrian Larumbe
On 17.05.2022 21:39, Ruhl, Michael J wrote:
> >-Original Message-
> >From: Intel-gfx  On Behalf Of
> >Adrian Larumbe
> >Sent: Tuesday, May 17, 2022 4:45 PM
> >To: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
> >Cc: adrian.laru...@collabora.com
> >Subject: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory
> >region and object backend with TTM
> >
> >Remove shmem region and object backend code as they are now
> >unnecessary.
> >In the case of objects placed in SMEM and backed by kernel shmem files, the
> >file pointer has to be retrieved from the ttm_tt structure, so change this
> >as well. This means accessing an shmem-backed BO's file pointer requires
> >getting its pages beforehand, unlike in the old shmem backend.
> >
> >Expand TTM BO creation by handling devices with no LLC so that their
> >caching and coherency properties are set accordingly.
> >
> >Adapt phys backend to put pages of original shmem object in a 'TTM way',
> >also making sure its pages are put when a TTM shmem object has no struct
> >page.
> >
> >Signed-off-by: Adrian Larumbe 
> >---
> > drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c   |  12 +-
> > drivers/gpu/drm/i915/gem/i915_gem_mman.c |  32 +-
> > drivers/gpu/drm/i915/gem/i915_gem_object.h   |   4 +-
> > drivers/gpu/drm/i915/gem/i915_gem_phys.c |  32 +-
> > drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 390 +--
> > drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 267 -
> > drivers/gpu/drm/i915/gem/i915_gem_ttm.h  |   3 +
> > drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |   9 +-
> > drivers/gpu/drm/i915/gt/shmem_utils.c|  64 ++-
> > drivers/gpu/drm/i915/intel_memory_region.c   |   7 +-
> > 10 files changed, 398 insertions(+), 422 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> >b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> >index f5062d0c6333..de04ce4210b3 100644
> >--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> >+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> >@@ -12,6 +12,7 @@
> > #include 
> >
> > #include "gem/i915_gem_dmabuf.h"
> >+#include "gem/i915_gem_ttm.h"
> > #include "i915_drv.h"
> > #include "i915_gem_object.h"
> > #include "i915_scatterlist.h"
> >@@ -94,22 +95,25 @@ static int i915_gem_dmabuf_mmap(struct dma_buf
> >*dma_buf, struct vm_area_struct *
> > {
> > struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
> > struct drm_i915_private *i915 = to_i915(obj->base.dev);
> >+struct file *filp = i915_gem_ttm_get_filep(obj);
> > int ret;
> >
> >+GEM_BUG_ON(obj->base.import_attach != NULL);
> >+
> > if (obj->base.size < vma->vm_end - vma->vm_start)
> > return -EINVAL;
> >
> > if (HAS_LMEM(i915))
> > return drm_gem_prime_mmap(&obj->base, vma);
> 
> Since all of the devices that will have device memory will be true for 
> HAS_LMEM,
> won't your code path always go to drm_gem_prime_mmap()?

This makes me wonder, how was mapping of a dmabuf BO working before, in the case
of DG2 and DG1, when an object is smem-bound, and therefore backed by shmem?

I guess in this case it might be more sensible to control for the case that it's
an lmem-only object on a discrete platform as follows:

static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct 
*vma)
{
struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
struct file *filp = i915_gem_ttm_get_filep(obj);
int ret;

if (obj->base.size < vma->vm_end - vma->vm_start)
return -EINVAL;

if (IS_DGFX(i915) && i915_ttm_cpu_maps_iomem(bo->resource))
return drm_gem_prime_mmap(&obj->base, vma);

if (IS_ERR(filp))
return PTR_ERR(filp);

ret = call_mmap(filp, vma);
if (ret)
return ret;

vma_set_file(vma, filp);

return 0;
}

> >-if (!obj->base.filp)
> >+if (!filp)
> > return -ENODEV;
> >
> >-ret = call_mmap(obj->base.filp, vma);
> >+ret = call_mmap(filp, vma);
> > if (ret)
> > return ret;
> >
> >-vma_set_file(vma, obj->base.filp);
> >+vma_set_file(vma, filp);
> >
> > return 0;
> > }
> >@@ -224,6 +228,8 @@ struct dma_buf

Re: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory region and object backend with TTM

2022-05-17 Thread Ruhl, Michael J
>-Original Message-
>From: Intel-gfx  On Behalf Of
>Adrian Larumbe
>Sent: Tuesday, May 17, 2022 4:45 PM
>To: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org
>Cc: adrian.laru...@collabora.com
>Subject: [Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory
>region and object backend with TTM
>
>Remove shmem region and object backend code as they are now
>unnecessary.
>In the case of objects placed in SMEM and backed by kernel shmem files, the
>file pointer has to be retrieved from the ttm_tt structure, so change this
>as well. This means accessing an shmem-backed BO's file pointer requires
>getting its pages beforehand, unlike in the old shmem backend.
>
>Expand TTM BO creation by handling devices with no LLC so that their
>caching and coherency properties are set accordingly.
>
>Adapt phys backend to put pages of original shmem object in a 'TTM way',
>also making sure its pages are put when a TTM shmem object has no struct
>page.
>
>Signed-off-by: Adrian Larumbe 
>---
> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c   |  12 +-
> drivers/gpu/drm/i915/gem/i915_gem_mman.c |  32 +-
> drivers/gpu/drm/i915/gem/i915_gem_object.h   |   4 +-
> drivers/gpu/drm/i915/gem/i915_gem_phys.c |  32 +-
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 390 +--
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 267 -
> drivers/gpu/drm/i915/gem/i915_gem_ttm.h  |   3 +
> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |   9 +-
> drivers/gpu/drm/i915/gt/shmem_utils.c|  64 ++-
> drivers/gpu/drm/i915/intel_memory_region.c   |   7 +-
> 10 files changed, 398 insertions(+), 422 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>index f5062d0c6333..de04ce4210b3 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>@@ -12,6 +12,7 @@
> #include 
>
> #include "gem/i915_gem_dmabuf.h"
>+#include "gem/i915_gem_ttm.h"
> #include "i915_drv.h"
> #include "i915_gem_object.h"
> #include "i915_scatterlist.h"
>@@ -94,22 +95,25 @@ static int i915_gem_dmabuf_mmap(struct dma_buf
>*dma_buf, struct vm_area_struct *
> {
>   struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
>   struct drm_i915_private *i915 = to_i915(obj->base.dev);
>+  struct file *filp = i915_gem_ttm_get_filep(obj);
>   int ret;
>
>+  GEM_BUG_ON(obj->base.import_attach != NULL);
>+
>   if (obj->base.size < vma->vm_end - vma->vm_start)
>   return -EINVAL;
>
>   if (HAS_LMEM(i915))
>   return drm_gem_prime_mmap(&obj->base, vma);

Since all of the devices that will have device memory will be true for HAS_LMEM,
won't your code path always go to drm_gem_prime_mmap()?

>-  if (!obj->base.filp)
>+  if (!filp)
>   return -ENODEV;
>
>-  ret = call_mmap(obj->base.filp, vma);
>+  ret = call_mmap(filp, vma);
>   if (ret)
>   return ret;
>
>-  vma_set_file(vma, obj->base.filp);
>+  vma_set_file(vma, filp);
>
>   return 0;
> }
>@@ -224,6 +228,8 @@ struct dma_buf *i915_gem_prime_export(struct
>drm_gem_object *gem_obj, int flags)
>   exp_info.priv = gem_obj;
>   exp_info.resv = obj->base.resv;
>
>+  GEM_BUG_ON(obj->base.import_attach != NULL);
>+
>   if (obj->ops->dmabuf_export) {
>   int ret = obj->ops->dmabuf_export(obj);
>   if (ret)
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>index 0c5c43852e24..d963cf35fdc9 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>@@ -64,7 +64,9 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void
>*data,
>   struct drm_i915_private *i915 = to_i915(dev);
>   struct drm_i915_gem_mmap *args = data;
>   struct drm_i915_gem_object *obj;
>+  struct file *filp;
>   unsigned long addr;
>+  int ret;
>
>   /*
>* mmap ioctl is disallowed for all discrete platforms,
>@@ -83,12 +85,20 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void
>*data,
>   if (!obj)
>   return -ENOENT;
>
>-  /* prime objects have no backing filp to GEM mmap
>-   * pages from.
>-   */
>-  if (!obj->base.filp) {
>-  addr = -ENXIO;
>-  goto err;
>+  if (obj->base.import_attach)
>+  filp = obj->base.filp;

Why is this now ok?  This is the imported object. And it used to give an error.

If yo

[Intel-gfx] [RFC PATCH v2 1/1] drm/i915: Replace shmem memory region and object backend with TTM

2022-05-17 Thread Adrian Larumbe
Remove shmem region and object backend code as they are now unnecessary.
In the case of objects placed in SMEM and backed by kernel shmem files, the
file pointer has to be retrieved from the ttm_tt structure, so change this
as well. This means accessing an shmem-backed BO's file pointer requires
getting its pages beforehand, unlike in the old shmem backend.

Expand TTM BO creation by handling devices with no LLC so that their
caching and coherency properties are set accordingly.

Adapt phys backend to put pages of original shmem object in a 'TTM way',
also making sure its pages are put when a TTM shmem object has no struct
page.

Signed-off-by: Adrian Larumbe 
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c   |  12 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c |  32 +-
 drivers/gpu/drm/i915/gem/i915_gem_object.h   |   4 +-
 drivers/gpu/drm/i915/gem/i915_gem_phys.c |  32 +-
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 390 +--
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 267 -
 drivers/gpu/drm/i915/gem/i915_gem_ttm.h  |   3 +
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c |   9 +-
 drivers/gpu/drm/i915/gt/shmem_utils.c|  64 ++-
 drivers/gpu/drm/i915/intel_memory_region.c   |   7 +-
 10 files changed, 398 insertions(+), 422 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c 
b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index f5062d0c6333..de04ce4210b3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -12,6 +12,7 @@
 #include 
 
 #include "gem/i915_gem_dmabuf.h"
+#include "gem/i915_gem_ttm.h"
 #include "i915_drv.h"
 #include "i915_gem_object.h"
 #include "i915_scatterlist.h"
@@ -94,22 +95,25 @@ static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, 
struct vm_area_struct *
 {
struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_i915_private *i915 = to_i915(obj->base.dev);
+   struct file *filp = i915_gem_ttm_get_filep(obj);
int ret;
 
+   GEM_BUG_ON(obj->base.import_attach != NULL);
+
if (obj->base.size < vma->vm_end - vma->vm_start)
return -EINVAL;
 
if (HAS_LMEM(i915))
return drm_gem_prime_mmap(&obj->base, vma);
 
-   if (!obj->base.filp)
+   if (!filp)
return -ENODEV;
 
-   ret = call_mmap(obj->base.filp, vma);
+   ret = call_mmap(filp, vma);
if (ret)
return ret;
 
-   vma_set_file(vma, obj->base.filp);
+   vma_set_file(vma, filp);
 
return 0;
 }
@@ -224,6 +228,8 @@ struct dma_buf *i915_gem_prime_export(struct drm_gem_object 
*gem_obj, int flags)
exp_info.priv = gem_obj;
exp_info.resv = obj->base.resv;
 
+   GEM_BUG_ON(obj->base.import_attach != NULL);
+
if (obj->ops->dmabuf_export) {
int ret = obj->ops->dmabuf_export(obj);
if (ret)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c 
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 0c5c43852e24..d963cf35fdc9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -64,7 +64,9 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
struct drm_i915_private *i915 = to_i915(dev);
struct drm_i915_gem_mmap *args = data;
struct drm_i915_gem_object *obj;
+   struct file *filp;
unsigned long addr;
+   int ret;
 
/*
 * mmap ioctl is disallowed for all discrete platforms,
@@ -83,12 +85,20 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
if (!obj)
return -ENOENT;
 
-   /* prime objects have no backing filp to GEM mmap
-* pages from.
-*/
-   if (!obj->base.filp) {
-   addr = -ENXIO;
-   goto err;
+   if (obj->base.import_attach)
+   filp = obj->base.filp;
+   else {
+   ret = i915_gem_object_pin_pages_unlocked(obj);
+   if (ret) {
+   addr = ret;
+   goto err_pin;
+   }
+
+   filp = i915_gem_ttm_get_filep(obj);
+   if (!filp) {
+   addr = -ENXIO;
+   goto err;
+   }
}
 
if (range_overflows(args->offset, args->size, (u64)obj->base.size)) {
@@ -96,7 +106,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
goto err;
}
 
-   addr = vm_mmap(obj->base.filp, 0, args->size,
+   addr = vm_mmap(filp, 0, args->size,
   PROT_READ | PROT_WRITE, MAP_SHARED,
   args->offset);
if (IS_ERR_VALUE(addr))
@@ -111,7 +121,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
goto err;
}
vma = find_vma(mm, addr);
-   if (vma && __vma_matches(vma, obj->base.filp, addr, args->size))
+   if (vma &&