[Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[Chris Wilson]

If we import a shmemfs object between devices, for example from
Tigerlake to DG1, we can simply reuse the native object and its backing
store.

Suggested-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index 04e9c04545ad..4816f08c4009 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -242,6 +242,13 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
 			 */
 			return &i915_gem_object_get(obj)->base;
 		}
+
+		/*
+		 * If the object is in plain system memory, we can reuse the
+		 * same backing store in any device.
+		 */
+		if (i915_gem_object_is_shmem(obj))
+			return &i915_gem_object_get(obj)->base;
 	}
 
 	/* need to attach */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[Matthew Auld]

On Wed, 20 Jan 2021 at 15:40, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> If we import a shmemfs object between devices, for example from
> Tigerlake to DG1, we can simply reuse the native object and its backing
> store.

Hmmm interesting, so does that include re-using the actual sg mapping
for the backing pages? Does that work out-of-the-box between different
devices assuming we have iommu enabled?

>
> Suggested-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> index 04e9c04545ad..4816f08c4009 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> @@ -242,6 +242,13 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
>                          */
>                         return &i915_gem_object_get(obj)->base;
>                 }
> +
> +               /*
> +                * If the object is in plain system memory, we can reuse the
> +                * same backing store in any device.
> +                */
> +               if (i915_gem_object_is_shmem(obj))
> +                       return &i915_gem_object_get(obj)->base;
>         }
>
>         /* need to attach */
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4184 bytes --]

Hi Chris,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip v5.11-rc4 next-20210120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-i915-gem-Allow-importing-of-shmemfs-objects-into-any-device/20210120-234237
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a013-20210120 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/ce377384dec1c18f1af3558d5d4624d300d160c4
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Chris-Wilson/drm-i915-gem-Allow-importing-of-shmemfs-objects-into-any-device/20210120-234237
        git checkout ce377384dec1c18f1af3558d5d4624d300d160c4
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c: In function 'i915_gem_prime_import':
>> drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c:250:7: error: implicit declaration of function 'i915_gem_object_is_shmem'; did you mean 'i915_gem_object_is_tiled'? [-Werror=implicit-function-declaration]
     250 |   if (i915_gem_object_is_shmem(obj))
         |       ^~~~~~~~~~~~~~~~~~~~~~~~
         |       i915_gem_object_is_tiled
   cc1: some warnings being treated as errors


vim +250 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c

   225	
   226	struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
   227						     struct dma_buf *dma_buf)
   228	{
   229		static struct lock_class_key lock_class;
   230		struct dma_buf_attachment *attach;
   231		struct drm_i915_gem_object *obj;
   232		int ret;
   233	
   234		/* is this one of own objects? */
   235		if (dma_buf->ops == &i915_dmabuf_ops) {
   236			obj = dma_buf_to_obj(dma_buf);
   237			/* is it from our device? */
   238			if (obj->base.dev == dev) {
   239				/*
   240				 * Importing dmabuf exported from out own gem increases
   241				 * refcount on gem itself instead of f_count of dmabuf.
   242				 */
   243				return &i915_gem_object_get(obj)->base;
   244			}
   245	
   246			/*
   247			 * If the object is in plain system memory, we can reuse the
   248			 * same backing store in any device.
   249			 */
 > 250			if (i915_gem_object_is_shmem(obj))
   251				return &i915_gem_object_get(obj)->base;
   252		}
   253	
   254		/* need to attach */
   255		attach = dma_buf_attach(dma_buf, dev->dev);
   256		if (IS_ERR(attach))
   257			return ERR_CAST(attach);
   258	
   259		get_dma_buf(dma_buf);
   260	
   261		obj = i915_gem_object_alloc();
   262		if (obj == NULL) {
   263			ret = -ENOMEM;
   264			goto fail_detach;
   265		}
   266	
   267		drm_gem_private_object_init(dev, &obj->base, dma_buf->size);
   268		i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops, &lock_class);
   269		obj->base.import_attach = attach;
   270		obj->base.resv = dma_buf->resv;
   271	
   272		/* We use GTT as shorthand for a coherent domain, one that is
   273		 * neither in the GPU cache nor in the CPU cache, where all
   274		 * writes are immediately visible in memory. (That's not strictly
   275		 * true, but it's close! There are internal buffers such as the
   276		 * write-combined buffer or a delay through the chipset for GTT
   277		 * writes that do require us to treat GTT as a separate cache domain.)
   278		 */
   279		obj->read_domains = I915_GEM_DOMAIN_GTT;
   280		obj->write_domain = 0;
   281	
   282		return &obj->base;
   283	
   284	fail_detach:
   285		dma_buf_detach(dma_buf, attach);
   286		dma_buf_put(dma_buf);
   287	
   288		return ERR_PTR(ret);
   289	}
   290	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34782 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[Chris Wilson]

