From: Chris Wilson <chris@chris-wilson.co.uk>
To: Imre Deak <imre.deak@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v8 2/3] drm/i915/gem: Add a helper to read data from a GEM object page
Date: Wed, 20 Jan 2021 12:02:49 +0000 [thread overview]
Message-ID: <161114416949.24677.9355507827845088518@build.alporthouse.com> (raw)
In-Reply-To: <20210115194101.1037430-1-imre.deak@intel.com>
Quoting Imre Deak (2021-01-15 19:41:00)
> Add a simple helper to read data with the CPU from the page of a GEM
> object. Do the read either via a kmap if the object has struct pages
> or an iomap otherwise. This is needed by the next patch, reading a u64
> value from the object (w/o requiring the obj to be mapped to the GPU).
>
> Suggested by Chris.
>
> v2 (Chris):
> - Sanitize the type and order of func params.
> - Avoid consts requiring too many casts.
> - Use BUG_ON instead of WARN_ON, simplify the conditions.
> - Fix __iomem sparse errors.
> - Leave locking/syncing/pinning up to the caller, require only that the
> caller has pinned the object pages.
> - Check for iomem backing store before reading via an iomap.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 64 ++++++++++++++++++++++
> drivers/gpu/drm/i915/gem/i915_gem_object.h | 8 +++
> 2 files changed, 72 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 00d24000b5e8..67956a5f5fe3 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -32,6 +32,7 @@
> #include "i915_gem_mman.h"
> #include "i915_gem_object.h"
> #include "i915_globals.h"
> +#include "i915_memcpy.h"
> #include "i915_trace.h"
>
> static struct i915_global_object {
> @@ -383,6 +384,69 @@ void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj,
> }
> }
>
> +static void
> +i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size)
> +{
> + void *src_map;
> + void *src_ptr;
> +
> + src_map = kmap_atomic(i915_gem_object_get_page(obj, offset >> PAGE_SHIFT));
> +
> + src_ptr = src_map + offset_in_page(offset);
> + if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
> + drm_clflush_virt_range(src_ptr, size);
> + memcpy(dst, src_ptr, size);
> +
> + kunmap_atomic(src_map);
> +}
> +
> +static void
> +i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size)
> +{
> + void __iomem *src_map;
> + void __iomem *src_ptr;
> +
> + src_map = io_mapping_map_wc(&obj->mm.region->iomap,
> + i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT),
I've been corrected in that one needs to use
dma_addr_t dma =
i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT);
src_map = io_mapping_map_wc(&obj->mm.region->iomap,
dma - obj->mm.region->region.start,
> + PAGE_SIZE);
> +
> + src_ptr = src_map + offset_in_page(offset);
> + if (!i915_memcpy_from_wc(dst, (void __force *)src_ptr, size))
> + memcpy_fromio(dst, src_ptr, size);
> +
> + io_mapping_unmap(src_map);
> +}
> +
> +/**
> + * i915_gem_object_read_from_page - read data from the page of a GEM object
> + * @obj: GEM object to read from
> + * @offset: offset within the object
> + * @dst: buffer to store the read data
> + * @size: size to read
> + *
> + * Reads data from @obj at the specified offset. The requested region to read
> + * from can't cross a page boundary. The caller must ensure that @obj pages
> + * are pinned and that @obj is synced wrt. any related writes.
> + *
> + * Returns 0 on sucess or -ENODEV if the type of @obj's backing store is
> + * unsupported.
> + */
> +int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64 offset, void *dst, int size)
> +{
> + GEM_BUG_ON(offset >= obj->base.size);
> + GEM_BUG_ON(offset_in_page(offset) > PAGE_SIZE - size);
> + GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
> +
> + if (i915_gem_object_has_struct_page(obj))
> + i915_gem_object_read_from_page_kmap(obj, offset, dst, size);
> + else if (i915_gem_object_has_iomem(obj))
> + i915_gem_object_read_from_page_iomap(obj, offset, dst, size);
> + else
> + return -ENODEV;
Otherwise, that looks to be as simple as possible (offloading the setup
to the caller where it is already done), so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
with the dma offset before Matthew corrects me, again.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-01-20 12:02 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-14 20:13 [Intel-gfx] [PATCH v7 0/3] drm/i915/gen12: Add display render clear color decompression support Imre Deak
2021-01-14 20:13 ` [Intel-gfx] [PATCH v7 1/3] drm/framebuffer: Format modifier for Intel Gen 12 render compression with Clear Color Imre Deak
2021-01-15 9:41 ` Kahola, Mika
2021-01-15 18:42 ` Chery, Nanley G
2021-01-14 20:13 ` [Intel-gfx] [PATCH v7 2/3] drm/i915/gem: Add a helper to read data from a GEM object page Imre Deak
2021-01-14 21:23 ` Chris Wilson
2021-01-15 17:01 ` Imre Deak
2021-01-15 19:41 ` [Intel-gfx] [PATCH v8 " Imre Deak
2021-01-20 12:02 ` Chris Wilson [this message]
2021-01-20 19:43 ` Imre Deak
2021-01-20 21:38 ` [Intel-gfx] [PATCH v9 " Imre Deak
2021-01-14 20:13 ` [Intel-gfx] [PATCH v7 3/3] drm/i915/tgl: Add Clear Color support for TGL Render Decompression Imre Deak
2021-01-15 19:41 ` [Intel-gfx] [PATCH v8 " Imre Deak
2021-01-15 21:39 ` [Intel-gfx] [PATCH v9 " Imre Deak
2021-01-21 20:48 ` Matt Roper
2021-01-19 8:29 ` [Intel-gfx] [PATCH v7 " Dan Carpenter
2021-01-19 14:20 ` Imre Deak
2021-01-14 21:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gen12: Add display render clear color decompression support Patchwork
2021-01-14 21:42 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-01-14 22:10 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-01-15 10:05 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-01-15 23:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gen12: Add display render clear color decompression support (rev4) Patchwork
2021-01-15 23:16 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-01-15 23:43 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-01-16 9:26 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-01-20 22:52 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gen12: Add display render clear color decompression support (rev5) Patchwork
2021-01-20 22:55 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-01-20 23:23 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-01-21 12:57 ` Imre Deak
2021-01-21 14:14 ` Saarinen, Jani
2021-01-22 3:33 ` Vudum, Lakshminarayana
2021-01-22 3:32 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-01-22 6:50 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-01-22 14:33 ` Imre Deak
2021-01-22 16:47 ` Vudum, Lakshminarayana
2021-01-22 16:43 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=161114416949.24677.9355507827845088518@build.alporthouse.com \
--to=chris@chris-wilson.co.uk \
--cc=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).