2021-04-23 19:12:59

by Jonathan Marek

[permalink] [raw]
Subject: [PATCH 0/5] drm/msm: add MSM_BO_CACHED_COHERENT (and related changes)

Add support for MSM_BO_CACHED_COHERENT, a coherent version of MSM_BO_CACHED
which is implemented by setting the IOMMU_CACHE flag.

Jonathan Marek (5):
drm/msm: remove unnecessary mmap logic for cached BOs
drm/msm: replace MSM_BO_UNCACHED with MSM_BO_WC for internal objects
drm/msm: use the right pgprot when mapping BOs in the kernel
drm/msm: add MSM_BO_CACHED_COHERENT
drm/msm: deprecate MSM_BO_UNCACHED (map as writecombine instead)

drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +--
drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +-
drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 4 +--
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 2 +-
drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
drivers/gpu/drm/msm/msm_drv.c | 3 +-
drivers/gpu/drm/msm/msm_drv.h | 1 +
drivers/gpu/drm/msm/msm_gem.c | 34 ++++++++++-----------
include/uapi/drm/msm_drm.h | 7 ++---
12 files changed, 33 insertions(+), 31 deletions(-)

--
2.26.1


2021-04-23 19:13:06

by Jonathan Marek

[permalink] [raw]
Subject: [PATCH 2/5] drm/msm: replace MSM_BO_UNCACHED with MSM_BO_WC for internal objects

msm_gem_get_vaddr() currently always maps as writecombine, so use the right
flag instead of relying on broken behavior (things don't actually work if
they are mapped as uncached).