Quoting Chris Wilson (2021-01-20 18:06:08)
> Quoting Matthew Auld (2021-01-20 17:46:10)
> > On Wed, 20 Jan 2021 at 15:40, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > >
> > > If we import a shmemfs object between devices, for example from
> > > Tigerlake to DG1, we can simply reuse the native object and its backing
> > > store.
> > 
> > Hmmm interesting, so does that include re-using the actual sg mapping
> > for the backing pages? Does that work out-of-the-box between different
> > devices assuming we have iommu enabled?
> 
> Indeed interesting; the dma_addr_t are supposed to be local to a device.

On reflection, we are expected to use cross-device dma_addr_t with
dma-buf. It's the exporter who assigns the dma_addr_t for the importer
to use, and they are always given from the original device.

Maybe not so bad. Definitely needs testing to see what happens in
practice.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/gem: Allow importing of shmemfs objects into any device

[Patchwork]

== Series Details ==

Series: drm/i915/gem: Allow importing of shmemfs objects into any device
URL   : https://patchwork.freedesktop.org/series/86093/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.o
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c: In function ‘i915_gem_prime_import’:
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c:250:7: error: implicit declaration of function ‘i915_gem_object_is_shmem’; did you mean ‘i915_gem_object_is_tiled’? [-Werror=implicit-function-declaration]
   if (i915_gem_object_is_shmem(obj))
       ^~~~~~~~~~~~~~~~~~~~~~~~
       i915_gem_object_is_tiled
cc1: all warnings being treated as errors
scripts/Makefile.build:279: recipe for target 'drivers/gpu/drm/i915/gem/i915_gem_dmabuf.o' failed
make[4]: *** [drivers/gpu/drm/i915/gem/i915_gem_dmabuf.o] Error 1
scripts/Makefile.build:496: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:496: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:496: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1805: recipe for target 'drivers' failed
make: *** [drivers] Error 2


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[Tvrtko Ursulin]

On 20/01/2021 20:59, Chris Wilson wrote:
> Quoting Chris Wilson (2021-01-20 18:06:08)
>> Quoting Matthew Auld (2021-01-20 17:46:10)
>>> On Wed, 20 Jan 2021 at 15:40, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>>>
>>>> If we import a shmemfs object between devices, for example from
>>>> Tigerlake to DG1, we can simply reuse the native object and its backing
>>>> store.
>>>
>>> Hmmm interesting, so does that include re-using the actual sg mapping
>>> for the backing pages? Does that work out-of-the-box between different
>>> devices assuming we have iommu enabled?
>>
>> Indeed interesting; the dma_addr_t are supposed to be local to a device.
> 
> On reflection, we are expected to use cross-device dma_addr_t with
> dma-buf. It's the exporter who assigns the dma_addr_t for the importer
> to use, and they are always given from the original device.
> 
> Maybe not so bad. Definitely needs testing to see what happens in
> practice.

What about object migration? I did not spot anything preventing it once 
object was exported like this so owning device could move it to device 
memory afterwards which would probably be bad.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[Chris Wilson]

