[PATCH 2/6] drm/amdgpu: Add separate mode for syncing DMA_RESV_USAGE_BOOKKEEP.
To prep for allowing different sync modes in a follow-up patch.
Signed-off-by: Bas Nieuwenhuizen
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 11 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 2 +-
10 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a6eb7697c936..746f44c1c3f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1158,7 +1158,7 @@ static int process_sync_pds_resv(struct
amdkfd_process_info *process_info,
struct amdgpu_bo *pd = peer_vm->root.bo;
ret = amdgpu_sync_resv(NULL, sync, pd->tbo.base.resv,
- AMDGPU_SYNC_NE_OWNER,
+ AMDGPU_SYNC_NE_OWNER,
AMDGPU_SYNC_NE_OWNER,
AMDGPU_FENCE_OWNER_KFD);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f1ceb25d1b84..91958e9db90b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -675,7 +675,7 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p)
sync_mode = amdgpu_bo_explicit_sync(bo) ?
AMDGPU_SYNC_EXPLICIT : AMDGPU_SYNC_NE_OWNER;
r = amdgpu_sync_resv(p->adev, >job->sync, resv, sync_mode,
->vm);
+AMDGPU_SYNC_EXPLICIT, >vm);
if (r)
return r;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 2c82b1d5a0d7..20c45f502536 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1410,7 +1410,8 @@ void amdgpu_bo_fence(struct amdgpu_bo *bo, struct
dma_fence *fence,
*
* @adev: amdgpu device pointer
* @resv: reservation object to sync to
- * @sync_mode: synchronization mode
+ * @implicit_sync_mode: synchronization mode for usage <= DMA_RESV_USAGE_READ
+ * @explicit_sync_mode: synchronization mode for usage DMA_RESV_USAGE_BOOKKEEP
* @owner: fence owner
* @intr: Whether the wait is interruptible
*
@@ -1420,14 +1421,15 @@ void amdgpu_bo_fence(struct amdgpu_bo *bo, struct
dma_fence *fence,
* 0 on success, errno otherwise.
*/
int amdgpu_bo_sync_wait_resv(struct amdgpu_device *adev, struct dma_resv *resv,
-enum amdgpu_sync_mode sync_mode, void *owner,
+enum amdgpu_sync_mode implicit_sync_mode,
+enum amdgpu_sync_mode explicit_sync_mode, void
*owner,
bool intr)
{
struct amdgpu_sync sync;
int r;
amdgpu_sync_create();
- amdgpu_sync_resv(adev, , resv, sync_mode, owner);
+ amdgpu_sync_resv(adev, , resv, implicit_sync_mode,
explicit_sync_mode, owner);
r = amdgpu_sync_wait(, intr);
amdgpu_sync_free();
return r;
@@ -1448,7 +1450,8 @@ int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void
*owner, bool intr)
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
return amdgpu_bo_sync_wait_resv(adev, bo->tbo.base.resv,
- AMDGPU_SYNC_NE_OWNER, owner, intr);
+ AMDGPU_SYNC_NE_OWNER,
AMDGPU_SYNC_EXPLICIT,
+ owner, intr);
}
/**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 147b79c10cbb..36ce9abb579c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -320,7 +320,8 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
ttm_buffer_object *bo);
void amdgpu_bo_fence(struct amdgpu_bo *bo, struct dma_fence *fence,
bool shared);
int amdgpu_bo_sync_wait_resv(struct amdgpu_device *adev, struct dma_resv *resv,
-enum amdgpu_sync_mode sync_mode, void *owner,
+enum amdgpu_sync_mode implicit_sync_mode,
+enum amdgpu_sync_mode explicit_sync_mode, void
*owner,
bool intr);
int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr);
u64
[PATCH 5/6] drm/amdgpu: Add option to disable implicit sync for a context.
This changes all BO usages in a submit to BOOKKEEP instead of READ,
which effectively disables implicit sync for these submits.
This is configured at a context level using the existing IOCTL.
Signed-off-by: Bas Nieuwenhuizen
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 13 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 32 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
include/uapi/drm/amdgpu_drm.h | 3 +++
4 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 175fc2c2feec..5246defa4de8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -500,6 +500,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
struct amdgpu_bo *gws;
struct amdgpu_bo *oa;
int r;
+ enum dma_resv_usage resv_usage;
INIT_LIST_HEAD(>validated);
@@ -522,16 +523,19 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser
*p,
mutex_lock(>bo_list->bo_list_mutex);
+ resv_usage = p->ctx->disable_implicit_sync ? DMA_RESV_USAGE_BOOKKEEP :
+DMA_RESV_USAGE_READ;
+
/* One for TTM and one for the CS job */
amdgpu_bo_list_for_each_entry(e, p->bo_list) {
e->tv.num_shared = 2;
- e->tv.usage = DMA_RESV_USAGE_READ;
+ e->tv.usage = resv_usage;
}
amdgpu_bo_list_get_list(p->bo_list, >validated);
INIT_LIST_HEAD();
- amdgpu_vm_get_pd_bo(>vm, >validated, >vm_pd,
DMA_RESV_USAGE_READ);
+ amdgpu_vm_get_pd_bo(>vm, >validated, >vm_pd, resv_usage);
if (p->uf_entry.tv.bo && !ttm_to_amdgpu_bo(p->uf_entry.tv.bo)->parent)
list_add(>uf_entry.tv.head, >validated);
@@ -672,7 +676,7 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p)
struct dma_resv *resv = bo->tbo.base.resv;
enum amdgpu_sync_mode sync_mode;
- sync_mode = amdgpu_bo_explicit_sync(bo) ?
+ sync_mode = (amdgpu_bo_explicit_sync(bo) ||
p->ctx->disable_implicit_sync) ?
AMDGPU_SYNC_EXPLICIT : AMDGPU_SYNC_NE_OWNER;
r = amdgpu_sync_resv(p->adev, >job->sync, resv, sync_mode,
AMDGPU_SYNC_EXPLICIT, >vm);
@@ -1287,7 +1291,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
/* Make sure all BOs are remembered as writers */
amdgpu_bo_list_for_each_entry(e, p->bo_list) {
e->tv.num_shared = 0;
- e->tv.usage = DMA_RESV_USAGE_WRITE;
+ e->tv.usage = p->ctx->disable_implicit_sync ?
DMA_RESV_USAGE_BOOKKEEP
+ :
DMA_RESV_USAGE_WRITE;
}
ttm_eu_fence_buffer_objects(>ticket, >validated, p->fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 7dc92ef36b2b..c01140a449da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -596,8 +596,6 @@ static int amdgpu_ctx_query2(struct amdgpu_device *adev,
return 0;
}
-
-
static int amdgpu_ctx_stable_pstate(struct amdgpu_device *adev,
struct amdgpu_fpriv *fpriv, uint32_t id,
bool set, u32 *stable_pstate)
@@ -626,6 +624,30 @@ static int amdgpu_ctx_stable_pstate(struct amdgpu_device
*adev,
return r;
}
+static int amdgpu_ctx_set_implicit_sync(struct amdgpu_device *adev,
+ struct amdgpu_fpriv *fpriv, uint32_t id,
+ bool enable)
+{
+ struct amdgpu_ctx *ctx;
+ struct amdgpu_ctx_mgr *mgr;
+
+ if (!fpriv)
+ return -EINVAL;
+
+ mgr = >ctx_mgr;
+ mutex_lock(>lock);
+ ctx = idr_find(>ctx_handles, id);
+ if (!ctx) {
+ mutex_unlock(>lock);
+ return -EINVAL;
+ }
+
+ ctx->disable_implicit_sync = !enable;
+
+ mutex_unlock(>lock);
+ return 0;
+}
+
int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
@@ -674,6 +696,12 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
return -EINVAL;
r = amdgpu_ctx_stable_pstate(adev, fpriv, id, true,
_pstate);
break;
+ case AMDGPU_CTX_OP_SET_IMPLICIT_SYNC:
+ if ((args->in.flags & ~AMDGPU_CTX_IMPICIT_SYNC_ENABLED) ||
args->in.priority)
+ return -EINVAL;
+ r = amdgpu_ctx_set_implicit_sync(adev, fpriv, id,
+args->in.flags &
~AMDGPU_CTX_IMPICIT_SYNC_ENABLED);
+ break;
default:
return -EINVAL;
}
diff --git
[PATCH 6/6] drm/amdgpu: Bump amdgpu driver version.
For detection of the new explicit sync functionality without having to try the ioctl. Signed-off-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 8890300766a5..6d92e8846b21 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -101,9 +101,10 @@ * - 3.45.0 - Add context ioctl stable pstate interface * - 3.46.0 - To enable hot plug amdgpu tests in libdrm * * 3.47.0 - Add AMDGPU_GEM_CREATE_DISCARDABLE and AMDGPU_VM_NOALLOC flags + * - 3.48.0 - Add AMDGPU_CTX_OP_SET_IMPLICIT_SYNC context operation. */ #define KMS_DRIVER_MAJOR 3 -#define KMS_DRIVER_MINOR 47 +#define KMS_DRIVER_MINOR 48 #define KMS_DRIVER_PATCHLEVEL 0 int amdgpu_vram_limit; -- 2.37.1
[PATCH 3/6] drm/amdgpu: Allow explicit sync for VM ops.
This should be okay because moves themselves use KERNEL usage and hence still sync with BOOKKEEP usage. Then any later submits still wait on any pending VM operations. (i.e. we only made VM ops not wait on BOOKKEEP submits, not the other way around) Signed-off-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c index f10332e1c6c0..e898a549f86d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c @@ -51,7 +51,8 @@ static int amdgpu_vm_cpu_prepare(struct amdgpu_vm_update_params *p, if (!resv) return 0; - return amdgpu_bo_sync_wait_resv(p->adev, resv, sync_mode, sync_mode, p->vm, true); + return amdgpu_bo_sync_wait_resv(p->adev, resv, sync_mode, + AMDGPU_SYNC_EXPLICIT, p->vm, true); } /** diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c index 6ec6217f0b0e..9233ea3c9404 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c @@ -75,7 +75,8 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p, if (!resv) return 0; - return amdgpu_sync_resv(p->adev, >job->sync, resv, sync_mode, sync_mode, p->vm); + return amdgpu_sync_resv(p->adev, >job->sync, resv, sync_mode, + AMDGPU_SYNC_EXPLICIT, p->vm); } /** -- 2.37.1
[PATCH 1/6] drm/ttm: Add usage to ttm_validate_buffer.
This way callsites can choose between READ/BOOKKEEP reservations.
Signed-off-by: Bas Nieuwenhuizen
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 5 +
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 9 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 8 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 1 +
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 1 +
drivers/gpu/drm/qxl/qxl_release.c| 1 +
drivers/gpu/drm/radeon/radeon_cs.c | 2 ++
drivers/gpu/drm/radeon/radeon_gem.c | 1 +
drivers/gpu/drm/radeon/radeon_vm.c | 2 ++
drivers/gpu/drm/ttm/ttm_execbuf_util.c | 3 +--
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 7 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 1 +
include/drm/ttm/ttm_execbuf_util.h | 2 ++
15 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 4608599ba6bb..a6eb7697c936 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -775,6 +775,7 @@ static void add_kgd_mem_to_kfd_bo_list(struct kgd_mem *mem,
INIT_LIST_HEAD(>head);
entry->num_shared = 1;
+ entry->usage = DMA_RESV_USAGE_READ;
entry->bo = >tbo;
mutex_lock(_info->lock);
if (userptr)
@@ -919,6 +920,7 @@ static int reserve_bo_and_vm(struct kgd_mem *mem,
ctx->kfd_bo.priority = 0;
ctx->kfd_bo.tv.bo = >tbo;
ctx->kfd_bo.tv.num_shared = 1;
+ ctx->kfd_bo.tv.usage = DMA_RESV_USAGE_READ;
list_add(>kfd_bo.tv.head, >list);
amdgpu_vm_get_pd_bo(vm, >list, >vm_pd[0]);
@@ -982,6 +984,7 @@ static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
ctx->kfd_bo.priority = 0;
ctx->kfd_bo.tv.bo = >tbo;
ctx->kfd_bo.tv.num_shared = 1;
+ ctx->kfd_bo.tv.usage = DMA_RESV_USAGE_READ;
list_add(>kfd_bo.tv.head, >list);
i = 0;
@@ -2207,6 +2210,7 @@ static int validate_invalid_user_pages(struct
amdkfd_process_info *process_info)
list_add_tail(>resv_list.head, _list);
mem->resv_list.bo = mem->validate_list.bo;
mem->resv_list.num_shared = mem->validate_list.num_shared;
+ mem->resv_list.usage = mem->validate_list.usage;
}
/* Reserve all BOs and page tables for validation */
@@ -2406,6 +2410,7 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info,
struct dma_fence **ef)
list_add_tail(>resv_list.head, );
mem->resv_list.bo = mem->validate_list.bo;
mem->resv_list.num_shared = mem->validate_list.num_shared;
+ mem->resv_list.usage = mem->validate_list.usage;
}
ret = ttm_eu_reserve_buffers(, ,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d8f1335bc68f..f1ceb25d1b84 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -57,6 +57,7 @@ static int amdgpu_cs_user_fence_chunk(struct amdgpu_cs_parser
*p,
p->uf_entry.tv.bo = >tbo;
/* One for TTM and two for the CS job */
p->uf_entry.tv.num_shared = 3;
+ p->uf_entry.tv.usage = DMA_RESV_USAGE_READ;
drm_gem_object_put(gobj);
@@ -522,8 +523,10 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
mutex_lock(>bo_list->bo_list_mutex);
/* One for TTM and one for the CS job */
- amdgpu_bo_list_for_each_entry(e, p->bo_list)
+ amdgpu_bo_list_for_each_entry(e, p->bo_list) {
e->tv.num_shared = 2;
+ e->tv.usage = DMA_RESV_USAGE_READ;
+ }
amdgpu_bo_list_get_list(p->bo_list, >validated);
@@ -1282,8 +1285,10 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
amdgpu_vm_move_to_lru_tail(p->adev, >vm);
/* Make sure all BOs are remembered as writers */
- amdgpu_bo_list_for_each_entry(e, p->bo_list)
+ amdgpu_bo_list_for_each_entry(e, p->bo_list) {
e->tv.num_shared = 0;
+ e->tv.usage = DMA_RESV_USAGE_WRITE;
+ }
ttm_eu_fence_buffer_objects(>ticket, >validated, p->fence);
mutex_unlock(>adev->notifier_lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index c6d4d41c4393..24941ed1a5ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -75,6 +75,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
INIT_LIST_HEAD(_tv.head);
csa_tv.bo = >tbo;
csa_tv.num_shared = 1;
+ csa_tv.usage = DMA_RESV_USAGE_READ;
list_add(_tv.head, );
[PATCH v6 7/8] drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large
The ttm_bo_init_reserved() functions returns -ENOSPC if the size is too big to add vma. The direct function that returns -ENOSPC is drm_mm_insert_node_in_range(). To handle the same error as other code returning -E2BIG when the size is too large, it converts return value to -E2BIG. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 30f488712abe..88d1bfd11f1d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1249,6 +1249,17 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, ret = ttm_bo_init_reserved(>bdev, i915_gem_to_ttm(obj), bo_type, _sys_placement, page_size >> PAGE_SHIFT, , NULL, NULL, i915_ttm_bo_destroy); + + /* +* XXX: The ttm_bo_init_reserved() functions returns -ENOSPC if the size +* is too big to add vma. The direct function that returns -ENOSPC is +* drm_mm_insert_node_in_range(). To handle the same error as other code +* that returns -E2BIG when the size is too large, it converts -ENOSPC to +* -E2BIG. +*/ + if (size >> PAGE_SHIFT > INT_MAX && ret == -ENOSPC) + ret = -E2BIG; + if (ret) return i915_ttm_err_to_gem(ret); -- 2.34.1
[PATCH v6 4/8] drm/i915: Check for integer truncation on scatterlist creation
From: Chris Wilson
There is an impedance mismatch between the scatterlist API using unsigned
int and our memory/page accounting in unsigned long. That is we may try
to create a scatterlist for a large object that overflows returning a
small table into which we try to fit very many pages. As the object size
is under control of userspace, we have to be prudent and catch the
conversion errors.
To catch the implicit truncation as we switch from unsigned long into the
scatterlist's unsigned int, we use overflows_type check and report
E2BIG prior to the operation. This is already used in our create ioctls to
indicate if the uABI request is simply too large for the backing store.
Failing that type check, we have a second check at sg_alloc_table time
to make sure the values we are passing into the scatterlist API are not
truncated.
It uses pgoff_t for locals that are dealing with page indices, in this
case, the page count is the limit of the page index.
And it uses safe_conversion() macro which performs a type conversion (cast)
of an integer value into a new variable, checking that the destination is
large enough to hold the source value.
v2: Move added i915_utils's macro into drm_util header (Jani N)
v5: Fix macros to be enclosed in parentheses for complex values
Fix too long line warning
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Brian Welty
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
Reviewed-by: Andrzej Hajda
---
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 --
drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ---
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4
drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 5 -
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 4
drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 -
drivers/gpu/drm/i915/gvt/dmabuf.c| 9 +
drivers/gpu/drm/i915/i915_scatterlist.h | 11 +++
8 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
index c698f95af15f..ff2e6e780631 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
@@ -37,10 +37,13 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
struct sg_table *st;
struct scatterlist *sg;
unsigned int sg_page_sizes;
- unsigned int npages;
+ pgoff_t npages; /* restricted by sg_alloc_table */
int max_order;
gfp_t gfp;
+ if (!safe_conversion(, obj->base.size >> PAGE_SHIFT))
+ return -E2BIG;
+
max_order = MAX_ORDER;
#ifdef CONFIG_SWIOTLB
if (is_swiotlb_active(obj->base.dev->dev)) {
@@ -67,7 +70,6 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
if (!st)
return -ENOMEM;
- npages = obj->base.size / PAGE_SIZE;
if (sg_alloc_table(st, npages, GFP_KERNEL)) {
kfree(st);
return -ENOMEM;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 5da872afc4ba..0cf31adbfd41 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -26,9 +26,6 @@ enum intel_region_id;
* this and catch if we ever need to fix it. In the meantime, if you do
* spot such a local variable, please consider fixing!
*
- * Aside from our own locals (for which we have no excuse!):
- * - sg_table embeds unsigned int for nents
- *
* We can check for invalidly typed locals with typecheck(), see for example
* i915_gem_object_get_sg().
*/
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index 0d0e46dae559..88ba7266a3a5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -28,6 +28,10 @@ static int i915_gem_object_get_pages_phys(struct
drm_i915_gem_object *obj)
void *dst;
int i;
+ /* Contiguous chunk, with a single scatterlist element */
+ if (overflows_type(obj->base.size, sg->length))
+ return -E2BIG;
+
if (GEM_WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index f42ca1179f37..4cb35808e431 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -193,13 +193,16 @@ static int shmem_get_pages(struct drm_i915_gem_object
*obj)
struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct intel_memory_region *mem = obj->mm.region;
struct address_space *mapping = obj->base.filp->f_mapping;
- const unsigned long page_count = obj->base.size / PAGE_SIZE;
unsigned
[PATCH v6 6/8] drm/i915: Check if the size is too big while creating shmem file
The __shmem_file_setup() function returns -EINVAL if size is greater than MAX_LFS_FILESIZE. To handle the same error as other code that returns -E2BIG when the size is too large, it add a code that returns -E2BIG when the size is larger than the size that can be handled. v4: If BITS_PER_LONG is 32, size > MAX_LFS_FILESIZE is always false, so it checks only when BITS_PER_LONG is 64. Signed-off-by: Gwan-gyeong Mun Cc: Chris Wilson Cc: Matthew Auld Cc: Thomas Hellström Reviewed-by: Nirmoy Das Reviewed-by: Mauro Carvalho Chehab Reported-by: kernel test robot Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 4cb35808e431..4a7a6d65fc7a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -541,6 +541,20 @@ static int __create_shmem(struct drm_i915_private *i915, drm_gem_private_object_init(>drm, obj, size); + /* XXX: The __shmem_file_setup() function returns -EINVAL if size is +* greater than MAX_LFS_FILESIZE. +* To handle the same error as other code that returns -E2BIG when +* the size is too large, we add a code that returns -E2BIG when the +* size is larger than the size that can be handled. +* If BITS_PER_LONG is 32, size > MAX_LFS_FILESIZE is always false, +* so we only needs to check when BITS_PER_LONG is 64. +* If BITS_PER_LONG is 32, E2BIG checks are processed when +* i915_gem_object_size_2big() is called before init_object() callback +* is called. +*/ + if (BITS_PER_LONG == 64 && size > MAX_LFS_FILESIZE) + return -E2BIG; + if (i915->mm.gemfs) filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size, flags); -- 2.34.1
[PATCH v6 3/8] drm/i915/gem: Typecheck page lookups
From: Chris Wilson
We need to check that we avoid integer overflows when looking up a page,
and so fix all the instances where we have mistakenly used a plain
integer instead of a more suitable long. Be pedantic and add integer
typechecking to the lookup so that we can be sure that we are safe.
And it also uses pgoff_t as our page lookups must remain compatible with
the page cache, pgoff_t is currently exactly unsigned long.
v2: Move added i915_utils's macro into drm_util header (Jani N)
v3: Make not use the same macro name on a function. (Mauro)
For kernel-doc, macros and functions are handled in the same namespace,
the same macro name on a function prevents ever adding documentation
for it.
v4: Add kernel-doc markups to the kAPI functions and macros (Mauoro)
v5: Fix an alignment to match open parenthesis
v6: Rebase
Signed-off-by: Chris Wilson
Signed-off-by: Gwan-gyeong Mun
Cc: Tvrtko Ursulin
Cc: Matthew Auld
Cc: Thomas Hellström
Reviewed-by: Nirmoy Das
Reviewed-by: Mauro Carvalho Chehab
Reviewed-by: Andrzej Hajda
---
drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +-
drivers/gpu/drm/i915/gem/i915_gem_object.h| 293 --
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 27 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
.../drm/i915/gem/selftests/i915_gem_context.c | 12 +-
.../drm/i915/gem/selftests/i915_gem_mman.c| 8 +-
.../drm/i915/gem/selftests/i915_gem_object.c | 8 +-
drivers/gpu/drm/i915/i915_gem.c | 18 +-
drivers/gpu/drm/i915/i915_utils.h | 1 +
drivers/gpu/drm/i915/i915_vma.c | 8 +-
10 files changed, 323 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 389e9f157ca5..b3861739c1eb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -413,10 +413,11 @@ void __i915_gem_object_invalidate_frontbuffer(struct
drm_i915_gem_object *obj,
static void
i915_gem_object_read_from_page_kmap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ pgoff_t idx = offset >> PAGE_SHIFT;
void *src_map;
void *src_ptr;
- src_map = kmap_atomic(i915_gem_object_get_page(obj, offset >>
PAGE_SHIFT));
+ src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
src_ptr = src_map + offset_in_page(offset);
if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
@@ -429,9 +430,10 @@ i915_gem_object_read_from_page_kmap(struct
drm_i915_gem_object *obj, u64 offset,
static void
i915_gem_object_read_from_page_iomap(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ pgoff_t idx = offset >> PAGE_SHIFT;
+ dma_addr_t dma = i915_gem_object_get_dma_address(obj, idx);
void __iomem *src_map;
void __iomem *src_ptr;
- dma_addr_t dma = i915_gem_object_get_dma_address(obj, offset >>
PAGE_SHIFT);
src_map = io_mapping_map_wc(>mm.region->iomap,
dma - obj->mm.region->region.start,
@@ -460,6 +462,7 @@ i915_gem_object_read_from_page_iomap(struct
drm_i915_gem_object *obj, u64 offset
*/
int i915_gem_object_read_from_page(struct drm_i915_gem_object *obj, u64
offset, void *dst, int size)
{
+ GEM_BUG_ON(overflows_type(offset >> PAGE_SHIFT, pgoff_t));
GEM_BUG_ON(offset >= obj->base.size);
GEM_BUG_ON(offset_in_page(offset) > PAGE_SIZE - size);
GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 6f0a3ce35567..5da872afc4ba 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -27,8 +27,10 @@ enum intel_region_id;
* spot such a local variable, please consider fixing!
*
* Aside from our own locals (for which we have no excuse!):
- * - sg_table embeds unsigned int for num_pages
- * - get_user_pages*() mixed ints with longs
+ * - sg_table embeds unsigned int for nents
+ *
+ * We can check for invalidly typed locals with typecheck(), see for example
+ * i915_gem_object_get_sg().
*/
#define GEM_CHECK_SIZE_OVERFLOW(sz) \
GEM_WARN_ON((sz) >> PAGE_SHIFT > INT_MAX)
@@ -363,44 +365,289 @@ i915_gem_object_get_tile_row_size(const struct
drm_i915_gem_object *obj)
int i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
unsigned int tiling, unsigned int stride);
+/**
+ * __i915_gem_object_page_iter_get_sg - helper to find the target scatterlist
+ * pointer and the target page position using pgoff_t n input argument and
+ * i915_gem_object_page_iter
+ * @obj: i915 GEM buffer object
+ * @iter: i915 GEM buffer object page iterator
+ * @n: page offset
+ * @offset: searched physical offset,
+ * it will be used for returning physical page offset value
+ *
+ * Context: Takes and
[PATCH v6 2/8] util_macros: Add exact_type macro to catch type mis-match while compiling
It adds exact_type and exactly_pgoff_t macro to catch type mis-match while
compiling. The existing typecheck() macro outputs build warnings, but the
newly added exact_type() macro uses the BUILD_BUG_ON() macro to generate
a build break when the types are different and can be used to detect
explicit build errors.
v6: Move macro addition location so that it can be used by other than drm
subsystem (Jani, Mauro, Andi)
Signed-off-by: Gwan-gyeong Mun
Cc: Thomas Hellström
Cc: Matthew Auld
Cc: Nirmoy Das
Cc: Jani Nikula
Cc: Andi Shyti
Cc: Mauro Carvalho Chehab
---
include/linux/util_macros.h | 25 +
1 file changed, 25 insertions(+)
diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
index 72299f261b25..b6624b275257 100644
--- a/include/linux/util_macros.h
+++ b/include/linux/util_macros.h
@@ -2,6 +2,9 @@
#ifndef _LINUX_HELPER_MACROS_H_
#define _LINUX_HELPER_MACROS_H_
+#include
+#include
+
#define __find_closest(x, a, as, op) \
({ \
typeof(as) __fc_i, __fc_as = (as) - 1; \
@@ -38,4 +41,26 @@
*/
#define find_closest_descending(x, a, as) __find_closest(x, a, as, >=)
+/**
+ * exact_type - break compile if source type and destination value's type are
+ * not the same
+ * @T: Source type
+ * @n: Destination value
+ *
+ * It is a helper macro for a poor man's -Wconversion: only allow variables of
+ * an exact type. It determines whether the source type and destination value's
+ * type are the same while compiling, and it breaks compile if two types are
+ * not the same
+ */
+#define exact_type(T, n) \
+ BUILD_BUG_ON(!__builtin_constant_p(n) &&
!__builtin_types_compatible_p(T, typeof(n)))
+
+/**
+ * exactly_pgoff_t - helper to check if the type of a value is pgoff_t
+ * @n: value to compare pgoff_t type
+ *
+ * It breaks compile if the argument value's type is not pgoff_t type.
+ */
+#define exactly_pgoff_t(n) exact_type(pgoff_t, n)
+
#endif
--
2.34.1
[PATCH v6 1/8] overflow: Move and add few utility macros into overflow
It moves overflows_type utility macro into overflow header from i915_utils
header. The overflows_type can be used to catch the truncation between data
types. And it adds safe_conversion() macro which performs a type conversion
(cast) of an source value into a new variable, checking that the
destination is large enough to hold the source value. And the functionality
of overflows_type has been improved to handle the signbit.
The is_unsigned_type macro has been added to check the sign bit of the
built-in type.
v3: Add is_type_unsigned() macro (Mauro)
Modify overflows_type() macro to consider signed data types (Mauro)
Fix the problem that safe_conversion() macro always returns true
v4: Fix kernel-doc markups
v6: Move macro addition location so that it can be used by other than drm
subsystem (Jani, Mauro, Andi)
Change is_type_unsigned to is_unsigned_type to have the same name form
as is_signed_type macro
Signed-off-by: Gwan-gyeong Mun
Cc: Thomas Hellström
Cc: Matthew Auld
Cc: Nirmoy Das
Cc: Jani Nikula
Cc: Andi Shyti
Reviewed-by: Mauro Carvalho Chehab (v5)
---
drivers/gpu/drm/i915/i915_utils.h | 5 +--
include/linux/overflow.h | 54 +++
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_utils.h
b/drivers/gpu/drm/i915/i915_utils.h
index c10d68cdc3ca..eb0ded23fa9c 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_X86
#include
@@ -111,10 +112,6 @@ bool i915_error_injected(void);
#define range_overflows_end_t(type, start, size, max) \
range_overflows_end((type)(start), (type)(size), (type)(max))
-/* Note we don't consider signbits :| */
-#define overflows_type(x, T) \
- (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T))
-
#define ptr_mask_bits(ptr, n) ({ \
unsigned long __v = (unsigned long)(ptr); \
(typeof(ptr))(__v & -BIT(n)); \
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index f1221d11f8e5..462a03454377 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -35,6 +35,60 @@
#define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
#define type_min(T) ((T)((T)-type_max(T)-(T)1))
+/**
+ * is_unsigned_type - helper for checking data type which is an unsigned data
+ * type or not
+ * @x: The data type to check
+ *
+ * Returns:
+ * True if the data type is an unsigned data type, false otherwise.
+ */
+#define is_unsigned_type(x) ((typeof(x))-1 >= (typeof(x))0)
+
+/**
+ * overflows_type - helper for checking the truncation between data types
+ * @x: Source for overflow type comparison
+ * @T: Destination for overflow type comparison
+ *
+ * It compares the values and size of each data type between the first and
+ * second argument to check whether truncation can occur when assigning the
+ * first argument to the variable of the second argument.
+ * Source and Destination can be used with or without sign bit.
+ * Composite data structures such as union and structure are not considered.
+ * Enum data types are not considered.
+ * Floating point data types are not considered.
+ *
+ * Returns:
+ * True if truncation can occur, false otherwise.
+ */
+#define overflows_type(x, T) \
+ (is_unsigned_type(x) ? \
+ is_unsigned_type(T) ? \
+ (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T)) ? 1
: 0 \
+ : (sizeof(x) >= sizeof(T) && (x) >> (BITS_PER_TYPE(T) -
1)) ? 1 : 0 \
+ : is_unsigned_type(T) ? \
+ ((x) < 0) ? 1 : (sizeof(x) > sizeof(T) && (x) >>
BITS_PER_TYPE(T)) ? 1 : 0 \
+ : (sizeof(x) > sizeof(T)) ? \
+ ((x) < 0) ? (((x) * -1) >> BITS_PER_TYPE(T)) ? 1 : 0 \
+ : ((x) >> BITS_PER_TYPE(T)) ? 1 : 0 \
+ : 0)
+
+/**
+ * safe_conversion - perform a type conversion (cast) of an source value into
+ * a new variable, checking that the destination is large enough to hold the
+ * source value.
+ * @ptr: Destination pointer address
+ * @value: Source value
+ *
+ * Returns:
+ * If the value would overflow the destination, it returns false.
+ */
+#define safe_conversion(ptr, value) ({ \
+ typeof(value) __v = (value); \
+ typeof(ptr) __ptr = (ptr); \
+ overflows_type(__v, *__ptr) ? 0 : ((*__ptr = (typeof(*__ptr))__v), 1); \
+})
+
/*
* Avoids triggering -Wtype-limits compilation warning,
* while using unsigned data types to check a < 0.
--
2.34.1
[PATCH v6 0/8] Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation
This patch series fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation, etc. We need to check that we avoid integer overflows when looking up a page, and so fix all the instances where we have mistakenly used a plain integer instead of a more suitable long. And there is an impedance mismatch between the scatterlist API using unsigned int and our memory/page accounting in unsigned long. That is we may try to create a scatterlist for a large object that overflows returning a small table into which we try to fit very many pages. As the object size is under the control of userspace, we have to be prudent and catch the conversion errors. To catch the implicit truncation as we switch from unsigned long into the scatterlist's unsigned int, we use our overflows_type check and report E2BIG prior to the operation. This is already used in our create ioctls to indicate if the uABI request is simply too large for the backing store. And ttm place also has the same problem with scatterlist creation, and we fix the integer truncation problem with the way approached by scatterlist creation. And It corrects the error code to return -E2BIG when creating gem objects using ttm or shmem, if the size is too large in each case. In order to provide a common macro, it moves and adds a few utility macros into overflow/util_macros header v6: Move macro addition location so that it can be used by other than drm subsystem (Jani, Mauro, Andi) Fix to follow general use case for GEM_BUG_ON(). (Jani) v5: Fix an alignment to match open parenthesis Fix macros to be enclosed in parentheses for complex values Fix too long line warning v4: Fix build warnins that reported by kernel test robot. (kernel test robot ) Add kernel-doc markups to the kAPI functions and macros (Mauoro) v3: Modify overflows_type() macro to consider signed data types and add is_type_unsigned() macro (Mauro) Make not use the same macro name on a function. (Mauro) For kernel-doc, macros and functions are handled in the same namespace, the same macro name on a function prevents ever adding documentation for it. Not to change execution inside a macro. (Mauro) Fix the problem that safe_conversion() macro always returns true (G.G) Add safe_conversion_gem_bug_on() macro and remove temporal SAFE_CONVERSION() macro. (G.G.) Chris Wilson (3): drm/i915/gem: Typecheck page lookups drm/i915: Check for integer truncation on scatterlist creation drm/i915: Remove truncation warning for large objects Gwan-gyeong Mun (5): overflow: Move and add few utility macros into overflow util_macros: Add exact_type macro to catch type mis-match while compiling drm/i915: Check for integer truncation on the configuration of ttm place drm/i915: Check if the size is too big while creating shmem file drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large drivers/gpu/drm/i915/gem/i915_gem_internal.c | 6 +- drivers/gpu/drm/i915/gem/i915_gem_object.c| 7 +- drivers/gpu/drm/i915/gem/i915_gem_object.h| 303 +++--- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 27 +- drivers/gpu/drm/i915/gem/i915_gem_phys.c | 4 + drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 19 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 23 +- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 5 +- .../drm/i915/gem/selftests/i915_gem_context.c | 12 +- .../drm/i915/gem/selftests/i915_gem_mman.c| 8 +- .../drm/i915/gem/selftests/i915_gem_object.c | 8 +- drivers/gpu/drm/i915/gvt/dmabuf.c | 9 +- drivers/gpu/drm/i915/i915_gem.c | 18 +- drivers/gpu/drm/i915/i915_scatterlist.h | 11 + drivers/gpu/drm/i915/i915_utils.h | 6 +- drivers/gpu/drm/i915/i915_vma.c | 8 +- drivers/gpu/drm/i915/intel_region_ttm.c | 22 +- include/linux/overflow.h | 54 include/linux/util_macros.h | 25 ++ 19 files changed, 482 insertions(+), 93 deletions(-) -- 2.34.1
[PATCH libdrm v2 1/2] tests/modetest: Allocate drmModeAtomicReq before setting properties
Fix null pointer deference caused by drmModeAtomicReq being
allocated before set_property was called when modetest was run
with the atomic flag.
Reviewed-by: Rob Clark
Signed-off-by: Rohith Iyer
---
tests/modetest/modetest.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 8ff6c80d..f33f303c 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -2188,11 +2188,13 @@ int main(int argc, char **argv)
dump_resource(, planes);
dump_resource(, framebuffers);
+ if (dev.use_atomic)
+ dev.req = drmModeAtomicAlloc();
+
for (i = 0; i < prop_count; ++i)
set_property(, _args[i]);
if (dev.use_atomic) {
- dev.req = drmModeAtomicAlloc();
if (set_preferred || (count && plane_count)) {
uint64_t cap = 0;
--
2.17.1
[PATCH libdrm v2 2/2] tests/modetest: Add support for writeback connector
Add writeback support to modetest with the below options:
- Passing in -c will now also show the writeback connector
- Test a built-in mode on writeback connector
- Test a custom mode from user input on writeback connector
Usage: "./modetest -M msm -x :
-a -P @:+0+0@RG24."
Refer to --help for exact syntax
- Dump the writeback output buffer to bitstream
Usage: "./modetest -M msm -s :
-a -o
-P @:+0+0@RG24"
This currently supports a singular writeback connector.
Changes made in V2:
- Added helper method that checks if user pipe has writeback connector
- Added error message for dump flag if no writeback connector is found
- Polls on the writeback fence fd until writeback is complete
Signed-off-by: Rohith Iyer
---
tests/modetest/buffers.c | 19
tests/modetest/buffers.h | 1 +
tests/modetest/modetest.c | 176 ++
3 files changed, 177 insertions(+), 19 deletions(-)
diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 8a8d9e01..279d7b28 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -353,3 +353,22 @@ void bo_destroy(struct bo *bo)
free(bo);
}
+
+void bo_dump(struct bo *bo, const char *filename)
+{
+ FILE *fp;
+
+ if (!bo || !filename)
+ return;
+
+ fp = fopen(filename, "wb");
+ if (fp) {
+ void *addr;
+
+ bo_map(bo, );
+ printf("Dumping buffer %p to file %s.\n", bo->ptr, filename);
+ fwrite(bo->ptr, 1, bo->size, fp);
+ bo_unmap(bo);
+ fclose(fp);
+ }
+}
diff --git a/tests/modetest/buffers.h b/tests/modetest/buffers.h
index 7f95396b..cbd54e9e 100644
--- a/tests/modetest/buffers.h
+++ b/tests/modetest/buffers.h
@@ -36,5 +36,6 @@ struct bo *bo_create(int fd, unsigned int format,
unsigned int handles[4], unsigned int pitches[4],
unsigned int offsets[4], enum util_fill_pattern pattern);
void bo_destroy(struct bo *bo);
+void bo_dump(struct bo *bo, const char *filename);
#endif
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index f33f303c..e8adc18e 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -70,6 +70,7 @@
static enum util_fill_pattern primary_fill = UTIL_PATTERN_SMPTE;
static enum util_fill_pattern secondary_fill = UTIL_PATTERN_TILES;
+static enum util_fill_pattern plain_fill = UTIL_PATTERN_PLAIN;
struct crtc {
drmModeCrtc *crtc;
@@ -128,6 +129,7 @@ struct device {
int use_atomic;
drmModeAtomicReq *req;
+ int32_t *writeback_fence_fd;
};
static inline int64_t U642I64(uint64_t val)
@@ -135,6 +137,11 @@ static inline int64_t U642I64(uint64_t val)
return (int64_t)*((int64_t *));
}
+static inline uint64_t to_user_pointer(const void *ptr)
+{
+ return (uintptr_t)ptr;
+}
+
static float mode_vrefresh(drmModeModeInfo *mode)
{
return mode->clock * 1000.00
@@ -813,6 +820,10 @@ struct pipe_arg {
struct crtc *crtc;
unsigned int fb_id[2], current_fb_id;
struct timeval start;
+ unsigned int out_fb_id;
+ struct bo *out_bo;
+ bool custom;
+ bool dump;
int swap_count;
};
@@ -919,27 +930,43 @@ static struct crtc *pipe_find_crtc(struct device *dev,
struct pipe_arg *pipe)
return >resources->crtcs[crtc_idx - 1];
}
+static int parse_mode_string(char *mode_string, drmModeModeInfo *user_mode)
+{
+ return sscanf(mode_string,
"%u,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu,%u,%s",
+ _mode->clock, _mode->hdisplay,
_mode->hsync_start,
+ _mode->hsync_end, _mode->htotal,
_mode->hskew,
+ _mode->vdisplay, _mode->vsync_start,
_mode->vsync_end,
+ _mode->vtotal, _mode->vscan,
_mode->vrefresh,
+ user_mode->name);
+}
+
static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe)
{
drmModeModeInfo *mode = NULL;
int i;
+ static drmModeModeInfo user_mode;
- pipe->mode = NULL;
-
- for (i = 0; i < (int)pipe->num_cons; i++) {
- mode = connector_find_mode(dev, pipe->con_ids[i],
- pipe->mode_str, pipe->vrefresh);
- if (mode == NULL) {
- if (pipe->vrefresh)
- fprintf(stderr,
- "failed to find mode "
- "\"%s-%.2fHz\" for connector %s\n",
- pipe->mode_str, pipe->vrefresh, pipe->cons[i]);
- else
- fprintf(stderr,
- "failed to find mode \"%s\" for connector %s\n",
- pipe->mode_str, pipe->cons[i]);
+ if (pipe->custom) {
+ if (parse_mode_string(pipe->mode_str, _mode)
[PATCH libdrm v2 0/2] Add Writeback Support for Modetest
Add writeback support to modetest with the below options: - Passing in -c will now also show the writeback connector - Test a built-in mode on writeback connector - Test a custom mode from user input on writeback connector Usage: "./modetest -M msm -x : -a -P @:+0+0@RG24." Refer to --help for exact syntax - Dump the writeback output buffer to bitstream Usage: "./modetest -M msm -s : -a -o -P @:+0+0@RG24" This currently supports a singular writeback connector. This patch also fixes a bug for running modetest with the atomic flag. Changes made in V2: - Added helper method that checks if user pipe has writeback connector - Added error message for dump flag if no writeback connector is found - Polls on the writeback fence fd until writeback is complete Rohith Iyer (2): tests/modetest: Allocate drmModeAtomicReq before setting properties tests/modetest: Add support for writeback connector tests/modetest/buffers.c | 19 + tests/modetest/buffers.h | 1 + tests/modetest/modetest.c | 165 +- 3 files changed, 165 insertions(+), 20 deletions(-) -- 2.17.1
Re: [PATCH 2/4] dt-bindings: display: sun6i-dsi: Add the A100 variant
Hi Krzysztof, On 8/12/22 5:49 AM, Krzysztof Kozlowski wrote: > On 12/08/2022 10:42, Samuel Holland wrote: >> The "40nm" MIPI DSI controller found in the A100 and D1 SoCs has the >> same register layout as previous SoC integrations. However, its module >> clock now comes from the TCON, which means it no longer runs at a fixed >> rate, so this needs to be distinguished in the driver. >> >> The controller also now uses pins on Port D instead of dedicated pins, >> so it drops the separate power domain. >> >> Signed-off-by: Samuel Holland >> --- >> Removal of the vcc-dsi-supply is maybe a bit questionable. Since there >> is no "VCC-DSI" pin anymore, it's not obvious which pin actually does >> power the DSI controller/PHY. Possibly power comes from VCC-PD or VCC-IO >> or VCC-LVDS. So far, all boards have all of these as always-on supplies, >> so it is hard to test. >> >> .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 28 +++ >> 1 file changed, 23 insertions(+), 5 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml >> >> b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml >> index ae55ef3fb1fe..c53c25b87bd4 100644 >> --- >> a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml >> +++ >> b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml >> @@ -12,9 +12,14 @@ maintainers: >> >> properties: >>compatible: >> -enum: >> - - allwinner,sun6i-a31-mipi-dsi >> - - allwinner,sun50i-a64-mipi-dsi >> +oneOf: >> + - enum: >> + - allwinner,sun6i-a31-mipi-dsi >> + - allwinner,sun50i-a64-mipi-dsi >> + - allwinner,sun50i-a100-mipi-dsi > > While you are moving code, how about bringing alphabetical order? I have put the sun*i prefix in numeric order, which matches (almost) all of our other bindings. It roughly corresponds to chronological order as well. It doesn't make much sense to me to sort sun50i (ARM64 SoCs) between sun5i and sun6i (early ARMv7 SoCs). Regards, Samuel
[PATCH] drm/virtio: Fix same-context optimization
From: Rob Clark
When VIRTGPU_EXECBUF_RING_IDX is used, we should be considering the
timeline that the EB if running on rather than the global driver fence
context.
Fixes: 85c83ea915ed ("drm/virtio: implement context init: allocate an array of
fence contexts")
Signed-off-by: Rob Clark
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index fa2f757583f7..9e6cb3c2666e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -168,7 +168,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
*dev, void *data,
* array contains any fence from a foreign context.
*/
ret = 0;
- if (!dma_fence_match_context(in_fence,
vgdev->fence_drv.context))
+ if (!dma_fence_match_context(in_fence, fence_ctx + ring_idx))
ret = dma_fence_wait(in_fence, true);
dma_fence_put(in_fence);
--
2.36.1
Re: Selecting CPUs for queuing work on
Hello, On Fri, Aug 12, 2022 at 04:54:04PM -0400, Felix Kuehling wrote: > In principle, I think IRQ routing to CPUs can change dynamically with > irqbalance. I wonder whether this is something which should be exposed to userland rather than trying to do dynamically in the kernel and let irqbalance or whatever deal with it. People use irq affinity to steer these handlings to specfic CPUs and the usual expectation is that the bottom half handling is gonna take place on the same cpu usually through softirq. It's kinda awkard to have this secondary assignment happening implicitly. > What we need is kind of the opposite of WQ_UNBOUND. As I understand it, > WQ_UNBOUND can schedule anywhere to maximize concurrency. What we need is to > schedule to very specific, predictable CPUs. We only have one work item per > GPU that processes all the interrupts in order, so we don't need the > concurrency of WQ_UNBOUND. Each WQ_UNBOUND workqueue has a cpumask associated with it and the cpumask can be changed dynamically, so it can be used for sth like this, but I'm not yet convinced that's the right thing to do. Thanks. -- tejun
[PATCH 3/5] drm/msm: Drop of_gpio header
These drivers include the deprecated OF GPIO header yet fail to use symbols from it, so drop the include. Cc: Rob Clark Cc: Abhinav Kumar Cc: Dmitry Baryshkov Cc: Sean Paul Signed-off-by: Maíra Canal --- drivers/gpu/drm/msm/dp/dp_parser.c | 2 +- drivers/gpu/drm/msm/hdmi/hdmi.c| 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c index f6ab3b5586ce..9a1bd92dcdc5 100644 --- a/drivers/gpu/drm/msm/dp/dp_parser.c +++ b/drivers/gpu/drm/msm/dp/dp_parser.c @@ -3,7 +3,7 @@ * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved. */ -#include +#include #include #include diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 93fe61b86967..f178c424257b 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -6,7 +6,6 @@ */ #include -#include #include #include -- 2.37.1
[PATCH 4/5] drm/sti: Drop of_gpio header
This driver includes the deprecated OF GPIO header yet fail to use symbols from it, so drop this include. Cc: Alain Volmat Signed-off-by: Maíra Canal --- drivers/gpu/drm/sti/sti_dvo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c index b6ee8a82e656..0fc7710b054a 100644 --- a/drivers/gpu/drm/sti/sti_dvo.c +++ b/drivers/gpu/drm/sti/sti_dvo.c @@ -8,7 +8,7 @@ #include #include #include -#include +#include #include #include -- 2.37.1
[PATCH 5/5] drm/vc4: Drop of_gpio header
This driver includes the deprecated OF GPIO header yet fail to use symbols from it, so drop the include. Cc: Emma Anholt Cc: Maxime Ripard Signed-off-by: Maíra Canal --- drivers/gpu/drm/vc4/vc4_hdmi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 6f61a1b8a1a3..84e5a91c2ea7 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -42,7 +42,6 @@ #include #include #include -#include #include #include #include -- 2.37.1
[PATCH 2/5] drm/mediatek: Drop of_gpio header
These drivers include the deprecated OF GPIO header yet fail to use symbols from it, so drop the include. Cc: Chun-Kuang Hu Cc: Philipp Zabel Cc: Matthias Brugger Cc: linux-media...@lists.infradead.org Signed-off-by: Maíra Canal --- drivers/gpu/drm/mediatek/mtk_dpi.c | 1 - drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 630a4e301ef6..508a6d994e83 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 3196189429bc..4c80b6896dc3 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include -- 2.37.1
[PATCH 1/5] drm/bridge: anx7625: Drop of_gpio header
This driver includes the deprecated OF GPIO header yet fail to use symbols from it, so drop the include. Cc: Andrzej Hajda Cc: Neil Armstrong Cc: Robert Foss Cc: Laurent Pinchart Cc: Jonas Karlman Cc: Jernej Skrabec Signed-off-by: Maíra Canal --- drivers/gpu/drm/bridge/analogix/anx7625.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 79fc7a50b497..d7d4ca1c8b30 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -17,7 +17,6 @@ #include #include -#include #include #include -- 2.37.1
[PATCH 0/5] Drop of_gpio header
The legacy GPIO header and the deprecated OF GPIO header should not be used anymore due to the existance of the new GPIO header . Currently, the DRM still holds seven OF GPIO header includes. That said, this series drops all the OF GPIO header includes from the DRM, replacing it, when proper, with the OF header. Best Regards, - Maíra Canal Cc: Andrzej Hajda Cc: Neil Armstrong Cc: Robert Foss Cc: Laurent Pinchart Cc: Jonas Karlman Cc: Jernej Skrabec Cc: Chun-Kuang Hu Cc: Philipp Zabel Cc: Matthias Brugger Cc: Rob Clark Cc: Abhinav Kumar Cc: Dmitry Baryshkov Cc: Sean Paul Cc: Alain Volmat Cc: Emma Anholt Cc: Maxime Ripard Cc: linux-media...@lists.infradead.org Maíra Canal (5): drm/bridge: anx7625: Drop of_gpio header drm/mediatek: Drop of_gpio header drm/msm: Drop of_gpio header drm/sti: Drop of_gpio header drm/vc4: Drop of_gpio header drivers/gpu/drm/bridge/analogix/anx7625.c | 1 - drivers/gpu/drm/mediatek/mtk_dpi.c| 1 - drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 - drivers/gpu/drm/msm/dp/dp_parser.c| 2 +- drivers/gpu/drm/msm/hdmi/hdmi.c | 1 - drivers/gpu/drm/sti/sti_dvo.c | 2 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 1 - 7 files changed, 2 insertions(+), 7 deletions(-) -- 2.37.1
Re: Selecting CPUs for queuing work on
On 2022-08-12 16:30, Tejun Heo wrote: On Fri, Aug 12, 2022 at 04:26:47PM -0400, Felix Kuehling wrote: Hi workqueue maintainers, In the KFD (amdgpu) driver we found a need to schedule bottom half interrupt handlers on CPU cores different from the one where the top-half interrupt handler runs to avoid the interrupt handler stalling the bottom half in extreme scenarios. See my latest patch that tries to use a different hyperthread on the same CPU core, or falls back to a different core in the same NUMA node if that fails: https://lore.kernel.org/all/20220811190433.1213179-1-felix.kuehl...@amd.com/ Dave pointed out that the driver may not be the best place to implement such logic and suggested that we should have an abstraction, maybe in the workqueue code. Do you feel this is something that could or should be provided by the core workqueue code? Or maybe some other place? I'm not necessarily against it. I guess it can be a flag on an unbound wq. Do the interrupts move across different CPUs tho? ie. why does this need to be a dynamic decision? In principle, I think IRQ routing to CPUs can change dynamically with irqbalance. If this were a flag, would there be a way to ensure all work queued to the same workqueue from the same CPU, or maybe all work associated with a work_struct always goes to the same CPU? One of the reasons for my latest patch was to get more predictable scheduling of the work to cores that are specifically reserved for interrupt handling by the system admin. This minimizes CPU scheduling noise that can compound to cause real performance issues in large scale distributed applications. What we need is kind of the opposite of WQ_UNBOUND. As I understand it, WQ_UNBOUND can schedule anywhere to maximize concurrency. What we need is to schedule to very specific, predictable CPUs. We only have one work item per GPU that processes all the interrupts in order, so we don't need the concurrency of WQ_UNBOUND. Regards, Felix Thanks.
Re: Selecting CPUs for queuing work on
On Fri, Aug 12, 2022 at 04:26:47PM -0400, Felix Kuehling wrote: > Hi workqueue maintainers, > > In the KFD (amdgpu) driver we found a need to schedule bottom half interrupt > handlers on CPU cores different from the one where the top-half interrupt > handler runs to avoid the interrupt handler stalling the bottom half in > extreme scenarios. See my latest patch that tries to use a different > hyperthread on the same CPU core, or falls back to a different core in the > same NUMA node if that fails: > https://lore.kernel.org/all/20220811190433.1213179-1-felix.kuehl...@amd.com/ > > Dave pointed out that the driver may not be the best place to implement such > logic and suggested that we should have an abstraction, maybe in the > workqueue code. Do you feel this is something that could or should be > provided by the core workqueue code? Or maybe some other place? I'm not necessarily against it. I guess it can be a flag on an unbound wq. Do the interrupts move across different CPUs tho? ie. why does this need to be a dynamic decision? Thanks. -- tejun
Selecting CPUs for queuing work on
Hi workqueue maintainers, In the KFD (amdgpu) driver we found a need to schedule bottom half interrupt handlers on CPU cores different from the one where the top-half interrupt handler runs to avoid the interrupt handler stalling the bottom half in extreme scenarios. See my latest patch that tries to use a different hyperthread on the same CPU core, or falls back to a different core in the same NUMA node if that fails: https://lore.kernel.org/all/20220811190433.1213179-1-felix.kuehl...@amd.com/ Dave pointed out that the driver may not be the best place to implement such logic and suggested that we should have an abstraction, maybe in the workqueue code. Do you feel this is something that could or should be provided by the core workqueue code? Or maybe some other place? Thank you, Felix -- F e l i x K u e h l i n g PMTS Software Development Engineer | Linux Compute Kernel 1 Commerce Valley Dr. East, Markham, ON L3T 7X6 Canada (O) +1(289)695-1597 _ _ _ _ _ / \ | \ / | | _ \ \ _ | / A \ | \M/ | | |D) ) /|_| | /_/ \_\ |_| |_| |_/ |__/ \| facebook.com/AMD | amd.com
Re: [PATCH] drm/amdgpu: use native mode for dp aux transfer
Hi Zhenneng, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on linus/master v5.19 next-20220812] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Zhenneng-Li/drm-amdgpu-use-native-mode-for-dp-aux-transfer/20220811-193443 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next config: s390-randconfig-r034-20220812 (https://download.01.org/0day-ci/archive/20220813/202208130320.ndvnbevl-...@intel.com/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install s390 cross compiling tool for clang build # apt-get install binutils-s390x-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/1098c6fecb4292d634dbdccff9e720400dc7138d git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Zhenneng-Li/drm-amdgpu-use-native-mode-for-dp-aux-transfer/20220811-193443 git checkout 1098c6fecb4292d634dbdccff9e720400dc7138d # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/gpu/drm/amd/amdgpu/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_dp_auxch.c:25: In file included from drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:52: In file included from include/linux/pci.h:39: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __raw_readb(PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x)) ^ include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16' #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x)) ^ In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_dp_auxch.c:25: In file included from drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:52: In file included from include/linux/pci.h:39: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) ^ include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32' #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) ^ In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_dp_auxch.c:25: In file included from drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:52: In file included from include/linux/pci.h:39: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writeb(value, PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:521:59: warning: perf
Re: [PATCH AUTOSEL 5.10 01/46] drm/r128: Fix undefined behavior due to shift overflowing the constant
Hi! In this series, I only received patches up-to 42/46. Is problem at sender or receiver? Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH 4/4] drm/format-helper: Add drm_fb_build_fourcc_list() helper
Hi Thomas,
On Wed, Aug 10, 2022 at 01:20:53PM +0200, Thomas Zimmermann wrote:
> Add drm_fb_build_fourcc_list() function that builds a list of supported
> formats from native and emulated ones. Helpful for all drivers that do
> format conversion as part of their plane updates. Update current caller.
>
> Signed-off-by: Thomas Zimmermann
A few comments in the following. Consider to add the warning and with it
added or not:
Reviewed-by: Sam Ravnborg
> ---
> drivers/gpu/drm/drm_format_helper.c | 94 +
> drivers/gpu/drm/tiny/simpledrm.c| 47 ++-
> include/drm/drm_format_helper.h | 11 +++-
> 3 files changed, 109 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_format_helper.c
> b/drivers/gpu/drm/drm_format_helper.c
> index 56642816fdff..dca5531615f3 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -793,3 +793,97 @@ void drm_fb_xrgb_to_mono(struct iosys_map *dst,
> const unsigned int *dst_pitc
> kfree(src32);
> }
> EXPORT_SYMBOL(drm_fb_xrgb_to_mono);
> +
> +static bool is_listed_fourcc(const uint32_t *fourccs, size_t nfourccs,
> uint32_t fourcc)
> +{
> + const uint32_t *fourccs_end = fourccs + nfourccs;
> +
> + while (fourccs < fourccs_end) {
> + if (*fourccs == fourcc)
> + return true;
> + ++fourccs;
> + }
> + return false;
> +}
> +
> +/**
> + * drm_fb_build_fourcc_list - Filters a list of supported color formats
> against
> + *the device's native formats
> + * @dev: DRM device
> + * @native_fourccs: 4CC codes of natively supported color formats
> + * @native_nfourccs: The number of entries in @native_fourccs
> + * @extra_fourccs: 4CC codes of additionally supported color formats
> + * @extra_nfourccs: The number of entries in @extra_fourccs
> + * @fourccs_out: Returns 4CC codes of supported color formats
> + * @nfourccs_out: The number of available entries in @fourccs_out
> + *
> + * This function create a list of supported color format from natively
> + * supported formats and the emulated formats. *
Stray '*' at the end of the line.
> + * At a minimum, most userspace programs expect at least support for
> + * XRGB on the primary plane. Devices that have to emulate the
> + * format, and possibly others, can use drm_fb_build_fourcc_list() to
> + * create a list of supported color formats. The returned list can
> + * be handed over to drm_universal_plane_init() et al. Native formats
> + * will go before emulated formats. Other heuristics might be applied
> + * to optimize the order. Formats near the beginning of the list are
> + * usually preferred over formats near the end of the list.
> + *
> + * Returns:
> + * The number of color-formats 4CC codes returned in @fourccs_out.
> + */
> +size_t drm_fb_build_fourcc_list(struct drm_device *dev,
> + const uint32_t *native_fourccs, size_t
> native_nfourccs,
> + const uint32_t *extra_fourccs, size_t
> extra_nfourccs,
> + uint32_t *fourccs_out, size_t nfourccs_out)
drm_fourcc.c uses the type u32 for fourcc codes, why no go with the same
here?
I wish we had a better way to express that we have a list of fourcc
codes, for example using list_head. But it is a bad mismatch with
the current drm_universal_plane_init() implementation.
The format negotiation operation in the bridges could benefit too.
> +{
> + uint32_t *fourccs = fourccs_out;
> + const uint32_t *fourccs_end = fourccs_out + nfourccs_out;
> + bool found_native = false;
> + size_t nfourccs, i;
> +
> + /* native formats go first */
> +
Drop extra line, capital start
> + nfourccs = min_t(size_t, native_nfourccs, nfourccs_out);
> +
> + for (i = 0; i < nfourccs; ++i) {
> + uint32_t fourcc = native_fourccs[i];
> +
> + drm_dbg_kms(dev, "adding native format %p4cc\n", );
> +
> + if (!found_native)
> + found_native = is_listed_fourcc(extra_fourccs,
> extra_nfourccs, fourcc);
> + *fourccs = fourcc;
> + ++fourccs;
> + }
> +
> + /*
> + * The plane's atomic_update helper converts the framebuffer's color
> format
> + * to the native format when copying them to device memory.
> + *
> + * If there is not a single format supported by both, device and
> + * plane, the native formats are likely not supported by the conversion
> + * helpers. Therefore *only* support the native formats and add a
> + * conversion helper ASAP.
> + */
Despite the nice comment I had to think twice. In the end I agree with
this.
> + if (!found_native) {
> + drm_warn(dev, "format conversion helpers required to add extra
> formats\n");
> + goto out;
> + }
> +
> + /* extra formats go second */
> +
Drop extra line, capital start.
> +
[PATCH] drm/amd/amdgpu: Replace kmap() with kmap_local_page()
kmap() is being deprecated in favor of kmap_local_page(). There are two main problems with kmap(): (1) It comes with an overhead as mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap’s pool wraps and it might block when the mapping space is fully utilized until a slot becomes available. With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored and are still valid. Since its use in amdgpu/amdgpu_ttm.c is safe, it should be preferred. Therefore, replace kmap() with kmap_local_page() in amdgpu/amdgpu_ttm.c. Suggested-by: Ira Weiny Signed-off-by: Fabio M. De Francesco --- 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 3b4c19412625..c11657b5915f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -2301,9 +2301,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; @@ -2352,9 +2352,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.37.1
Re: [PATCH] drm/amd/display: remove unreachable code
Hi, On 12/08/2022 00:19, Jiapeng Chong wrote: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_util_32.c:1658 dml32_TruncToValidBPP() warn: ignoring unreachable code. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=1894 Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- .../drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c index 05fc14a47fba..0758e1da55a9 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c @@ -1654,10 +1654,6 @@ double dml32_TruncToValidBPP( else return DesiredBPP; } - - *RequiredSlots = dml_ceil(DesiredBPP / MaxLinkBPP * 64, 1); - - return BPP_INVALID; } // TruncToValidBPP double dml32_RequiredDTBCLK( Seems correct. Reviewed-by: Tales Aparecida I feel like RequiredSlots is not actually used anywhere in the code, just passed around dml32_TruncToValidBPP() and dml32_CalculateOutputLink(). I've looked for any mentions of it in the mailing list, but could not find anything that implied it's part of ground working. I wonder if it's something outside the Linux tree for other platforms or related to HW gospel.
Re: [PATCH 2/4] drm/probe-helper: Add drm_crtc_helper_mode_valid_static()
On Fri, Aug 12, 2022 at 06:37:17PM +0200, Thomas Zimmermann wrote: > Hi Sam > > Am 10.08.22 um 21:26 schrieb Sam Ravnborg: > > Hi Thomas, > > > > On Wed, Aug 10, 2022 at 01:20:51PM +0200, Thomas Zimmermann wrote: > > > Add drm_crtc_helper_mode_valid_static(), which validates a given mode > > > against a display hardware's mode. Convert simpledrm and use it in a > > > few other drivers with static modes. > > > > > > Signed-off-by: Thomas Zimmermann > > > > With the header file fixed, > > The include statement is required for enum drm_mode_status. Obviously - missed that. Sam
Re: [PATCH] spi/panel: dt-bindings: drop 3-wire from common properties
On Wed, 10 Aug 2022 16:13:11 +0300, Krzysztof Kozlowski wrote: > The spi-3wire property is device specific and should be accepted only if > device really needs them. Drop it from common spi-peripheral-props.yaml > schema, mention in few panel drivers which use it and include instead in > the SPI controller bindings. The controller bindings will provide > spi-3wire type validation and one place for description. Each device > schema must list the property if it is applicable. > > The Samsung S6E63M0 panel uses also spi-cpha/cpol properties on at least > one board (ste-ux500-samsung-janice/dts), so add also these to the > panel's bindings. > > Signed-off-by: Krzysztof Kozlowski > --- > .../bindings/display/panel/kingdisplay,kd035g6-54nt.yaml | 2 ++ > .../bindings/display/panel/leadtek,ltk035c5444t.yaml | 2 ++ > .../devicetree/bindings/display/panel/samsung,s6e63m0.yaml | 4 > Documentation/devicetree/bindings/spi/spi-controller.yaml| 5 + > .../devicetree/bindings/spi/spi-peripheral-props.yaml| 5 - > 5 files changed, 13 insertions(+), 5 deletions(-) > Reviewed-by: Rob Herring
Re: [EXT] Re: [PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support
Hi,
On Mon, Aug 08, 2022 at 02:39:53PM +, Olivier Masse wrote:
> Hi Brian,
>
> On ven., 2022-08-05 at 16:41 +0100, Brian Starkey wrote:
> > Caution: EXT Email
> >
> > Hi Olivier,
> >
> > Thanks, I think this is looking much better.
> >
> > I'd like to know how others feel about landing this heap; there's
> > been
> > push-back in the past about heaps in device-tree and discussions
> > around how "custom" heaps should be treated (though IMO this is quite
> > a generic one).
> >
> > On Fri, Aug 05, 2022 at 03:53:28PM +0200, Olivier Masse wrote:
> > > add Linaro secure heap bindings: linaro,secure-heap
> > > use genalloc to allocate/free buffer from buffer pool.
> > > buffer pool info is from dts.
> > > use sg_table instore the allocated memory info, the length of
> > > sg_table is 1.
> > > implement secure_heap_buf_ops to implement buffer share in
> > > difference device:
> > > 1. Userspace passes this fd to all drivers it wants this buffer
> > > to share with: First the filedescriptor is converted to a _buf
> > > using
> > > dma_buf_get(). Then the buffer is attached to the device using
> > > dma_buf_attach().
> > > 2. Once the buffer is attached to all devices userspace can
> > > initiate DMA
> > > access to the shared buffer. In the kernel this is done by calling
> > > dma_buf_map_attachment()
> > > 3. get sg_table with dma_buf_map_attachment in difference device.
> > >
> >
> > I think this commit message could use a little rework. A few
> > thoughts:
> >
> > * The bindings are in a separate commit, so seems strange to mention
> > here.
>
> what about:
> "add Linaro secure heap compatible reserved memory: linaro,secure-heap"
>
I'd say something like:
Add a dma-buf heap to allocate secure buffers from a reserved-memory
region.
..snip
> > > +
> > > +static struct sg_table *secure_heap_map_dma_buf(struct
> > > dma_buf_attachment *attachment,
> > > + enum
> > > dma_data_direction direction)
> > > +{
> > > + struct secure_heap_attachment *a = attachment->priv;
> > > +
> > > + return a->table;
> >
> > I think you still need to implement mapping and unmapping using the
> > DMA APIs. For example devices might be behind IOMMUs and the buffer
> > will need mapping into the IOMMU.
>
> Devices that will need access to the buffer must be in secure.
> The tee driver will only need the scatter-list table to get dma address
> and len. Mapping will be done in the TEE.
> Please find tee_shm_register_fd in the following commit
> https://github.com/linaro-swg/linux/commit/41e21e5c405530590dc2dd10b2a8dbe64589840f
>
> This patch need to be up-streamed as well.
>
Interesting, that's not how the devices I've worked on operated.
Are you saying that you have to have a display controller driver
running in the TEE to display one of these buffers? If everything
needs to be in the TEE, then why even have these buffers allocated
by non-secure Linux at all?
I would have expected there to be HW enforcement of buffer access,
but for the display driver to be in non-secure Linux. That's how
TZMP1 works: https://static.linaro.org/connect/hkg18/presentations/hkg18-408.pdf
Looking at this SDP presentation, that also seems to be the case
there: https://static.linaro.org/connect/san19/presentations/san19-107.pdf
Based on those presentations, I think this heap should be implementing
map_dma_buf in the "normal" way, using the DMA API to map the buffer
to the device. It's up to the TEE and HW firewall to prevent access
to those mappings from non-secure devices.
My understanding is:
* The memory region should never be mapped or accessed from the host
CPU. This is not a security requirement - the CPU will be denied
access by whatever hardware is enforcing security - but any CPU
accesses will fail, so there is no point in ever having a CPU
mapping.
* The allocated buffers _should_ be mapped to devices via map_dma_buf.
Again the HW enforcement will prevent access from devices which
aren't permitted access, but for example a display controller
may be allowed to read the secure buffer, composite it with other
buffers, and display it on the screen.
Am I wrong? Even if SDP doesn't work this way, I think we should make
the heap as generic as possible so that it can work with different
secure video implementations.
>
> >
.. snip
> > > +
> > > +RESERVEDMEM_OF_DECLARE(secure_heap, "linaro,secure-heap",
> > > rmem_secure_heap_setup);
> >
> > Is there anything linaro-specific about this? Could it be
> > linux,secure-heap?
>
> for now, it's specific to Linaro OPTEE OS.
> but in a more generic way, it could be
> linux,unmapped-heap ?
If these buffers can never be mapped, not to the CPU nor to devices,
then actually I don't see why it should be a dma-buf heap at all.
If this is just an interface to associate some identifier (in this
case an fd) with a region of physical address space, then why is it
useful to pretend that it's a dma-buf, if none of the
Re: [PATCH 2/4] drm/probe-helper: Add drm_crtc_helper_mode_valid_static()
Hi Sam
Am 10.08.22 um 21:26 schrieb Sam Ravnborg:
Hi Thomas,
On Wed, Aug 10, 2022 at 01:20:51PM +0200, Thomas Zimmermann wrote:
Add drm_crtc_helper_mode_valid_static(), which validates a given mode
against a display hardware's mode. Convert simpledrm and use it in a
few other drivers with static modes.
Signed-off-by: Thomas Zimmermann
With the header file fixed,
The include statement is required for enum drm_mode_status.
Best regards
Thomas
Reviewed-by: Sam Ravnborg
---
drivers/gpu/drm/drm_mipi_dbi.c | 18 ++
drivers/gpu/drm/drm_probe_helper.c | 25
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 1 +
drivers/gpu/drm/tiny/hx8357d.c | 1 +
drivers/gpu/drm/tiny/ili9163.c | 1 +
drivers/gpu/drm/tiny/ili9341.c | 1 +
drivers/gpu/drm/tiny/ili9486.c | 1 +
drivers/gpu/drm/tiny/mi0283qt.c | 1 +
drivers/gpu/drm/tiny/panel-mipi-dbi.c| 1 +
drivers/gpu/drm/tiny/repaper.c | 10
drivers/gpu/drm/tiny/simpledrm.c | 10 +---
drivers/gpu/drm/tiny/st7735r.c | 1 +
include/drm/drm_mipi_dbi.h | 2 ++
include/drm/drm_probe_helper.h | 8 +--
14 files changed, 70 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index b67ec9a5cda9..d544a99df9df 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -309,6 +309,24 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
struct drm_rect *rect)
drm_dev_exit(idx);
}
+/**
+ * mipi_dbi_pipe_mode_valid - MIPI DBI mode-valid helper
+ * @pipe: Simple display pipe
+ * @mode: The mode to test
+ *
+ * This function validates a given display mode against the MIPI DBI's hardware
+ * display. Drivers can use this as their
_simple_display_pipe_funcs->mode_valid
+ * callback.
+ */
+enum drm_mode_status mipi_dbi_pipe_mode_valid(struct drm_simple_display_pipe
*pipe,
+ const struct drm_display_mode
*mode)
+{
+ struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+
+ return drm_crtc_helper_mode_valid_static(>crtc, mode,
>mode);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_mode_valid);
+
/**
* mipi_dbi_pipe_update - Display pipe update helper
* @pipe: Simple display pipe
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 809187377e4e..bc3876853fca 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -1014,6 +1014,31 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_helper_hpd_irq_event);
+/**
+ * drm_crtc_helper_mode_valid_static - Validates a display mode
+ * @crtc: the crtc
+ * @mode: the mode to validate
+ * @hw_mode: the display hardware's mode
+ *
+ * Returns:
+ * MODE_OK on success, or another mode-status code otherwise.
+ */
+enum drm_mode_status drm_crtc_helper_mode_valid_static(struct drm_crtc *crtc,
+ const struct
drm_display_mode *mode,
+ const struct
drm_display_mode *hw_mode)
+{
+
+ if (mode->hdisplay != hw_mode->hdisplay && mode->vdisplay !=
hw_mode->vdisplay)
+ return MODE_ONE_SIZE;
+ else if (mode->hdisplay != hw_mode->hdisplay)
+ return MODE_ONE_WIDTH;
+ else if (mode->vdisplay != hw_mode->vdisplay)
+ return MODE_ONE_HEIGHT;
+
+ return MODE_OK;
+}
+EXPORT_SYMBOL(drm_crtc_helper_mode_valid_static);
+
/**
* drm_connector_helper_get_modes_from_ddc - Updates the connector's EDID
* property from the connector's
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 7da09e34385d..39dc40cf681f 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -576,6 +576,7 @@ static void ili9341_dbi_enable(struct
drm_simple_display_pipe *pipe,
}
static const struct drm_simple_display_pipe_funcs ili9341_dbi_funcs = {
+ .mode_valid = mipi_dbi_pipe_mode_valid,
.enable = ili9341_dbi_enable,
.disable = mipi_dbi_pipe_disable,
.update = mipi_dbi_pipe_update,
diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
index 57f229a785bf..48c24aa8c28a 100644
--- a/drivers/gpu/drm/tiny/hx8357d.c
+++ b/drivers/gpu/drm/tiny/hx8357d.c
@@ -181,6 +181,7 @@ static void yx240qv29_enable(struct drm_simple_display_pipe
*pipe,
}
static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
+ .mode_valid = mipi_dbi_pipe_mode_valid,
.enable = yx240qv29_enable,
.disable = mipi_dbi_pipe_disable,
.update = mipi_dbi_pipe_update,
diff --git
Re: [git pull] drm fixes for 6.0-rc1
The pull request you sent on Fri, 12 Aug 2022 06:09:44 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-next-2022-08-12-1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/7ce2aa6d7fe121e243e1c8a8093911fecdf1c88e Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [Intel-gfx] [PATCH] drm/i915/guc: clear stalled request after a reset
On 8/12/2022 12:29 AM, Tvrtko Ursulin wrote:
On 11/08/2022 22:08, Daniele Ceraolo Spurio wrote:
If the GuC CTs are full and we need to stall the request submission
while waiting for space, we save the stalled request and where the stall
occurred; when the CTs have space again we pick up the request
submission
from where we left off.
How serious is it? Statement always was CT buffers can never get full
outside the pathological IGT test cases. So I am wondering if this is
in the category of fix for correctness or actually the CT buffers can
get full in normal use so it is imperative to fix.
The CT buffers being full is indeed something that is normally only
observed with IGTs that hammer the submission path, but it is still
something that a user can do so IMO we do have to fix it. However, the
bug is still extremely unlikely to happen out in the wild as it needs 2
relatively rare things to happen:
- We need to hit the pathological case of the GuC CTs being full and the
stall kicking in
- Something needs to go wrong and escalated to a full GT reset
The bug report that triggered my investigation into this came from what
look like faulty HW: the HW seems to suddenly just stop with no errors
anywhere, which leads to the buffers filling up because the GuC is no
longer processing them, followed by a GT reset as we try to recover the
HW. To replicate this locally I had to add a debugfs to kill the GuC in
the middle of the test to simulate this "HW silently dies" scenario.
Daniele
Regards,
Tvrtko
If a full GT reset occurs, the state of all contexts is cleared and all
non-guilty requests are unsubmitted, therefore we need to restart the
stalled request submission from scratch. To make sure that we do so,
clear the saved request after a reset.
Fixes note: the patch that introduced the bug is in 5.15, but no
officially supported platform had GuC submission enabled by default
in that kernel, so the backport to that particular version (and only
that one) can potentially be skipped.
Fixes: 925dc1cf58ed ("drm/i915/guc: Implement GuC submission tasklet")
Signed-off-by: Daniele Ceraolo Spurio
Cc: Matthew Brost
Cc: John Harrison
Cc: # v5.15+
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 0d17da77e787..0d56b615bf78 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -4002,6 +4002,13 @@ static inline void guc_init_lrc_mapping(struct
intel_guc *guc)
/* make sure all descriptors are clean... */
xa_destroy(>context_lookup);
+ /*
+ * A reset might have occurred while we had a pending stalled
request,
+ * so make sure we clean that up.
+ */
+ guc->stalled_request = NULL;
+ guc->submission_stall_reason = STALL_NONE;
+
/*
* Some contexts might have been pinned before we enabled GuC
* submission, so we need to add them to the GuC bookeeping.
Re: [PATCH 1/3] dt-bindings: display/msm: Add binding for SC8280XP MDSS
On Wed, 10 Aug 2022 21:01:19 -0700, Bjorn Andersson wrote: > Add binding for the display subsystem and display processing unit in the > Qualcomm SC8280XP platform. > > Signed-off-by: Bjorn Andersson > --- > .../bindings/display/msm/dpu-sc8280xp.yaml| 284 ++ > 1 file changed, 284 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-sc8280xp.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/display/msm/dpu-sc8280xp.example.dts:21:18: fatal error: dt-bindings/clock/qcom,dispcc-sc8280xp.h: No such file or directory 21 | #include | ^~ compilation terminated. make[1]: *** [scripts/Makefile.lib:383: Documentation/devicetree/bindings/display/msm/dpu-sc8280xp.example.dtb] Error 1 make[1]: *** Waiting for unfinished jobs make: *** [Makefile:1404: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
Re: [PATCH v8 2/2] drm/gem: Don't map imported GEMs
On Fri, Aug 12, 2022 at 7:57 AM Rob Clark wrote:
>
> On Fri, Aug 12, 2022 at 4:26 AM Dmitry Osipenko
> wrote:
> >
> > On 8/11/22 02:19, Rob Clark wrote:
> > > On Wed, Aug 10, 2022 at 3:23 PM Dmitry Osipenko
> > > wrote:
> > >>
> > >> On 8/11/22 01:03, Rob Clark wrote:
> > >>> On Wed, Aug 10, 2022 at 12:26 PM Dmitry Osipenko
> > >>> wrote:
> >
> > On 8/10/22 18:08, Rob Clark wrote:
> > > On Wed, Aug 10, 2022 at 4:47 AM Daniel Vetter wrote:
> > >>
> > >> On Wed, Jul 06, 2022 at 10:02:07AM +0300, Dmitry Osipenko wrote:
> > >>> On 7/6/22 00:48, Rob Clark wrote:
> > On Tue, Jul 5, 2022 at 4:51 AM Christian König
> > wrote:
> > >
> > > Am 01.07.22 um 11:02 schrieb Dmitry Osipenko:
> > >> Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers
> > >> don't
> > >> handle imported dma-bufs properly, which results in mapping of
> > >> something
> > >> else than the imported dma-buf. On NVIDIA Tegra we get a hard
> > >> lockup when
> > >> userspace writes to the memory mapping of a dma-buf that was
> > >> imported into
> > >> Tegra's DRM GEM.
> > >>
> > >> Majority of DRM drivers prohibit mapping of the imported GEM
> > >> objects.
> > >> Mapping of imported GEMs require special care from userspace
> > >> since it
> > >> should sync dma-buf because mapping coherency of the exporter
> > >> device may
> > >> not match the DRM device. Let's prohibit the mapping for all DRM
> > >> drivers
> > >> for consistency.
> > >>
> > >> Suggested-by: Thomas Hellström
> > >> Signed-off-by: Dmitry Osipenko
> > >
> > > I'm pretty sure that this is the right approach, but it's
> > > certainly more
> > > than possible that somebody abused this already.
> >
> > I suspect that this is abused if you run deqp cts on android.. ie.
> > all
> > winsys buffers are dma-buf imports from gralloc. And then when you
> > hit readpix...
> >
> > You might only hit this in scenarios with separate gpu and display
> > (or
> > dGPU+iGPU) because self-imports are handled differently in
> > drm_gem_prime_import_dev().. and maybe not in cases where you end
> > up
> > with a blit from tiled/compressed to linear.. maybe that narrows
> > the
> > scope enough to just fix it in userspace?
> > >>>
> > >>> Given that that only drivers which use DRM-SHMEM potentially
> > >>> could've
> > >>> map imported dma-bufs (Panfrost, Lima) and they already don't allow
> > >>> to
> > >>> do that, I think we're good.
> > >>
> > >> So can I have an ack from Rob here or are there still questions that
> > >> this
> > >> might go boom?
> > >>
> > >> Dmitry, since you have a bunch of patches merged now I think would
> > >> also be
> > >> good to get commit rights so you can drive this more yourself. I've
> > >> asked
> > >> Daniel Stone to help you out with getting that.
> > >
> > > I *think* we'd be ok with this on msm, mostly just by dumb luck.
> > > Because the dma-buf's we import will be self-import. I'm less sure
> > > about panfrost (src/panfrost/lib/pan_bo.c doesn't seem to have a
> > > special path for imported dma-bufs either, and in that case they won't
> > > be self-imports.. but I guess no one has tried to run android cts on
> > > panfrost).
> >
> > The last time I tried to mmap dma-buf imported to Panfrost didn't work
> > because Panfrost didn't implement something needed for that. I'll need
> > to take a look again because can't recall what it was.
> > Upd: I re-checked Panfrost using today's linux-next and mapping of
> > imported dma-buf works, I mapped imported buf from video decoder.
> > Apparently previously I had some local kernel change that broke the mapping.
> >
> > > What about something less drastic to start, like (apologies for
> > > hand-edited patch):
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > index 86d670c71286..fc9ec42fa0ab 100644
> > > --- a/drivers/gpu/drm/drm_gem.c
> > > +++ b/drivers/gpu/drm/drm_gem.c
> > > @@ -1034,6 +1034,10 @@ int drm_gem_mmap_obj(struct drm_gem_object
> > > *obj, unsigned long obj_size,
> > > {
> > > int ret;
> > >
> > > + WARN_ON_ONCE(obj->import_attach);
> >
> > This will hang NVIDIA Tegra, which is what this patch fixed initially.
> > If neither of upstream DRM drivers need to map imported dma-bufs and
> > never needed, then why do we need this?
> > >>>
> > >>> oh, tegra isn't using shmem helpers? I assumed it was. Well my point
> > >>> was to make a more targeted
Re: [PATCH v8 2/2] drm/gem: Don't map imported GEMs
On Fri, Aug 12, 2022 at 4:26 AM Dmitry Osipenko
wrote:
>
> On 8/11/22 02:19, Rob Clark wrote:
> > On Wed, Aug 10, 2022 at 3:23 PM Dmitry Osipenko
> > wrote:
> >>
> >> On 8/11/22 01:03, Rob Clark wrote:
> >>> On Wed, Aug 10, 2022 at 12:26 PM Dmitry Osipenko
> >>> wrote:
>
> On 8/10/22 18:08, Rob Clark wrote:
> > On Wed, Aug 10, 2022 at 4:47 AM Daniel Vetter wrote:
> >>
> >> On Wed, Jul 06, 2022 at 10:02:07AM +0300, Dmitry Osipenko wrote:
> >>> On 7/6/22 00:48, Rob Clark wrote:
> On Tue, Jul 5, 2022 at 4:51 AM Christian König
> wrote:
> >
> > Am 01.07.22 um 11:02 schrieb Dmitry Osipenko:
> >> Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers
> >> don't
> >> handle imported dma-bufs properly, which results in mapping of
> >> something
> >> else than the imported dma-buf. On NVIDIA Tegra we get a hard
> >> lockup when
> >> userspace writes to the memory mapping of a dma-buf that was
> >> imported into
> >> Tegra's DRM GEM.
> >>
> >> Majority of DRM drivers prohibit mapping of the imported GEM
> >> objects.
> >> Mapping of imported GEMs require special care from userspace since
> >> it
> >> should sync dma-buf because mapping coherency of the exporter
> >> device may
> >> not match the DRM device. Let's prohibit the mapping for all DRM
> >> drivers
> >> for consistency.
> >>
> >> Suggested-by: Thomas Hellström
> >> Signed-off-by: Dmitry Osipenko
> >
> > I'm pretty sure that this is the right approach, but it's certainly
> > more
> > than possible that somebody abused this already.
>
> I suspect that this is abused if you run deqp cts on android.. ie.
> all
> winsys buffers are dma-buf imports from gralloc. And then when you
> hit readpix...
>
> You might only hit this in scenarios with separate gpu and display
> (or
> dGPU+iGPU) because self-imports are handled differently in
> drm_gem_prime_import_dev().. and maybe not in cases where you end up
> with a blit from tiled/compressed to linear.. maybe that narrows the
> scope enough to just fix it in userspace?
> >>>
> >>> Given that that only drivers which use DRM-SHMEM potentially could've
> >>> map imported dma-bufs (Panfrost, Lima) and they already don't allow to
> >>> do that, I think we're good.
> >>
> >> So can I have an ack from Rob here or are there still questions that
> >> this
> >> might go boom?
> >>
> >> Dmitry, since you have a bunch of patches merged now I think would
> >> also be
> >> good to get commit rights so you can drive this more yourself. I've
> >> asked
> >> Daniel Stone to help you out with getting that.
> >
> > I *think* we'd be ok with this on msm, mostly just by dumb luck.
> > Because the dma-buf's we import will be self-import. I'm less sure
> > about panfrost (src/panfrost/lib/pan_bo.c doesn't seem to have a
> > special path for imported dma-bufs either, and in that case they won't
> > be self-imports.. but I guess no one has tried to run android cts on
> > panfrost).
>
> The last time I tried to mmap dma-buf imported to Panfrost didn't work
> because Panfrost didn't implement something needed for that. I'll need
> to take a look again because can't recall what it was.
> Upd: I re-checked Panfrost using today's linux-next and mapping of
> imported dma-buf works, I mapped imported buf from video decoder.
> Apparently previously I had some local kernel change that broke the mapping.
>
> > What about something less drastic to start, like (apologies for
> > hand-edited patch):
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index 86d670c71286..fc9ec42fa0ab 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -1034,6 +1034,10 @@ int drm_gem_mmap_obj(struct drm_gem_object
> > *obj, unsigned long obj_size,
> > {
> > int ret;
> >
> > + WARN_ON_ONCE(obj->import_attach);
>
> This will hang NVIDIA Tegra, which is what this patch fixed initially.
> If neither of upstream DRM drivers need to map imported dma-bufs and
> never needed, then why do we need this?
> >>>
> >>> oh, tegra isn't using shmem helpers? I assumed it was. Well my point
> >>> was to make a more targeted fail on tegra, and a WARN_ON for everyone
> >>> else to make it clear that what they are doing is undefined behavior.
> >>> Because so far existing userspace (or well, panfrost and freedreno at
> >>> least, those are the two I know or checked) don't make special cases
> >>> for mmap'ing against the dmabuf fd against the
[RFC 4/4] phy/rockchip: inno-dsidphy: Add support for rk3568
From: Chris Morgan
Add support for the Rockchip RK3568 DSI-DPHY. Registers were taken from
the BSP kernel driver and wherever possible cross referenced with the
TRM.
Signed-off-by: Chris Morgan
---
.../phy/rockchip/phy-rockchip-inno-dsidphy.c | 204 ++
1 file changed, 158 insertions(+), 46 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
index 630e01b5c19b..2c5847faff63 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
@@ -84,9 +84,25 @@
#define DATA_LANE_0_SKEW_PHASE_MASKGENMASK(2, 0)
#define DATA_LANE_0_SKEW_PHASE(x) UPDATE(x, 2, 0)
/* Analog Register Part: reg08 */
+#define PLL_POST_DIV_ENABLE_MASK BIT(5)
+#define PLL_POST_DIV_ENABLEBIT(5)
#define SAMPLE_CLOCK_DIRECTION_MASKBIT(4)
#define SAMPLE_CLOCK_DIRECTION_REVERSE BIT(4)
#define SAMPLE_CLOCK_DIRECTION_FORWARD 0
+#define LOWFRE_EN_MASK BIT(5)
+#define PLL_OUTPUT_FREQUENCY_DIV_BY_1 0
+#define PLL_OUTPUT_FREQUENCY_DIV_BY_2 1
+/* Analog Register Part: reg0b */
+#define CLOCK_LANE_VOD_RANGE_SET_MASK GENMASK(3, 0)
+#define CLOCK_LANE_VOD_RANGE_SET(x)UPDATE(x, 3, 0)
+#define VOD_MIN_RANGE 0x1
+#define VOD_MID_RANGE 0x3
+#define VOD_BIG_RANGE 0x7
+#define VOD_MAX_RANGE 0xf
+/* Analog Register Part: reg1E */
+#define PLL_MODE_SEL_MASK GENMASK(6, 5)
+#define PLL_MODE_SEL_LVDS_MODE 0
+#define PLL_MODE_SEL_MIPI_MODE BIT(5)
/* Digital Register Part: reg00 */
#define REG_DIG_RSTN_MASK BIT(0)
#define REG_DIG_RSTN_NORMALBIT(0)
@@ -102,20 +118,22 @@
#define T_LPX_CNT_MASK GENMASK(5, 0)
#define T_LPX_CNT(x) UPDATE(x, 5, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg06 */
+#define T_HS_ZERO_CNT_HI_MASK BIT(7)
+#define T_HS_ZERO_CNT_HI(x)UPDATE(x, 7, 7)
#define T_HS_PREPARE_CNT_MASK GENMASK(6, 0)
#define T_HS_PREPARE_CNT(x)UPDATE(x, 6, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg07 */
-#define T_HS_ZERO_CNT_MASK GENMASK(5, 0)
-#define T_HS_ZERO_CNT(x) UPDATE(x, 5, 0)
+#define T_HS_ZERO_CNT_LO_MASK GENMASK(5, 0)
+#define T_HS_ZERO_CNT_LO(x)UPDATE(x, 5, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg08 */
#define T_HS_TRAIL_CNT_MASKGENMASK(6, 0)
#define T_HS_TRAIL_CNT(x) UPDATE(x, 6, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg09 */
-#define T_HS_EXIT_CNT_MASK GENMASK(4, 0)
-#define T_HS_EXIT_CNT(x) UPDATE(x, 4, 0)
+#define T_HS_EXIT_CNT_LO_MASK GENMASK(4, 0)
+#define T_HS_EXIT_CNT_LO(x)UPDATE(x, 4, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg0a */
-#define T_CLK_POST_CNT_MASKGENMASK(3, 0)
-#define T_CLK_POST_CNT(x) UPDATE(x, 3, 0)
+#define T_CLK_POST_CNT_LO_MASK GENMASK(3, 0)
+#define T_CLK_POST_CNT_LO(x) UPDATE(x, 3, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg0c */
#define LPDT_TX_PPI_SYNC_MASK BIT(2)
#define LPDT_TX_PPI_SYNC_ENABLEBIT(2)
@@ -129,9 +147,13 @@
#define T_CLK_PRE_CNT_MASK GENMASK(3, 0)
#define T_CLK_PRE_CNT(x) UPDATE(x, 3, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg10 */
+#define T_CLK_POST_CNT_HI_MASK GENMASK(7, 6)
+#define T_CLK_POST_CNT_HI(x) UPDATE(x, 7, 6)
#define T_TA_GO_CNT_MASK GENMASK(5, 0)
#define T_TA_GO_CNT(x) UPDATE(x, 5, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg11 */
+#define T_HS_EXIT_CNT_HI_MASK BIT(6)
+#define T_HS_EXIT_CNT_HI(x)UPDATE(x, 6, 6)
#define T_TA_SURE_CNT_MASK GENMASK(5, 0)
#define T_TA_SURE_CNT(x) UPDATE(x, 5, 0)
/* Clock/Data0/Data1/Data2/Data3 Lane Register Part: reg12 */
@@ -169,11 +191,23 @@
#define DSI_PHY_STATUS 0xb0
#define PHY_LOCK BIT(0)
+enum phy_max_rate {
+ MAX_1GHZ,
+ MAX_2_5GHZ,
+};
+
+struct inno_video_phy_plat_data {
+ const struct inno_mipi_dphy_timing *inno_mipi_dphy_timing_table;
+ const unsigned int num_timings;
+ enum phy_max_rate max_rate;
+};
+
struct inno_dsidphy {
struct device *dev;
struct clk *ref_clk;
struct clk *pclk_phy;
struct
[RFC 3/4] drm/rockchip: dsi: add rk3568 support
From: Chris Morgan
Add the compatible and GRF definitions for the RK3568 soc.
Signed-off-by: Chris Morgan
---
.../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 51 ++-
1 file changed, 49 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index 110e83aad9bb..bf6948125b84 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -179,6 +179,23 @@
#define RK3399_TXRX_SRC_SEL_ISP0 BIT(4)
#define RK3399_TXRX_TURNREQUESTGENMASK(3, 0)
+#define RK3568_GRF_VO_CON2 0x0368
+#define RK3568_DSI0_SKEWCALHS (0x1f << 11)
+#define RK3568_DSI0_FORCETXSTOPMODE(0xf << 4)
+#define RK3568_DSI0_TURNDISABLEBIT(2)
+#define RK3568_DSI0_FORCERXMODEBIT(0)
+
+/*
+ * Note these registers do not appear in the datasheet, they are
+ * however present in the BSP driver which is where these values
+ * come from. Name GRF_VO_CON3 is assumed.
+ */
+#define RK3568_GRF_VO_CON3 0x36c
+#define RK3568_DSI1_SKEWCALHS (0x1f << 11)
+#define RK3568_DSI1_FORCETXSTOPMODE(0xf << 4)
+#define RK3568_DSI1_TURNDISABLEBIT(2)
+#define RK3568_DSI1_FORCERXMODEBIT(0)
+
#define HIWORD_UPDATE(val, mask) (val | (mask) << 16)
enum {
@@ -735,8 +752,9 @@ static void dw_mipi_dsi_rockchip_config(struct
dw_mipi_dsi_rockchip *dsi)
static void dw_mipi_dsi_rockchip_set_lcdsel(struct dw_mipi_dsi_rockchip *dsi,
int mux)
{
- regmap_write(dsi->grf_regmap, dsi->cdata->lcdsel_grf_reg,
- mux ? dsi->cdata->lcdsel_lit : dsi->cdata->lcdsel_big);
+ if (dsi->cdata->lcdsel_grf_reg < 0)
+ regmap_write(dsi->grf_regmap, dsi->cdata->lcdsel_grf_reg,
+ mux ? dsi->cdata->lcdsel_lit : dsi->cdata->lcdsel_big);
}
static int
@@ -963,6 +981,8 @@ static int dw_mipi_dsi_rockchip_bind(struct device *dev,
DRM_DEV_ERROR(dev, "Failed to create drm encoder\n");
goto out_pll_clk;
}
+ rockchip_drm_encoder_set_crtc_endpoint_id(>encoder,
+ dev->of_node, 0, 0);
ret = dw_mipi_dsi_bind(dsi->dmd, >encoder.encoder);
if (ret) {
@@ -1612,6 +1632,30 @@ static const struct rockchip_dw_dsi_chip_data
rk3399_chip_data[] = {
{ /* sentinel */ }
};
+static const struct rockchip_dw_dsi_chip_data rk3568_chip_data[] = {
+ {
+ .reg = 0xfe06,
+ .lcdsel_grf_reg = -1,
+ .lanecfg1_grf_reg = RK3568_GRF_VO_CON2,
+ .lanecfg1 = HIWORD_UPDATE(0, RK3568_DSI0_SKEWCALHS |
+ RK3568_DSI0_FORCETXSTOPMODE |
+ RK3568_DSI0_TURNDISABLE |
+ RK3568_DSI0_FORCERXMODE),
+ .max_data_lanes = 4,
+ },
+ {
+ .reg = 0xfe07,
+ .lcdsel_grf_reg = -1,
+ .lanecfg1_grf_reg = RK3568_GRF_VO_CON3,
+ .lanecfg1 = HIWORD_UPDATE(0, RK3568_DSI1_SKEWCALHS |
+ RK3568_DSI1_FORCETXSTOPMODE |
+ RK3568_DSI1_TURNDISABLE |
+ RK3568_DSI1_FORCERXMODE),
+ .max_data_lanes = 4,
+ },
+ { /* sentinel */ }
+};
+
static const struct of_device_id dw_mipi_dsi_rockchip_dt_ids[] = {
{
.compatible = "rockchip,px30-mipi-dsi",
@@ -1622,6 +1666,9 @@ static const struct of_device_id
dw_mipi_dsi_rockchip_dt_ids[] = {
}, {
.compatible = "rockchip,rk3399-mipi-dsi",
.data = _chip_data,
+ }, {
+.compatible = "rockchip,rk3568-mipi-dsi",
+.data = _chip_data,
},
{ /* sentinel */ }
};
--
2.25.1
[RFC 1/4] dt-bindings: display: rockchip-dsi: add rk3568 compatible
From: Chris Morgan The rk3568 uses the same dw-mipi-dsi controller as previous Rockchip SOCs, so add a compatible string for it. Signed-off-by: Chris Morgan --- .../bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt index 39792f051d2d..9a223df8530c 100644 --- a/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt +++ b/Documentation/devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt @@ -8,6 +8,7 @@ Required properties: "rockchip,px30-mipi-dsi", "snps,dw-mipi-dsi" "rockchip,rk3288-mipi-dsi", "snps,dw-mipi-dsi" "rockchip,rk3399-mipi-dsi", "snps,dw-mipi-dsi" + "rockchip,rk3568-mipi-dsi", "snps,dw-mipi-dsi" - reg: Represent the physical address range of the controller. - interrupts: Represent the controller's interrupt to the CPU(s). - clocks, clock-names: Phandles to the controller's pll reference -- 2.25.1
[RFC 2/4] dt-bindings: phy: phy-rockchip-inno-dsidphy: add compatible for rk3568
From: Chris Morgan Add a compatible string for the rk3568 dsi-dphy. Signed-off-by: Chris Morgan --- .../devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml index 8a3032a3bd73..5c35e5ceec0b 100644 --- a/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml +++ b/Documentation/devicetree/bindings/phy/rockchip,px30-dsi-dphy.yaml @@ -18,6 +18,7 @@ properties: - rockchip,px30-dsi-dphy - rockchip,rk3128-dsi-dphy - rockchip,rk3368-dsi-dphy + - rockchip,rk3568-dsi-dphy reg: maxItems: 1 -- 2.25.1
[RFC 0/4] rockchip-dsi for rk3568
From: Chris Morgan This series adds support for the dsi and dphy controllers on the Rockchip RK3568. I can confirm that for the Rockchip RK3568 this current series DOES NOT WORK properly yet. The image on the screen is shifted about 100 pixels to the right and does not appear to be a timing issue. This behavior was observed on both the Anbernic RG503 and RG353 portable gaming devices with different screens. These changes were also tested on an RK3326 based device (an Odroid Go Advance) with no noticeable regressions. An example of the issue on multiple devices: https://media.discordapp.net/attachments/973914035890290718/1007407064647221299/IMG_1999.jpg https://media.discordapp.net/attachments/995430498677571604/1003754966932008960/AB25898E-73EC-40A9-BD47-3FB970DDFB31.jpg Given the fact that the DSI controller is identical on the PX30 and RK3568 aside from different grf registers I am assuming the PHY is likely where the bugs are currently. I'm posting this as an RFC in the hopes that someone more knowledgeable than I can help identify the problem. Chris Morgan (4): dt-bindings: display: rockchip-dsi: add rk3568 compatible dt-bindings: phy: phy-rockchip-inno-dsidphy: add compatible for rk3568 drm/rockchip: dsi: add rk3568 support phy/rockchip: inno-dsidphy: Add support for rk3568 .../display/rockchip/dw_mipi_dsi_rockchip.txt | 1 + .../bindings/phy/rockchip,px30-dsi-dphy.yaml | 1 + .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 51 - .../phy/rockchip/phy-rockchip-inno-dsidphy.c | 204 ++ 4 files changed, 209 insertions(+), 48 deletions(-) -- 2.25.1
[PATCH 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
From: Etienne Carriere
This change allows userland to create a tee_shm object that refers
to a dmabuf reference.
Userland provides a dmabuf file descriptor as buffer reference.
The created tee_shm object exported as a brand new dmabuf reference
used to provide a clean fd to userland. Userland shall closed this new
fd to release the tee_shm object resources. The initial dmabuf resources
are tracked independently through original dmabuf file descriptor.
Once the buffer is registered and until it is released, TEE driver
keeps a refcount on the registered dmabuf structure.
This change only support dmabuf references that relates to physically
contiguous memory buffers.
New tee_shm flag to identify tee_shm objects built from a registered
dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged with
TEE_SHM_EXT_DMA_BUF.
Co-Developed-by: Etienne Carriere
Signed-off-by: Olivier Masse
Reported-by: kernel test robot
From: https://github.com/linaro-swg/linux.git
(cherry picked from commit 41e21e5c405530590dc2dd10b2a8dbe64589840f)
---
drivers/tee/tee_core.c | 38 +++
drivers/tee/tee_shm.c| 99 +++-
include/linux/tee_drv.h | 11 +
include/uapi/linux/tee.h | 29
4 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 8aa1a4836b92..7c45cbf85eb9 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context *ctx,
return ret;
}
+static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
+struct tee_ioctl_shm_register_fd_data
__user *udata)
+{
+ struct tee_ioctl_shm_register_fd_data data;
+ struct tee_shm *shm;
+ long ret;
+
+ if (copy_from_user(, udata, sizeof(data)))
+ return -EFAULT;
+
+ /* Currently no input flags are supported */
+ if (data.flags)
+ return -EINVAL;
+
+ shm = tee_shm_register_fd(ctx, data.fd);
+ if (IS_ERR(shm))
+ return -EINVAL;
+
+ data.id = shm->id;
+ data.flags = shm->flags;
+ data.size = shm->size;
+
+ if (copy_to_user(udata, , sizeof(data)))
+ ret = -EFAULT;
+ else
+ ret = tee_shm_get_fd(shm);
+
+ /*
+* When user space closes the file descriptor the shared memory
+* should be freed or if tee_shm_get_fd() failed then it will
+* be freed immediately.
+*/
+ tee_shm_put(shm);
+ return ret;
+}
+
static int params_from_user(struct tee_context *ctx, struct tee_param *params,
size_t num_params,
struct tee_ioctl_param __user *uparams)
@@ -829,6 +865,8 @@ static long tee_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
return tee_ioctl_shm_alloc(ctx, uarg);
case TEE_IOC_SHM_REGISTER:
return tee_ioctl_shm_register(ctx, uarg);
+ case TEE_IOC_SHM_REGISTER_FD:
+ return tee_ioctl_shm_register_fd(ctx, uarg);
case TEE_IOC_OPEN_SESSION:
return tee_ioctl_open_session(ctx, uarg);
case TEE_IOC_INVOKE:
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 836872467dc6..55a3fbbb022e 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -4,6 +4,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -12,6 +13,14 @@
#include
#include "tee_private.h"
+/* extra references appended to shm object for registered shared memory */
+struct tee_shm_dmabuf_ref {
+ struct tee_shm shm;
+ struct dma_buf *dmabuf;
+ struct dma_buf_attachment *attach;
+ struct sg_table *sgt;
+};
+
static void shm_put_kernel_pages(struct page **pages, size_t page_count)
{
size_t n;
@@ -71,7 +80,16 @@ static void release_registered_pages(struct tee_shm *shm)
static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm)
{
- if (shm->flags & TEE_SHM_POOL) {
+ if (shm->flags & TEE_SHM_EXT_DMA_BUF) {
+ struct tee_shm_dmabuf_ref *ref;
+
+ ref = container_of(shm, struct tee_shm_dmabuf_ref, shm);
+ dma_buf_unmap_attachment(ref->attach, ref->sgt,
+DMA_BIDIRECTIONAL);
+
+ dma_buf_detach(ref->dmabuf, ref->attach);
+ dma_buf_put(ref->dmabuf);
+ } else if (shm->flags & TEE_SHM_POOL) {
teedev->pool->ops->free(teedev->pool, shm);
} else if (shm->flags & TEE_SHM_DYNAMIC) {
int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm);
@@ -195,7 +213,7 @@ struct tee_shm *tee_shm_alloc_user_buf(struct tee_context
*ctx, size_t size)
* tee_client_invoke_func(). The memory allocated is later freed with a
* call to tee_shm_free().
*
- * @returns a pointer to 'struct tee_shm'
+ * @returns a pointer
[PATCH v2 0/1] tee: Add tee_shm_register_fd
Add a new ioctl called TEE_IOC_SHM_REGISTER_FD to register a shared memory from a dmabuf file descriptor. This new ioctl will allow the Linux Kernel to register a buffer to be used by the Secure Data Path OPTEE OS feature. Please find more information here: https://static.linaro.org/connect/san19/presentations/san19-107.pdf Patch tested on Hikey 6220. Etienne Carriere (1): tee: new ioctl to a register tee_shm from a dmabuf file descriptor drivers/tee/tee_core.c | 38 +++ drivers/tee/tee_shm.c| 99 +++- include/linux/tee_drv.h | 11 + include/uapi/linux/tee.h | 29 4 files changed, 175 insertions(+), 2 deletions(-) -- 2.25.0
Re: [EXT] Re: [PATCH 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
Hi Sumit,
Thanks for your comments, please find my answer below.
On ven., 2022-08-12 at 19:36 +0530, Sumit Garg wrote:
> Caution: EXT Email
>
> Hi Olivier,
>
> On Thu, 11 Aug 2022 at 19:26, Olivier Masse
> wrote:
> >
> > From: Etienne Carriere
> >
> > This change allows userland to create a tee_shm object that refers
> > to a dmabuf reference.
> >
> > Userland provides a dmabuf file descriptor as buffer reference.
> > The created tee_shm object exported as a brand new dmabuf reference
> > used to provide a clean fd to userland. Userland shall closed this
> > new
> > fd to release the tee_shm object resources. The initial dmabuf
> > resources
> > are tracked independently through original dmabuf file descriptor.
> >
> > Once the buffer is registered and until it is released, TEE driver
> > keeps a refcount on the registered dmabuf structure.
> >
> > This change only support dmabuf references that relates to
> > physically
> > contiguous memory buffers.
>
> What limits you to extend this feature to non-contiguous memory
> buffers? I believe that should be possible with OP-TEE dynamic shared
> memory which gives you the granularity to register a list of pages.
But it will need more logic in OPTEE OS to verify that all pages are in
the Secure Data Path protected area.
Our solution use a fixed protected reserved memory region and do not
rely on a dynamic protection set in secure.
Best regards,
Olivier
>
> -Sumit
>
> >
> > New tee_shm flag to identify tee_shm objects built from a
> > registered
> > dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged
> > with
> > TEE_SHM_EXT_DMA_BUF.
> >
> > Co-Developed-by: Etienne Carriere
> > Signed-off-by: Olivier Masse
> > From:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flinaro-swg%2Flinux.gitdata=05%7C01%7Colivier.masse%40nxp.com%7C204e0821d1c84355ed4208da7c6be5c6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637959100100176447%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=MRfnQNbwAIj%2Bnd9tSjSb5Nzrv3sQVpVMIdOxxRfu6U4%3Dreserved=0
> > (cherry picked from commit
> > 41e21e5c405530590dc2dd10b2a8dbe64589840f)
> > ---
> > drivers/tee/tee_core.c | 38 +++
> > drivers/tee/tee_shm.c| 99
> > +++-
> > include/linux/tee_drv.h | 11 +
> > include/uapi/linux/tee.h | 29
> > 4 files changed, 175 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 8aa1a4836b92..7c45cbf85eb9 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context
> > *ctx,
> > return ret;
> > }
> >
> > +static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
> > +struct
> > tee_ioctl_shm_register_fd_data __user *udata)
> > +{
> > + struct tee_ioctl_shm_register_fd_data data;
> > + struct tee_shm *shm;
> > + long ret;
> > +
> > + if (copy_from_user(, udata, sizeof(data)))
> > + return -EFAULT;
> > +
> > + /* Currently no input flags are supported */
> > + if (data.flags)
> > + return -EINVAL;
> > +
> > + shm = tee_shm_register_fd(ctx, data.fd);
> > + if (IS_ERR(shm))
> > + return -EINVAL;
> > +
> > + data.id = shm->id;
> > + data.flags = shm->flags;
> > + data.size = shm->size;
> > +
> > + if (copy_to_user(udata, , sizeof(data)))
> > + ret = -EFAULT;
> > + else
> > + ret = tee_shm_get_fd(shm);
> > +
> > + /*
> > +* When user space closes the file descriptor the shared
> > memory
> > +* should be freed or if tee_shm_get_fd() failed then it
> > will
> > +* be freed immediately.
> > +*/
> > + tee_shm_put(shm);
> > + return ret;
> > +}
> > +
> > static int params_from_user(struct tee_context *ctx, struct
> > tee_param *params,
> > size_t num_params,
> > struct tee_ioctl_param __user *uparams)
> > @@ -829,6 +865,8 @@ static long tee_ioctl(struct file *filp,
> > unsigned int cmd, unsigned long arg)
> > return tee_ioctl_shm_alloc(ctx, uarg);
> > case TEE_IOC_SHM_REGISTER:
> > return tee_ioctl_shm_register(ctx, uarg);
> > + case TEE_IOC_SHM_REGISTER_FD:
> > + return tee_ioctl_shm_register_fd(ctx, uarg);
> > case TEE_IOC_OPEN_SESSION:
> > return tee_ioctl_open_session(ctx, uarg);
> > case TEE_IOC_INVOKE:
> > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> > index 836872467dc6..55a3fbbb022e 100644
> > --- a/drivers/tee/tee_shm.c
> > +++ b/drivers/tee/tee_shm.c
> > @@ -4,6 +4,7 @@
> > */
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
>
[PATCH] drm/panel: simple: Add Multi-Inno Technology MI0800FT-9
Add Multi-Inno Technology MI0800FT-9 8" 800x600 DPI panel support.
Signed-off-by: Christoph Niedermaier
Cc: Sam Ravnborg
To: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/panel/panel-simple.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index ff5e1a44c43a..f5b632989285 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2703,6 +2703,36 @@ static const struct panel_desc multi_inno_mi0700s4t_6 = {
.connector_type = DRM_MODE_CONNECTOR_DPI,
};
+static const struct display_timing multi_inno_mi0800ft_9_timing = {
+ .pixelclock = { 3200, 4000, 5000 },
+ .hactive = { 800, 800, 800 },
+ .hfront_porch = { 16, 210, 354 },
+ .hback_porch = { 6, 26, 45 },
+ .hsync_len = { 1, 20, 40 },
+ .vactive = { 600, 600, 600 },
+ .vfront_porch = { 1, 12, 77 },
+ .vback_porch = { 3, 13, 22 },
+ .vsync_len = { 1, 10, 20 },
+ .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE |
+DISPLAY_FLAGS_SYNC_POSEDGE,
+};
+
+static const struct panel_desc multi_inno_mi0800ft_9 = {
+ .timings = _inno_mi0800ft_9_timing,
+ .num_timings = 1,
+ .bpc = 8,
+ .size = {
+ .width = 162,
+ .height = 122,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+ .bus_flags = DRM_BUS_FLAG_DE_HIGH |
+DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
+DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE,
+ .connector_type = DRM_MODE_CONNECTOR_DPI,
+};
+
static const struct display_timing multi_inno_mi1010ait_1cp_timing = {
.pixelclock = { 6890, 7000, 7340 },
.hactive = { 1280, 1280, 1280 },
@@ -4104,6 +4134,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "multi-inno,mi0700s4t-6",
.data = _inno_mi0700s4t_6,
}, {
+ .compatible = "multi-inno,mi0800ft-9",
+ .data = _inno_mi0800ft_9,
+ }, {
.compatible = "multi-inno,mi1010ait-1cp",
.data = _inno_mi1010ait_1cp,
}, {
--
2.11.0
Re: [PATCH v1 34/35] drm/modes: Introduce the tv_mode property as a command-line option
Hi Maxime,
On Fri, Jul 29, 2022 at 6:37 PM Maxime Ripard wrote:
> Our new tv mode option allows to specify the TV mode from a property.
> However, it can still be useful, for example to avoid any boot time
> artifact, to set that property directly from the kernel command line.
>
> Let's add some code to allow it, and some unit tests to exercise that code.
>
> Signed-off-by: Maxime Ripard
Thanks for your patch!
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1677,6 +1677,80 @@ static int drm_mode_parse_panel_orientation(const char
> *delim,
> return 0;
> }
>
> +#define TV_OPTION_EQUAL(value, len, option) \
> + ((strlen(option) == len) && !strncmp(value, option, len))
> +
> +static int drm_mode_parse_tv_mode(const char *delim,
> + struct drm_cmdline_mode *mode)
> +{
> + const char *value;
> + unsigned int len;
> +
> + if (*delim != '=')
> + return -EINVAL;
> +
> + value = delim + 1;
> + delim = strchr(value, ',');
> + if (!delim)
> + delim = value + strlen(value);
> +
> + len = delim - value;
> + if (TV_OPTION_EQUAL(value, len, "NTSC-443"))
> + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_443;
> + else if (TV_OPTION_EQUAL(value, len, "NTSC-J"))
> + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_J;
> + else if (TV_OPTION_EQUAL(value, len, "NTSC-M"))
> + mode->tv_mode = DRM_MODE_TV_NORM_NTSC_M;
[...]
You already have the array tv_norm_values[] from "[PATCH v1 05/35]
drm/connector: Add TV standard property". Can't you export that, and
loop over that array instead?
> + else if (TV_OPTION_EQUAL(value, len, "HD480I"))
> + mode->tv_mode = DRM_MODE_TV_NORM_HD480I;
> + else if (TV_OPTION_EQUAL(value, len, "HD480P"))
> + mode->tv_mode = DRM_MODE_TV_NORM_HD480P;
> + else if (TV_OPTION_EQUAL(value, len, "HD576I"))
> + mode->tv_mode = DRM_MODE_TV_NORM_HD576I;
> + else if (TV_OPTION_EQUAL(value, len, "HD576P"))
> + mode->tv_mode = DRM_MODE_TV_NORM_HD576P;
> + else if (TV_OPTION_EQUAL(value, len, "HD720P"))
> + mode->tv_mode = DRM_MODE_TV_NORM_HD720P;
> + else if (TV_OPTION_EQUAL(value, len, "HD1080I"))
> + mode->tv_mode = DRM_MODE_TV_NORM_HD1080I;
The names in tv_norm_values[] use lower-case, while you use upper-case
here.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v6 6/6] drm/ttm: Switch to using the new res callback
Apply new intersect and compatible callback instead
of having a generic placement range verfications.
v2: Added a separate callback for compatiblilty
checks (Christian)
v3: Cleanups and removal of workarounds
Signed-off-by: Christian König
Signed-off-by: Arunpravin Paneer Selvam
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 45 +++--
drivers/gpu/drm/ttm/ttm_resource.c | 17 ++
2 files changed, 15 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 170935c294f5..7d25a10395c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1328,11 +1328,12 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct amdgpu_device
*adev, struct ttm_tt *ttm,
static bool amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
const struct ttm_place *place)
{
- unsigned long num_pages = bo->resource->num_pages;
struct dma_resv_iter resv_cursor;
- struct amdgpu_res_cursor cursor;
struct dma_fence *f;
+ if (!amdgpu_bo_is_amdgpu_bo(bo))
+ return ttm_bo_eviction_valuable(bo, place);
+
/* Swapout? */
if (bo->resource->mem_type == TTM_PL_SYSTEM)
return true;
@@ -1351,40 +1352,20 @@ static bool amdgpu_ttm_bo_eviction_valuable(struct
ttm_buffer_object *bo,
return false;
}
- switch (bo->resource->mem_type) {
- case AMDGPU_PL_PREEMPT:
- /* Preemptible BOs don't own system resources managed by the
-* driver (pages, VRAM, GART space). They point to resources
-* owned by someone else (e.g. pageable memory in user mode
-* or a DMABuf). They are used in a preemptible context so we
-* can guarantee no deadlocks and good QoS in case of MMU
-* notifiers or DMABuf move notifiers from the resource owner.
-*/
+ /* Preemptible BOs don't own system resources managed by the
+* driver (pages, VRAM, GART space). They point to resources
+* owned by someone else (e.g. pageable memory in user mode
+* or a DMABuf). They are used in a preemptible context so we
+* can guarantee no deadlocks and good QoS in case of MMU
+* notifiers or DMABuf move notifiers from the resource owner.
+*/
+ if (bo->resource->mem_type == AMDGPU_PL_PREEMPT)
return false;
- case TTM_PL_TT:
- if (amdgpu_bo_is_amdgpu_bo(bo) &&
- amdgpu_bo_encrypted(ttm_to_amdgpu_bo(bo)))
- return false;
- return true;
- case TTM_PL_VRAM:
- /* Check each drm MM node individually */
- amdgpu_res_first(bo->resource, 0, (u64)num_pages << PAGE_SHIFT,
-);
- while (cursor.remaining) {
- if (place->fpfn < PFN_DOWN(cursor.start + cursor.size)
- && !(place->lpfn &&
-place->lpfn <= PFN_DOWN(cursor.start)))
- return true;
-
- amdgpu_res_next(, cursor.size);
- }
+ if (bo->resource->mem_type == TTM_PL_TT &&
+ amdgpu_bo_encrypted(ttm_to_amdgpu_bo(bo)))
return false;
- default:
- break;
- }
-
return ttm_bo_eviction_valuable(bo, place);
}
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
b/drivers/gpu/drm/ttm/ttm_resource.c
index 0d1f862a582b..a729c32a1e48 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -276,17 +276,9 @@ bool ttm_resource_intersects(struct ttm_device *bdev,
if (!res)
return false;
- if (!place)
- return true;
-
man = ttm_manager_type(bdev, res->mem_type);
- if (!man->func->intersects) {
- if (place->fpfn >= (res->start + res->num_pages) ||
- (place->lpfn && place->lpfn <= res->start))
- return false;
-
+ if (!place || !man->func->intersects)
return true;
- }
return man->func->intersects(man, res, place, size);
}
@@ -314,13 +306,8 @@ bool ttm_resource_compatible(struct ttm_device *bdev,
return false;
man = ttm_manager_type(bdev, res->mem_type);
- if (!man->func->compatible) {
- if (res->start < place->fpfn ||
- (place->lpfn && (res->start + res->num_pages) >
place->lpfn))
- return false;
-
+ if (!man->func->compatible)
return true;
- }
return man->func->compatible(man, res, place, size);
}
--
2.25.1
[PATCH v6 5/6] drm/nouveau: Implement intersect/compatible functions
Implemented a new intersect and compatible callback function
fetching the start offset from struct ttm_resource.
Signed-off-by: Christian König
Signed-off-by: Arunpravin Paneer Selvam
---
drivers/gpu/drm/nouveau/nouveau_mem.c | 29 +++
drivers/gpu/drm/nouveau/nouveau_mem.h | 6 ++
drivers/gpu/drm/nouveau/nouveau_ttm.c | 24 ++
3 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.c
b/drivers/gpu/drm/nouveau/nouveau_mem.c
index 2e517cdc24c9..76f8edefa637 100644
--- a/drivers/gpu/drm/nouveau/nouveau_mem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_mem.c
@@ -187,3 +187,32 @@ nouveau_mem_new(struct nouveau_cli *cli, u8 kind, u8 comp,
*res = >base;
return 0;
}
+
+bool
+nouveau_mem_intersects(struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ u32 num_pages = PFN_UP(size);
+
+ /* Don't evict BOs outside of the requested placement range */
+ if (place->fpfn >= (res->start + num_pages) ||
+ (place->lpfn && place->lpfn <= res->start))
+ return false;
+
+ return true;
+}
+
+bool
+nouveau_mem_compatible(struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ u32 num_pages = PFN_UP(size);
+
+ if (res->start < place->fpfn ||
+ (place->lpfn && (res->start + num_pages) > place->lpfn))
+ return false;
+
+ return true;
+}
diff --git a/drivers/gpu/drm/nouveau/nouveau_mem.h
b/drivers/gpu/drm/nouveau/nouveau_mem.h
index 325551eba5cd..1ee6cdb9ad9b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_mem.h
+++ b/drivers/gpu/drm/nouveau/nouveau_mem.h
@@ -25,6 +25,12 @@ int nouveau_mem_new(struct nouveau_cli *, u8 kind, u8 comp,
struct ttm_resource **);
void nouveau_mem_del(struct ttm_resource_manager *man,
struct ttm_resource *);
+bool nouveau_mem_intersects(struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size);
+bool nouveau_mem_compatible(struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size);
int nouveau_mem_vram(struct ttm_resource *, bool contig, u8 page);
int nouveau_mem_host(struct ttm_resource *, struct ttm_tt *);
void nouveau_mem_fini(struct nouveau_mem *);
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 85f1f5a0fe5d..9602c30928f2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -42,6 +42,24 @@ nouveau_manager_del(struct ttm_resource_manager *man,
nouveau_mem_del(man, reg);
}
+static bool
+nouveau_manager_intersects(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ return nouveau_mem_intersects(res, place, size);
+}
+
+static bool
+nouveau_manager_compatible(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ return nouveau_mem_compatible(res, place, size);
+}
+
static int
nouveau_vram_manager_new(struct ttm_resource_manager *man,
struct ttm_buffer_object *bo,
@@ -73,6 +91,8 @@ nouveau_vram_manager_new(struct ttm_resource_manager *man,
const struct ttm_resource_manager_func nouveau_vram_manager = {
.alloc = nouveau_vram_manager_new,
.free = nouveau_manager_del,
+ .intersects = nouveau_manager_intersects,
+ .compatible = nouveau_manager_compatible,
};
static int
@@ -97,6 +117,8 @@ nouveau_gart_manager_new(struct ttm_resource_manager *man,
const struct ttm_resource_manager_func nouveau_gart_manager = {
.alloc = nouveau_gart_manager_new,
.free = nouveau_manager_del,
+ .intersects = nouveau_manager_intersects,
+ .compatible = nouveau_manager_compatible,
};
static int
@@ -130,6 +152,8 @@ nv04_gart_manager_new(struct ttm_resource_manager *man,
const struct ttm_resource_manager_func nv04_gart_manager = {
.alloc = nv04_gart_manager_new,
.free = nouveau_manager_del,
+ .intersects = nouveau_manager_intersects,
+ .compatible = nouveau_manager_compatible,
};
static int
--
2.25.1
[PATCH v6 4/6] drm/i915: Implement intersect/compatible functions
Implemented a new intersect and compatible callback function
fetching start offset from drm buddy allocator.
v3: move the bits that are specific to buddy_man (Matthew)
v4: consider the block size /range (Matthew)
Signed-off-by: Christian König
Signed-off-by: Arunpravin Paneer Selvam
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 41 +--
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 73 +++
2 files changed, 74 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 5a5cf332d8a5..bc9c432edffe 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -361,7 +361,6 @@ static bool i915_ttm_eviction_valuable(struct
ttm_buffer_object *bo,
const struct ttm_place *place)
{
struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
- struct ttm_resource *res = bo->resource;
if (!obj)
return false;
@@ -378,45 +377,7 @@ static bool i915_ttm_eviction_valuable(struct
ttm_buffer_object *bo,
if (!i915_gem_object_evictable(obj))
return false;
- switch (res->mem_type) {
- case I915_PL_LMEM0: {
- struct ttm_resource_manager *man =
- ttm_manager_type(bo->bdev, res->mem_type);
- struct i915_ttm_buddy_resource *bman_res =
- to_ttm_buddy_resource(res);
- struct drm_buddy *mm = bman_res->mm;
- struct drm_buddy_block *block;
-
- if (!place->fpfn && !place->lpfn)
- return true;
-
- GEM_BUG_ON(!place->lpfn);
-
- /*
-* If we just want something mappable then we can quickly check
-* if the current victim resource is using any of the CPU
-* visible portion.
-*/
- if (!place->fpfn &&
- place->lpfn == i915_ttm_buddy_man_visible_size(man))
- return bman_res->used_visible_size > 0;
-
- /* Real range allocation */
- list_for_each_entry(block, _res->blocks, link) {
- unsigned long fpfn =
- drm_buddy_block_offset(block) >> PAGE_SHIFT;
- unsigned long lpfn = fpfn +
- (drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
-
- if (place->fpfn < lpfn && place->lpfn > fpfn)
- return true;
- }
- return false;
- } default:
- break;
- }
-
- return true;
+ return ttm_bo_eviction_valuable(bo, place);
}
static void i915_ttm_evict_flags(struct ttm_buffer_object *bo,
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 427de1aaab36..e19452f0e100 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -173,6 +173,77 @@ static void i915_ttm_buddy_man_free(struct
ttm_resource_manager *man,
kfree(bman_res);
}
+static bool i915_ttm_buddy_man_intersects(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res);
+ struct i915_ttm_buddy_manager *bman = to_buddy_manager(man);
+ struct drm_buddy *mm = >mm;
+ struct drm_buddy_block *block;
+
+ if (!place->fpfn && !place->lpfn)
+ return true;
+
+ GEM_BUG_ON(!place->lpfn);
+
+ /*
+* If we just want something mappable then we can quickly check
+* if the current victim resource is using any of the CPU
+* visible portion.
+*/
+ if (!place->fpfn &&
+ place->lpfn == i915_ttm_buddy_man_visible_size(man))
+ return bman_res->used_visible_size > 0;
+
+ /* Check each drm buddy block individually */
+ list_for_each_entry(block, _res->blocks, link) {
+ unsigned long fpfn =
+ drm_buddy_block_offset(block) >> PAGE_SHIFT;
+ unsigned long lpfn = fpfn +
+ (drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
+
+ if (place->fpfn < lpfn && place->lpfn > fpfn)
+ return true;
+ }
+
+ return false;
+}
+
+static bool i915_ttm_buddy_man_compatible(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ struct i915_ttm_buddy_resource
[PATCH v6 3/6] drm/amdgpu: Implement intersect/compatible functions
Implemented a new intersect and compatible callback function
fetching start offset from backend drm buddy allocator.
Signed-off-by: Christian König
Signed-off-by: Arunpravin Paneer Selvam
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 38 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 68
2 files changed, 106 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 8c6b2284cf56..1f3302aebeff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -204,6 +204,42 @@ void amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr)
amdgpu_gart_invalidate_tlb(adev);
}
+/**
+ * amdgpu_gtt_mgr_intersects - test for intersection
+ *
+ * @man: Our manager object
+ * @res: The resource to test
+ * @place: The place for the new allocation
+ * @size: The size of the new allocation
+ *
+ * Simplified intersection test, only interesting if we need GART or not.
+ */
+static bool amdgpu_gtt_mgr_intersects(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ return !place->lpfn || amdgpu_gtt_mgr_has_gart_addr(res);
+}
+
+/**
+ * amdgpu_gtt_mgr_compatible - test for compatibility
+ *
+ * @man: Our manager object
+ * @res: The resource to test
+ * @place: The place for the new allocation
+ * @size: The size of the new allocation
+ *
+ * Simplified compatibility test.
+ */
+static bool amdgpu_gtt_mgr_compatible(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ return !place->lpfn || amdgpu_gtt_mgr_has_gart_addr(res);
+}
+
/**
* amdgpu_gtt_mgr_debug - dump VRAM table
*
@@ -225,6 +261,8 @@ static void amdgpu_gtt_mgr_debug(struct
ttm_resource_manager *man,
static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
.alloc = amdgpu_gtt_mgr_new,
.free = amdgpu_gtt_mgr_del,
+ .intersects = amdgpu_gtt_mgr_intersects,
+ .compatible = amdgpu_gtt_mgr_compatible,
.debug = amdgpu_gtt_mgr_debug
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 28ec5f8ac1c1..d1a2619fa89f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -720,6 +720,72 @@ uint64_t amdgpu_vram_mgr_vis_usage(struct amdgpu_vram_mgr
*mgr)
return atomic64_read(>vis_usage);
}
+/**
+ * amdgpu_vram_mgr_intersects - test each drm buddy block for intersection
+ *
+ * @man: TTM memory type manager
+ * @res: The resource to test
+ * @place: The place to test against
+ * @size: Size of the new allocation
+ *
+ * Test each drm buddy block for intersection for eviction decision.
+ */
+static bool amdgpu_vram_mgr_intersects(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ struct amdgpu_vram_mgr_resource *mgr = to_amdgpu_vram_mgr_resource(res);
+ struct drm_buddy_block *block;
+
+ /* Check each drm buddy block individually */
+ list_for_each_entry(block, >blocks, link) {
+ unsigned long fpfn =
+ amdgpu_vram_mgr_block_start(block) >> PAGE_SHIFT;
+ unsigned long lpfn = fpfn +
+ (amdgpu_vram_mgr_block_size(block) >> PAGE_SHIFT);
+
+ if (place->fpfn < lpfn &&
+ (place->lpfn && place->lpfn > fpfn))
+ return true;
+ }
+
+ return false;
+}
+
+/**
+ * amdgpu_vram_mgr_compatible - test each drm buddy block for compatibility
+ *
+ * @man: TTM memory type manager
+ * @res: The resource to test
+ * @place: The place to test against
+ * @size: Size of the new allocation
+ *
+ * Test each drm buddy block for placement compatibility.
+ */
+static bool amdgpu_vram_mgr_compatible(struct ttm_resource_manager *man,
+ struct ttm_resource *res,
+ const struct ttm_place *place,
+ size_t size)
+{
+ struct amdgpu_vram_mgr_resource *mgr = to_amdgpu_vram_mgr_resource(res);
+ struct drm_buddy_block *block;
+
+ /* Check each drm buddy block individually */
+ list_for_each_entry(block, >blocks, link) {
+ unsigned long fpfn =
+ amdgpu_vram_mgr_block_start(block) >> PAGE_SHIFT;
+ unsigned long lpfn = fpfn +
+ (amdgpu_vram_mgr_block_size(block) >> PAGE_SHIFT);
+
+ if
[PATCH v6 2/6] drm/ttm: Implement intersect/compatible functions
Implemented a new intersect and compatible callback functions
to ttm range manager fetching start offset from drm mm range
allocator.
Signed-off-by: Christian König
Signed-off-by: Arunpravin Paneer Selvam
---
drivers/gpu/drm/ttm/ttm_range_manager.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c
b/drivers/gpu/drm/ttm/ttm_range_manager.c
index d91666721dc6..4cfef2b3514d 100644
--- a/drivers/gpu/drm/ttm/ttm_range_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
@@ -113,6 +113,37 @@ static void ttm_range_man_free(struct ttm_resource_manager
*man,
kfree(node);
}
+static bool ttm_range_man_intersects(struct ttm_resource_manager *man,
+struct ttm_resource *res,
+const struct ttm_place *place,
+size_t size)
+{
+ struct drm_mm_node *node = _ttm_range_mgr_node(res)->mm_nodes[0];
+ u32 num_pages = PFN_UP(size);
+
+ /* Don't evict BOs outside of the requested placement range */
+ if (place->fpfn >= (node->start + num_pages) ||
+ (place->lpfn && place->lpfn <= node->start))
+ return false;
+
+ return true;
+}
+
+static bool ttm_range_man_compatible(struct ttm_resource_manager *man,
+struct ttm_resource *res,
+const struct ttm_place *place,
+size_t size)
+{
+ struct drm_mm_node *node = _ttm_range_mgr_node(res)->mm_nodes[0];
+ u32 num_pages = PFN_UP(size);
+
+ if (node->start < place->fpfn ||
+ (place->lpfn && (node->start + num_pages) > place->lpfn))
+ return false;
+
+ return true;
+}
+
static void ttm_range_man_debug(struct ttm_resource_manager *man,
struct drm_printer *printer)
{
@@ -126,6 +157,8 @@ static void ttm_range_man_debug(struct ttm_resource_manager
*man,
static const struct ttm_resource_manager_func ttm_range_manager_func = {
.alloc = ttm_range_man_alloc,
.free = ttm_range_man_free,
+ .intersects = ttm_range_man_intersects,
+ .compatible = ttm_range_man_compatible,
.debug = ttm_range_man_debug
};
--
2.25.1
[PATCH v6 1/6] drm/ttm: Add new callbacks to ttm res mgr
We are adding two new callbacks to ttm resource manager
function to handle intersection and compatibility of
placement and resources.
v2: move the amdgpu and ttm_range_manager changes to
separate patches (Christian)
v3: rename "intersect" to "intersects" (Matthew)
v4: move !place check to the !res if and return false
in ttm_resource_compatible() function (Christian)
v5: move bits of code from patch number 6 to avoid
temporary driver breakup (Christian)
Signed-off-by: Christian König
Signed-off-by: Arunpravin Paneer Selvam
---
drivers/gpu/drm/ttm/ttm_bo.c | 9 ++--
drivers/gpu/drm/ttm/ttm_resource.c | 77 +-
include/drm/ttm/ttm_resource.h | 40
3 files changed, 119 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c1bd006a5525..f066e8124c50 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
const struct ttm_place *place)
{
+ struct ttm_resource *res = bo->resource;
+ struct ttm_device *bdev = bo->bdev;
+
dma_resv_assert_held(bo->base.resv);
if (bo->resource->mem_type == TTM_PL_SYSTEM)
return true;
@@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
/* Don't evict this BO if it's outside of the
* requested placement range
*/
- if (place->fpfn >= (bo->resource->start + bo->resource->num_pages) ||
- (place->lpfn && place->lpfn <= bo->resource->start))
- return false;
-
- return true;
+ return ttm_resource_intersects(bdev, res, place, bo->base.size);
}
EXPORT_SYMBOL(ttm_bo_eviction_valuable);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
b/drivers/gpu/drm/ttm/ttm_resource.c
index 20f9adcc3235..0d1f862a582b 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -253,10 +253,84 @@ void ttm_resource_free(struct ttm_buffer_object *bo,
struct ttm_resource **res)
}
EXPORT_SYMBOL(ttm_resource_free);
+/**
+ * ttm_resource_intersects - test for intersection
+ *
+ * @bdev: TTM device structure
+ * @res: The resource to test
+ * @place: The placement to test
+ * @size: How many bytes the new allocation needs.
+ *
+ * Test if @res intersects with @place and @size. Used for testing if evictions
+ * are valueable or not.
+ *
+ * Returns true if the res placement intersects with @place and @size.
+ */
+bool ttm_resource_intersects(struct ttm_device *bdev,
+struct ttm_resource *res,
+const struct ttm_place *place,
+size_t size)
+{
+ struct ttm_resource_manager *man;
+
+ if (!res)
+ return false;
+
+ if (!place)
+ return true;
+
+ man = ttm_manager_type(bdev, res->mem_type);
+ if (!man->func->intersects) {
+ if (place->fpfn >= (res->start + res->num_pages) ||
+ (place->lpfn && place->lpfn <= res->start))
+ return false;
+
+ return true;
+ }
+
+ return man->func->intersects(man, res, place, size);
+}
+
+/**
+ * ttm_resource_compatible - test for compatibility
+ *
+ * @bdev: TTM device structure
+ * @res: The resource to test
+ * @place: The placement to test
+ * @size: How many bytes the new allocation needs.
+ *
+ * Test if @res compatible with @place and @size.
+ *
+ * Returns true if the res placement compatible with @place and @size.
+ */
+bool ttm_resource_compatible(struct ttm_device *bdev,
+struct ttm_resource *res,
+const struct ttm_place *place,
+size_t size)
+{
+ struct ttm_resource_manager *man;
+
+ if (!res || !place)
+ return false;
+
+ man = ttm_manager_type(bdev, res->mem_type);
+ if (!man->func->compatible) {
+ if (res->start < place->fpfn ||
+ (place->lpfn && (res->start + res->num_pages) >
place->lpfn))
+ return false;
+
+ return true;
+ }
+
+ return man->func->compatible(man, res, place, size);
+}
+
static bool ttm_resource_places_compat(struct ttm_resource *res,
const struct ttm_place *places,
unsigned num_placement)
{
+ struct ttm_buffer_object *bo = res->bo;
+ struct ttm_device *bdev = bo->bdev;
unsigned i;
if (res->placement & TTM_PL_FLAG_TEMPORARY)
@@ -265,8 +339,7 @@ static bool ttm_resource_places_compat(struct ttm_resource
*res,
for (i = 0; i < num_placement; i++) {
const struct ttm_place *heap = [i];
- if (res->start < heap->fpfn
Re: [PATCH v1 09/35] drm/modes: Move named modes parsing to a separate function
Hi Maxime,
On Fri, Jul 29, 2022 at 6:36 PM Maxime Ripard wrote:
> The current construction of the named mode parsing doesn't allow to extend
> it easily. Let's move it to a separate function so we can add more
> parameters and modes.
>
> Signed-off-by: Maxime Ripard
Thanks for your patch, which looks similar to my "[PATCH v2 2/5]
drm/modes: Extract drm_mode_parse_cmdline_named_mode()"
(https://lore.kernel.org/dri-devel/1371554419ae63cb54c2a377db0c1016fcf200bb.1657788997.git.ge...@linux-m68k.org
;-)
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1773,6 +1773,28 @@ static const char * const drm_named_modes_whitelist[]
> = {
> "PAL",
> };
>
> +static bool drm_mode_parse_cmdline_named_mode(const char *name,
> + unsigned int name_end,
> + struct drm_cmdline_mode
> *cmdline_mode)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
> + int ret;
> +
> + ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
> + if (ret != name_end)
> + continue;
> +
> + strcpy(cmdline_mode->name, drm_named_modes_whitelist[i]);
> + cmdline_mode->specified = true;
> +
> + return true;
> + }
> +
> + return false;
What's the point in returning a value, if it is never checked?
Just make this function return void?
> +}
> +
> /**
> * drm_mode_parse_command_line_for_connector - parse command line modeline
> for connector
> * @mode_option: optional per connector mode option
> @@ -1848,18 +1870,14 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
> parse_extras = true;
> }
>
> - /* First check for a named mode */
> - for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) {
> - ret = str_has_prefix(name, drm_named_modes_whitelist[i]);
> - if (ret == mode_end) {
> - if (refresh_ptr)
> - return false; /* named + refresh is invalid */
> + /*
> +* Having a mode that starts by a letter (and thus is named) and
> +* an at-sign (used to specify a refresh rate) is disallowed.
> +*/
> + if (!isdigit(name[0]) && refresh_ptr)
This condition may have to be relaxed, if we want to support e.g.
"hd720p@50", cfr. my comments on "[PATCH v1 05/35] drm/connector:
Add TV standard property".
> + return false;
>
> - strcpy(mode->name, drm_named_modes_whitelist[i]);
> - mode->specified = true;
> - break;
> - }
> - }
> + drm_mode_parse_cmdline_named_mode(name, mode_end, mode);
This call needs to be conditional on mode_end being non-zero, cfr. my
patch "[PATCH v2 1/5] drm/modes: parse_cmdline: Handle empty mode name
part"
(https://lore.kernel.org/dri-devel/302d0737539daa2053134e8f24fdf37e3d939e1e.1657788997.git.ge...@linux-m68k.org).
>
> /* No named mode? Check for a normal mode argument, e.g. 1024x768 */
> if (!mode->specified && isdigit(name[0])) {
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v1 07/35] drm/modes: Only consider bpp and refresh before options
Hi Maxime,
On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard wrote:
> Some video= options might have a value that contains a dash. However, the
> command line parsing mode considers all dashes as the separator between the
> mode and the bpp count.
>
> Let's rework the parsing code a bit to only consider a dash as the bpp
> separator if it before a comma, the options separator.
>
> A follow-up patch will add a unit-test for this once such an option is
> introduced.
>
> Signed-off-by: Maxime Ripard
Thanks for your patch!
Reviewed-by: Geert Uytterhoeven
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1819,20 +1819,22 @@ bool drm_mode_parse_command_line_for_connector(const
> char *mode_option,
>
> name = mode_option;
>
> + /* Locate the start of named options */
> + options_ptr = strchr(name, ',');
> + if (options_ptr)
> + options_off = options_ptr - name;
> + else
> + options_off = strlen(name);
> +
> /* Try to locate the bpp and refresh specifiers, if any */
> - bpp_ptr = strchr(name, '-');
> + bpp_ptr = strnchr(name, options_off, '-');
Probably you still want to add a check that the next character
is actually a digit, cfr. my "[PATCH v2 5/5] drm/modes:
parse_cmdline: Add support for named modes containing dashes"
(https://lore.kernel.org/dri-devel/2eb205da88c3cb19ddf04d167ece4e16a330948b.1657788997.git.ge...@linux-m68k.org)?
> if (bpp_ptr)
> bpp_off = bpp_ptr - name;
>
> - refresh_ptr = strchr(name, '@');
> + refresh_ptr = strnchr(name, options_off, '@');
> if (refresh_ptr)
> refresh_off = refresh_ptr - name;
>
> - /* Locate the start of named options */
> - options_ptr = strchr(name, ',');
> - if (options_ptr)
> - options_off = options_ptr - name;
> -
> /* Locate the end of the name / resolution, and parse it */
> if (bpp_ptr) {
> mode_end = bpp_off;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v1 05/35] drm/connector: Add TV standard property
Hi Maxime,
On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard wrote:
> The TV mode property has been around for a while now to select and get the
> current TV mode output on an analog TV connector.
>
> Despite that property name being generic, its content isn't and has been
> driver-specific which makes it hard to build any generic behaviour on top
> of it, both in kernel and user-space.
>
> Let's create a new bitmask tv norm property, that can contain any of the
> analog TV standards currently supported by kernel drivers. Each driver can
> then pass in a bitmask of the modes it supports.
>
> We'll then be able to phase out the older tv mode property.
>
> Signed-off-by: Maxime Ripard
Thanks for your patch!
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1649,11 +1650,40 @@ EXPORT_SYMBOL(drm_mode_create_tv_margin_properties);
> * 0 on success or a negative error code on failure.
> */
> int drm_mode_create_tv_properties(struct drm_device *dev,
> + unsigned int supported_tv_norms,
> unsigned int num_modes,
> const char * const modes[])
> {
> + static const struct drm_prop_enum_list tv_norm_values[] = {
> + { __builtin_ffs(DRM_MODE_TV_NORM_NTSC_443) - 1, "NTSC-443" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_NTSC_J) - 1, "NTSC-J" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_NTSC_M) - 1, "NTSC-M" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_60) - 1, "PAL-60" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_B) - 1, "PAL-B" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_D) - 1, "PAL-D" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_G) - 1, "PAL-G" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_H) - 1, "PAL-H" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_I) - 1, "PAL-I" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_M) - 1, "PAL-M" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_N) - 1, "PAL-N" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_PAL_NC) - 1, "PAL-Nc" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_60) - 1, "SECAM-60" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_B) - 1, "SECAM-B" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_D) - 1, "SECAM-D" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_G) - 1, "SECAM-G" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_K) - 1, "SECAM-K" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_K1) - 1, "SECAM-K1" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_SECAM_L) - 1, "SECAM-L" },
The above are analog standards, with a variable horizontal resolution.
> + { __builtin_ffs(DRM_MODE_TV_NORM_HD480I) - 1, "hd480i" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_HD480P) - 1, "hd480p" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_HD576I) - 1, "hd576i" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_HD576P) - 1, "hd576p" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_HD720P) - 1, "hd720p" },
> + { __builtin_ffs(DRM_MODE_TV_NORM_HD1080I) - 1, "hd1080i" },
The above are digital standards, with a fixed resolution.
You seem to have missed "hd1080p"?
In addition, "hd720p", "hd080i", and "hd1080p" are available in both 50
and 60 (actually 59.94) Hz, while "hd1080p" can also use 24 or 25 Hz.
Either you have to add them here (e.g. "hd720p50" and "hd720p60"), or
handle them through "@". The latter would impact "[PATCH v1
09/35] drm/modes: Move named modes parsing to a separate function", as
currently a named mode and a refresh rate can't be specified both.
As "[PATCH v1 34/35] drm/modes: Introduce the tv_mode property as a
command-line option" uses a separate "tv_mode" option, and not the main
mode name, I think you want to add them here.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [EXT] Re: [PATCH 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
Hi Christian,
On ven., 2022-08-12 at 12:18 +0200, Christian König wrote:
> Caution: EXT Email
>
> Hi Etienne,
>
> at least I don't see anything obvious broken.
>
> Just two comments:
>
> 1. Dmitry is working on a change which renames some functions and
> makes
> it mandatory to call them with the dma_resv lock held.
>
> Depending on how you want to upstream this change you will certainly
> run
> into conflicts with that.
ok could you send me some link to his changes ?
>
> 2. Would it be possible to do this dynamically? In other words does
> the
> tee driver has a concept of buffers moving around?
I'm not sure to understand what you mean by a moving buffer ?
For information the TEE need to map the buffer in secure which is done
when calling the invoke operation function.
>
> Am 11.08.22 um 15:56 schrieb Olivier Masse:
> > From: Etienne Carriere
> >
> > This change allows userland to create a tee_shm object that refers
> > to a dmabuf reference.
> >
> > Userland provides a dmabuf file descriptor as buffer reference.
> > The created tee_shm object exported as a brand new dmabuf reference
> > used to provide a clean fd to userland. Userland shall closed this
> > new
> > fd to release the tee_shm object resources. The initial dmabuf
> > resources
> > are tracked independently through original dmabuf file descriptor.
> >
> > Once the buffer is registered and until it is released, TEE driver
> > keeps a refcount on the registered dmabuf structure.
> >
> > This change only support dmabuf references that relates to
> > physically
> > contiguous memory buffers.
>
> That sounds like a pretty hard restriction, but I obviously don't see
> how to avoid it either.
>
> Regards,
> Christian.
>
> >
> > New tee_shm flag to identify tee_shm objects built from a
> > registered
> > dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged
> > with
> > TEE_SHM_EXT_DMA_BUF.
> >
> > Co-Developed-by: Etienne Carriere
> > Signed-off-by: Olivier Masse
> > From:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flinaro-swg%2Flinux.gitdata=05%7C01%7Colivier.masse%40nxp.com%7C03c2d3cc95b8429693c108da7c4bf7a7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637958962989857231%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=vz8Y7ZwKXteePpWQVO8EhmZ1oyIX0xOCRd%2BGH7xHjII%3Dreserved=0
> > (cherry picked from commit
> > 41e21e5c405530590dc2dd10b2a8dbe64589840f)
> > ---
> > drivers/tee/tee_core.c | 38 +++
> > drivers/tee/tee_shm.c| 99
> > +++-
> > include/linux/tee_drv.h | 11 +
> > include/uapi/linux/tee.h | 29
> > 4 files changed, 175 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 8aa1a4836b92..7c45cbf85eb9 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context
> > *ctx,
> > return ret;
> > }
> >
> > +static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
> > + struct
> > tee_ioctl_shm_register_fd_data __user *udata)
> > +{
> > + struct tee_ioctl_shm_register_fd_data data;
> > + struct tee_shm *shm;
> > + long ret;
> > +
> > + if (copy_from_user(, udata, sizeof(data)))
> > + return -EFAULT;
> > +
> > + /* Currently no input flags are supported */
> > + if (data.flags)
> > + return -EINVAL;
> > +
> > + shm = tee_shm_register_fd(ctx, data.fd);
> > + if (IS_ERR(shm))
> > + return -EINVAL;
> > +
> > + data.id = shm->id;
> > + data.flags = shm->flags;
> > + data.size = shm->size;
> > +
> > + if (copy_to_user(udata, , sizeof(data)))
> > + ret = -EFAULT;
> > + else
> > + ret = tee_shm_get_fd(shm);
> > +
> > + /*
> > + * When user space closes the file descriptor the shared
> > memory
> > + * should be freed or if tee_shm_get_fd() failed then it will
> > + * be freed immediately.
> > + */
> > + tee_shm_put(shm);
> > + return ret;
> > +}
> > +
> > static int params_from_user(struct tee_context *ctx, struct
> > tee_param *params,
> > size_t num_params,
> > struct tee_ioctl_param __user *uparams)
> > @@ -829,6 +865,8 @@ static long tee_ioctl(struct file *filp,
> > unsigned int cmd, unsigned long arg)
> > return tee_ioctl_shm_alloc(ctx, uarg);
> > case TEE_IOC_SHM_REGISTER:
> > return tee_ioctl_shm_register(ctx, uarg);
> > + case TEE_IOC_SHM_REGISTER_FD:
> > + return tee_ioctl_shm_register_fd(ctx, uarg);
> > case TEE_IOC_OPEN_SESSION:
> > return tee_ioctl_open_session(ctx, uarg);
> > case TEE_IOC_INVOKE:
> > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> > index
Re: [PATCH v1 04/35] drm/modes: Introduce 480i and 576i modes
Hi Maxime,
Thanks for your patch!
On Fri, Jul 29, 2022 at 6:35 PM Maxime Ripard wrote:
> Multiple drivers (meson, vc4) define the analog TV 525-lines and 625-lines
> modes in the drivers.
Nit: strictly speaking these are not analog modes, but the digital
variants (ITU-R BT.656 and DVD-Video D1) of NTSC and PAL, using a
13.5 MHz sampling frequency for pixels.
In analog modes, the only discrete values are the number of lines, and
the frame/field rate (fixing the horizontal sync rate when combined).
The number of (in)visible pixels per line depends on the available
bandwidth. In a digital variant (which is anything generated by a
digital computer system), the latter depends on the pixel clock, which
can wildly differ from the 13.5 MHz used in the BT.656 standard. (e.g.
Amiga uses 7.09/14.19/28.38 MHz (PAL) or 7.16/14.32/28.64 MHz (NTSC)).
So I think we probably need some way to generate a PAL/NTSC-compatible
mode based not only on resolution, but also on pixel clock.
>
> Since those modes are fairly standards, and that we'll need to use them in
> more places in the future, let's move the meson definition into the
> framework.
>
> The meson one was chosen because vc4's isn't accurate and doesn't amount to
> 525 and 625 lines.
>
> Signed-off-by: Maxime Ripard
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -48,6 +48,24 @@
>
> #include "drm_crtc_internal.h"
>
> +const struct drm_display_mode drm_mode_480i = {
> + DRM_MODE("720x480i", DRM_MODE_TYPE_DRIVER, 13500,
> +720, 739, 801, 858, 0,
> +480, 488, 494, 525, 0,
> +DRM_MODE_FLAG_INTERLACE),
> + .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> +};
> +EXPORT_SYMBOL_GPL(drm_mode_480i);
> +
> +const struct drm_display_mode drm_mode_576i = {
> + DRM_MODE("720x576i", DRM_MODE_TYPE_DRIVER, 13500,
> +720, 732, 795, 864, 0,
> +576, 580, 586, 625, 0,
> +DRM_MODE_FLAG_INTERLACE),
> + .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3,
> +};
> +EXPORT_SYMBOL_GPL(drm_mode_576i);
> +
> /**
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [Linaro-mm-sig] [PATCH v2 3/5] dma-buf: Move all dma-bufs to dynamic locking specification
On 8/12/22 14:34, Christian König wrote: > > > Am 10.08.22 um 20:53 schrieb Dmitry Osipenko: >> On 8/10/22 21:25, Christian König wrote: >>> Am 10.08.22 um 19:49 schrieb Dmitry Osipenko: On 8/10/22 14:30, Christian König wrote: > Am 25.07.22 um 17:18 schrieb Dmitry Osipenko: >> This patch moves the non-dynamic dma-buf users over to the dynamic >> locking specification. The strict locking convention prevents >> deadlock >> situation for dma-buf importers and exporters. >> >> Previously the "unlocked" versions of the dma-buf API functions >> weren't >> taking the reservation lock and this patch makes them to take the >> lock. >> >> Intel and AMD GPU drivers already were mapping imported dma-bufs >> under >> the held lock, hence the "locked" variant of the functions are added >> for them and the drivers are updated to use the "locked" versions. > In general "Yes, please", but that won't be that easy. > > You not only need to change amdgpu and i915, but all drivers > implementing the map_dma_buf(), unmap_dma_buf() callbacks. > > Auditing all that code is a huge bunch of work. Hm, neither of drivers take the resv lock in map_dma_buf/unmap_dma_buf. It's easy to audit them all and I did it. So either I'm missing something or it doesn't take much time to check them all. Am I really missing something? >>> Ok, so this is only changing map/unmap now? >> It also vmap/vunmap and attach/detach: In the previous patch I added the >> _unlocked postfix to the func names and in this patch I made them all to >> actually take the lock. > > > Take your patch "[PATCH v2 2/5] drm/gem: Take reservation lock for > vmap/vunmap operations" as a blueprint on how to approach it. > > E.g. one callback at a time and then document the result in the end. Yeah, I'll do it for v3. I'm vaguely recalling that there was a problem when I wanted to split this patch in the past, but don't remember what it was.. maybe that problem is gone now, will see :) -- Best regards, Dmitry
Re: [Linaro-mm-sig] [PATCH v2 3/5] dma-buf: Move all dma-bufs to dynamic locking specification
Am 10.08.22 um 20:53 schrieb Dmitry Osipenko: On 8/10/22 21:25, Christian König wrote: Am 10.08.22 um 19:49 schrieb Dmitry Osipenko: On 8/10/22 14:30, Christian König wrote: Am 25.07.22 um 17:18 schrieb Dmitry Osipenko: This patch moves the non-dynamic dma-buf users over to the dynamic locking specification. The strict locking convention prevents deadlock situation for dma-buf importers and exporters. Previously the "unlocked" versions of the dma-buf API functions weren't taking the reservation lock and this patch makes them to take the lock. Intel and AMD GPU drivers already were mapping imported dma-bufs under the held lock, hence the "locked" variant of the functions are added for them and the drivers are updated to use the "locked" versions. In general "Yes, please", but that won't be that easy. You not only need to change amdgpu and i915, but all drivers implementing the map_dma_buf(), unmap_dma_buf() callbacks. Auditing all that code is a huge bunch of work. Hm, neither of drivers take the resv lock in map_dma_buf/unmap_dma_buf. It's easy to audit them all and I did it. So either I'm missing something or it doesn't take much time to check them all. Am I really missing something? Ok, so this is only changing map/unmap now? It also vmap/vunmap and attach/detach: In the previous patch I added the _unlocked postfix to the func names and in this patch I made them all to actually take the lock. Take your patch "[PATCH v2 2/5] drm/gem: Take reservation lock for vmap/vunmap operations" as a blueprint on how to approach it. E.g. one callback at a time and then document the result in the end. Regards, Christian. In this case please separate this from the documentation change. I'll factor out the doc in the v3. I would also drop the _locked postfix from the function name, just having _unlocked on all functions which are supposed to be called with the lock held should be sufficient. Noted for the v3. Thanks for looking into this, Christian. Thank you for the review.
[PATCH v3] drm/fb-helper: add virtual screen size check to drm_fb_helper_check_var()
Make sure that virtual screen size is not less than physical screen one. Reviewed-by: Geert Uytterhoeven Signed-off-by: Andrey Strachuk --- drivers/gpu/drm/drm_fb_helper.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 2d4cee6a10ff..ea07c97bc5a9 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1355,6 +1355,12 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, (drm_format_info_block_height(fb->format, 0) > 1)) return -EINVAL; + /* make sure that virtual resolution >= physical resolution */ + if (var->xres_virtual < var->xres) + var->xres_virtual = var->xres; + if (var->yres_virtual < var->yres) + var->yres_virtual = var->yres; + /* * Changes struct fb_var_screeninfo are currently not pushed back * to KMS, hence fail if different settings are requested. -- 2.25.1
Re: [PATCH v8 2/2] drm/gem: Don't map imported GEMs
On 8/11/22 02:19, Rob Clark wrote:
> On Wed, Aug 10, 2022 at 3:23 PM Dmitry Osipenko
> wrote:
>>
>> On 8/11/22 01:03, Rob Clark wrote:
>>> On Wed, Aug 10, 2022 at 12:26 PM Dmitry Osipenko
>>> wrote:
On 8/10/22 18:08, Rob Clark wrote:
> On Wed, Aug 10, 2022 at 4:47 AM Daniel Vetter wrote:
>>
>> On Wed, Jul 06, 2022 at 10:02:07AM +0300, Dmitry Osipenko wrote:
>>> On 7/6/22 00:48, Rob Clark wrote:
On Tue, Jul 5, 2022 at 4:51 AM Christian König
wrote:
>
> Am 01.07.22 um 11:02 schrieb Dmitry Osipenko:
>> Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers don't
>> handle imported dma-bufs properly, which results in mapping of
>> something
>> else than the imported dma-buf. On NVIDIA Tegra we get a hard lockup
>> when
>> userspace writes to the memory mapping of a dma-buf that was
>> imported into
>> Tegra's DRM GEM.
>>
>> Majority of DRM drivers prohibit mapping of the imported GEM objects.
>> Mapping of imported GEMs require special care from userspace since it
>> should sync dma-buf because mapping coherency of the exporter device
>> may
>> not match the DRM device. Let's prohibit the mapping for all DRM
>> drivers
>> for consistency.
>>
>> Suggested-by: Thomas Hellström
>> Signed-off-by: Dmitry Osipenko
>
> I'm pretty sure that this is the right approach, but it's certainly
> more
> than possible that somebody abused this already.
I suspect that this is abused if you run deqp cts on android.. ie. all
winsys buffers are dma-buf imports from gralloc. And then when you
hit readpix...
You might only hit this in scenarios with separate gpu and display (or
dGPU+iGPU) because self-imports are handled differently in
drm_gem_prime_import_dev().. and maybe not in cases where you end up
with a blit from tiled/compressed to linear.. maybe that narrows the
scope enough to just fix it in userspace?
>>>
>>> Given that that only drivers which use DRM-SHMEM potentially could've
>>> map imported dma-bufs (Panfrost, Lima) and they already don't allow to
>>> do that, I think we're good.
>>
>> So can I have an ack from Rob here or are there still questions that this
>> might go boom?
>>
>> Dmitry, since you have a bunch of patches merged now I think would also
>> be
>> good to get commit rights so you can drive this more yourself. I've asked
>> Daniel Stone to help you out with getting that.
>
> I *think* we'd be ok with this on msm, mostly just by dumb luck.
> Because the dma-buf's we import will be self-import. I'm less sure
> about panfrost (src/panfrost/lib/pan_bo.c doesn't seem to have a
> special path for imported dma-bufs either, and in that case they won't
> be self-imports.. but I guess no one has tried to run android cts on
> panfrost).
The last time I tried to mmap dma-buf imported to Panfrost didn't work
because Panfrost didn't implement something needed for that. I'll need
to take a look again because can't recall what it was.
Upd: I re-checked Panfrost using today's linux-next and mapping of
imported dma-buf works, I mapped imported buf from video decoder.
Apparently previously I had some local kernel change that broke the mapping.
> What about something less drastic to start, like (apologies for
> hand-edited patch):
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 86d670c71286..fc9ec42fa0ab 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1034,6 +1034,10 @@ int drm_gem_mmap_obj(struct drm_gem_object
> *obj, unsigned long obj_size,
> {
> int ret;
>
> + WARN_ON_ONCE(obj->import_attach);
This will hang NVIDIA Tegra, which is what this patch fixed initially.
If neither of upstream DRM drivers need to map imported dma-bufs and
never needed, then why do we need this?
>>>
>>> oh, tegra isn't using shmem helpers? I assumed it was. Well my point
>>> was to make a more targeted fail on tegra, and a WARN_ON for everyone
>>> else to make it clear that what they are doing is undefined behavior.
>>> Because so far existing userspace (or well, panfrost and freedreno at
>>> least, those are the two I know or checked) don't make special cases
>>> for mmap'ing against the dmabuf fd against the dmabuf fd instead of
>>> the drm device fd.
>>
>> It's not clear to me what bad Android does form yours comments. Does it
>> export dma-buf from GPU and then import it to GPU? If yes, then DRM core
>> has a check for the self-importing [1].
>>
>> [1]
>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_prime.c#L918
>>
>> If
Re: [PATCH] drm/amdgpu: remove useless condition in amdgpu_job_stop_all_jobs_on_sched()
@Alex was that one already picked up?
Am 25.07.22 um 18:40 schrieb Andrey Grodzovsky:
Reviewed-by: Andrey Grodzovsky
Andrey
On 2022-07-19 06:39, Andrey Strachuk wrote:
Local variable 'rq' is initialized by an address
of field of drm_sched_job, so it does not make
sense to compare 'rq' with NULL.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Andrey Strachuk
Fixes: 7c6e68c777f1 ("drm/amdgpu: Avoid HW GPU reset for RAS.")
---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 67f66f2f1809..600401f2a98f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -285,10 +285,6 @@ void amdgpu_job_stop_all_jobs_on_sched(struct
drm_gpu_scheduler *sched)
/* Signal all jobs not yet scheduled */
for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >=
DRM_SCHED_PRIORITY_MIN; i--) {
struct drm_sched_rq *rq = >sched_rq[i];
-
- if (!rq)
- continue;
-
spin_lock(>lock);
list_for_each_entry(s_entity, >entities, list) {
while ((s_job =
to_drm_sched_job(spsc_queue_pop(_entity->job_queue {
Re: [PATCH] drm/radeon: add a force flush to delay work when radeon
Am 11.08.22 um 09:25 schrieb Zhenneng Li:
Although radeon card fence and wait for gpu to finish processing current batch
rings,
there is still a corner case that radeon lockup work queue may not be fully
flushed,
and meanwhile the radeon_suspend_kms() function has called
pci_set_power_state() to
put device in D3hot state.
If I'm not completely mistaken the reset worker uses the suspend/resume
functionality as well to get the hardware into a working state again.
So if I'm not completely mistaken this here would lead to a deadlock,
please double check that.
Regards,
Christian.
Per PCI spec rev 4.0 on 5.3.1.4.1 D3hot State.
Configuration and Message requests are the only TLPs accepted by a Function in
the D3hot state. All other received Requests must be handled as Unsupported
Requests,
and all received Completions may optionally be handled as Unexpected
Completions.
This issue will happen in following logs:
Unable to handle kernel paging request at virtual address 8800e0008010
CPU 0 kworker/0:3(131): Oops 0
pc = [] ra = [] ps = Tainted: G
W
pc is at si_gpu_check_soft_reset+0x3c/0x240
ra is at si_dma_is_lockup+0x34/0xd0
v0 = t0 = fff08800e0008010 t1 = 0001
t2 = 8010 t3 = fff7e3c0 t4 = fff7e3c00258
t5 = t6 = 0001 t7 = fff7ef078000
s0 = fff7e3c016e8 s1 = fff7e3c0 s2 = fff7e3c00018
s3 = fff7e3c0 s4 = fff7fff59d80 s5 =
s6 = fff7ef07bd98
a0 = fff7e3c0 a1 = fff7e3c016e8 a2 = 0008
a3 = 0001 a4 = 8f5c28f5c28f5c29 a5 = 810f4338
t8 = 0275 t9 = 809b66f8 t10 = ff6769c5d964b800
t11= b886 pv = 811bea20 at =
gp = 81d89690 sp = aa814126
Disabling lock debugging due to kernel taint
Trace:
[] si_dma_is_lockup+0x34/0xd0
[] radeon_fence_check_lockup+0xd0/0x290
[] process_one_work+0x280/0x550
[] worker_thread+0x70/0x7c0
[] worker_thread+0x130/0x7c0
[] kthread+0x200/0x210
[] worker_thread+0x0/0x7c0
[] kthread+0x14c/0x210
[] ret_from_kernel_thread+0x18/0x20
[] kthread+0x0/0x210
Code: ad3e0008 43f0074a ad7e0018 ad9e0020 8c3001e8 40230101
<8821> 4821ed21
So force lockup work queue flush to fix this problem.
Signed-off-by: Zhenneng Li
---
drivers/gpu/drm/radeon/radeon_device.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 15692cb241fc..e608ca26780a 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1604,6 +1604,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool
suspend,
if (r) {
/* delay GPU reset to resume */
radeon_fence_driver_force_completion(rdev, i);
+ } else {
+ /* finish executing delayed work */
+ flush_delayed_work(>fence_drv[i].lockup_work);
}
}
Re: [PATCH 2/4] dt-bindings: display: sun6i-dsi: Add the A100 variant
On 12/08/2022 10:42, Samuel Holland wrote: > The "40nm" MIPI DSI controller found in the A100 and D1 SoCs has the > same register layout as previous SoC integrations. However, its module > clock now comes from the TCON, which means it no longer runs at a fixed > rate, so this needs to be distinguished in the driver. > > The controller also now uses pins on Port D instead of dedicated pins, > so it drops the separate power domain. > > Signed-off-by: Samuel Holland > --- > Removal of the vcc-dsi-supply is maybe a bit questionable. Since there > is no "VCC-DSI" pin anymore, it's not obvious which pin actually does > power the DSI controller/PHY. Possibly power comes from VCC-PD or VCC-IO > or VCC-LVDS. So far, all boards have all of these as always-on supplies, > so it is hard to test. > > .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 28 +++ > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml > b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml > index ae55ef3fb1fe..c53c25b87bd4 100644 > --- > a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml > +++ > b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml > @@ -12,9 +12,14 @@ maintainers: > > properties: >compatible: > -enum: > - - allwinner,sun6i-a31-mipi-dsi > - - allwinner,sun50i-a64-mipi-dsi > +oneOf: > + - enum: > + - allwinner,sun6i-a31-mipi-dsi > + - allwinner,sun50i-a64-mipi-dsi > + - allwinner,sun50i-a100-mipi-dsi While you are moving code, how about bringing alphabetical order? > + - items: > + - const: allwinner,sun20i-d1-mipi-dsi > + - const: allwinner,sun50i-a100-mipi-dsi > >reg: > maxItems: 1 > @@ -59,7 +64,6 @@ required: >- phys >- phy-names >- resets > - - vcc-dsi-supply >- port > > allOf: > @@ -68,7 +72,9 @@ allOf: >properties: > compatible: >contains: > -const: allwinner,sun6i-a31-mipi-dsi > +enum: > + - allwinner,sun6i-a31-mipi-dsi > + - allwinner,sun50i-a100-mipi-dsi Here as well > > then: >properties: > @@ -83,6 +89,18 @@ allOf: > clocks: >maxItems: 1 > > + - if: > + properties: > +compatible: > + contains: > +enum: > + - allwinner,sun6i-a31-mipi-dsi > + - allwinner,sun50i-a64-mipi-dsi and here Best regards, Krzysztof
Re: [PATCH 1/4] dt-bindings: display: sun6i-dsi: Fix clock conditional
On 12/08/2022 10:42, Samuel Holland wrote:
> The A64 case should have limited maxItems, instead of duplicating the
> minItems value from the main binding. While here, simplify the binding
> by making this an "else" case of the two-clock conditional block.
>
> Fixes: fe5040f2843a ("dt-bindings: sun6i-dsi: Document A64 MIPI-DSI
> controller")
> Signed-off-by: Samuel Holland
Acked-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
Hi Etienne,
at least I don't see anything obvious broken.
Just two comments:
1. Dmitry is working on a change which renames some functions and makes
it mandatory to call them with the dma_resv lock held.
Depending on how you want to upstream this change you will certainly run
into conflicts with that.
2. Would it be possible to do this dynamically? In other words does the
tee driver has a concept of buffers moving around?
Am 11.08.22 um 15:56 schrieb Olivier Masse:
From: Etienne Carriere
This change allows userland to create a tee_shm object that refers
to a dmabuf reference.
Userland provides a dmabuf file descriptor as buffer reference.
The created tee_shm object exported as a brand new dmabuf reference
used to provide a clean fd to userland. Userland shall closed this new
fd to release the tee_shm object resources. The initial dmabuf resources
are tracked independently through original dmabuf file descriptor.
Once the buffer is registered and until it is released, TEE driver
keeps a refcount on the registered dmabuf structure.
This change only support dmabuf references that relates to physically
contiguous memory buffers.
That sounds like a pretty hard restriction, but I obviously don't see
how to avoid it either.
Regards,
Christian.
New tee_shm flag to identify tee_shm objects built from a registered
dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged with
TEE_SHM_EXT_DMA_BUF.
Co-Developed-by: Etienne Carriere
Signed-off-by: Olivier Masse
From: https://github.com/linaro-swg/linux.git
(cherry picked from commit 41e21e5c405530590dc2dd10b2a8dbe64589840f)
---
drivers/tee/tee_core.c | 38 +++
drivers/tee/tee_shm.c| 99 +++-
include/linux/tee_drv.h | 11 +
include/uapi/linux/tee.h | 29
4 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 8aa1a4836b92..7c45cbf85eb9 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context *ctx,
return ret;
}
+static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
+struct tee_ioctl_shm_register_fd_data
__user *udata)
+{
+ struct tee_ioctl_shm_register_fd_data data;
+ struct tee_shm *shm;
+ long ret;
+
+ if (copy_from_user(, udata, sizeof(data)))
+ return -EFAULT;
+
+ /* Currently no input flags are supported */
+ if (data.flags)
+ return -EINVAL;
+
+ shm = tee_shm_register_fd(ctx, data.fd);
+ if (IS_ERR(shm))
+ return -EINVAL;
+
+ data.id = shm->id;
+ data.flags = shm->flags;
+ data.size = shm->size;
+
+ if (copy_to_user(udata, , sizeof(data)))
+ ret = -EFAULT;
+ else
+ ret = tee_shm_get_fd(shm);
+
+ /*
+* When user space closes the file descriptor the shared memory
+* should be freed or if tee_shm_get_fd() failed then it will
+* be freed immediately.
+*/
+ tee_shm_put(shm);
+ return ret;
+}
+
static int params_from_user(struct tee_context *ctx, struct tee_param *params,
size_t num_params,
struct tee_ioctl_param __user *uparams)
@@ -829,6 +865,8 @@ static long tee_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
return tee_ioctl_shm_alloc(ctx, uarg);
case TEE_IOC_SHM_REGISTER:
return tee_ioctl_shm_register(ctx, uarg);
+ case TEE_IOC_SHM_REGISTER_FD:
+ return tee_ioctl_shm_register_fd(ctx, uarg);
case TEE_IOC_OPEN_SESSION:
return tee_ioctl_open_session(ctx, uarg);
case TEE_IOC_INVOKE:
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 836872467dc6..55a3fbbb022e 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -4,6 +4,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -12,6 +13,14 @@
#include
#include "tee_private.h"
+/* extra references appended to shm object for registered shared memory */
+struct tee_shm_dmabuf_ref {
+ struct tee_shm shm;
+ struct dma_buf *dmabuf;
+ struct dma_buf_attachment *attach;
+ struct sg_table *sgt;
+};
+
static void shm_put_kernel_pages(struct page **pages, size_t page_count)
{
size_t n;
@@ -71,7 +80,16 @@ static void release_registered_pages(struct tee_shm *shm)
static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm)
{
- if (shm->flags & TEE_SHM_POOL) {
+ if (shm->flags & TEE_SHM_EXT_DMA_BUF) {
+ struct tee_shm_dmabuf_ref *ref;
+
+ ref = container_of(shm, struct tee_shm_dmabuf_ref, shm);
+ dma_buf_unmap_attachment(ref->attach, ref->sgt,
+DMA_BIDIRECTIONAL);
+
+
[PATCH v2 5/7] gpu: drm: simplify drivers using devm_regulator_*get_enable*()
Simplify drivers using managed "regulator get and enable".
meson:
Use the devm_regulator_get_enable_optional(). Also drop the seemingly
unused struct member 'hdmi_supply'.
sii902x:
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
---
RFCv1 => v2:
- Change also sii902x to use devm_regulator_bulk_get_enable()
Please note - this is only compile-tested due to the lack of HW. Careful
review and testing is _highly_ appreciated.
---
drivers/gpu/drm/bridge/sii902x.c | 22 +++---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
2 files changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 65549fbfdc87..4bf572b7ca77 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -170,7 +170,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1070,6 +1069,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = >dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1120,27 +1120,13 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(>mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies),
supplies);
if (ret < 0) {
dev_err_probe(dev, ret, "Failed to enable supplies");
return ret;
}
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static int sii902x_remove(struct i2c_client *client)
@@ -1150,8 +1136,6 @@ static int sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(>bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
return 0;
}
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
*meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = _dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
2.37.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde
[PATCH v2 0/7] Devm helpers for regulator get and enable
Devm helpers for regulator get and enable First patch in the series is actually just a simple documentation fix which could be taken in as it is now. A few* drivers seem to use pattern demonstrated by pseudocode: - devm_regulator_get() - regulator_enable() - devm_add_action_or_reset(regulator_disable()) Introducing devm helpers for this pattern would remove bunch of code from drivers. Typically following: - replace 3 calls (devm_regulator_get[_optional](), regulator_enable(), devm_add_action_or_reset()) with just one (devm_regulator_get_enable[_optional]()). - drop disable callback. - remove stored pointer to struct regulator - which can lead to problem when an devm action for regulator_disable is used. I believe this simplifies things by removing some dublicated code. The suggested managed 'get_enable' APIs do not return the pointer to regulators for user because any call to regulator_disable() (or regulator_enable()) may easily lead to regulator enable count imbalance upon device detach. (Eg, if someone calls regulator_disable() and the device is then detached before user has re-enabled the regulator). Not returning the pointer to obtained regulator to caller is a good hint that the enable/disable should not be manually handled when these APIs are used. OTOH, not returning the pointer reduces the use-cases by not allowing the consumers to perform other regulator actions. For example request the voltages. A few drivers which used the "get, enable, devm_action_to_disable" did also query the voltages. The API does not suit needs of such users. This series reowrks only a few drivers as I am short of time. So, there is still plenty of fish in the sea for people who like to improve the code (or count the beans ;]). Finally - most of the converted drivers have not been tested (other than compile-tested) due to lack of HW. All reviews and testing is _highly_ appreciated (as always!). Revision history: RFCv1 => v2: - Add devm_regulator_bulk_get_enable() and devm_regulator_bulk_put() - Convert a couple of drivers to use the new devm_regulator_bulk_get_enable(). - Squash all IIO patches into one. Patch 1: Fix docmentation (devres API list) for regulator APIs Patch 2: The new devm helpers. Patch 3: Add new devm-helper APIs to docs. Patch 4: simplified CLK driver(s) Patch 5: simplified GPU driver(s) Patch 6: simplified hwmon driver(s) Patch 7: simplified IIO driver(s) --- Matti Vaittinen (7): docs: devres: regulator: Add missing devm_* functions to devres.rst regulator: Add devm helpers for get and enable docs: devres: regulator: Add new get_enable functions to devres.rst clk: cdce925: simplify using devm_regulator_get_enable() gpu: drm: simplify drivers using devm_regulator_*get_enable*() hwmon: lm90: simplify using devm_regulator_get_enable() iio: Simplify drivers using devm_regulator_*get_enable() .../driver-api/driver-model/devres.rst| 11 ++ drivers/clk/clk-cdce925.c | 21 +-- drivers/gpu/drm/bridge/sii902x.c | 22 +-- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +-- drivers/hwmon/lm90.c | 21 +-- drivers/iio/adc/ad7192.c | 15 +- drivers/iio/dac/ltc2688.c | 23 +-- drivers/iio/gyro/bmg160_core.c| 24 +-- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 - drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 30 +--- drivers/regulator/devres.c| 164 ++ include/linux/regulator/consumer.h| 27 +++ 12 files changed, 227 insertions(+), 156 deletions(-) -- 2.37.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =] signature.asc Description: PGP signature
Re: [PATCH] dt-bindings: display: sun4i: Add D1 TCONs to conditionals
On 12/08/2022 10:37, Samuel Holland wrote:
> When adding the D1 TCON bindings, I missed the conditional blocks that
> restrict the binding for TCON LCD vs TCON TV hardware. Add the D1 TCON
> variants to the appropriate blocks for DE2 TCON LCDs and TCON TVs.
>
> Fixes: ae5a5d26c15c ("dt-bindings: display: Add D1 display engine
> compatibles")
> Signed-off-by: Samuel Holland
Acked-by: Krzysztof Kozlowski
Best regards,
Krzysztof
[PATCH 4/4] drm/sun4i: dsi: Add the A100 variant
The A100 variant of the MIPI DSI controller now gets its module clock
from the TCON via the TCON TOP, so the clock rate cannot be set to a
fixed value. Otherwise, it appears to be the same as the A31 variant.
Signed-off-by: Samuel Holland
---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 6479ade416b9..5db5ecdc2fc6 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1222,6 +1222,10 @@ static const struct sun6i_dsi_variant
sun6i_a31_mipi_dsi_variant = {
static const struct sun6i_dsi_variant sun50i_a64_mipi_dsi_variant = {
};
+static const struct sun6i_dsi_variant sun50i_a100_mipi_dsi_variant = {
+ .has_mod_clk= true,
+};
+
static const struct of_device_id sun6i_dsi_of_table[] = {
{
.compatible = "allwinner,sun6i-a31-mipi-dsi",
@@ -1231,6 +1235,10 @@ static const struct of_device_id sun6i_dsi_of_table[] = {
.compatible = "allwinner,sun50i-a64-mipi-dsi",
.data = _a64_mipi_dsi_variant,
},
+ {
+ .compatible = "allwinner,sun50i-a100-mipi-dsi",
+ .data = _a100_mipi_dsi_variant,
+ },
{ }
};
MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
--
2.35.1
[PATCH 3/4] drm/sun4i: dsi: Add a variant structure
Replace the ad-hoc calls to of_device_is_compatible() with a structure
describing the differences between variants. This is in preparation for
adding more variants to the driver.
Signed-off-by: Samuel Holland
---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 50 +-
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 7
2 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index b4dfa166eccd..6479ade416b9 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1101,12 +1101,16 @@ static const struct component_ops sun6i_dsi_ops = {
static int sun6i_dsi_probe(struct platform_device *pdev)
{
+ const struct sun6i_dsi_variant *variant;
struct device *dev = >dev;
- const char *bus_clk_name = NULL;
struct sun6i_dsi *dsi;
void __iomem *base;
int ret;
+ variant = device_get_match_data(dev);
+ if (!variant)
+ return -EINVAL;
+
dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
if (!dsi)
return -ENOMEM;
@@ -1114,10 +1118,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
dsi->dev = dev;
dsi->host.ops = _dsi_host_ops;
dsi->host.dev = dev;
-
- if (of_device_is_compatible(dev->of_node,
- "allwinner,sun6i-a31-mipi-dsi"))
- bus_clk_name = "bus";
+ dsi->variant = variant;
base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base)) {
@@ -1142,7 +1143,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
return PTR_ERR(dsi->regs);
}
- dsi->bus_clk = devm_clk_get(dev, bus_clk_name);
+ dsi->bus_clk = devm_clk_get(dev, variant->has_mod_clk ? "bus" : NULL);
if (IS_ERR(dsi->bus_clk))
return dev_err_probe(dev, PTR_ERR(dsi->bus_clk),
"Couldn't get the DSI bus clock\n");
@@ -1151,21 +1152,21 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
if (ret)
return ret;
- if (of_device_is_compatible(dev->of_node,
- "allwinner,sun6i-a31-mipi-dsi")) {
+ if (variant->has_mod_clk) {
dsi->mod_clk = devm_clk_get(dev, "mod");
if (IS_ERR(dsi->mod_clk)) {
dev_err(dev, "Couldn't get the DSI mod clock\n");
ret = PTR_ERR(dsi->mod_clk);
goto err_attach_clk;
}
- }
- /*
-* In order to operate properly, that clock seems to be always
-* set to 297MHz.
-*/
- clk_set_rate_exclusive(dsi->mod_clk, 29700);
+ /*
+* In order to operate properly, the module clock on the
+* A31 variant always seems to be set to 297MHz.
+*/
+ if (variant->set_mod_clk)
+ clk_set_rate_exclusive(dsi->mod_clk, 29700);
+ }
dsi->dphy = devm_phy_get(dev, "dphy");
if (IS_ERR(dsi->dphy)) {
@@ -1205,16 +1206,31 @@ static int sun6i_dsi_remove(struct platform_device
*pdev)
component_del(>dev, _dsi_ops);
mipi_dsi_host_unregister(>host);
- clk_rate_exclusive_put(dsi->mod_clk);
+ if (dsi->variant->has_mod_clk && dsi->variant->set_mod_clk)
+ clk_rate_exclusive_put(dsi->mod_clk);
regmap_mmio_detach_clk(dsi->regs);
return 0;
}
+static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi_variant = {
+ .has_mod_clk= true,
+ .set_mod_clk= true,
+};
+
+static const struct sun6i_dsi_variant sun50i_a64_mipi_dsi_variant = {
+};
+
static const struct of_device_id sun6i_dsi_of_table[] = {
- { .compatible = "allwinner,sun6i-a31-mipi-dsi" },
- { .compatible = "allwinner,sun50i-a64-mipi-dsi" },
+ {
+ .compatible = "allwinner,sun6i-a31-mipi-dsi",
+ .data = _a31_mipi_dsi_variant,
+ },
+ {
+ .compatible = "allwinner,sun50i-a64-mipi-dsi",
+ .data = _a64_mipi_dsi_variant,
+ },
{ }
};
MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index c863900ae3b4..f1ddefe0f554 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -15,6 +15,11 @@
#define SUN6I_DSI_TCON_DIV 4
+struct sun6i_dsi_variant {
+ boolhas_mod_clk;
+ boolset_mod_clk;
+};
+
struct sun6i_dsi {
struct drm_connectorconnector;
struct drm_encoder encoder;
@@ -31,6 +36,8 @@ struct sun6i_dsi {
struct mipi_dsi_device *device;
struct drm_device *drm;
struct
[PATCH 2/4] dt-bindings: display: sun6i-dsi: Add the A100 variant
The "40nm" MIPI DSI controller found in the A100 and D1 SoCs has the same register layout as previous SoC integrations. However, its module clock now comes from the TCON, which means it no longer runs at a fixed rate, so this needs to be distinguished in the driver. The controller also now uses pins on Port D instead of dedicated pins, so it drops the separate power domain. Signed-off-by: Samuel Holland --- Removal of the vcc-dsi-supply is maybe a bit questionable. Since there is no "VCC-DSI" pin anymore, it's not obvious which pin actually does power the DSI controller/PHY. Possibly power comes from VCC-PD or VCC-IO or VCC-LVDS. So far, all boards have all of these as always-on supplies, so it is hard to test. .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 28 +++ 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml index ae55ef3fb1fe..c53c25b87bd4 100644 --- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml @@ -12,9 +12,14 @@ maintainers: properties: compatible: -enum: - - allwinner,sun6i-a31-mipi-dsi - - allwinner,sun50i-a64-mipi-dsi +oneOf: + - enum: + - allwinner,sun6i-a31-mipi-dsi + - allwinner,sun50i-a64-mipi-dsi + - allwinner,sun50i-a100-mipi-dsi + - items: + - const: allwinner,sun20i-d1-mipi-dsi + - const: allwinner,sun50i-a100-mipi-dsi reg: maxItems: 1 @@ -59,7 +64,6 @@ required: - phys - phy-names - resets - - vcc-dsi-supply - port allOf: @@ -68,7 +72,9 @@ allOf: properties: compatible: contains: -const: allwinner,sun6i-a31-mipi-dsi +enum: + - allwinner,sun6i-a31-mipi-dsi + - allwinner,sun50i-a100-mipi-dsi then: properties: @@ -83,6 +89,18 @@ allOf: clocks: maxItems: 1 + - if: + properties: +compatible: + contains: +enum: + - allwinner,sun6i-a31-mipi-dsi + - allwinner,sun50i-a64-mipi-dsi + +then: + required: +- vcc-dsi-supply + unevaluatedProperties: false examples: -- 2.35.1
[PATCH 0/4] drm/sun4i: dsi: Support the A100/D1 controller variant
This series adds support for the digital part of the DSI controller found in the A100 and D1 SoCs (plus T7, which is not supported by mainline Linux). There are two changes to the hardware integration: 1) the module clock routes through the TCON TOP, and 2) the separate I/O domain is removed. The actual register interface appears to be the same as before. The register definitions in the D1 BSP exactly match the A64 BSP. The BSP describes this as the "40nm" DSI controller variant. There is also a "28nm" variant with a different register interface; that one is found in a different subset of SoCs (V5 and A50). A100/D1 also come with an updated DPHY, described by the BSP as a "combo" PHY, which is now also used for LVDS channel 0. (LVDS and DSI share the same pins on Port D.) Since that is a different subsystem, I am sending that as a separate series. Samuel Holland (4): dt-bindings: display: sun6i-dsi: Fix clock conditional dt-bindings: display: sun6i-dsi: Add the A100 variant drm/sun4i: dsi: Add a variant structure drm/sun4i: dsi: Add the A100 variant .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 30 +++--- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c| 58 +-- drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h| 7 +++ 3 files changed, 69 insertions(+), 26 deletions(-) -- 2.35.1
[PATCH 1/4] dt-bindings: display: sun6i-dsi: Fix clock conditional
The A64 case should have limited maxItems, instead of duplicating the
minItems value from the main binding. While here, simplify the binding
by making this an "else" case of the two-clock conditional block.
Fixes: fe5040f2843a ("dt-bindings: sun6i-dsi: Document A64 MIPI-DSI controller")
Signed-off-by: Samuel Holland
---
.../bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
index bf0bdf54e5f9..ae55ef3fb1fe 100644
---
a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
+++
b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
@@ -78,16 +78,10 @@ allOf:
required:
- clock-names
- - if:
- properties:
-compatible:
- contains:
-const: allwinner,sun50i-a64-mipi-dsi
-
-then:
+else:
properties:
clocks:
- minItems: 1
+ maxItems: 1
unevaluatedProperties: false
--
2.35.1
[PATCH] dt-bindings: display: sun4i: Add D1 TCONs to conditionals
When adding the D1 TCON bindings, I missed the conditional blocks that
restrict the binding for TCON LCD vs TCON TV hardware. Add the D1 TCON
variants to the appropriate blocks for DE2 TCON LCDs and TCON TVs.
Fixes: ae5a5d26c15c ("dt-bindings: display: Add D1 display engine compatibles")
Signed-off-by: Samuel Holland
---
.../devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml | 4
1 file changed, 4 insertions(+)
diff --git
a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
index 4a92a4c7dcd7..f8168986a0a9 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
@@ -224,43 +224,45 @@ allOf:
- const: ahb
- const: tcon-ch0
- const: lvds-alt
- if:
properties:
compatible:
contains:
enum:
- allwinner,sun8i-a83t-tcon-lcd
- allwinner,sun8i-v3s-tcon
- allwinner,sun9i-a80-tcon-lcd
+ - allwinner,sun20i-d1-tcon-lcd
then:
properties:
clocks:
minItems: 2
clock-names:
items:
- const: ahb
- const: tcon-ch0
- if:
properties:
compatible:
contains:
enum:
- allwinner,sun8i-a83t-tcon-tv
- allwinner,sun8i-r40-tcon-tv
- allwinner,sun9i-a80-tcon-tv
+ - allwinner,sun20i-d1-tcon-tv
then:
properties:
clocks:
minItems: 2
clock-names:
items:
- const: ahb
- const: tcon-ch1
- if:
@@ -269,40 +271,42 @@ allOf:
contains:
enum:
- allwinner,sun5i-a13-tcon
- allwinner,sun6i-a31-tcon
- allwinner,sun6i-a31s-tcon
- allwinner,sun7i-a20-tcon
- allwinner,sun8i-a23-tcon
- allwinner,sun8i-a33-tcon
- allwinner,sun8i-v3s-tcon
- allwinner,sun9i-a80-tcon-lcd
- allwinner,sun4i-a10-tcon
- allwinner,sun8i-a83t-tcon-lcd
+ - allwinner,sun20i-d1-tcon-lcd
then:
required:
- "#clock-cells"
- clock-output-names
- if:
properties:
compatible:
contains:
enum:
- allwinner,sun6i-a31-tcon
- allwinner,sun6i-a31s-tcon
- allwinner,sun8i-a23-tcon
- allwinner,sun8i-a33-tcon
- allwinner,sun8i-a83t-tcon-lcd
+ - allwinner,sun20i-d1-tcon-lcd
then:
properties:
resets:
minItems: 2
reset-names:
items:
- const: lcd
- const: lvds
- if:
--
2.35.1
Re: [Intel-gfx] [PATCH] drm/i915/guc: clear stalled request after a reset
On 11/08/2022 22:08, Daniele Ceraolo Spurio wrote:
If the GuC CTs are full and we need to stall the request submission
while waiting for space, we save the stalled request and where the stall
occurred; when the CTs have space again we pick up the request submission
from where we left off.
How serious is it? Statement always was CT buffers can never get full
outside the pathological IGT test cases. So I am wondering if this is in
the category of fix for correctness or actually the CT buffers can get
full in normal use so it is imperative to fix.
Regards,
Tvrtko
If a full GT reset occurs, the state of all contexts is cleared and all
non-guilty requests are unsubmitted, therefore we need to restart the
stalled request submission from scratch. To make sure that we do so,
clear the saved request after a reset.
Fixes note: the patch that introduced the bug is in 5.15, but no
officially supported platform had GuC submission enabled by default
in that kernel, so the backport to that particular version (and only
that one) can potentially be skipped.
Fixes: 925dc1cf58ed ("drm/i915/guc: Implement GuC submission tasklet")
Signed-off-by: Daniele Ceraolo Spurio
Cc: Matthew Brost
Cc: John Harrison
Cc: # v5.15+
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 0d17da77e787..0d56b615bf78 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -4002,6 +4002,13 @@ static inline void guc_init_lrc_mapping(struct intel_guc
*guc)
/* make sure all descriptors are clean... */
xa_destroy(>context_lookup);
+ /*
+* A reset might have occurred while we had a pending stalled request,
+* so make sure we clean that up.
+*/
+ guc->stalled_request = NULL;
+ guc->submission_stall_reason = STALL_NONE;
+
/*
* Some contexts might have been pinned before we enabled GuC
* submission, so we need to add them to the GuC bookeeping.
[PATCH] drm/vmwgfx: Fix comment typo
The double `to' is duplicated in line 384, remove one,The double `to' is duplicated in line 412, remove one. Signed-off-by: min tang --- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c index d49de4905efa..82b9d33ecc84 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c @@ -381,7 +381,7 @@ static int vmw_execbuf_res_noctx_val_add(struct vmw_sw_context *sw_context, } /** - * vmw_view_res_val_add - Add a view and the surface it's pointing to to the + * vmw_view_res_val_add - Add a view and the surface it's pointing to the * validation list * * @sw_context: The software context holding the validation list. @@ -409,7 +409,7 @@ static int vmw_view_res_val_add(struct vmw_sw_context *sw_context, /** * vmw_view_id_val_add - Look up a view and add it and the surface it's pointing - * to to the validation list. + * to the validation list. * * @sw_context: The software context holding the validation list. * @view_type: The view type to look up. -- 2.17.1
Re: [ldv-project] [PATCH v2] drm/fb-helper: add virtual screen size check to drm_fb_helper_check_var()
For v2 I would suggest to update description to something like this:
Make sure that virtual screen size is not less than physical screen one.
and comment to:
/* make sure that virtual resolution >= physical resolution */
--
Alexey
On 11.08.2022 17:54, Geert Uytterhoeven wrote:
> Hi Andrey,
>
> On Thu, Aug 11, 2022 at 4:49 PM Andrey Strachuk wrote:
>> Add virtual screen size check to drm_fb_helper_check_var() in
>> order to validate userspace input.
>>
>> Found by Linux Verification Center (linuxtesting.org) with syzkaller.
>>
>> Signed-off-by: Andrey Strachuk
>
> Thanks for the update!
>
>> Fixes: 785b93ef8c30 ("drm/kms: move driver specific fb common code to helper
>> functions (v2)")
>
> I'd drop the Fixes tag completely, as the bug was present in the
> intel and radeon drivers before. But probably it doesn't matter, as no one
> is gonna backport this to v2.6.31 and earlier ;-)
>
> Reviewed-by: Geert Uytterhoeven
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
>
> ___
> ldv-project mailing list
> ldv-proj...@linuxtesting.org
> http://linuxtesting.org/cgi-bin/mailman/listinfo/ldv-project
>
[PATCH] drm/vmwgfx: Fix comment typo
The double `should' is duplicated in line 72, remove one.
Signed-off-by: min tang
---
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 1d1c8b82c898..7c04e8150fe2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -69,7 +69,7 @@ struct vmw_du_update_plane {
*
* Some surface resource or buffer object need some extra cmd submission
* like update GB image for proxy surface and define a GMRFB for screen
-* object. That should should be done here as this callback will be
+* object. That should be done here as this callback will be
* called after FIFO allocation with the address of command buufer.
*
* This callback is optional.
--
2.17.1
[PATCH 2/3] drm/i915/gt: Replace kmap() with kmap_local_page()
kmap() is being deprecated in favor of kmap_local_page().
There are two main problems with kmap(): (1) It comes with an overhead as
mapping space is restricted and protected by a global lock for
synchronization and (2) it also requires global TLB invalidation when the
kmap’s pool wraps and it might block when the mapping space is fully
utilized until a slot becomes available.
With kmap_local_page() the mappings are per thread, CPU local, can take
page faults, and can be called from any context (including interrupts).
It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore,
the tasks can be preempted and, when they are scheduled to run again, the
kernel virtual addresses are restored and are still valid.
Since its use in i915/gt is safe everywhere, it should be preferred.
Therefore, replace kmap() with kmap_local_page() in i915/gt
Suggested-by: Ira Weiny
Signed-off-by: Fabio M. De Francesco
---
drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
drivers/gpu/drm/i915/gt/shmem_utils.c| 11 ---
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
index 6ebda3d65086..21d8ce40b897 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
@@ -747,7 +747,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, [i], 64);
@@ -755,7 +755,7 @@ static void swizzle_page(struct page *page)
memcpy([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 402f085f3a02..48edbb8a33e5 100644
--- a/drivers/gpu/drm/i915/gt/shmem_utils.c
+++ b/drivers/gpu/drm/i915/gt/shmem_utils.c
@@ -98,22 +98,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;
@@ -140,11 +137,11 @@ int shmem_read_to_iosys_map(struct file *file, loff_t off,
if (IS_ERR(page))
return PTR_ERR(page);
- vaddr = kmap(page);
+ vaddr = kmap_local_page(page);
iosys_map_memcpy_to(map, map_off, vaddr + offset_in_page(off),
this);
mark_page_accessed(page);
- kunmap(page);
+ kunmap_local(vaddr);
put_page(page);
len -= this;
--
2.37.1
[PATCH 0/3] drm/i915: Replace kmap() with kmap_local_page()
kmap() is being deprecated in favor of kmap_local_page(). There are two main problems with kmap(): (1) It comes with an overhead as mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap’s pool wraps and it might block when the mapping space is fully utilized until a slot becomes available. With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored and still valid. Since its use in drm/i915 is safe everywhere, it should be preferred. Therefore, replace kmap() with kmap_local_page() in drm/i915. These changes should be tested in an 32 bits system, booting a kernel with HIGHMEM enabled. Unfortunately I have no i915 based hardware, therefore any help with testing would be greatly appreciated. Suggested-by: Ira Weiny Signed-off-by: Fabio M. De Francesco Fabio M. De Francesco (3): drm/i915: Replace kmap() with kmap_local_page() drm/i915/gt: Replace kmap() with kmap_local_page() drm/i915/gem: Replace kmap() with kmap_local_page() 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 | 11 --- drivers/gpu/drm/i915/i915_gem.c| 8 5 files changed, 16 insertions(+), 21 deletions(-) -- 2.37.1
[PATCH] drm/ttm: Fix comment typo
The double `sure' is duplicated in line 454, remove one. Signed-off-by: min tang --- drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c index 7bc99b1279f7..2e9349e01e8e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c @@ -450,7 +450,7 @@ vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf) } /* -* If we don't track dirty using the MKWRITE method, make sure +* If we don't track dirty using the MKWRITE method, make * sure the page protection is write-enabled so we don't get * a lot of unnecessary write faults. */ -- 2.17.1
[PATCH] drm/i915: Fix comment typo
The double `for' is duplicated in line 2537, remove one. Signed-off-by: min tang --- drivers/gpu/drm/i915/i915_reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ccb67eec1bd..617a33e4bbb6 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2534,7 +2534,7 @@ * HDMI/DP bits are g4x+ * * WARNING: Bspec for hpd status bits on gen4 seems to be completely confused. - * Please check the detailed lore in the commit message for for experimental + * Please check the detailed lore in the commit message for experimental * evidence. */ /* Bspec says GM45 should match G4X/VLV/CHV, but reality disagrees */ -- 2.17.1
Re: [PATCH] drm/bridge_connector: enable HPD by default if supported
Hi, Nikita, All We have one X15 Android build based on the android mainline kernel, and it failed to boot with this change because of one kernel panic. And it would boot to the home screen if this change is reverted. Could you please help have a look and give suggestions on what should be done next to work with this change? The kernel panic is something like the following, for details please check the link here: http://ix.io/47mc [ 38.784301] Internal error: : 1211 [#2] PREEMPT SMP ARM [ 38.784301] Modules linked in: [ 38.788940] Kernel panic - not syncing: Fatal exception in interrupt [ 38.794189] snd_soc_omap_hdmi ti_tpd12s015 pvrsrvkm(O) display_connector omap_wdt omap_aes_driver ti_vpe ti_vpdma ti_csc ti_sc v4l2_mem2mem videobuf2_dma_contig omap_sham rtc_omap snd_soc_tlv320aic3x_i2c snd_soc_tlv320aic3x at24 rtc_palmas omap_des omap_crypto libdes crypto_engine rtc_ds1307 omap_hdq wire phy_gmii_sel ahci_platform libahci_platform libahci libata scsi_mod bsg scsi_common snd_soc_simple_card snd_soc_simple_card_utils [ 38.842315] CPU: 1 PID: 454 Comm: vsync Tainted: G D W O 5.19.0-02213-g3aeacdb764a2 #1 [ 38.851226] Hardware name: Generic DRA74X (Flattened Device Tree) [ 38.857360] PC is at dispc_write_irqenable+0x1c/0x38 [ 38.862335] LR is at omap_irq_enable_vblank+0xbc/0xd8 [ 38.867431] pc : []lr : []psr: 60080093 [ 38.873718] sp : ea281d40 ip : fp : c3ae3008 [ 38.878967] r10: r9 : c3a89cb8 r8 : 60080093 [ 38.884216] r7 : c398f000 r6 : 0812d64c r5 : c398f000 r4 : c398f184 [ 38.890777] r3 : e6137018 r2 : 0812d64c r1 : 0812d64c r0 : c3846000 [ 38.897338] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user [ 38.904571] Control: 30c5387d Table: 84e801c0 DAC: [ 38.910339] Register r0 information: slab kmalloc-8k start c3846000 pointer offset 0 size 8192 [ 38.919006] Register r1 information: non-paged memory [ 38.924102] Register r2 information: non-paged memory [ 38.929168] Register r3 information: 0-page vmalloc region starting at 0xe6137000 allocated at __devm_ioremap_resource+0x108/0x1f0 [ 38.940979] Register r4 information: slab kmalloc-512 start c398f000 pointer offset 388 size 512 [ 38.949829] Register r5 information: slab kmalloc-512 start c398f000 pointer offset 0 size 512 [ 38.958496] Register r6 information: non-paged memory [ 38.963562] Register r7 information: slab kmalloc-512 start c398f000 pointer offset 0 size 512 [ 38.972229] Register r8 information: non-paged memory [ 38.977294] Register r9 information: slab kmalloc-1k start c3a89c00 pointer offset 184 size 1024 [ 38.986145] Register r10 information: NULL pointer [ 38.990966] Register r11 information: slab kmalloc-2k start c3ae3000 pointer offset 8 size 2048 [ 38.999725] Register r12 information: NULL pointer [ 39.004547] Process vsync (pid: 454, stack limit = 0xd381b509) [ 39.010406] Stack: (0xea281d40 to 0xea282000) [ 39.014770] 1d40: 0001 c398fb0c c3a89c00 c398fa40 c0913a98 [ 39.022979] 1d60: c191b239 c5f7ea80 0001 c398fa40 c398fa80 c3a89c00 [ 39.031219] 1d80: 20080013 c3a89cbc c091397c c5f7e800 c39d0280 0001 c557ff00 [ 39.039428] 1da0: c398fa40 ea281e68 c5f8a300 0001 ffea c3a89c00 c3ae3008 c09150f0 [ 39.047637] 1dc0: 01140657 ea281e68 0001 [ 39.055847] 1de0: c39d 693113f4 0010 fff3 c5f8a300 [ 39.064086] 1e00: c3a89c00 c0914f88 ea281e68 0010 c5f8a300 c08ef2b0 693113f4 [ 39.072296] 1e20: b44fb158 ea281e68 0010 c0914f88 ea281e68 c5f21840 c5f8a300 c08ef53c [ 39.080505] 1e40: e200 0001 c1364999 0010 b44fb158 c010643a 003a [ 39.088745] 1e60: c10730ac 0001 0001 [ 39.096954] 1e80: [ 39.105163] 1ea0: [ 39.113372] 1ec0: [ 39.121612] 1ee0: 693113f4 c5f21840 c3c00a20 ea281f60 000c b44fb158 [ 39.129821] 1f00: c5f21841 0036 c010643a c048c1c4 ea281f48 c0f24c4c c02fa044 [ 39.138031] 1f20: 693113f4 c5f7e800 60080013 ea281f94 c0200b9c [ 39.146240] 1f40: c5f7e800 30c5387d ea281f58 c0f25138 c02001f0 30c5387d c0200bb4 [ 39.154449] 1f60: 000c c010643a b44fb158 b44fb124 b4647348 b44fb158 000c 0036 [ 39.162689] 1f80: 693113f4 b4647348 b44fb158 000c 0036 c0200298 c5f7e800 0036 [ 39.170898] 1fa0: c0200060 b4647348 b44fb158 000c c010643a b44fb158 b44fb124 [ 39.179107] 1fc0: b4647348 b44fb158 000c 0036 b44fb128 c010643a beb2aaa8 [ 39.187316] 1fe0: 0078 b44fb110 b5f8301b b5fb7ce4 80080010 000c
[PATCH 3/3] drm/i915/gem: Replace kmap() with kmap_local_page()
kmap() is being deprecated in favor of kmap_local_page().
There are two main problems with kmap(): (1) It comes with an overhead as
mapping space is restricted and protected by a global lock for
synchronization and (2) it also requires global TLB invalidation when the
kmap’s pool wraps and it might block when the mapping space is fully
utilized until a slot becomes available.
With kmap_local_page() the mappings are per thread, CPU local, can take
page faults, and can be called from any context (including interrupts).
It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore,
the tasks can be preempted and, when they are scheduled to run again, the
kernel virtual addresses are restored and are still valid.
Since its use in i915/gem is safe everywhere, it should be preferred.
Therefore, replace kmap() with kmap_local_page() in i915/gem.
Suggested-by: Ira Weiny
Signed-off-by: Fabio M. De Francesco
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++
drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 4eed3dd90ba8..2bc6ab9964ff 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -640,16 +640,14 @@ 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 = aops->write_begin(file, file->f_mapping, offset, len,
, );
if (err < 0)
goto fail;
- vaddr = kmap(page);
- memcpy(vaddr, data, len);
- kunmap(page);
+ memcpy_to_page(page, 0, data, len);
err = aops->write_end(file, file->f_mapping, offset, len, len,
page, pgdata);
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 3ced9948a331..bb25b50b5688 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -153,7 +153,7 @@ static int check_partial_mapping(struct drm_i915_gem_object
*obj,
intel_gt_flush_ggtt_writes(to_gt(i915));
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",
@@ -171,7 +171,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_gem_object_lock(obj, NULL);
@@ -249,7 +249,7 @@ static int check_partial_mappings(struct
drm_i915_gem_object *obj,
intel_gt_flush_ggtt_writes(to_gt(i915));
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",
@@ -267,7 +267,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;
--
2.37.1
Re: [PATCH 3/3] drm: omapdrm: Do no allocate non-scanout GEMs through DMM/TILER
Hi, Ivaylo, Tomi We have one X15 Android AOSP master build, it could not have the home screen displayed on the hdmi monitor connected with this change, with the following message printed on the serial console [ 607.404205] omapdrm omapdrm.0: Failed to setup plane plane-0 [ 607.410522] omapdrm omapdrm.0: Failed to setup plane plane-1 [ 607.416381] omapdrm omapdrm.0: Failed to setup plane plane-2 [ 607.422088] omapdrm omapdrm.0: Failed to setup plane plane-3 # for details, please check the link here: http://ix.io/47m1 It will work with home screen displayed on the hdmi monitor if this change is reverted. Is this the broken problem you talked about here? And could you please give some suggestions on how to have the x15 Android build work with this change? Thanks, Yongqin Liu On Thu, 17 Feb 2022 at 23:29, Ivaylo Dimitrov wrote: > > > > On 17.02.22 г. 14:46 ч., Tomi Valkeinen wrote: > > Hi, > > > > On 19/01/2022 12:23, Ivaylo Dimitrov wrote: > >> On devices with DMM, all allocations are done through either DMM or > >> TILER. > >> DMM/TILER being a limited resource means that such allocations will start > >> to fail before actual free memory is exhausted. What is even worse is > >> that > >> with time DMM/TILER space gets fragmented to the point that even if we > >> have > >> enough free DMM/TILER space and free memory, allocation fails because > >> there > >> is no big enough free block in DMM/TILER space. > >> > >> Such failures can be easily observed with OMAP xorg DDX, for example - > >> starting few GUI applications (so buffers for their windows are > >> allocated) > >> and then rotating landscape<->portrait while closing and opening new > >> windows soon results in allocation failures. > >> > >> Fix that by mapping buffers through DMM/TILER only when really needed, > >> like, for scanout buffers. > > > > Doesn't this break users that get a buffer from omapdrm and expect it to > > be contiguous? > > > > If you mean dumb buffer, then no, this does not break users as dumb > buffers are allocated as scanout: > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/omapdrm/omap_gem.c#L603 > > If you mean omap_bo allocated buffers, then if users want > linear(scanout) buffer, then they request it explicitly by passing > OMAP_BO_SCANOUT. > > Ivo -- Best Regards, Yongqin Liu --- #mailing list linaro-andr...@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android
[PATCH 1/3] drm/i915: Replace kmap() with kmap_local_page()
kmap() is being deprecated in favor of kmap_local_page(). There are two main problems with kmap(): (1) It comes with an overhead as mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap’s pool wraps and it might block when the mapping space is fully utilized until a slot becomes available. With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored and are still valid. Since its use in i915_gem.c is safe everywhere, it should be preferred. Therefore, replace kmap() with kmap_local_page() in i915_gem.c Suggested-by: Ira Weiny Signed-off-by: Fabio M. De Francesco --- drivers/gpu/drm/i915/i915_gem.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 702e5b89be22..43effce60e1b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -212,14 +212,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; } @@ -634,7 +634,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); @@ -643,7 +643,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; } -- 2.37.1
[PATCH] drm: Fix comment typo
The double `buffer' is duplicated in line 96, remove one. Signed-off-by: min tang --- drivers/gpu/drm/drm_vblank.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 2ff31717a3de..25660c30bbbf 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -92,7 +92,7 @@ * corrected timestamp. * * On a lot of display hardware, programming needs to take effect during the - * vertical blanking period so that settings like gamma, the image buffer + * vertical blanking period so that settings like gamma, the image * buffer to be scanned out, etc. can safely be changed without showing * any visual artifacts on the screen. In some unforgiving hardware, some of * this programming has to both start and end in the same vblank. To help -- 2.17.1
RE: [PATCH v3 2/3] drm/amdgpu_dm: Rely on split out luminance calculation function
On Thu, 11 Aug 2022, "Deucher, Alexander" wrote:
> [Public]
>
>> -Original Message-
>> From: amd-gfx On Behalf Of Jani
>> Nikula
>> Sent: Thursday, August 4, 2022 5:55 AM
>> To: Jouni Högander ; dri-
>> de...@lists.freedesktop.org; intel-...@lists.freedesktop.org; amd-
>> g...@lists.freedesktop.org
>> Cc: Siqueira, Rodrigo ; Li, Roman
>> ; Manasi Navare ; Mika
>> Kahola ; Jouni Högander
>> ; Wentland, Harry
>>
>> Subject: Re: [PATCH v3 2/3] drm/amdgpu_dm: Rely on split out luminance
>> calculation function
>>
>> On Tue, 19 Jul 2022, Jouni Högander wrote:
>> > Luminance range calculation was split out into drm_edid.c and is now
>> > part of edid parsing. Rely on values calculated during edid parsing
>> > and use these for caps->aux_max_input_signal and caps-
>> >aux_min_input_signal.
>>
>> Harry, I'll merge patches 1 & 3 in this series through drm-misc-next,
>> because I
>> think they're good to go, and fix stuff in i915.
>>
>> Can I get your rb/ack to merge this patch as well, or do you want to take
>> this
>> later via your tree?
>
> You can take this via drm-misc.
> Acked-by: Alex Deucher
Thanks, pushed the series to drm-misc-next.
BR,
Jani.
>
>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> > v2: Use values calculated during edid parsing
>> >
>> > Cc: Roman Li
>> > Cc: Rodrigo Siqueira
>> > Cc: Harry Wentland
>> > Cc: Lyude Paul
>> > Cc: Mika Kahola
>> > Cc: Jani Nikula
>> > Cc: Manasi Navare
>> > Signed-off-by: Jouni Högander
>> > ---
>> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35
>> > +++
>> > 1 file changed, 4 insertions(+), 31 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > index 3e83fed540e8..eb7abdeb8653 100644
>> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> > @@ -2903,15 +2903,12 @@ static struct drm_mode_config_helper_funcs
>> > amdgpu_dm_mode_config_helperfuncs = {
>> >
>> > static void update_connector_ext_caps(struct amdgpu_dm_connector
>> > *aconnector) {
>> > - u32 max_avg, min_cll, max, min, q, r;
>> >struct amdgpu_dm_backlight_caps *caps;
>> >struct amdgpu_display_manager *dm;
>> >struct drm_connector *conn_base;
>> >struct amdgpu_device *adev;
>> >struct dc_link *link = NULL;
>> > - static const u8 pre_computed_values[] = {
>> > - 50, 51, 52, 53, 55, 56, 57, 58, 59, 61, 62, 63, 65, 66, 68, 69,
>> > - 71, 72, 74, 75, 77, 79, 81, 82, 84, 86, 88, 90, 92, 94, 96, 98};
>> > + struct drm_luminance_range_info *luminance_range;
>> >int i;
>> >
>> >if (!aconnector || !aconnector->dc_link) @@ -2933,8 +2930,6 @@
>> > static void update_connector_ext_caps(struct amdgpu_dm_connector
>> *aconnector)
>> >caps = >backlight_caps[i];
>> >caps->ext_caps = >dc_link->dpcd_sink_ext_caps;
>> >caps->aux_support = false;
>> > - max_avg = conn_base->hdr_sink_metadata.hdmi_type1.max_fall;
>> > - min_cll = conn_base->hdr_sink_metadata.hdmi_type1.min_cll;
>> >
>> >if (caps->ext_caps->bits.oled == 1 /*||
>> >caps->ext_caps->bits.sdr_aux_backlight_control == 1 || @@
>> > -2946,31 +2941,9 @@ static void update_connector_ext_caps(struct
>> amdgpu_dm_connector *aconnector)
>> >else if (amdgpu_backlight == 1)
>> >caps->aux_support = true;
>> >
>> > - /* From the specification (CTA-861-G), for calculating the maximum
>> > - * luminance we need to use:
>> > - * Luminance = 50*2**(CV/32)
>> > - * Where CV is a one-byte value.
>> > - * For calculating this expression we may need float point precision;
>> > - * to avoid this complexity level, we take advantage that CV is divided
>> > - * by a constant. From the Euclids division algorithm, we know that
>> CV
>> > - * can be written as: CV = 32*q + r. Next, we replace CV in the
>> > - * Luminance expression and get 50*(2**q)*(2**(r/32)), hence we
>> just
>> > - * need to pre-compute the value of r/32. For pre-computing the
>> values
>> > - * We just used the following Ruby line:
>> > - * (0...32).each {|cv| puts (50*2**(cv/32.0)).round}
>> > - * The results of the above expressions can be verified at
>> > - * pre_computed_values.
>> > - */
>> > - q = max_avg >> 5;
>> > - r = max_avg % 32;
>> > - max = (1 << q) * pre_computed_values[r];
>> > -
>> > - // min luminance: maxLum * (CV/255)^2 / 100
>> > - q = DIV_ROUND_CLOSEST(min_cll, 255);
>> > - min = max * DIV_ROUND_CLOSEST((q * q), 100);
>> > -
>> > - caps->aux_max_input_signal = max;
>> > - caps->aux_min_input_signal = min;
>> > + luminance_range = _base->display_info.luminance_range;
>> > + caps->aux_min_input_signal = luminance_range->min_luminance;
>> > + caps->aux_max_input_signal = luminance_range->max_luminance;
>> > }
>> >
>> > void amdgpu_dm_update_connector_after_detect(
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open
Re: [PATCH v4 00/41] DYNDBG: opt-in class'd debug for modules, use in drm.
On Thu, Aug 11, 2022 at 06:52:40PM +0200, Daniel Vetter wrote: > On Wed, Aug 03, 2022 at 04:13:05PM -0400, Jason Baron wrote: > > > > > > On 8/3/22 15:56, jim.cro...@gmail.com wrote: > > > On Wed, Jul 20, 2022 at 9:32 AM Jim Cromie wrote: > > >> > > > > > >> Hi Jason, Greg, DRM-folk, > > >> > > >> This adds 'typed' "class FOO" support to dynamic-debug, where 'typed' > > >> means either DISJOINT (like drm debug categories), or VERBOSE (like > > >> nouveau debug-levels). Use it in DRM modules: core, helpers, and in > > >> drivers i915, amdgpu, nouveau. > > >> > > > > > > This revision fell over, on a conflict with something in drm-MUMBLE > > > > > > Error: patch > > > https://urldefense.com/v3/__https://patchwork.freedesktop.org/api/1.0/series/106427/revisions/2/mbox/__;!!GjvTz_vk!UCPl5Uf32cDVwwysMTfaLwoGLWomargFXuR8HjBA3xsUOjxXHXC5hneAkP4iWK91yc-LjjJxWW89-51Z$ > > > > > > not applied > > > Applying: dyndbg: fix static_branch manipulation > > > Applying: dyndbg: fix module.dyndbg handling > > > Applying: dyndbg: show both old and new in change-info > > > Applying: dyndbg: reverse module walk in cat control > > > Applying: dyndbg: reverse module.callsite walk in cat control > > > Applying: dyndbg: use ESCAPE_SPACE for cat control > > > Applying: dyndbg: let query-modname override actual module name > > > Applying: dyndbg: add test_dynamic_debug module > > > Applying: dyndbg: drop EXPORTed dynamic_debug_exec_queries > > > > > > Jason, > > > those above are decent maintenance patches, particularly the drop export. > > > It would be nice to trim this unused api this cycle. > > > > Hi Jim, > > > > Agreed - I was thinking the same thing. Feel free to add > > Acked-by: Jason Baron to those first 9. > > Does Greg KH usually pick up dyndbg patches or someone else or do I need > to do something? Would be great to get some movement here since -rc1 goes > out and merging will restart next week. Yes, I can take these into my tree after -rc1 is out. thanks, greg k-h