Signed-off-by: Jonathan Marek <[email protected]>
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++--
drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +-
drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 4 ++--
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 2 +-
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index ce13d49e615b..eb0f884eaf30 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -902,7 +902,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
if (!a5xx_gpu->shadow_bo) {
a5xx_gpu->shadow = msm_gem_kernel_new(gpu->dev,
sizeof(u32) * gpu->nr_rings,
- MSM_BO_UNCACHED | MSM_BO_MAP_PRIV,
+ MSM_BO_WC | MSM_BO_MAP_PRIV,
gpu->aspace, &a5xx_gpu->shadow_bo,
&a5xx_gpu->shadow_iova);

@@ -1407,7 +1407,7 @@ static int a5xx_crashdumper_init(struct msm_gpu *gpu,
struct a5xx_crashdumper *dumper)
{
dumper->ptr = msm_gem_kernel_new_locked(gpu->dev,
- SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
+ SZ_1M, MSM_BO_WC, gpu->aspace,
&dumper->bo, &dumper->iova);

if (!IS_ERR(dumper->ptr))
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c b/drivers/gpu/drm/msm/adreno/a5xx_power.c
index c35b06b46fcc..cdb165236a88 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_power.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c
@@ -363,7 +363,7 @@ void a5xx_gpmu_ucode_init(struct msm_gpu *gpu)
bosize = (cmds_size + (cmds_size / TYPE4_MAX_PAYLOAD) + 1) << 2;

ptr = msm_gem_kernel_new_locked(drm, bosize,
- MSM_BO_UNCACHED | MSM_BO_GPU_READONLY, gpu->aspace,
+ MSM_BO_WC | MSM_BO_GPU_READONLY, gpu->aspace,
&a5xx_gpu->gpmu_bo, &a5xx_gpu->gpmu_iova);
if (IS_ERR(ptr))
return;
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
index 42eaef7ad7c7..ee72510ff8ce 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
@@ -230,7 +230,7 @@ static int preempt_init_ring(struct a5xx_gpu *a5xx_gpu,

ptr = msm_gem_kernel_new(gpu->dev,
A5XX_PREEMPT_RECORD_SIZE + A5XX_PREEMPT_COUNTER_SIZE,
- MSM_BO_UNCACHED | MSM_BO_MAP_PRIV, gpu->aspace, &bo, &iova);
+ MSM_BO_WC | MSM_BO_MAP_PRIV, gpu->aspace, &bo, &iova);

if (IS_ERR(ptr))
return PTR_ERR(ptr);
@@ -238,7 +238,7 @@ static int preempt_init_ring(struct a5xx_gpu *a5xx_gpu,
/* The buffer to store counters needs to be unprivileged */
counters = msm_gem_kernel_new(gpu->dev,
A5XX_PREEMPT_COUNTER_SIZE,
- MSM_BO_UNCACHED, gpu->aspace, &counters_bo, &counters_iova);
+ MSM_BO_WC, gpu->aspace, &counters_bo, &counters_iova);
if (IS_ERR(counters)) {
msm_gem_kernel_put(bo, gpu->aspace, true);
return PTR_ERR(counters);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 5214a15db95f..1716984c68a8 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -852,7 +852,7 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
if (!a6xx_gpu->shadow_bo) {
a6xx_gpu->shadow = msm_gem_kernel_new_locked(gpu->dev,
sizeof(u32) * gpu->nr_rings,
- MSM_BO_UNCACHED | MSM_BO_MAP_PRIV,
+ MSM_BO_WC | MSM_BO_MAP_PRIV,
gpu->aspace, &a6xx_gpu->shadow_bo,
&a6xx_gpu->shadow_iova);

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
index c1699b4f9a89..21c49c5b4519 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
@@ -113,7 +113,7 @@ static int a6xx_crashdumper_init(struct msm_gpu *gpu,
struct a6xx_crashdumper *dumper)
{
dumper->ptr = msm_gem_kernel_new_locked(gpu->dev,
- SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
+ SZ_1M, MSM_BO_WC, gpu->aspace,
&dumper->bo, &dumper->iova);

if (!IS_ERR(dumper->ptr))
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 17f3e45fd5ff..c1332b2459ec 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -391,7 +391,7 @@ struct drm_gem_object *adreno_fw_create_bo(struct msm_gpu *gpu,
void *ptr;

ptr = msm_gem_kernel_new_locked(gpu->dev, fw->size - 4,
- MSM_BO_UNCACHED | MSM_BO_GPU_READONLY, gpu->aspace, &bo, iova);
+ MSM_BO_WC | MSM_BO_GPU_READONLY, gpu->aspace, &bo, iova);

if (IS_ERR(ptr))
return ERR_CAST(ptr);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 055cd1c7c9fe..18c80744e331 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1155,7 +1155,7 @@ int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size)
uint64_t iova;
u8 *data;

- data = msm_gem_kernel_new(dev, size, MSM_BO_UNCACHED,
+ data = msm_gem_kernel_new(dev, size, MSM_BO_WC,
priv->kms->aspace,
&msm_host->tx_gem_obj, &iova);

--
2.26.1

2021-04-23 19:13:16

by Jonathan Marek

[permalink] [raw]
Subject: [PATCH 3/5] drm/msm: use the right pgprot when mapping BOs in the kernel

Use the same logic as the userspace mapping.

This fixes msm_rd with cached BOs.

Signed-off-by: Jonathan Marek <[email protected]>
---
drivers/gpu/drm/msm/msm_gem.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 09abda42d764..0f58937be0a9 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -199,6 +199,15 @@ void msm_gem_put_pages(struct drm_gem_object *obj)
/* when we start tracking the pin count, then do something here */
}

+static pgprot_t msm_gem_pgprot(struct msm_gem_object *msm_obj, pgprot_t prot)
+{
+ if (msm_obj->flags & MSM_BO_WC)
+ return pgprot_writecombine(prot);
+ if (msm_obj->flags & MSM_BO_UNCACHED)
+ return pgprot_noncached(prot);
+ return prot;
+}
+
int msm_gem_mmap_obj(struct drm_gem_object *obj,
struct vm_area_struct *vma)
{
@@ -206,13 +215,7 @@ int msm_gem_mmap_obj(struct drm_gem_object *obj,

vma->vm_flags &= ~VM_PFNMAP;
vma->vm_flags |= VM_MIXEDMAP;
-
- if (msm_obj->flags & MSM_BO_WC)
- vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
- else if (msm_obj->flags & MSM_BO_UNCACHED)
- vma->vm_page_prot = pgprot_noncached(vm_get_page_prot(vma->vm_flags));
- else
- vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+ vma->vm_page_prot = msm_gem_pgprot(msm_obj, vm_get_page_prot(vma->vm_flags));

return 0;
}
@@ -632,7 +635,7 @@ static void *get_vaddr(struct drm_gem_object *obj, unsigned madv)
goto fail;
}
msm_obj->vaddr = vmap(pages, obj->size >> PAGE_SHIFT,
- VM_MAP, pgprot_writecombine(PAGE_KERNEL));
+ VM_MAP, msm_gem_pgprot(msm_obj, PAGE_KERNEL));
if (msm_obj->vaddr == NULL) {
ret = -ENOMEM;
goto fail;
--
2.26.1

2021-04-23 19:13:25

by Jonathan Marek

[permalink] [raw]
Subject: [PATCH 4/5] drm/msm: add MSM_BO_CACHED_COHERENT

Add a new cache mode for creating coherent host-cached BOs.

Signed-off-by: Jonathan Marek <[email protected]>
Reviewed-by: Jordan Crouse <[email protected]>
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
drivers/gpu/drm/msm/msm_drv.c | 3 ++-
drivers/gpu/drm/msm/msm_drv.h | 1 +
drivers/gpu/drm/msm/msm_gem.c | 8 ++++++++
include/uapi/drm/msm_drm.h | 5 ++---
5 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 9770fd81c614..155c10ffda6e 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -469,6 +469,7 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
config.rev.minor, config.rev.patchid);

priv->is_a2xx = config.rev.core == 2;
+ priv->has_cached_coherent = config.rev.core >= 6;

gpu = info->init(drm);
if (IS_ERR(gpu)) {
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index a63e969e5efb..7576a987dccc 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -40,9 +40,10 @@
* - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
* - 1.6.0 - Syncobj support
* - 1.7.0 - Add MSM_PARAM_SUSPENDS to access suspend count
+ * - 1.8.0 - Add MSM_BO_CACHED_COHERENT for supported GPUs (a6xx)
*/
#define MSM_VERSION_MAJOR 1
-#define MSM_VERSION_MINOR 7
+#define MSM_VERSION_MINOR 8
#define MSM_VERSION_PATCHLEVEL 0

static const struct drm_mode_config_funcs mode_config_funcs = {
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index ea78154c3c24..ffc9092b87b2 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -168,6 +168,7 @@ struct msm_drm_private {
struct msm_file_private *lastctx;
/* gpu is only set on open(), but we need this info earlier */
bool is_a2xx;
+ bool has_cached_coherent;

struct drm_fb_helper *fbdev;

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 0f58937be0a9..2e92e80009c8 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -433,6 +433,9 @@ static int msm_gem_pin_iova(struct drm_gem_object *obj,
if (msm_obj->flags & MSM_BO_MAP_PRIV)
prot |= IOMMU_PRIV;

+ if (msm_obj->flags & MSM_BO_CACHED_COHERENT)
+ prot |= IOMMU_CACHE;
+
GEM_WARN_ON(!msm_gem_is_locked(obj));

if (GEM_WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED))
@@ -1144,6 +1147,7 @@ static int msm_gem_new_impl(struct drm_device *dev,
uint32_t size, uint32_t flags,
struct drm_gem_object **obj)
{
+ struct msm_drm_private *priv = dev->dev_private;
struct msm_gem_object *msm_obj;

switch (flags & MSM_BO_CACHE_MASK) {
@@ -1151,6 +1155,10 @@ static int msm_gem_new_impl(struct drm_device *dev,
case MSM_BO_CACHED:
case MSM_BO_WC:
break;
+ case MSM_BO_CACHED_COHERENT:
+ if (priv->has_cached_coherent)
+ break;
+ /* fallthrough */
default:
DRM_DEV_ERROR(dev->dev, "invalid cache flag: %x\n",
(flags & MSM_BO_CACHE_MASK));
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index 5596d7c37f9e..a92d90a6d96f 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -95,12 +95,11 @@ struct drm_msm_param {
#define MSM_BO_CACHED 0x00010000
#define MSM_BO_WC 0x00020000
#define MSM_BO_UNCACHED 0x00040000
+#define MSM_BO_CACHED_COHERENT 0x080000

#define MSM_BO_FLAGS (MSM_BO_SCANOUT | \
MSM_BO_GPU_READONLY | \
- MSM_BO_CACHED | \
- MSM_BO_WC | \
- MSM_BO_UNCACHED)
+ MSM_BO_CACHE_MASK)

struct drm_msm_gem_new {
__u64 size; /* in */
--
2.26.1

2021-04-23 19:13:44

by Jonathan Marek

[permalink] [raw]
Subject: [PATCH 5/5] drm/msm: deprecate MSM_BO_UNCACHED (map as writecombine instead)

There shouldn't be any reason to ever use uncached over writecombine,
so just use writecombine for MSM_BO_UNCACHED.

Note: userspace never used MSM_BO_UNCACHED anyway

Signed-off-by: Jonathan Marek <[email protected]>
---
drivers/gpu/drm/msm/msm_gem.c | 4 +---
include/uapi/drm/msm_drm.h | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 2e92e80009c8..56bca9178253 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -201,10 +201,8 @@ void msm_gem_put_pages(struct drm_gem_object *obj)

static pgprot_t msm_gem_pgprot(struct msm_gem_object *msm_obj, pgprot_t prot)
{
- if (msm_obj->flags & MSM_BO_WC)
+ if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED))
return pgprot_writecombine(prot);
- if (msm_obj->flags & MSM_BO_UNCACHED)
- return pgprot_noncached(prot);
return prot;
}

diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index a92d90a6d96f..f075851021c3 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -94,7 +94,7 @@ struct drm_msm_param {
/* cache modes */
#define MSM_BO_CACHED 0x00010000
#define MSM_BO_WC 0x00020000
-#define MSM_BO_UNCACHED 0x00040000
+#define MSM_BO_UNCACHED 0x00040000 /* deprecated, use MSM_BO_WC */
#define MSM_BO_CACHED_COHERENT 0x080000

#define MSM_BO_FLAGS (MSM_BO_SCANOUT | \
--
2.26.1

2021-04-23 19:15:35

by Jonathan Marek

[permalink] [raw]
Subject: [PATCH 1/5] drm/msm: remove unnecessary mmap logic for cached BOs

No one knows what this is for anymore, so just remove it.

Signed-off-by: Jonathan Marek <[email protected]>
---
drivers/gpu/drm/msm/msm_gem.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index b199942266a2..09abda42d764 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -207,21 +207,12 @@ int msm_gem_mmap_obj(struct drm_gem_object *obj,
vma->vm_flags &= ~VM_PFNMAP;
vma->vm_flags |= VM_MIXEDMAP;

- if (msm_obj->flags & MSM_BO_WC) {
+ if (msm_obj->flags & MSM_BO_WC)
vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
- } else if (msm_obj->flags & MSM_BO_UNCACHED) {
+ else if (msm_obj->flags & MSM_BO_UNCACHED)
vma->vm_page_prot = pgprot_noncached(vm_get_page_prot(vma->vm_flags));
- } else {
- /*
- * Shunt off cached objs to shmem file so they have their own
- * address_space (so unmap_mapping_range does what we want,
- * in particular in the case of mmap'd dmabufs)
- */
- vma->vm_pgoff = 0;
- vma_set_file(vma, obj->filp);
-
+ else
vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
- }

return 0;
}
--
2.26.1

2021-05-02 19:53:34

by Jordan Crouse

[permalink] [raw]
Subject: Re: [PATCH 1/5] drm/msm: remove unnecessary mmap logic for cached BOs

On Fri, Apr 23, 2021 at 03:08:17PM -0400, Jonathan Marek wrote:
> No one knows what this is for anymore, so just remove it.

Acked-by: Jordan Crouse <[email protected]>

> Signed-off-by: Jonathan Marek <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_gem.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index b199942266a2..09abda42d764 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -207,21 +207,12 @@ int msm_gem_mmap_obj(struct drm_gem_object *obj,
> vma->vm_flags &= ~VM_PFNMAP;
> vma->vm_flags |= VM_MIXEDMAP;
>
> - if (msm_obj->flags & MSM_BO_WC) {
> + if (msm_obj->flags & MSM_BO_WC)
> vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> - } else if (msm_obj->flags & MSM_BO_UNCACHED) {
> + else if (msm_obj->flags & MSM_BO_UNCACHED)
> vma->vm_page_prot = pgprot_noncached(vm_get_page_prot(vma->vm_flags));
> - } else {
> - /*
> - * Shunt off cached objs to shmem file so they have their own
> - * address_space (so unmap_mapping_range does what we want,
> - * in particular in the case of mmap'd dmabufs)
> - */
> - vma->vm_pgoff = 0;
> - vma_set_file(vma, obj->filp);
> -
> + else
> vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> - }
>
> return 0;
> }
> --
> 2.26.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2021-05-02 19:54:53

by Jordan Crouse

[permalink] [raw]
Subject: Re: [PATCH 2/5] drm/msm: replace MSM_BO_UNCACHED with MSM_BO_WC for internal objects

On Fri, Apr 23, 2021 at 03:08:18PM -0400, Jonathan Marek wrote:
> msm_gem_get_vaddr() currently always maps as writecombine, so use the right
> flag instead of relying on broken behavior (things don't actually work if
> they are mapped as uncached).

Ugh - I can't believe this was stil in there.

Acked-by: Jordan Crouse <[email protected]>

> Signed-off-by: Jonathan Marek <[email protected]>
> ---
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++--
> drivers/gpu/drm/msm/adreno/a5xx_power.c | 2 +-
> drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 4 ++--
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 2 +-
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
> 7 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index ce13d49e615b..eb0f884eaf30 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -902,7 +902,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
> if (!a5xx_gpu->shadow_bo) {
> a5xx_gpu->shadow = msm_gem_kernel_new(gpu->dev,
> sizeof(u32) * gpu->nr_rings,
> - MSM_BO_UNCACHED | MSM_BO_MAP_PRIV,
> + MSM_BO_WC | MSM_BO_MAP_PRIV,
> gpu->aspace, &a5xx_gpu->shadow_bo,
> &a5xx_gpu->shadow_iova);
>
> @@ -1407,7 +1407,7 @@ static int a5xx_crashdumper_init(struct msm_gpu *gpu,
> struct a5xx_crashdumper *dumper)
> {
> dumper->ptr = msm_gem_kernel_new_locked(gpu->dev,
> - SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
> + SZ_1M, MSM_BO_WC, gpu->aspace,
> &dumper->bo, &dumper->iova);
>
> if (!IS_ERR(dumper->ptr))
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c b/drivers/gpu/drm/msm/adreno/a5xx_power.c
> index c35b06b46fcc..cdb165236a88 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_power.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c
> @@ -363,7 +363,7 @@ void a5xx_gpmu_ucode_init(struct msm_gpu *gpu)
> bosize = (cmds_size + (cmds_size / TYPE4_MAX_PAYLOAD) + 1) << 2;
>
> ptr = msm_gem_kernel_new_locked(drm, bosize,
> - MSM_BO_UNCACHED | MSM_BO_GPU_READONLY, gpu->aspace,
> + MSM_BO_WC | MSM_BO_GPU_READONLY, gpu->aspace,
> &a5xx_gpu->gpmu_bo, &a5xx_gpu->gpmu_iova);
> if (IS_ERR(ptr))
> return;
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> index 42eaef7ad7c7..ee72510ff8ce 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> @@ -230,7 +230,7 @@ static int preempt_init_ring(struct a5xx_gpu *a5xx_gpu,
>
> ptr = msm_gem_kernel_new(gpu->dev,
> A5XX_PREEMPT_RECORD_SIZE + A5XX_PREEMPT_COUNTER_SIZE,
> - MSM_BO_UNCACHED | MSM_BO_MAP_PRIV, gpu->aspace, &bo, &iova);
> + MSM_BO_WC | MSM_BO_MAP_PRIV, gpu->aspace, &bo, &iova);
>
> if (IS_ERR(ptr))
> return PTR_ERR(ptr);
> @@ -238,7 +238,7 @@ static int preempt_init_ring(struct a5xx_gpu *a5xx_gpu,
> /* The buffer to store counters needs to be unprivileged */
> counters = msm_gem_kernel_new(gpu->dev,
> A5XX_PREEMPT_COUNTER_SIZE,
> - MSM_BO_UNCACHED, gpu->aspace, &counters_bo, &counters_iova);
> + MSM_BO_WC, gpu->aspace, &counters_bo, &counters_iova);
> if (IS_ERR(counters)) {
> msm_gem_kernel_put(bo, gpu->aspace, true);
> return PTR_ERR(counters);
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 5214a15db95f..1716984c68a8 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -852,7 +852,7 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> if (!a6xx_gpu->shadow_bo) {
> a6xx_gpu->shadow = msm_gem_kernel_new_locked(gpu->dev,
> sizeof(u32) * gpu->nr_rings,
> - MSM_BO_UNCACHED | MSM_BO_MAP_PRIV,
> + MSM_BO_WC | MSM_BO_MAP_PRIV,
> gpu->aspace, &a6xx_gpu->shadow_bo,
> &a6xx_gpu->shadow_iova);
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> index c1699b4f9a89..21c49c5b4519 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> @@ -113,7 +113,7 @@ static int a6xx_crashdumper_init(struct msm_gpu *gpu,
> struct a6xx_crashdumper *dumper)
> {
> dumper->ptr = msm_gem_kernel_new_locked(gpu->dev,
> - SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
> + SZ_1M, MSM_BO_WC, gpu->aspace,
> &dumper->bo, &dumper->iova);
>
> if (!IS_ERR(dumper->ptr))
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 17f3e45fd5ff..c1332b2459ec 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -391,7 +391,7 @@ struct drm_gem_object *adreno_fw_create_bo(struct msm_gpu *gpu,
> void *ptr;
>
> ptr = msm_gem_kernel_new_locked(gpu->dev, fw->size - 4,
> - MSM_BO_UNCACHED | MSM_BO_GPU_READONLY, gpu->aspace, &bo, iova);
> + MSM_BO_WC | MSM_BO_GPU_READONLY, gpu->aspace, &bo, iova);
>
> if (IS_ERR(ptr))
> return ERR_CAST(ptr);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 055cd1c7c9fe..18c80744e331 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1155,7 +1155,7 @@ int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size)
> uint64_t iova;
> u8 *data;
>
> - data = msm_gem_kernel_new(dev, size, MSM_BO_UNCACHED,
> + data = msm_gem_kernel_new(dev, size, MSM_BO_WC,
> priv->kms->aspace,
> &msm_host->tx_gem_obj, &iova);
>
> --
> 2.26.1
>

2021-05-02 19:55:16

by Jordan Crouse

[permalink] [raw]
Subject: Re: [PATCH 3/5] drm/msm: use the right pgprot when mapping BOs in the kernel

On Fri, Apr 23, 2021 at 03:08:19PM -0400, Jonathan Marek wrote:
> Use the same logic as the userspace mapping.
>
> This fixes msm_rd with cached BOs.
>

Acked-by: Jordan Crouse <[email protected]>

> Signed-off-by: Jonathan Marek <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_gem.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 09abda42d764..0f58937be0a9 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -199,6 +199,15 @@ void msm_gem_put_pages(struct drm_gem_object *obj)
> /* when we start tracking the pin count, then do something here */
> }
>
> +static pgprot_t msm_gem_pgprot(struct msm_gem_object *msm_obj, pgprot_t prot)
> +{
> + if (msm_obj->flags & MSM_BO_WC)
> + return pgprot_writecombine(prot);
> + if (msm_obj->flags & MSM_BO_UNCACHED)
> + return pgprot_noncached(prot);
> + return prot;
> +}
> +
> int msm_gem_mmap_obj(struct drm_gem_object *obj,
> struct vm_area_struct *vma)
> {
> @@ -206,13 +215,7 @@ int msm_gem_mmap_obj(struct drm_gem_object *obj,
>
> vma->vm_flags &= ~VM_PFNMAP;
> vma->vm_flags |= VM_MIXEDMAP;
> -
> - if (msm_obj->flags & MSM_BO_WC)
> - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> - else if (msm_obj->flags & MSM_BO_UNCACHED)
> - vma->vm_page_prot = pgprot_noncached(vm_get_page_prot(vma->vm_flags));
> - else
> - vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> + vma->vm_page_prot = msm_gem_pgprot(msm_obj, vm_get_page_prot(vma->vm_flags));
>
> return 0;
> }
> @@ -632,7 +635,7 @@ static void *get_vaddr(struct drm_gem_object *obj, unsigned madv)
> goto fail;
> }
> msm_obj->vaddr = vmap(pages, obj->size >> PAGE_SHIFT,
> - VM_MAP, pgprot_writecombine(PAGE_KERNEL));
> + VM_MAP, msm_gem_pgprot(msm_obj, PAGE_KERNEL));
> if (msm_obj->vaddr == NULL) {
> ret = -ENOMEM;
> goto fail;
> --
> 2.26.1
>

2021-05-02 19:57:43

by Jordan Crouse

[permalink] [raw]
Subject: Re: [Freedreno] [PATCH 5/5] drm/msm: deprecate MSM_BO_UNCACHED (map as writecombine instead)

On Fri, Apr 23, 2021 at 03:08:21PM -0400, Jonathan Marek wrote:
> There shouldn't be any reason to ever use uncached over writecombine,
> so just use writecombine for MSM_BO_UNCACHED.

Extremely correct.

>
> Note: userspace never used MSM_BO_UNCACHED anyway
>

Acked-by: Jordan Crouse <[email protected]>

> Signed-off-by: Jonathan Marek <[email protected]>
> ---
> drivers/gpu/drm/msm/msm_gem.c | 4 +---
> include/uapi/drm/msm_drm.h | 2 +-
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 2e92e80009c8..56bca9178253 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -201,10 +201,8 @@ void msm_gem_put_pages(struct drm_gem_object *obj)
>
> static pgprot_t msm_gem_pgprot(struct msm_gem_object *msm_obj, pgprot_t prot)
> {
> - if (msm_obj->flags & MSM_BO_WC)
> + if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED))
> return pgprot_writecombine(prot);
> - if (msm_obj->flags & MSM_BO_UNCACHED)
> - return pgprot_noncached(prot);
> return prot;
> }
>
> diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
> index a92d90a6d96f..f075851021c3 100644
> --- a/include/uapi/drm/msm_drm.h
> +++ b/include/uapi/drm/msm_drm.h
> @@ -94,7 +94,7 @@ struct drm_msm_param {
> /* cache modes */
> #define MSM_BO_CACHED 0x00010000
> #define MSM_BO_WC 0x00020000
> -#define MSM_BO_UNCACHED 0x00040000
> +#define MSM_BO_UNCACHED 0x00040000 /* deprecated, use MSM_BO_WC */
> #define MSM_BO_CACHED_COHERENT 0x080000
>
> #define MSM_BO_FLAGS (MSM_BO_SCANOUT | \
> --
> 2.26.1
>
> _______________________________________________
> Freedreno mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/freedreno