Quoting Tvrtko Ursulin (2021-01-21 11:00:25)
> 
> On 20/01/2021 20:59, Chris Wilson wrote:
> > Quoting Chris Wilson (2021-01-20 18:06:08)
> >> Quoting Matthew Auld (2021-01-20 17:46:10)
> >>> On Wed, 20 Jan 2021 at 15:40, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >>>>
> >>>> If we import a shmemfs object between devices, for example from
> >>>> Tigerlake to DG1, we can simply reuse the native object and its backing
> >>>> store.
> >>>
> >>> Hmmm interesting, so does that include re-using the actual sg mapping
> >>> for the backing pages? Does that work out-of-the-box between different
> >>> devices assuming we have iommu enabled?
> >>
> >> Indeed interesting; the dma_addr_t are supposed to be local to a device.
> > 
> > On reflection, we are expected to use cross-device dma_addr_t with
> > dma-buf. It's the exporter who assigns the dma_addr_t for the importer
> > to use, and they are always given from the original device.
> > 
> > Maybe not so bad. Definitely needs testing to see what happens in
> > practice.
> 
> What about object migration? I did not spot anything preventing it once 
> object was exported like this so owning device could move it to device 
> memory afterwards which would probably be bad.

Depends on how you do your migration, but your migration should be
checking that it is allowed to migrate the object first.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[Tvrtko Ursulin]

On 21/01/2021 11:03, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2021-01-21 11:00:25)
>>
>> On 20/01/2021 20:59, Chris Wilson wrote:
>>> Quoting Chris Wilson (2021-01-20 18:06:08)
>>>> Quoting Matthew Auld (2021-01-20 17:46:10)
>>>>> On Wed, 20 Jan 2021 at 15:40, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>>>>>
>>>>>> If we import a shmemfs object between devices, for example from
>>>>>> Tigerlake to DG1, we can simply reuse the native object and its backing
>>>>>> store.
>>>>>
>>>>> Hmmm interesting, so does that include re-using the actual sg mapping
>>>>> for the backing pages? Does that work out-of-the-box between different
>>>>> devices assuming we have iommu enabled?
>>>>
>>>> Indeed interesting; the dma_addr_t are supposed to be local to a device.
>>>
>>> On reflection, we are expected to use cross-device dma_addr_t with
>>> dma-buf. It's the exporter who assigns the dma_addr_t for the importer
>>> to use, and they are always given from the original device.
>>>
>>> Maybe not so bad. Definitely needs testing to see what happens in
>>> practice.
>>
>> What about object migration? I did not spot anything preventing it once
>> object was exported like this so owning device could move it to device
>> memory afterwards which would probably be bad.
> 
> Depends on how you do your migration, but your migration should be
> checking that it is allowed to migrate the object first.

Okay agreed, meaning to be handled once migration gets to upstream.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs objects into any device

[Ruhl, Michael J]

>-----Original Message-----
>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
>Matthew Auld
>Sent: Wednesday, January 20, 2021 12:46 PM
>To: Chris Wilson <chris@chris-wilson.co.uk>
>Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>; Auld,
>Matthew <matthew.auld@intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915/gem: Allow importing of shmemfs
>objects into any device
>
>On Wed, 20 Jan 2021 at 15:40, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>
>> If we import a shmemfs object between devices, for example from
>> Tigerlake to DG1, we can simply reuse the native object and its backing
>> store.
>
>Hmmm interesting, so does that include re-using the actual sg mapping
>for the backing pages? Does that work out-of-the-box between different
>devices assuming we have iommu enabled?
>
>>
>> Suggested-by: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> index 04e9c04545ad..4816f08c4009 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
>> @@ -242,6 +242,13 @@ struct drm_gem_object
>*i915_gem_prime_import(struct drm_device *dev,
>>                          */
>>                         return &i915_gem_object_get(obj)->base;
>>                 }
>> +
>> +               /*
>> +                * If the object is in plain system memory, we can reuse the
>> +                * same backing store in any device.
>> +                */
>> +               if (i915_gem_object_is_shmem(obj))
>> +                       return &i915_gem_object_get(obj)->base;
>>         }

So this would mean sharing all of the object attributes between devices (pin count, etc)?

I.e. vma list etc?

Would that work?

Mike

>>         /* need to attach */
>> --
>> 2.20.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx