* [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
2021-12-10 23:23 [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions ira.weiny
@ 2021-12-10 23:23 ` ira.weiny
2021-12-13 9:04 ` Christoph Hellwig
` (2 more replies)
2021-12-10 23:23 ` [PATCH 2/7] drm/amd: " ira.weiny
` (6 subsequent siblings)
7 siblings, 3 replies; 24+ messages in thread
From: ira.weiny @ 2021-12-10 23:23 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: Ira Weiny, amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
From: Ira Weiny <ira.weiny@intel.com>
kmap() is being deprecated and these usages are all local to the thread
so there is no reason kmap_local_page() can't be used.
Replace kmap() calls with kmap_local_page().
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++--
drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
6 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index d77da59fae04..fa8b820e14aa 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -597,9 +597,9 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv,
if (err < 0)
goto fail;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
memcpy(vaddr, data, len);
- kunmap(page);
+ kunmap_local(vaddr);
err = pagecache_write_end(file, file->f_mapping,
offset, len, len,
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 6d30cdfa80f3..e59e1725e29d 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -144,7 +144,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
intel_gt_flush_ggtt_writes(&to_i915(obj->base.dev)->gt);
p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
- cpu = kmap(p) + offset_in_page(offset);
+ cpu = kmap_local_page(p) + offset_in_page(offset);
drm_clflush_virt_range(cpu, sizeof(*cpu));
if (*cpu != (u32)page) {
pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
@@ -162,7 +162,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
}
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap(p);
+ kunmap_local(cpu);
out:
__i915_vma_put(vma);
@@ -237,7 +237,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj,
intel_gt_flush_ggtt_writes(&to_i915(obj->base.dev)->gt);
p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
- cpu = kmap(p) + offset_in_page(offset);
+ cpu = kmap_local_page(p) + offset_in_page(offset);
drm_clflush_virt_range(cpu, sizeof(*cpu));
if (*cpu != (u32)page) {
pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
@@ -255,7 +255,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj,
}
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap(p);
+ kunmap_local(cpu);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
index f8948de72036..743a414f86f3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
@@ -743,7 +743,7 @@ static void swizzle_page(struct page *page)
char *vaddr;
int i;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
for (i = 0; i < PAGE_SIZE; i += 128) {
memcpy(temp, &vaddr[i], 64);
@@ -751,7 +751,7 @@ static void swizzle_page(struct page *page)
memcpy(&vaddr[i + 64], temp, 64);
}
- kunmap(page);
+ kunmap_local(vaddr);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c b/drivers/gpu/drm/i915/gt/shmem_utils.c
index 0683b27a3890..cba4f210d093 100644
--- a/drivers/gpu/drm/i915/gt/shmem_utils.c
+++ b/drivers/gpu/drm/i915/gt/shmem_utils.c
@@ -104,7 +104,7 @@ static int __shmem_rw(struct file *file, loff_t off,
if (IS_ERR(page))
return PTR_ERR(page);
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
if (write) {
memcpy(vaddr + offset_in_page(off), ptr, this);
set_page_dirty(page);
@@ -112,7 +112,7 @@ static int __shmem_rw(struct file *file, loff_t off,
memcpy(ptr, vaddr + offset_in_page(off), this);
}
mark_page_accessed(page);
- kunmap(page);
+ kunmap_local(vaddr);
put_page(page);
len -= this;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 981e383d1a5d..af5adb187ca4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -196,14 +196,14 @@ shmem_pread(struct page *page, int offset, int len, char __user *user_data,
char *vaddr;
int ret;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
if (needs_clflush)
drm_clflush_virt_range(vaddr + offset, len);
ret = __copy_to_user(user_data, vaddr + offset, len);
- kunmap(page);
+ kunmap_local(vaddr);
return ret ? -EFAULT : 0;
}
@@ -618,7 +618,7 @@ shmem_pwrite(struct page *page, int offset, int len, char __user *user_data,
char *vaddr;
int ret;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
if (needs_clflush_before)
drm_clflush_virt_range(vaddr + offset, len);
@@ -627,7 +627,7 @@ shmem_pwrite(struct page *page, int offset, int len, char __user *user_data,
if (!ret && needs_clflush_after)
drm_clflush_virt_range(vaddr + offset, len);
- kunmap(page);
+ kunmap_local(vaddr);
return ret ? -EFAULT : 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 2a2d7643b551..c526d7892081 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1094,9 +1094,9 @@ i915_vma_coredump_create(const struct intel_gt *gt,
drm_clflush_pages(&page, 1);
- s = kmap(page);
+ s = kmap_local_page(page);
ret = compress_page(compress, s, dst, false);
- kunmap(page);
+ kunmap_local(s);
drm_clflush_pages(&page, 1);
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
2021-12-10 23:23 ` [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page() ira.weiny
@ 2021-12-13 9:04 ` Christoph Hellwig
2021-12-13 16:02 ` Ira Weiny
2021-12-13 12:39 ` Ville Syrjälä
2021-12-22 6:08 ` [PATCH V2] " ira.weiny
2 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2021-12-13 9:04 UTC (permalink / raw)
To: ira.weiny
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, amd-gfx, dri-devel, linux-kernel, intel-gfx,
linux-arm-msm
On Fri, Dec 10, 2021 at 03:23:58PM -0800, ira.weiny@intel.com wrote:
> - vaddr = kmap(page);
> + vaddr = kmap_local_page(page);
> memcpy(vaddr, data, len);
> - kunmap(page);
> + kunmap_local(vaddr);
memcpy_to_page?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
2021-12-13 9:04 ` Christoph Hellwig
@ 2021-12-13 16:02 ` Ira Weiny
0 siblings, 0 replies; 24+ messages in thread
From: Ira Weiny @ 2021-12-13 16:02 UTC (permalink / raw)
To: Christoph Hellwig
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, amd-gfx, dri-devel, linux-kernel, intel-gfx,
linux-arm-msm
On Mon, Dec 13, 2021 at 01:04:07AM -0800, Christoph Hellwig wrote:
> On Fri, Dec 10, 2021 at 03:23:58PM -0800, ira.weiny@intel.com wrote:
> > - vaddr = kmap(page);
> > + vaddr = kmap_local_page(page);
> > memcpy(vaddr, data, len);
> > - kunmap(page);
> > + kunmap_local(vaddr);
>
> memcpy_to_page?
Opps! Yea!
David, Daniel,
Do you prefer me to resent the entire series or reply to this message with a
V2?
Ira
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
2021-12-10 23:23 ` [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page() ira.weiny
2021-12-13 9:04 ` Christoph Hellwig
@ 2021-12-13 12:39 ` Ville Syrjälä
2021-12-13 15:45 ` Ira Weiny
2021-12-22 6:08 ` [PATCH V2] " ira.weiny
2 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2021-12-13 12:39 UTC (permalink / raw)
To: ira.weiny
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, linux-arm-msm, intel-gfx, linux-kernel, dri-devel,
amd-gfx
On Fri, Dec 10, 2021 at 03:23:58PM -0800, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
>
> kmap() is being deprecated and these usages are all local to the thread
> so there is no reason kmap_local_page() can't be used.
>
> Replace kmap() calls with kmap_local_page().
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
> drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
> drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
> drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++--
> drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
> drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
> 6 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index d77da59fae04..fa8b820e14aa 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -597,9 +597,9 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv,
> if (err < 0)
> goto fail;
>
> - vaddr = kmap(page);
> + vaddr = kmap_local_page(page);
> memcpy(vaddr, data, len);
> - kunmap(page);
> + kunmap_local(vaddr);
>
> err = pagecache_write_end(file, file->f_mapping,
> offset, len, len,
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> index 6d30cdfa80f3..e59e1725e29d 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -144,7 +144,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
> intel_gt_flush_ggtt_writes(&to_i915(obj->base.dev)->gt);
>
> p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
> - cpu = kmap(p) + offset_in_page(offset);
> + cpu = kmap_local_page(p) + offset_in_page(offset);
Does kunmap_local() do some magic to make it work even when you
don't pass in the same value you got from kmap_local_page()?
> drm_clflush_virt_range(cpu, sizeof(*cpu));
> if (*cpu != (u32)page) {
> pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
> @@ -162,7 +162,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
> }
> *cpu = 0;
> drm_clflush_virt_range(cpu, sizeof(*cpu));
> - kunmap(p);
> + kunmap_local(cpu);
>
> out:
> __i915_vma_put(vma);
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page()
2021-12-13 12:39 ` Ville Syrjälä
@ 2021-12-13 15:45 ` Ira Weiny
0 siblings, 0 replies; 24+ messages in thread
From: Ira Weiny @ 2021-12-13 15:45 UTC (permalink / raw)
To: Ville Syrjälä
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, linux-arm-msm, intel-gfx, linux-kernel, dri-devel,
amd-gfx
On Mon, Dec 13, 2021 at 02:39:59PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 10, 2021 at 03:23:58PM -0800, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > kmap() is being deprecated and these usages are all local to the thread
> > so there is no reason kmap_local_page() can't be used.
> >
> > Replace kmap() calls with kmap_local_page().
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
> > drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
> > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
> > drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++--
> > drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
> > drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
> > 6 files changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > index d77da59fae04..fa8b820e14aa 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > @@ -597,9 +597,9 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv,
> > if (err < 0)
> > goto fail;
> >
> > - vaddr = kmap(page);
> > + vaddr = kmap_local_page(page);
> > memcpy(vaddr, data, len);
> > - kunmap(page);
> > + kunmap_local(vaddr);
> >
> > err = pagecache_write_end(file, file->f_mapping,
> > offset, len, len,
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > index 6d30cdfa80f3..e59e1725e29d 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> > @@ -144,7 +144,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
> > intel_gt_flush_ggtt_writes(&to_i915(obj->base.dev)->gt);
> >
> > p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
> > - cpu = kmap(p) + offset_in_page(offset);
> > + cpu = kmap_local_page(p) + offset_in_page(offset);
>
> Does kunmap_local() do some magic to make it work even when you
> don't pass in the same value you got from kmap_local_page()?
Yes. It sounds like a patch like this would be nice to clarify?
Ira
diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h
index 0a0b2b09b1b8..fb2d3e033c01 100644
--- a/include/linux/highmem-internal.h
+++ b/include/linux/highmem-internal.h
@@ -246,6 +246,17 @@ do { \
__kunmap_atomic(__addr); \
} while (0)
+/**
+ * kunmap_local - Unmap a page mapped via kmap_local_page().
+ * @__addr: An address within the page mapped
+ *
+ * __addr is often an address returned from kmap_local_page(). However,
+ * this address can be any address within the mapped page. It does not need to
+ * be the exact address returned from kmap_local_page()
+ *
+ * Unmapping should be done in the reverse order of the mapping. See
+ * kmap_local_page() for details.
+ */
#define kunmap_local(__addr) \
do { \
BUILD_BUG_ON(__same_type((__addr), struct page *)); \
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH V2] drm/i915: Replace kmap() with kmap_local_page()
2021-12-10 23:23 ` [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page() ira.weiny
2021-12-13 9:04 ` Christoph Hellwig
2021-12-13 12:39 ` Ville Syrjälä
@ 2021-12-22 6:08 ` ira.weiny
2 siblings, 0 replies; 24+ messages in thread
From: ira.weiny @ 2021-12-22 6:08 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: Ira Weiny, amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
From: Ira Weiny <ira.weiny@intel.com>
kmap() is being deprecated and these usages are all local to the thread
so there is no reason kmap_local_page() can't be used.
Replace kmap() calls with kmap_local_page().
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
NOTE: I'm sending as a follow on to the V1 patch. Please let me know if you
prefer the entire series instead.
Changes for V2:
From Christoph Helwig
Prefer the use of memcpy_*_page() where appropriate.
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++----
drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
drivers/gpu/drm/i915/gt/shmem_utils.c | 7 ++-----
drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
6 files changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index d77da59fae04..842e089aaaa5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -589,7 +589,7 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv,
do {
unsigned int len = min_t(typeof(size), size, PAGE_SIZE);
struct page *page;
- void *pgdata, *vaddr;
+ void *pgdata;
err = pagecache_write_begin(file, file->f_mapping,
offset, len, 0,
@@ -597,9 +597,7 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv,
if (err < 0)
goto fail;
- vaddr = kmap(page);
- memcpy(vaddr, data, len);
- kunmap(page);
+ memcpy_to_page(page, 0, data, len);
err = pagecache_write_end(file, file->f_mapping,
offset, len, len,
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 6d30cdfa80f3..e59e1725e29d 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -144,7 +144,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
intel_gt_flush_ggtt_writes(&to_i915(obj->base.dev)->gt);
p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
- cpu = kmap(p) + offset_in_page(offset);
+ cpu = kmap_local_page(p) + offset_in_page(offset);
drm_clflush_virt_range(cpu, sizeof(*cpu));
if (*cpu != (u32)page) {
pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
@@ -162,7 +162,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
}
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap(p);
+ kunmap_local(cpu);
out:
__i915_vma_put(vma);
@@ -237,7 +237,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj,
intel_gt_flush_ggtt_writes(&to_i915(obj->base.dev)->gt);
p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
- cpu = kmap(p) + offset_in_page(offset);
+ cpu = kmap_local_page(p) + offset_in_page(offset);
drm_clflush_virt_range(cpu, sizeof(*cpu));
if (*cpu != (u32)page) {
pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
@@ -255,7 +255,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj,
}
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
- kunmap(p);
+ kunmap_local(cpu);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
index f8948de72036..743a414f86f3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
@@ -743,7 +743,7 @@ static void swizzle_page(struct page *page)
char *vaddr;
int i;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
for (i = 0; i < PAGE_SIZE; i += 128) {
memcpy(temp, &vaddr[i], 64);
@@ -751,7 +751,7 @@ static void swizzle_page(struct page *page)
memcpy(&vaddr[i + 64], temp, 64);
}
- kunmap(page);
+ kunmap_local(vaddr);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c b/drivers/gpu/drm/i915/gt/shmem_utils.c
index 0683b27a3890..d47f262d2f07 100644
--- a/drivers/gpu/drm/i915/gt/shmem_utils.c
+++ b/drivers/gpu/drm/i915/gt/shmem_utils.c
@@ -97,22 +97,19 @@ static int __shmem_rw(struct file *file, loff_t off,
unsigned int this =
min_t(size_t, PAGE_SIZE - offset_in_page(off), len);
struct page *page;
- void *vaddr;
page = shmem_read_mapping_page_gfp(file->f_mapping, pfn,
GFP_KERNEL);
if (IS_ERR(page))
return PTR_ERR(page);
- vaddr = kmap(page);
if (write) {
- memcpy(vaddr + offset_in_page(off), ptr, this);
+ memcpy_to_page(page, offset_in_page(off), ptr, this);
set_page_dirty(page);
} else {
- memcpy(ptr, vaddr + offset_in_page(off), this);
+ memcpy_from_page(ptr, page, offset_in_page(off), this);
}
mark_page_accessed(page);
- kunmap(page);
put_page(page);
len -= this;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 981e383d1a5d..af5adb187ca4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -196,14 +196,14 @@ shmem_pread(struct page *page, int offset, int len, char __user *user_data,
char *vaddr;
int ret;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
if (needs_clflush)
drm_clflush_virt_range(vaddr + offset, len);
ret = __copy_to_user(user_data, vaddr + offset, len);
- kunmap(page);
+ kunmap_local(vaddr);
return ret ? -EFAULT : 0;
}
@@ -618,7 +618,7 @@ shmem_pwrite(struct page *page, int offset, int len, char __user *user_data,
char *vaddr;
int ret;
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
if (needs_clflush_before)
drm_clflush_virt_range(vaddr + offset, len);
@@ -627,7 +627,7 @@ shmem_pwrite(struct page *page, int offset, int len, char __user *user_data,
if (!ret && needs_clflush_after)
drm_clflush_virt_range(vaddr + offset, len);
- kunmap(page);
+ kunmap_local(vaddr);
return ret ? -EFAULT : 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 2a2d7643b551..c526d7892081 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1094,9 +1094,9 @@ i915_vma_coredump_create(const struct intel_gt *gt,
drm_clflush_pages(&page, 1);
- s = kmap(page);
+ s = kmap_local_page(page);
ret = compress_page(compress, s, dst, false);
- kunmap(page);
+ kunmap_local(s);
drm_clflush_pages(&page, 1);
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/7] drm/amd: Replace kmap() with kmap_local_page()
2021-12-10 23:23 [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions ira.weiny
2021-12-10 23:23 ` [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page() ira.weiny
@ 2021-12-10 23:23 ` ira.weiny
2021-12-10 23:24 ` [PATCH 3/7] drm/gma: Remove calls to kmap() ira.weiny
` (5 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: ira.weiny @ 2021-12-10 23:23 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: Ira Weiny, amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
From: Ira Weiny <ira.weiny@intel.com>
kmap() is being deprecated. These maps are thread local and can be
replaced with kmap_local_page().
Replace kmap() with kmap_local_page()
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c875f1cdd2af..a3a2c73a44bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2281,9 +2281,9 @@ static ssize_t amdgpu_iomem_read(struct file *f, char __user *buf,
if (p->mapping != adev->mman.bdev.dev_mapping)
return -EPERM;
- ptr = kmap(p);
+ ptr = kmap_local_page(p);
r = copy_to_user(buf, ptr + off, bytes);
- kunmap(p);
+ kunmap_local(ptr);
if (r)
return -EFAULT;
@@ -2332,9 +2332,9 @@ static ssize_t amdgpu_iomem_write(struct file *f, const char __user *buf,
if (p->mapping != adev->mman.bdev.dev_mapping)
return -EPERM;
- ptr = kmap(p);
+ ptr = kmap_local_page(p);
r = copy_from_user(ptr + off, buf, bytes);
- kunmap(p);
+ kunmap_local(ptr);
if (r)
return -EFAULT;
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 3/7] drm/gma: Remove calls to kmap()
2021-12-10 23:23 [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions ira.weiny
2021-12-10 23:23 ` [PATCH 1/7] drm/i915: Replace kmap() with kmap_local_page() ira.weiny
2021-12-10 23:23 ` [PATCH 2/7] drm/amd: " ira.weiny
@ 2021-12-10 23:24 ` ira.weiny
2021-12-10 23:24 ` [PATCH 4/7] drm/radeon: Replace kmap() with kmap_local_page() ira.weiny
` (4 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: ira.weiny @ 2021-12-10 23:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: Ira Weiny, amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
From: Ira Weiny <ira.weiny@intel.com>
kmap() is being deprecated and these instances are easy to convert to
kmap_local_page().
Furthermore, in gma_crtc_cursor_set() use the memcpy_from_page() helper
instead of an open coded use of kmap_local_page().
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/gpu/drm/gma500/gma_display.c | 6 ++----
drivers/gpu/drm/gma500/mmu.c | 8 ++++----
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index cbcecbaa041b..caf7c7b321a9 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -334,7 +334,7 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
struct gtt_range *gt;
struct gtt_range *cursor_gt = gma_crtc->cursor_gt;
struct drm_gem_object *obj;
- void *tmp_dst, *tmp_src;
+ void *tmp_dst;
int ret = 0, i, cursor_pages;
/* If we didn't get a handle then turn the cursor off */
@@ -400,9 +400,7 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
/* Copy the cursor to cursor mem */
tmp_dst = dev_priv->vram_addr + cursor_gt->offset;
for (i = 0; i < cursor_pages; i++) {
- tmp_src = kmap(gt->pages[i]);
- memcpy(tmp_dst, tmp_src, PAGE_SIZE);
- kunmap(gt->pages[i]);
+ memcpy_from_page(tmp_dst, gt->pages[i], 0, PAGE_SIZE);
tmp_dst += PAGE_SIZE;
}
diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c
index fe9ace2a7967..a70b01ccdf70 100644
--- a/drivers/gpu/drm/gma500/mmu.c
+++ b/drivers/gpu/drm/gma500/mmu.c
@@ -184,17 +184,17 @@ struct psb_mmu_pd *psb_mmu_alloc_pd(struct psb_mmu_driver *driver,
pd->invalid_pte = 0;
}
- v = kmap(pd->dummy_pt);
+ v = kmap_local_page(pd->dummy_pt);
for (i = 0; i < (PAGE_SIZE / sizeof(uint32_t)); ++i)
v[i] = pd->invalid_pte;
- kunmap(pd->dummy_pt);
+ kunmap_local(v);
- v = kmap(pd->p);
+ v = kmap_local_page(pd->p);
for (i = 0; i < (PAGE_SIZE / sizeof(uint32_t)); ++i)
v[i] = pd->invalid_pde;
- kunmap(pd->p);
+ kunmap_local(v);
clear_page(kmap(pd->dummy_page));
kunmap(pd->dummy_page);
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 4/7] drm/radeon: Replace kmap() with kmap_local_page()
2021-12-10 23:23 [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions ira.weiny
` (2 preceding siblings ...)
2021-12-10 23:24 ` [PATCH 3/7] drm/gma: Remove calls to kmap() ira.weiny
@ 2021-12-10 23:24 ` ira.weiny
2021-12-10 23:24 ` [PATCH 5/7] drm/msm: Alter comment to use kmap_local_page() ira.weiny
` (3 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: ira.weiny @ 2021-12-10 23:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: Ira Weiny, amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
From: Ira Weiny <ira.weiny@intel.com>
kmap() is being deprecated and this usage is local to the thread. Use
kmap_local_page() instead.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 11b21d605584..76d7906e1785 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -907,11 +907,11 @@ static ssize_t radeon_ttm_gtt_read(struct file *f, char __user *buf,
page = rdev->gart.pages[p];
if (page) {
- ptr = kmap(page);
+ ptr = kmap_local_page(page);
ptr += off;
r = copy_to_user(buf, ptr, cur_size);
- kunmap(rdev->gart.pages[p]);
+ kunmap_local(ptr);
} else
r = clear_user(buf, cur_size);
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 5/7] drm/msm: Alter comment to use kmap_local_page()
2021-12-10 23:23 [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions ira.weiny
` (3 preceding siblings ...)
2021-12-10 23:24 ` [PATCH 4/7] drm/radeon: Replace kmap() with kmap_local_page() ira.weiny
@ 2021-12-10 23:24 ` ira.weiny
2021-12-10 23:24 ` [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error ira.weiny
` (2 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: ira.weiny @ 2021-12-10 23:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: Ira Weiny, amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
From: Ira Weiny <ira.weiny@intel.com>
kmap() is being deprecated. So this comment could be misleading in the
future.
Change this comment to point to using kmap_local_page(). While here
remove 'we' from the comment.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 282628d6b72c..654ae0d13eaf 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -438,8 +438,8 @@ static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *ob
return -EINVAL;
}
- /* For now, just map the entire thing. Eventually we probably
- * to do it page-by-page, w/ kmap() if not vmap()d..
+ /* For now, just map the entire thing. Eventually it should probably
+ * be done page-by-page, w/ kmap_local_page() if not vmap()d..
*/
ptr = msm_gem_get_vaddr_locked(&obj->base);
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error
2021-12-10 23:23 [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions ira.weiny
` (4 preceding siblings ...)
2021-12-10 23:24 ` [PATCH 5/7] drm/msm: Alter comment to use kmap_local_page() ira.weiny
@ 2021-12-10 23:24 ` ira.weiny
2021-12-13 20:37 ` Christian König
2021-12-10 23:24 ` [PATCH 7/7] drm/radeon: " ira.weiny
2022-01-19 16:53 ` [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions Ira Weiny
7 siblings, 1 reply; 24+ messages in thread
From: ira.weiny @ 2021-12-10 23:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: Ira Weiny, amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
From: Ira Weiny <ira.weiny@intel.com>
The default case leaves the buffer object mapped in error.
Add amdgpu_bo_kunmap() to that case to ensure the mapping is cleaned up.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
NOTE: It seems like this function could use a fair bit of refactoring
but this is the easiest way to fix the actual bug.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index 6f8de11a17f1..b3ffd0f6b35f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -889,6 +889,7 @@ static int amdgpu_uvd_cs_msg(struct amdgpu_uvd_cs_ctx *ctx,
return 0;
default:
+ amdgpu_bo_kunmap(bo);
DRM_ERROR("Illegal UVD message type (%d)!\n", msg_type);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error
2021-12-10 23:24 ` [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error ira.weiny
@ 2021-12-13 20:37 ` Christian König
2021-12-14 3:37 ` Ira Weiny
0 siblings, 1 reply; 24+ messages in thread
From: Christian König @ 2021-12-13 20:37 UTC (permalink / raw)
To: ira.weiny, David Airlie, Daniel Vetter, Patrik Jakobsson,
Rob Clark, Sean Paul
Cc: linux-arm-msm, intel-gfx, linux-kernel, dri-devel, amd-gfx
Am 11.12.21 um 00:24 schrieb ira.weiny@intel.com:
> From: Ira Weiny <ira.weiny@intel.com>
>
> The default case leaves the buffer object mapped in error.
>
> Add amdgpu_bo_kunmap() to that case to ensure the mapping is cleaned up.
Mhm, good catch. But why do you want to do this in the first place?
Christian.
>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>
> ---
> NOTE: It seems like this function could use a fair bit of refactoring
> but this is the easiest way to fix the actual bug.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
> 1 file changed, 1 insertion(+)
> nice
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index 6f8de11a17f1..b3ffd0f6b35f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -889,6 +889,7 @@ static int amdgpu_uvd_cs_msg(struct amdgpu_uvd_cs_ctx *ctx,
> return 0;
>
> default:
> + amdgpu_bo_kunmap(bo);
> DRM_ERROR("Illegal UVD message type (%d)!\n", msg_type);
> }
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error
2021-12-13 20:37 ` Christian König
@ 2021-12-14 3:37 ` Ira Weiny
2021-12-14 7:09 ` Christian König
0 siblings, 1 reply; 24+ messages in thread
From: Ira Weiny @ 2021-12-14 3:37 UTC (permalink / raw)
To: Christian König
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, linux-arm-msm, intel-gfx, linux-kernel, dri-devel,
amd-gfx
On Mon, Dec 13, 2021 at 09:37:32PM +0100, Christian König wrote:
> Am 11.12.21 um 00:24 schrieb ira.weiny@intel.com:
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > The default case leaves the buffer object mapped in error.
> >
> > Add amdgpu_bo_kunmap() to that case to ensure the mapping is cleaned up.
>
> Mhm, good catch. But why do you want to do this in the first place?
I'm not sure I understand the question.
Any mapping of memory should be paired with an unmapping when no longer needed.
And this is supported by the call to amdgpu_bo_kunmap() in the other
non-default cases.
Do you believe the mapping is not needed?
Ira
>
> Christian.
>
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> >
> > ---
> > NOTE: It seems like this function could use a fair bit of refactoring
> > but this is the easiest way to fix the actual bug.
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
> > 1 file changed, 1 insertion(+)
> > nice
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > index 6f8de11a17f1..b3ffd0f6b35f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > @@ -889,6 +889,7 @@ static int amdgpu_uvd_cs_msg(struct amdgpu_uvd_cs_ctx *ctx,
> > return 0;
> > default:
> > + amdgpu_bo_kunmap(bo);
> > DRM_ERROR("Illegal UVD message type (%d)!\n", msg_type);
> > }
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error
2021-12-14 3:37 ` Ira Weiny
@ 2021-12-14 7:09 ` Christian König
2021-12-15 21:09 ` Ira Weiny
0 siblings, 1 reply; 24+ messages in thread
From: Christian König @ 2021-12-14 7:09 UTC (permalink / raw)
To: Ira Weiny
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, linux-arm-msm, intel-gfx, linux-kernel, dri-devel,
amd-gfx
Am 14.12.21 um 04:37 schrieb Ira Weiny:
> On Mon, Dec 13, 2021 at 09:37:32PM +0100, Christian König wrote:
>> Am 11.12.21 um 00:24 schrieb ira.weiny@intel.com:
>>> From: Ira Weiny <ira.weiny@intel.com>
>>>
>>> The default case leaves the buffer object mapped in error.
>>>
>>> Add amdgpu_bo_kunmap() to that case to ensure the mapping is cleaned up.
>> Mhm, good catch. But why do you want to do this in the first place?
> I'm not sure I understand the question.
>
> Any mapping of memory should be paired with an unmapping when no longer needed.
> And this is supported by the call to amdgpu_bo_kunmap() in the other
> non-default cases.
>
> Do you believe the mapping is not needed?
No, the unmapping is not needed here. See the function amdgpu_bo_kmap(),
it either creates the mapping or return the cached pointer.
A call to amdgpu_bo_kunmap() is only done in a few places where we know
that the created mapping most likely won't be needed any more. If that's
not done the mapping is automatically destroyed when the BO is moved or
freed up.
I mean good bug fix, but you seem to see this as some kind of
prerequisite to some follow up work converting TTM to use kmap_local()
which most likely won't work in the first place.
Regards,
Christian.
>
> Ira
>
>> Christian.
>>
>>> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>>>
>>> ---
>>> NOTE: It seems like this function could use a fair bit of refactoring
>>> but this is the easiest way to fix the actual bug.
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>> nice
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> index 6f8de11a17f1..b3ffd0f6b35f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> @@ -889,6 +889,7 @@ static int amdgpu_uvd_cs_msg(struct amdgpu_uvd_cs_ctx *ctx,
>>> return 0;
>>> default:
>>> + amdgpu_bo_kunmap(bo);
>>> DRM_ERROR("Illegal UVD message type (%d)!\n", msg_type);
>>> }
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error
2021-12-14 7:09 ` Christian König
@ 2021-12-15 21:09 ` Ira Weiny
2021-12-16 8:32 ` Christian König
0 siblings, 1 reply; 24+ messages in thread
From: Ira Weiny @ 2021-12-15 21:09 UTC (permalink / raw)
To: Christian König
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, linux-arm-msm, intel-gfx, linux-kernel, dri-devel,
amd-gfx
On Tue, Dec 14, 2021 at 08:09:29AM +0100, Christian König wrote:
> Am 14.12.21 um 04:37 schrieb Ira Weiny:
> > On Mon, Dec 13, 2021 at 09:37:32PM +0100, Christian König wrote:
> > > Am 11.12.21 um 00:24 schrieb ira.weiny@intel.com:
> > > > From: Ira Weiny <ira.weiny@intel.com>
> > > >
> > > > The default case leaves the buffer object mapped in error.
> > > >
> > > > Add amdgpu_bo_kunmap() to that case to ensure the mapping is cleaned up.
> > > Mhm, good catch. But why do you want to do this in the first place?
> > I'm not sure I understand the question.
> >
> > Any mapping of memory should be paired with an unmapping when no longer needed.
> > And this is supported by the call to amdgpu_bo_kunmap() in the other
> > non-default cases.
> >
> > Do you believe the mapping is not needed?
>
> No, the unmapping is not needed here. See the function amdgpu_bo_kmap(), it
> either creates the mapping or return the cached pointer.
Ah I missed that. Thanks.
>
> A call to amdgpu_bo_kunmap() is only done in a few places where we know that
> the created mapping most likely won't be needed any more. If that's not done
> the mapping is automatically destroyed when the BO is moved or freed up.
>
> I mean good bug fix, but you seem to see this as some kind of prerequisite
> to some follow up work converting TTM to use kmap_local() which most likely
> won't work in the first place.
Sure. I see now that it is more complicated than I thought but I never thought
of this as a strict prerequisite. Just something I found while trying to
figure out how this works.
How much of a speed up is it to maintain the ttm_bo_map_kmap map type? Could
this all be done with vmap and just remove the kmap stuff?
Ira
>
> Regards,
> Christian.
>
> >
> > Ira
> >
> > > Christian.
> > >
> > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > > >
> > > > ---
> > > > NOTE: It seems like this function could use a fair bit of refactoring
> > > > but this is the easiest way to fix the actual bug.
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > > nice
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > > > index 6f8de11a17f1..b3ffd0f6b35f 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> > > > @@ -889,6 +889,7 @@ static int amdgpu_uvd_cs_msg(struct amdgpu_uvd_cs_ctx *ctx,
> > > > return 0;
> > > > default:
> > > > + amdgpu_bo_kunmap(bo);
> > > > DRM_ERROR("Illegal UVD message type (%d)!\n", msg_type);
> > > > }
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error
2021-12-15 21:09 ` Ira Weiny
@ 2021-12-16 8:32 ` Christian König
0 siblings, 0 replies; 24+ messages in thread
From: Christian König @ 2021-12-16 8:32 UTC (permalink / raw)
To: Ira Weiny
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, linux-arm-msm, intel-gfx, linux-kernel, dri-devel,
amd-gfx
Am 15.12.21 um 22:09 schrieb Ira Weiny:
> On Tue, Dec 14, 2021 at 08:09:29AM +0100, Christian König wrote:
>> Am 14.12.21 um 04:37 schrieb Ira Weiny:
>>> On Mon, Dec 13, 2021 at 09:37:32PM +0100, Christian König wrote:
>>>> Am 11.12.21 um 00:24 schrieb ira.weiny@intel.com:
>>>>> From: Ira Weiny <ira.weiny@intel.com>
>>>>>
>>>>> The default case leaves the buffer object mapped in error.
>>>>>
>>>>> Add amdgpu_bo_kunmap() to that case to ensure the mapping is cleaned up.
>>>> Mhm, good catch. But why do you want to do this in the first place?
>>> I'm not sure I understand the question.
>>>
>>> Any mapping of memory should be paired with an unmapping when no longer needed.
>>> And this is supported by the call to amdgpu_bo_kunmap() in the other
>>> non-default cases.
>>>
>>> Do you believe the mapping is not needed?
>> No, the unmapping is not needed here. See the function amdgpu_bo_kmap(), it
>> either creates the mapping or return the cached pointer.
> Ah I missed that. Thanks.
>
>> A call to amdgpu_bo_kunmap() is only done in a few places where we know that
>> the created mapping most likely won't be needed any more. If that's not done
>> the mapping is automatically destroyed when the BO is moved or freed up.
>>
>> I mean good bug fix, but you seem to see this as some kind of prerequisite
>> to some follow up work converting TTM to use kmap_local() which most likely
>> won't work in the first place.
> Sure. I see now that it is more complicated than I thought but I never thought
> of this as a strict prerequisite. Just something I found while trying to
> figure out how this works.
>
> How much of a speed up is it to maintain the ttm_bo_map_kmap map type?
Good question. I don't really know.
This used to be pretty important for older drivers since there the
kernel needs to kmap individual pages and patch them up before sending
the command stream to the hardware.
It most likely doesn't matter for modern hardware.
> Could this all be done with vmap and just remove the kmap stuff?
Maybe, but I wouldn't bet on it and I don't really want to touch any of
the old drivers to figure that out.
Christian.
>
> Ira
>
>> Regards,
>> Christian.
>>
>>> Ira
>>>
>>>> Christian.
>>>>
>>>>> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>>>>>
>>>>> ---
>>>>> NOTE: It seems like this function could use a fair bit of refactoring
>>>>> but this is the easiest way to fix the actual bug.
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>> nice
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>>>> index 6f8de11a17f1..b3ffd0f6b35f 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>>>> @@ -889,6 +889,7 @@ static int amdgpu_uvd_cs_msg(struct amdgpu_uvd_cs_ctx *ctx,
>>>>> return 0;
>>>>> default:
>>>>> + amdgpu_bo_kunmap(bo);
>>>>> DRM_ERROR("Illegal UVD message type (%d)!\n", msg_type);
>>>>> }
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 7/7] drm/radeon: Ensure kunmap is called on error
2021-12-10 23:23 [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions ira.weiny
` (5 preceding siblings ...)
2021-12-10 23:24 ` [PATCH 6/7] drm/amdgpu: Ensure kunmap is called on error ira.weiny
@ 2021-12-10 23:24 ` ira.weiny
2022-01-19 16:53 ` [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions Ira Weiny
7 siblings, 0 replies; 24+ messages in thread
From: ira.weiny @ 2021-12-10 23:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: Ira Weiny, amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
From: Ira Weiny <ira.weiny@intel.com>
The default case leaves the buffer object mapped in error.
Add radeon_bo_kunmap() to that case to ensure the mapping is cleaned up.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
NOTE: It seems like this function could use a fair bit of refactoring
but this is the easiest way to fix the actual bug.
---
drivers/gpu/drm/radeon/radeon_uvd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c b/drivers/gpu/drm/radeon/radeon_uvd.c
index 2ea86919d953..7462010e0e6d 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -563,6 +563,7 @@ static int radeon_uvd_cs_msg(struct radeon_cs_parser *p, struct radeon_bo *bo,
default:
+ radeon_bo_kunmap(bo);
DRM_ERROR("Illegal UVD message type (%d)!\n", msg_type);
return -EINVAL;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions
2021-12-10 23:23 [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions ira.weiny
` (6 preceding siblings ...)
2021-12-10 23:24 ` [PATCH 7/7] drm/radeon: " ira.weiny
@ 2022-01-19 16:53 ` Ira Weiny
2022-01-19 17:24 ` Daniel Vetter
7 siblings, 1 reply; 24+ messages in thread
From: Ira Weiny @ 2022-01-19 16:53 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark, Sean Paul
Cc: amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
On Fri, Dec 10, 2021 at 03:23:57PM -0800, 'Ira Weiny' wrote:
> From: Ira Weiny <ira.weiny@intel.com>
>
> This series starts by converting the last easy kmap() uses to
> kmap_local_page().
>
> There is one more call to kmap() wrapped in ttm_bo_kmap_ttm(). Unfortunately,
> ttm_bo_kmap_ttm() is called in a number of different ways including some which
> are not thread local. I have a patch to convert that call. However, it is not
> straight forward so it is not included in this series.
>
> The final 2 patches fix bugs found while working on the ttm_bo_kmap_ttm()
> conversion.
Gentile ping on this series? Will it make this merge window?
Thanks,
Ira
>
>
> Ira Weiny (7):
> drm/i915: Replace kmap() with kmap_local_page()
> drm/amd: Replace kmap() with kmap_local_page()
> drm/gma: Remove calls to kmap()
> drm/radeon: Replace kmap() with kmap_local_page()
> drm/msm: Alter comment to use kmap_local_page()
> drm/amdgpu: Ensure kunmap is called on error
> drm/radeon: Ensure kunmap is called on error
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++----
> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
> drivers/gpu/drm/gma500/gma_display.c | 6 ++----
> drivers/gpu/drm/gma500/mmu.c | 8 ++++----
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
> drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
> drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
> drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++--
> drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
> drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
> drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++--
> drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
> drivers/gpu/drm/radeon/radeon_uvd.c | 1 +
> 13 files changed, 32 insertions(+), 32 deletions(-)
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions
2022-01-19 16:53 ` [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions Ira Weiny
@ 2022-01-19 17:24 ` Daniel Vetter
2022-01-19 23:55 ` Ira Weiny
0 siblings, 1 reply; 24+ messages in thread
From: Daniel Vetter @ 2022-01-19 17:24 UTC (permalink / raw)
To: Ira Weiny
Cc: David Airlie, Daniel Vetter, Patrik Jakobsson, Rob Clark,
Sean Paul, amd-gfx, dri-devel, linux-kernel, intel-gfx,
linux-arm-msm
On Wed, Jan 19, 2022 at 08:53:56AM -0800, Ira Weiny wrote:
> On Fri, Dec 10, 2021 at 03:23:57PM -0800, 'Ira Weiny' wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > This series starts by converting the last easy kmap() uses to
> > kmap_local_page().
> >
> > There is one more call to kmap() wrapped in ttm_bo_kmap_ttm(). Unfortunately,
> > ttm_bo_kmap_ttm() is called in a number of different ways including some which
> > are not thread local. I have a patch to convert that call. However, it is not
> > straight forward so it is not included in this series.
> >
> > The final 2 patches fix bugs found while working on the ttm_bo_kmap_ttm()
> > conversion.
>
> Gentile ping on this series? Will it make this merge window?
I think this fell through the cracks and so no. Note that generally we
feature-freeze drm tree around -rc6 anyway for the upcoming merge window,
so you were cutting this all a bit close anyway. Also looks like the ttm
kmap caching question didn't get resolved?
Anyway if patches are stuck resend with RESEND and if people still don't
pick them up poke me and I'll apply as fallback.
Cheers, Daniel
>
> Thanks,
> Ira
>
> >
> >
> > Ira Weiny (7):
> > drm/i915: Replace kmap() with kmap_local_page()
> > drm/amd: Replace kmap() with kmap_local_page()
> > drm/gma: Remove calls to kmap()
> > drm/radeon: Replace kmap() with kmap_local_page()
> > drm/msm: Alter comment to use kmap_local_page()
> > drm/amdgpu: Ensure kunmap is called on error
> > drm/radeon: Ensure kunmap is called on error
> >
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++----
> > drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
> > drivers/gpu/drm/gma500/gma_display.c | 6 ++----
> > drivers/gpu/drm/gma500/mmu.c | 8 ++++----
> > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
> > drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
> > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
> > drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++--
> > drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
> > drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
> > drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++--
> > drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
> > drivers/gpu/drm/radeon/radeon_uvd.c | 1 +
> > 13 files changed, 32 insertions(+), 32 deletions(-)
> >
> > --
> > 2.31.1
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions
2022-01-19 17:24 ` Daniel Vetter
@ 2022-01-19 23:55 ` Ira Weiny
2022-01-20 8:16 ` Christian König
0 siblings, 1 reply; 24+ messages in thread
From: Ira Weiny @ 2022-01-19 23:55 UTC (permalink / raw)
To: David Airlie, Patrik Jakobsson, Rob Clark, Sean Paul, amd-gfx,
dri-devel, linux-kernel, intel-gfx, linux-arm-msm,
Christian K�nig
On Wed, Jan 19, 2022 at 06:24:22PM +0100, Daniel Vetter wrote:
> On Wed, Jan 19, 2022 at 08:53:56AM -0800, Ira Weiny wrote:
> > On Fri, Dec 10, 2021 at 03:23:57PM -0800, 'Ira Weiny' wrote:
> > > From: Ira Weiny <ira.weiny@intel.com>
> > >
> > > This series starts by converting the last easy kmap() uses to
> > > kmap_local_page().
> > >
> > > There is one more call to kmap() wrapped in ttm_bo_kmap_ttm(). Unfortunately,
> > > ttm_bo_kmap_ttm() is called in a number of different ways including some which
> > > are not thread local. I have a patch to convert that call. However, it is not
> > > straight forward so it is not included in this series.
> > >
> > > The final 2 patches fix bugs found while working on the ttm_bo_kmap_ttm()
> > > conversion.
> >
> > Gentile ping on this series? Will it make this merge window?
>
> I think this fell through the cracks and so no. Note that generally we
> feature-freeze drm tree around -rc6 anyway for the upcoming merge window,
> so you were cutting this all a bit close anyway.
Ok, No problem. I just had not heard if this was picked up or not.
> Also looks like the ttm
> kmap caching question didn't get resolved?
I'm sorry I thought it was resolve for this series. Christian said the patches
in this series were "a good bug fix" even if not strictly necessary.[1] Beyond
this series I was discussing where to go from here, and is it possible to go
further with more changes.[2] At the moment I don't think I will.
Christian did I misunderstand? I can drop patch 6 and 7 if they are not proper
bug fixes or at least clarifications to the code.
Ira
[1] https://lore.kernel.org/lkml/c3b173ea-6509-ebbe-b5f9-eeb29f1ce57e@amd.com/
[2] https://lore.kernel.org/lkml/20211215210949.GW3538886@iweiny-DESK2.sc.intel.com/
>
> Anyway if patches are stuck resend with RESEND and if people still don't
> pick them up poke me and I'll apply as fallback.
>
> Cheers, Daniel
>
> >
> > Thanks,
> > Ira
> >
> > >
> > >
> > > Ira Weiny (7):
> > > drm/i915: Replace kmap() with kmap_local_page()
> > > drm/amd: Replace kmap() with kmap_local_page()
> > > drm/gma: Remove calls to kmap()
> > > drm/radeon: Replace kmap() with kmap_local_page()
> > > drm/msm: Alter comment to use kmap_local_page()
> > > drm/amdgpu: Ensure kunmap is called on error
> > > drm/radeon: Ensure kunmap is called on error
> > >
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++----
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
> > > drivers/gpu/drm/gma500/gma_display.c | 6 ++----
> > > drivers/gpu/drm/gma500/mmu.c | 8 ++++----
> > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
> > > drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
> > > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
> > > drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++--
> > > drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
> > > drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
> > > drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++--
> > > drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
> > > drivers/gpu/drm/radeon/radeon_uvd.c | 1 +
> > > 13 files changed, 32 insertions(+), 32 deletions(-)
> > >
> > > --
> > > 2.31.1
> > >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions
2022-01-19 23:55 ` Ira Weiny
@ 2022-01-20 8:16 ` Christian König
2022-01-20 15:48 ` [Intel-gfx] " Daniel Vetter
0 siblings, 1 reply; 24+ messages in thread
From: Christian König @ 2022-01-20 8:16 UTC (permalink / raw)
To: Ira Weiny, David Airlie, Patrik Jakobsson, Rob Clark, Sean Paul,
amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
Am 20.01.22 um 00:55 schrieb Ira Weiny:
> On Wed, Jan 19, 2022 at 06:24:22PM +0100, Daniel Vetter wrote:
>> On Wed, Jan 19, 2022 at 08:53:56AM -0800, Ira Weiny wrote:
>>> On Fri, Dec 10, 2021 at 03:23:57PM -0800, 'Ira Weiny' wrote:
>>>> From: Ira Weiny <ira.weiny@intel.com>
>>>>
>>>> This series starts by converting the last easy kmap() uses to
>>>> kmap_local_page().
>>>>
>>>> There is one more call to kmap() wrapped in ttm_bo_kmap_ttm(). Unfortunately,
>>>> ttm_bo_kmap_ttm() is called in a number of different ways including some which
>>>> are not thread local. I have a patch to convert that call. However, it is not
>>>> straight forward so it is not included in this series.
>>>>
>>>> The final 2 patches fix bugs found while working on the ttm_bo_kmap_ttm()
>>>> conversion.
>>> Gentile ping on this series? Will it make this merge window?
>> I think this fell through the cracks and so no. Note that generally we
>> feature-freeze drm tree around -rc6 anyway for the upcoming merge window,
>> so you were cutting this all a bit close anyway.
> Ok, No problem. I just had not heard if this was picked up or not.
>
>> Also looks like the ttm
>> kmap caching question didn't get resolved?
> I'm sorry I thought it was resolve for this series. Christian said the patches
> in this series were "a good bug fix" even if not strictly necessary.[1] Beyond
> this series I was discussing where to go from here, and is it possible to go
> further with more changes.[2] At the moment I don't think I will.
>
> Christian did I misunderstand? I can drop patch 6 and 7 if they are not proper
> bug fixes or at least clarifications to the code.
Yeah, it is indeed a correct cleanup. I would just *not* put a CC stable
on it because it doesn't really fix anything.
Christian.
>
> Ira
>
> [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2Fc3b173ea-6509-ebbe-b5f9-eeb29f1ce57e%40amd.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C5e0192210d4640adb88b08d9dba734b1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637782333459591089%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=4p7jCB6pB4nlcUtLWh6K2Sso9X%2BsRSK7mcD8UavzztQ%3D&reserved=0
> [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20211215210949.GW3538886%40iweiny-DESK2.sc.intel.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C5e0192210d4640adb88b08d9dba734b1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637782333459591089%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=6%2BGfSKshg8Xr%2FXJshiU28yHzbg2HcVisVJLDU6tVUT4%3D&reserved=0
>
>> Anyway if patches are stuck resend with RESEND and if people still don't
>> pick them up poke me and I'll apply as fallback.
>>
>> Cheers, Daniel
>>
>>> Thanks,
>>> Ira
>>>
>>>>
>>>> Ira Weiny (7):
>>>> drm/i915: Replace kmap() with kmap_local_page()
>>>> drm/amd: Replace kmap() with kmap_local_page()
>>>> drm/gma: Remove calls to kmap()
>>>> drm/radeon: Replace kmap() with kmap_local_page()
>>>> drm/msm: Alter comment to use kmap_local_page()
>>>> drm/amdgpu: Ensure kunmap is called on error
>>>> drm/radeon: Ensure kunmap is called on error
>>>>
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++----
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
>>>> drivers/gpu/drm/gma500/gma_display.c | 6 ++----
>>>> drivers/gpu/drm/gma500/mmu.c | 8 ++++----
>>>> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
>>>> drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
>>>> drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
>>>> drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++--
>>>> drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
>>>> drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
>>>> drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++--
>>>> drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
>>>> drivers/gpu/drm/radeon/radeon_uvd.c | 1 +
>>>> 13 files changed, 32 insertions(+), 32 deletions(-)
>>>>
>>>> --
>>>> 2.31.1
>>>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C5e0192210d4640adb88b08d9dba734b1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637782333459591089%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=NLqm91HCdllhW%2BrQ8aHMLXhYGkOJrYffpjsIJZWaFBc%3D&reserved=0
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Intel-gfx] [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions
2022-01-20 8:16 ` Christian König
@ 2022-01-20 15:48 ` Daniel Vetter
2022-01-20 16:57 ` Ira Weiny
0 siblings, 1 reply; 24+ messages in thread
From: Daniel Vetter @ 2022-01-20 15:48 UTC (permalink / raw)
To: Christian König
Cc: Ira Weiny, David Airlie, Patrik Jakobsson, Rob Clark, Sean Paul,
amd-gfx, dri-devel, linux-kernel, intel-gfx, linux-arm-msm
On Thu, Jan 20, 2022 at 09:16:35AM +0100, Christian König wrote:
> Am 20.01.22 um 00:55 schrieb Ira Weiny:
> > On Wed, Jan 19, 2022 at 06:24:22PM +0100, Daniel Vetter wrote:
> > > On Wed, Jan 19, 2022 at 08:53:56AM -0800, Ira Weiny wrote:
> > > > On Fri, Dec 10, 2021 at 03:23:57PM -0800, 'Ira Weiny' wrote:
> > > > > From: Ira Weiny <ira.weiny@intel.com>
> > > > >
> > > > > This series starts by converting the last easy kmap() uses to
> > > > > kmap_local_page().
> > > > >
> > > > > There is one more call to kmap() wrapped in ttm_bo_kmap_ttm(). Unfortunately,
> > > > > ttm_bo_kmap_ttm() is called in a number of different ways including some which
> > > > > are not thread local. I have a patch to convert that call. However, it is not
> > > > > straight forward so it is not included in this series.
> > > > >
> > > > > The final 2 patches fix bugs found while working on the ttm_bo_kmap_ttm()
> > > > > conversion.
> > > > Gentile ping on this series? Will it make this merge window?
> > > I think this fell through the cracks and so no. Note that generally we
> > > feature-freeze drm tree around -rc6 anyway for the upcoming merge window,
> > > so you were cutting this all a bit close anyway.
> > Ok, No problem. I just had not heard if this was picked up or not.
> >
> > > Also looks like the ttm
> > > kmap caching question didn't get resolved?
> > I'm sorry I thought it was resolve for this series. Christian said the patches
> > in this series were "a good bug fix" even if not strictly necessary.[1] Beyond
> > this series I was discussing where to go from here, and is it possible to go
> > further with more changes.[2] At the moment I don't think I will.
> >
> > Christian did I misunderstand? I can drop patch 6 and 7 if they are not proper
> > bug fixes or at least clarifications to the code.
>
> Yeah, it is indeed a correct cleanup. I would just *not* put a CC stable on
> it because it doesn't really fix anything.
Ok can you pls get the amd/radeon ones stuffed into alex' tree? Or do we
want to put all the ttm ones into drm-misc instead?
-Daniel
>
> Christian.
>
> >
> > Ira
> >
> > [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2Fc3b173ea-6509-ebbe-b5f9-eeb29f1ce57e%40amd.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C5e0192210d4640adb88b08d9dba734b1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637782333459591089%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=4p7jCB6pB4nlcUtLWh6K2Sso9X%2BsRSK7mcD8UavzztQ%3D&reserved=0
> > [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20211215210949.GW3538886%40iweiny-DESK2.sc.intel.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C5e0192210d4640adb88b08d9dba734b1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637782333459591089%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=6%2BGfSKshg8Xr%2FXJshiU28yHzbg2HcVisVJLDU6tVUT4%3D&reserved=0
> >
> > > Anyway if patches are stuck resend with RESEND and if people still don't
> > > pick them up poke me and I'll apply as fallback.
> > >
> > > Cheers, Daniel
> > >
> > > > Thanks,
> > > > Ira
> > > >
> > > > >
> > > > > Ira Weiny (7):
> > > > > drm/i915: Replace kmap() with kmap_local_page()
> > > > > drm/amd: Replace kmap() with kmap_local_page()
> > > > > drm/gma: Remove calls to kmap()
> > > > > drm/radeon: Replace kmap() with kmap_local_page()
> > > > > drm/msm: Alter comment to use kmap_local_page()
> > > > > drm/amdgpu: Ensure kunmap is called on error
> > > > > drm/radeon: Ensure kunmap is called on error
> > > > >
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 ++++----
> > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 +
> > > > > drivers/gpu/drm/gma500/gma_display.c | 6 ++----
> > > > > drivers/gpu/drm/gma500/mmu.c | 8 ++++----
> > > > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 4 ++--
> > > > > drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8 ++++----
> > > > > drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
> > > > > drivers/gpu/drm/i915/gt/shmem_utils.c | 4 ++--
> > > > > drivers/gpu/drm/i915/i915_gem.c | 8 ++++----
> > > > > drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++--
> > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++--
> > > > > drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
> > > > > drivers/gpu/drm/radeon/radeon_uvd.c | 1 +
> > > > > 13 files changed, 32 insertions(+), 32 deletions(-)
> > > > >
> > > > > --
> > > > > 2.31.1
> > > > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C5e0192210d4640adb88b08d9dba734b1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637782333459591089%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=NLqm91HCdllhW%2BrQ8aHMLXhYGkOJrYffpjsIJZWaFBc%3D&reserved=0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Intel-gfx] [PATCH 0/7] DRM kmap() fixes and kmap_local_page() conversions
2022-01-20 15:48 ` [Intel-gfx] " Daniel Vetter
@ 2022-01-20 16:57 ` Ira Weiny
0 siblings, 0 replies; 24+ messages in thread
From: Ira Weiny @ 2022-01-20 16:57 UTC (permalink / raw)
To: Christian König, David Airlie, Patrik Jakobsson, Rob Clark,
Sean Paul, amd-gfx, dri-devel, linux-kernel, intel-gfx,
linux-arm-msm
On Thu, Jan 20, 2022 at 04:48:50PM +0100, Daniel Vetter wrote:
> On Thu, Jan 20, 2022 at 09:16:35AM +0100, Christian König wrote:
> > Am 20.01.22 um 00:55 schrieb Ira Weiny:
> > > On Wed, Jan 19, 2022 at 06:24:22PM +0100, Daniel Vetter wrote:
> > > > On Wed, Jan 19, 2022 at 08:53:56AM -0800, Ira Weiny wrote:
> > > > > On Fri, Dec 10, 2021 at 03:23:57PM -0800, 'Ira Weiny' wrote:
> > > > > > From: Ira Weiny <ira.weiny@intel.com>
> > > > > >
> > > > > > This series starts by converting the last easy kmap() uses to
> > > > > > kmap_local_page().
> > > > > >
> > > > > > There is one more call to kmap() wrapped in ttm_bo_kmap_ttm(). Unfortunately,
> > > > > > ttm_bo_kmap_ttm() is called in a number of different ways including some which
> > > > > > are not thread local. I have a patch to convert that call. However, it is not
> > > > > > straight forward so it is not included in this series.
> > > > > >
> > > > > > The final 2 patches fix bugs found while working on the ttm_bo_kmap_ttm()
> > > > > > conversion.
> > > > > Gentile ping on this series? Will it make this merge window?
> > > > I think this fell through the cracks and so no. Note that generally we
> > > > feature-freeze drm tree around -rc6 anyway for the upcoming merge window,
> > > > so you were cutting this all a bit close anyway.
> > > Ok, No problem. I just had not heard if this was picked up or not.
> > >
> > > > Also looks like the ttm
> > > > kmap caching question didn't get resolved?
> > > I'm sorry I thought it was resolve for this series. Christian said the patches
> > > in this series were "a good bug fix" even if not strictly necessary.[1] Beyond
> > > this series I was discussing where to go from here, and is it possible to go
> > > further with more changes.[2] At the moment I don't think I will.
> > >
> > > Christian did I misunderstand? I can drop patch 6 and 7 if they are not proper
> > > bug fixes or at least clarifications to the code.
> >
> > Yeah, it is indeed a correct cleanup. I would just *not* put a CC stable on
> > it because it doesn't really fix anything.
>
> Ok can you pls get the amd/radeon ones stuffed into alex' tree? Or do we
> want to put all the ttm ones into drm-misc instead?
I just updated to the latest master and there is a minor conflict. Since this
is not going in this window. Let me rebase and resend.
Ira
> -Daniel
>
^ permalink raw reply [flat|nested] 24+ messages in thread