Re: [PATCH libdrm] amdgpu/test: Add illegal register and memory access test.
Am 01.11.18 um 02:44 schrieb Alex Deucher:
On Wed, Oct 31, 2018 at 4:05 PM Grodzovsky, Andrey
wrote:
On 10/31/2018 03:49 PM, Alex Deucher wrote:
On Wed, Oct 31, 2018 at 2:33 PM Andrey Grodzovsky
wrote:
Illegal access will cause CP hang followed by job timeout and
recovery kicking in.
Also, disable the suite for all APU ASICs until GPU
reset issues for them will be resolved and GPU reset recovery
will be enabled by default.
Signed-off-by: Andrey Grodzovsky
---
tests/amdgpu/deadlock_tests.c | 118
+-
1 file changed, 117 insertions(+), 1 deletion(-)
diff --git a/tests/amdgpu/deadlock_tests.c b/tests/amdgpu/deadlock_tests.c
index 292ec4e..c565f7a 100644
--- a/tests/amdgpu/deadlock_tests.c
+++ b/tests/amdgpu/deadlock_tests.c
@@ -73,6 +73,29 @@
* 1 - pfp
*/
+#definePACKET3_WRITE_DATA 0x37
+#defineWRITE_DATA_DST_SEL(x) ((x) << 8)
+ /* 0 - register
+* 1 - memory (sync - via GRBM)
+* 2 - gl2
+* 3 - gds
+* 4 - reserved
+* 5 - memory (async - direct)
+*/
+#defineWR_ONE_ADDR (1 << 16)
+#defineWR_CONFIRM (1 << 20)
+#defineWRITE_DATA_CACHE_POLICY(x) ((x) << 25)
+ /* 0 - LRU
+* 1 - Stream
+*/
+#defineWRITE_DATA_ENGINE_SEL(x)((x) << 30)
+ /* 0 - me
+* 1 - pfp
+* 2 - ce
+*/
+
+#define mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR
0x54f
+
static amdgpu_device_handle device_handle;
static uint32_t major_version;
static uint32_t minor_version;
@@ -85,6 +108,8 @@ int use_uc_mtype = 0;
static void amdgpu_deadlock_helper(unsigned ip_type);
static void amdgpu_deadlock_gfx(void);
static void amdgpu_deadlock_compute(void);
+static void amdgpu_illegal_reg_access();
+static void amdgpu_illegal_mem_access();
CU_BOOL suite_deadlock_tests_enable(void)
{
@@ -94,7 +119,9 @@ CU_BOOL suite_deadlock_tests_enable(void)
&minor_version, &device_handle))
return CU_FALSE;
- if (device_handle->info.family_id == AMDGPU_FAMILY_SI) {
+ if (device_handle->info.family_id == AMDGPU_FAMILY_SI ||
Add AMDGPU_FAMILY_KV for CI based APUs as well.
+ device_handle->info.family_id == AMDGPU_FAMILY_CZ ||
+ device_handle->info.family_id == AMDGPU_FAMILY_RV) {
printf("\n\nCurrently hangs the CP on this ASIC, deadlock suite
disabled\n");
enable = CU_FALSE;
}
@@ -140,6 +167,8 @@ int suite_deadlock_tests_clean(void)
CU_TestInfo deadlock_tests[] = {
{ "gfx ring block test", amdgpu_deadlock_gfx },
{ "compute ring block test", amdgpu_deadlock_compute },
+ { "illegal reg access test", amdgpu_illegal_reg_access },
+ { "illegal mem access test", amdgpu_illegal_mem_access },
Won't this illegal mem access just result in a page fault? Is the
idea to set vm_debug to force an MC halt to test reset?
Alex
For this test to hang the CP amdgpu.vm_fault_stop=2 needs to be set.
With the KV added above and a comment about vm_fault_stop added, this patch is:
Reviewed-by: Alex Deucher
Seconded, patch is Reviewed-by: Christian König
as well.
Christian.
Andrey
CU_TEST_INFO_NULL,
};
@@ -257,3 +286,90 @@ static void amdgpu_deadlock_helper(unsigned ip_type)
r = amdgpu_cs_ctx_free(context_handle);
CU_ASSERT_EQUAL(r, 0);
}
+
+static void bad_access_helper(int reg_access)
+{
+ amdgpu_context_handle context_handle;
+ amdgpu_bo_handle ib_result_handle;
+ void *ib_result_cpu;
+ uint64_t ib_result_mc_address;
+ struct amdgpu_cs_request ibs_request;
+ struct amdgpu_cs_ib_info ib_info;
+ struct amdgpu_cs_fence fence_status;
+ uint32_t expired;
+ int i, r;
+ amdgpu_bo_list_handle bo_list;
+ amdgpu_va_handle va_handle;
+
+ r = amdgpu_cs_ctx_create(device_handle, &context_handle);
+ CU_ASSERT_EQUAL(r, 0);
+
+ r = amdgpu_bo_alloc_and_map_raw(device_handle, 4096, 4096,
+ AMDGPU_GEM_DOMAIN_GTT, 0, 0,
+ &ib_result_handle,
&ib_result_cpu,
+ &ib_result_mc_address,
&va_handle);
+ CU_ASSERT_EQUAL(r, 0);
+
+ r = amdgpu_get_bo_list(device_handle, ib_result_handle, NULL,
+ &bo_list);
+ CU_ASSERT_EQUAL(r, 0);
+
+ ptr = ib_result_cpu;
+ i = 0;
+
+ ptr[i++] = PACKET3(PACKET3_WRITE_DATA, 3);
+
[PATCH 1/5] drm: add support of syncobj timeline point wait v4
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.
v3:
userspace can specify two kinds waits::
a. Wait for time point to be completed.
b. and wait for time point to become available
v4:
rebase
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
---
drivers/gpu/drm/drm_internal.h | 2 +
drivers/gpu/drm/drm_ioctl.c| 2 +
drivers/gpu/drm/drm_syncobj.c | 115 -
include/uapi/drm/drm.h | 18 ++
4 files changed, 121 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_private);
int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 94bd872d56c4..a9a17ed35cc4 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -675,6 +675,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 4dca5f7e8c4b..9dc54a345480 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -153,11 +153,12 @@ static void drm_syncobj_add_callback_locked(struct
drm_syncobj *syncobj,
}
static void drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+ uint64_t point,
struct dma_fence **fence,
struct drm_syncobj_cb *cb,
drm_syncobj_func_t func)
{
- u64 pt_value = 0;
+ u64 pt_value = point;
WARN_ON(*fence);
@@ -854,6 +855,7 @@ struct syncobj_wait_entry {
struct dma_fence *fence;
struct dma_fence_cb fence_cb;
struct drm_syncobj_cb syncobj_cb;
+ u64point;
};
static void syncobj_wait_fence_func(struct dma_fence *fence,
@@ -871,12 +873,13 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj
*syncobj,
struct syncobj_wait_entry *wait =
container_of(cb, struct syncobj_wait_entry, syncobj_cb);
- drm_syncobj_search_fence(syncobj, 0, 0, &wait->fence);
+ drm_syncobj_search_fence(syncobj, wait->point, 0, &wait->fence);
wake_up_process(wait->task);
}
static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj
**syncobjs,
+ void __user *user_points,
uint32_t count,
uint32_t flags,
signed long timeout,
@@ -884,12 +887,37 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
{
struct syncobj_wait_entry *entries;
struct dma_fence *fence;
+ uint64_t *points;
uint32_t signaled_count, i;
- entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
- if (!entries)
+ points = kmalloc_array(count, sizeof(*points), GFP_KERNEL);
+ if (points == NULL)
return -ENOMEM;
+ if (!user_points) {
+ memset(points, 0, count * sizeof(uint64_t));
+ } else if (copy_from_user(points, user_points, sizeof(uint64_t) *
count)) {
+ timeout = -EFAULT;
+ goto err_free_points;
+ }
+
+
+ if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE) {
+ struct dma_fence *fence;
+ for (i = 0; i < count; ++i) {
+ timeout = drm_syncobj_search_fence(syncobjs[i],
points[i],
+
[PATCH 2/5] drm: add timeline syncobj payload query ioctl v2
user mode can query timeline payload.
v2: check return value of copy_to_user
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
---
drivers/gpu/drm/drm_internal.h | 2 ++
drivers/gpu/drm/drm_ioctl.c| 2 ++
drivers/gpu/drm/drm_syncobj.c | 52 ++
include/uapi/drm/drm.h | 11 +++
4 files changed, 67 insertions(+)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 566d44e3c782..9c4826411a3c 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -189,6 +189,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
/* drm_framebuffer.c */
void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a9a17ed35cc4..7578ef6dc1d1 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -681,6 +681,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl,
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE,
drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl,
DRM_MASTER|DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 9dc54a345480..9ad58d0d21cd 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1291,3 +1291,55 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void
*data,
return ret;
}
+
+static void drm_syncobj_timeline_query_payload(struct drm_syncobj *syncobj,
+ uint64_t *point)
+{
+ if (syncobj->type != DRM_SYNCOBJ_TYPE_TIMELINE) {
+ DRM_ERROR("Normal syncobj cann't be queried!");
+ *point = 0;
+ return;
+ }
+ *point = syncobj->signal_point;
+}
+
+int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private)
+{
+ struct drm_syncobj_timeline_query *args = data;
+ struct drm_syncobj **syncobjs;
+ uint64_t *points;
+ uint32_t i;
+ int ret;
+
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+ return -ENODEV;
+
+ if (args->count_handles == 0)
+ return -EINVAL;
+
+ ret = drm_syncobj_array_find(file_private,
+u64_to_user_ptr(args->handles),
+args->count_handles,
+&syncobjs);
+ if (ret < 0)
+ return ret;
+
+
+ points = kmalloc_array(args->count_handles, sizeof(*points),
+ GFP_KERNEL);
+ if (points == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ for (i = 0; i < args->count_handles; i++)
+ drm_syncobj_timeline_query_payload(syncobjs[i], &points[i]);
+ ret = copy_to_user(u64_to_user_ptr(args->points), points,
+ sizeof(uint64_t) * args->count_handles) ? -EFAULT :
0;
+
+ kfree(points);
+out:
+ drm_syncobj_array_free(syncobjs, args->count_handles);
+
+ return ret;
+}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index c8bc1414753d..23c4979d8a1c 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -771,6 +771,15 @@ struct drm_syncobj_array {
__u32 pad;
};
+struct drm_syncobj_timeline_query {
+ __u64 handles;
+ /* points are timeline syncobjs payloads returned by query ioctl */
+ __u64 points;
+ __u32 count_handles;
+ __u32 pad;
+};
+
+
/* Query current scanout sequence number */
struct drm_crtc_get_sequence {
__u32 crtc_id; /* requested crtc_id */
@@ -928,6 +937,8 @@ extern "C" {
#define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct
drm_mode_revoke_lease)
#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAITDRM_IOWR(0xCA, struct
drm_syncobj_timeline_wait)
+#define DRM_IOCTL_SYNCOBJ_QUERYDRM_IOWR(0xCB, struct
drm_syncobj_timeline_query)
+
/**
* Device specific ioctls should only be in their respective headers
[PATCH 3/5] drm: add timeline support for syncobj export/import
user space can specify timeline point fence to export/import.
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
---
drivers/gpu/drm/drm_internal.h | 4 ++
drivers/gpu/drm/drm_ioctl.c| 4 ++
drivers/gpu/drm/drm_syncobj.c | 76 ++
include/uapi/drm/drm.h | 11 +
4 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 9c4826411a3c..5ad6cbdb68ab 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -181,6 +181,10 @@ int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_private);
int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl2(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl2(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7578ef6dc1d1..364d26e949cf 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -673,6 +673,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE,
drm_syncobj_fd_to_handle_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2,
drm_syncobj_handle_to_fd_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2,
drm_syncobj_fd_to_handle_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 9ad58d0d21cd..dffc42ba2f91 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -677,7 +677,7 @@ static int drm_syncobj_fd_to_handle(struct drm_file
*file_private,
}
static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
- int fd, int handle)
+ int fd, int handle, uint64_t
point)
{
struct dma_fence *fence = sync_file_get_fence(fd);
struct drm_syncobj *syncobj;
@@ -691,14 +691,14 @@ static int drm_syncobj_import_sync_file_fence(struct
drm_file *file_private,
return -ENOENT;
}
- drm_syncobj_replace_fence(syncobj, 0, fence);
+ drm_syncobj_replace_fence(syncobj, point, fence);
dma_fence_put(fence);
drm_syncobj_put(syncobj);
return 0;
}
static int drm_syncobj_export_sync_file(struct drm_file *file_private,
- int handle, int *p_fd)
+ int handle, uint64_t point, int *p_fd)
{
int ret;
struct dma_fence *fence;
@@ -708,7 +708,7 @@ static int drm_syncobj_export_sync_file(struct drm_file
*file_private,
if (fd < 0)
return fd;
- ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
+ ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence);
if (ret)
goto err_put_fd;
@@ -817,9 +817,14 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
return -EINVAL;
- if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+ if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) {
+ struct drm_syncobj *syncobj = drm_syncobj_find(file_private,
+ args->handle);
+ if (!syncobj || syncobj->type != DRM_SYNCOBJ_TYPE_BINARY)
+ return -EINVAL;
return drm_syncobj_export_sync_file(file_private, args->handle,
- &args->fd);
+ 0, &args->fd);
+ }
return drm_syncobj_handle_to_fd(file_private, args->handle,
&args->fd);
@@ -841,15 +846,72 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev,
void *data,
[PATCH 4/5] drm/amdgpu: add timeline support in amdgpu CS v2
syncobj wait/signal operation is appending in command submission.
v2: separate to two kinds in/out_deps functions
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h| 8 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 111 +
include/uapi/drm/amdgpu_drm.h | 9 ++
3 files changed, 112 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index d0102cfc8efb..081e9b973252 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -543,6 +543,11 @@ struct amdgpu_cs_chunk {
void*kdata;
};
+struct amdgpu_cs_syncobj_post_dep {
+ struct drm_syncobj *post_dep_syncobj;
+ u64 point;
+};
+
struct amdgpu_cs_parser {
struct amdgpu_device*adev;
struct drm_file *filp;
@@ -571,9 +576,8 @@ struct amdgpu_cs_parser {
/* user fence */
struct amdgpu_bo_list_entry uf_entry;
-
+ struct amdgpu_cs_syncobj_post_dep *post_dep_syncobjs;
unsigned num_post_dep_syncobjs;
- struct drm_syncobj **post_dep_syncobjs;
};
static inline u32 amdgpu_get_ib_value(struct amdgpu_cs_parser *p,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 35bc8fc3bc70..6a823b58b3b8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -213,6 +213,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
case AMDGPU_CHUNK_ID_DEPENDENCIES:
case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
+ case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT:
+ case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL:
break;
default:
@@ -793,7 +795,7 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser
*parser, int error,
&parser->validated);
for (i = 0; i < parser->num_post_dep_syncobjs; i++)
- drm_syncobj_put(parser->post_dep_syncobjs[i]);
+ drm_syncobj_put(parser->post_dep_syncobjs[i].post_dep_syncobj);
kfree(parser->post_dep_syncobjs);
dma_fence_put(parser->fence);
@@ -1100,13 +1102,17 @@ static int amdgpu_cs_process_fence_dep(struct
amdgpu_cs_parser *p,
}
static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
-uint32_t handle)
+uint32_t handle, u64 point,
+u64 flags)
{
int r;
struct dma_fence *fence;
- r = drm_syncobj_find_fence(p->filp, handle, 0, 0, &fence);
- if (r)
+
+ r = drm_syncobj_find_fence(p->filp, handle, point, flags, &fence);
+ if (r) {
+ DRM_ERROR("syncobj %u failed to find fence!\n", handle);
return r;
+ }
r = amdgpu_sync_fence(p->adev, &p->job->sync, fence, true);
dma_fence_put(fence);
@@ -1117,46 +1123,108 @@ static int
amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p,
struct amdgpu_cs_chunk *chunk)
{
+ struct drm_amdgpu_cs_chunk_sem *deps;
unsigned num_deps;
int i, r;
- struct drm_amdgpu_cs_chunk_sem *deps;
deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata;
num_deps = chunk->length_dw * 4 /
sizeof(struct drm_amdgpu_cs_chunk_sem);
+ for (i = 0; i < num_deps; ++i) {
+ r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle,
+ 0, 0);
+ if (r)
+ return r;
+ }
+
+ return 0;
+}
+
+
+static int amdgpu_cs_process_syncobj_timeline_in_dep(struct amdgpu_cs_parser
*p,
+struct amdgpu_cs_chunk
*chunk)
+{
+ struct drm_amdgpu_cs_chunk_syncobj *syncobj_deps;
+ unsigned num_deps;
+ int i, r;
+ syncobj_deps = (struct drm_amdgpu_cs_chunk_syncobj *)chunk->kdata;
+ num_deps = chunk->length_dw * 4 /
+ sizeof(struct drm_amdgpu_cs_chunk_syncobj);
for (i = 0; i < num_deps; ++i) {
- r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle);
+ r = amdgpu_syncobj_lookup_and_add_to_sync(p,
+
syncobj_deps[i].handle,
+ syncobj_deps[i].point,
+
syncobj_deps[i].flags);
[PATCH 5/5] drm/amdgpu: update version for timeline syncobj support in amdgpu
Signed-off-by: Chunming Zhou --- 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 28781414d71c..dba529d65ccd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -71,9 +71,10 @@ * - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk). * - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE. * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation. + * - 3.28.0 - Add syncobj timeline support to AMDGPU_CS. */ #define KMS_DRIVER_MAJOR 3 -#define KMS_DRIVER_MINOR 27 +#define KMS_DRIVER_MINOR 28 #define KMS_DRIVER_PATCHLEVEL 0 int amdgpu_vram_limit = 0; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 2/5] addr cs chunk for syncobj timeline
Signed-off-by: Chunming Zhou
---
include/drm/amdgpu_drm.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
index 1ceec56d..a3c067dd 100644
--- a/include/drm/amdgpu_drm.h
+++ b/include/drm/amdgpu_drm.h
@@ -517,6 +517,8 @@ struct drm_amdgpu_gem_va {
#define AMDGPU_CHUNK_ID_SYNCOBJ_IN 0x04
#define AMDGPU_CHUNK_ID_SYNCOBJ_OUT 0x05
#define AMDGPU_CHUNK_ID_BO_HANDLES 0x06
+#define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT0x07
+#define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL 0x08
struct drm_amdgpu_cs_chunk {
__u32 chunk_id;
@@ -592,6 +594,13 @@ struct drm_amdgpu_cs_chunk_sem {
__u32 handle;
};
+struct drm_amdgpu_cs_chunk_syncobj {
+ __u32 handle;
+ __u32 flags;
+ __u64 point;
+};
+
+
#define AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ 0
#define AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ_FD 1
#define AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD2
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/5] [libdrm] new syncobj extension
Signed-off-by: Chunming Zhou
---
include/drm/drm.h | 38 ++
1 file changed, 38 insertions(+)
diff --git a/include/drm/drm.h b/include/drm/drm.h
index 85c685a2..43d7420a 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -711,6 +711,7 @@ struct drm_prime_handle {
struct drm_syncobj_create {
__u32 handle;
#define DRM_SYNCOBJ_CREATE_SIGNALED (1 << 0)
+#define DRM_SYNCOBJ_CREATE_TYPE_TIMELINE (1 << 1)
__u32 flags;
};
@@ -728,9 +729,20 @@ struct drm_syncobj_handle {
__s32 fd;
__u32 pad;
};
+struct drm_syncobj_handle2 {
+ __u32 handle;
+ __u32 flags;
+ __u64 point;
+
+ __s32 fd;
+ __u32 pad;
+};
+
#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
+#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_COMPLETED (1 << 2)
+#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 3)
struct drm_syncobj_wait {
__u64 handles;
/* absolute timeout */
@@ -741,12 +753,32 @@ struct drm_syncobj_wait {
__u32 pad;
};
+struct drm_syncobj_timeline_wait {
+__u64 handles;
+/* wait on specific timeline point for every handles*/
+__u64 points;
+/* absolute timeout */
+__s64 timeout_nsec;
+__u32 count_handles;
+__u32 flags;
+__u32 first_signaled; /* only valid when not waiting all */
+__u32 pad;
+};
+
struct drm_syncobj_array {
__u64 handles;
__u32 count_handles;
__u32 pad;
};
+struct drm_syncobj_timeline_query {
+__u64 handles;
+/* points are timeline syncobjs payloads returned by query ioctl */
+__u64 points;
+__u32 count_handles;
+__u32 pad;
+};
+
/* Query current scanout sequence number */
struct drm_crtc_get_sequence {
__u32 crtc_id; /* requested crtc_id */
@@ -903,6 +935,12 @@ extern "C" {
#define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct
drm_mode_get_lease)
#define DRM_IOCTL_MODE_REVOKE_LEASEDRM_IOWR(0xC9, struct
drm_mode_revoke_lease)
+#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct
drm_syncobj_timeline_wait)
+#define DRM_IOCTL_SYNCOBJ_QUERY DRM_IOWR(0xCB, struct
drm_syncobj_timeline_query)
+#define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2DRM_IOWR(0xCC, struct
drm_syncobj_handle2)
+#define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2DRM_IOWR(0xCD, struct
drm_syncobj_handle2)
+
+
/**
* Device specific ioctls should only be in their respective headers
* The device specific ioctl range is from 0x40 to 0x9f.
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 3/5] [libdrm]: add timeline wait/query/export/import ioctl
Signed-off-by: Chunming Zhou
---
xf86drm.c | 78 +++
xf86drm.h | 8 ++
2 files changed, 86 insertions(+)
diff --git a/xf86drm.c b/xf86drm.c
index 49150d74..13f36b04 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -4212,6 +4212,40 @@ drm_public int drmSyncobjExportSyncFile(int fd, uint32_t
handle,
return 0;
}
+drm_public int drmSyncobjImportSyncFile2(int fd, uint32_t handle,
+uint64_t point,
+int sync_file_fd)
+{
+struct drm_syncobj_handle2 args;
+
+memclear(args);
+args.fd = sync_file_fd;
+args.handle = handle;
+args.point = point;
+args.flags = DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE;
+return drmIoctl(fd, DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2, &args);
+}
+
+drm_public int drmSyncobjExportSyncFile2(int fd, uint32_t handle,
+uint64_t point,
+int *sync_file_fd)
+{
+struct drm_syncobj_handle2 args;
+int ret;
+
+memclear(args);
+args.fd = -1;
+args.handle = handle;
+args.point = point;
+args.flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE;
+ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2, &args);
+if (ret)
+return ret;
+*sync_file_fd = args.fd;
+return 0;
+}
+
+
drm_public int drmSyncobjWait(int fd, uint32_t *handles, unsigned num_handles,
int64_t timeout_nsec, unsigned flags,
uint32_t *first_signaled)
@@ -4261,3 +4295,47 @@ drm_public int drmSyncobjSignal(int fd, const uint32_t
*handles,
ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_SIGNAL, &args);
return ret;
}
+
+drm_public int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t
*points,
+ unsigned num_handles,
+ int64_t timeout_nsec, unsigned flags,
+ uint32_t *first_signaled)
+{
+struct drm_syncobj_timeline_wait args;
+int ret;
+
+memclear(args);
+args.handles = (uintptr_t)handles;
+args.points = (uint64_t)(uintptr_t)points;
+args.timeout_nsec = timeout_nsec;
+args.count_handles = num_handles;
+args.flags = flags;
+
+ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, &args);
+if (ret < 0)
+return -errno;
+
+if (first_signaled)
+*first_signaled = args.first_signaled;
+return ret;
+}
+
+
+drm_public int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points,
+ uint32_t handle_count)
+{
+struct drm_syncobj_timeline_query args;
+int ret;
+
+memclear(args);
+args.handles = (uintptr_t)handles;
+args.points = (uint64_t)(uintptr_t)points;
+args.count_handles = handle_count;
+
+ret = drmIoctl(fd, DRM_IOCTL_SYNCOBJ_QUERY, &args);
+if (ret)
+return ret;
+return 0;
+}
+
+
diff --git a/xf86drm.h b/xf86drm.h
index 7773d71a..2dae1694 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -870,11 +870,19 @@ extern int drmSyncobjFDToHandle(int fd, int obj_fd,
uint32_t *handle);
extern int drmSyncobjImportSyncFile(int fd, uint32_t handle, int sync_file_fd);
extern int drmSyncobjExportSyncFile(int fd, uint32_t handle, int
*sync_file_fd);
+extern int drmSyncobjImportSyncFile2(int fd, uint32_t handle, uint64_t point,
int sync_file_fd);
+extern int drmSyncobjExportSyncFile2(int fd, uint32_t handle, uint64_t point,
int *sync_file_fd);
extern int drmSyncobjWait(int fd, uint32_t *handles, unsigned num_handles,
int64_t timeout_nsec, unsigned flags,
uint32_t *first_signaled);
extern int drmSyncobjReset(int fd, const uint32_t *handles, uint32_t
handle_count);
extern int drmSyncobjSignal(int fd, const uint32_t *handles, uint32_t
handle_count);
+extern int drmSyncobjTimelineWait(int fd, uint32_t *handles, uint64_t *points,
+ unsigned num_handles,
+ int64_t timeout_nsec, unsigned flags,
+ uint32_t *first_signaled);
+extern int drmSyncobjQuery(int fd, uint32_t *handles, uint64_t *points,
+ uint32_t handle_count);
#if defined(__cplusplus)
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 4/5] [libdrm]: wrap syncobj timeline query/wait/export/import APIs for amdgpu v2
v2: symbos are stored in lexical order. Signed-off-by: Chunming Zhou --- amdgpu/amdgpu-symbol-check | 4 +++ amdgpu/amdgpu.h| 73 ++ amdgpu/amdgpu_cs.c | 62 3 files changed, 139 insertions(+) diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check index 6f5e0f95..c94813f8 100755 --- a/amdgpu/amdgpu-symbol-check +++ b/amdgpu/amdgpu-symbol-check @@ -48,9 +48,13 @@ amdgpu_cs_signal_semaphore amdgpu_cs_submit amdgpu_cs_submit_raw amdgpu_cs_syncobj_export_sync_file +amdgpu_cs_syncobj_export_sync_file2 amdgpu_cs_syncobj_import_sync_file +amdgpu_cs_syncobj_import_sync_file2 +amdgpu_cs_syncobj_query amdgpu_cs_syncobj_reset amdgpu_cs_syncobj_signal +amdgpu_cs_syncobj_timeline_wait amdgpu_cs_syncobj_wait amdgpu_cs_wait_fences amdgpu_cs_wait_semaphore diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index dc51659a..aea31fdc 100644 --- a/amdgpu/amdgpu.h +++ b/amdgpu/amdgpu.h @@ -1489,6 +1489,45 @@ int amdgpu_cs_syncobj_wait(amdgpu_device_handle dev, int64_t timeout_nsec, unsigned flags, uint32_t *first_signaled); +/** + * Wait for one or all sync objects on their points to signal. + * + * \param dev- \c [in] self-explanatory + * \param handles - \c [in] array of sync object handles + * \param points - \c [in] array of sync points to wait + * \param num_handles - \c [in] self-explanatory + * \param timeout_nsec - \c [in] self-explanatory + * \param flags - \c [in] a bitmask of DRM_SYNCOBJ_WAIT_FLAGS_* + * \param first_signaled - \c [in] self-explanatory + * + * \return 0 on success\n + * -ETIME - Timeout + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_timeline_wait(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, + uint32_t *first_signaled); +/** + * Query sync objects payloads. + * + * \param dev- \c [in] self-explanatory + * \param handles - \c [in] array of sync object handles + * \param points - \c [out] array of sync points returned, which presents + * syncobj payload. + * \param num_handles - \c [in] self-explanatory + * + * \return 0 on success\n + * -ETIME - Timeout + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_query(amdgpu_device_handle dev, + uint32_t *handles, uint64_t **points, + unsigned num_handles); + /** * Export kernel sync object to shareable fd. * @@ -1547,6 +1586,40 @@ int amdgpu_cs_syncobj_export_sync_file(amdgpu_device_handle dev, int amdgpu_cs_syncobj_import_sync_file(amdgpu_device_handle dev, uint32_t syncobj, int sync_file_fd); +/** + * Export kernel timeline sync object point to a sync_file. + * + * \param dev - \c [in] device handle + * \param syncobj- \c [in] sync object handle + * \param point - \c [in] sync points to export + * \param sync_file_fd - \c [out] sync_file file descriptor. + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_export_sync_file2(amdgpu_device_handle dev, + uint32_t syncobj, + uint64_t point, + int *sync_file_fd); + +/** + * Import to kernel timeline sync object point from a sync_file. + * + * \param dev - \c [in] device handle + * \param syncobj- \c [in] sync object handle + * \param point - \c [in] sync points to import + * \param sync_file_fd - \c [in] sync_file file descriptor. + * + * \return 0 on success\n + * <0 - Negative POSIX Error code + * + */ +int amdgpu_cs_syncobj_import_sync_file2(amdgpu_device_handle dev, + uint32_t syncobj, + uint64_t point, + int sync_file_fd); + /** * Export an amdgpu fence as a handle (syncobj or fd). diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index 3b8231aa..6a9ed3c3 100644 --- a/amdgpu/amdgpu_cs.c +++ b/amdgpu/amdgpu_cs.c @@ -661,6 +661,45 @@ drm_public int amdgpu_cs_syncobj_wait(amdgpu_device_handle dev, flags, first_signaled); } +drm_public int amdgpu_cs_syncobj_timeline_wait(amdgpu_device_handle dev, + uint32_t *handles, uint64_t *points, + unsigned num_handles, + int64_t timeout_nsec, unsigned flags, +
[PATCH libdrm 5/5] [libdrm] add syncobj timeline tests
Signed-off-by: Chunming Zhou
---
tests/amdgpu/Makefile.am | 3 +-
tests/amdgpu/amdgpu_test.c | 12 ++
tests/amdgpu/amdgpu_test.h | 21 +++
tests/amdgpu/meson.build | 2 +-
tests/amdgpu/syncobj_tests.c | 263 +++
5 files changed, 299 insertions(+), 2 deletions(-)
create mode 100644 tests/amdgpu/syncobj_tests.c
diff --git a/tests/amdgpu/Makefile.am b/tests/amdgpu/Makefile.am
index 447ff217..d3fbe2bb 100644
--- a/tests/amdgpu/Makefile.am
+++ b/tests/amdgpu/Makefile.am
@@ -33,4 +33,5 @@ amdgpu_test_SOURCES = \
vcn_tests.c \
uve_ib.h \
deadlock_tests.c \
- vm_tests.c
+ vm_tests.c \
+ syncobj_tests.c
diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
index 96fcd687..cdcb93a5 100644
--- a/tests/amdgpu/amdgpu_test.c
+++ b/tests/amdgpu/amdgpu_test.c
@@ -56,6 +56,7 @@
#define UVD_ENC_TESTS_STR "UVD ENC Tests"
#define DEADLOCK_TESTS_STR "Deadlock Tests"
#define VM_TESTS_STR "VM Tests"
+#define SYNCOBJ_TIMELINE_TESTS_STR "SYNCOBJ TIMELINE Tests"
/**
* Open handles for amdgpu devices
@@ -116,6 +117,12 @@ static CU_SuiteInfo suites[] = {
.pCleanupFunc = suite_vm_tests_clean,
.pTests = vm_tests,
},
+ {
+ .pName = SYNCOBJ_TIMELINE_TESTS_STR,
+ .pInitFunc = suite_syncobj_timeline_tests_init,
+ .pCleanupFunc = suite_syncobj_timeline_tests_clean,
+ .pTests = syncobj_timeline_tests,
+ },
CU_SUITE_INFO_NULL,
};
@@ -165,6 +172,11 @@ static Suites_Active_Status suites_active_stat[] = {
.pName = VM_TESTS_STR,
.pActive = suite_vm_tests_enable,
},
+ {
+ .pName = SYNCOBJ_TIMELINE_TESTS_STR,
+ .pActive = suite_syncobj_timeline_tests_enable,
+ },
+
};
diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h
index 0609a74b..946e91c2 100644
--- a/tests/amdgpu/amdgpu_test.h
+++ b/tests/amdgpu/amdgpu_test.h
@@ -194,6 +194,27 @@ CU_BOOL suite_vm_tests_enable(void);
*/
extern CU_TestInfo vm_tests[];
+/**
+ * Initialize syncobj timeline test suite
+ */
+int suite_syncobj_timeline_tests_init();
+
+/**
+ * Deinitialize syncobj timeline test suite
+ */
+int suite_syncobj_timeline_tests_clean();
+
+/**
+ * Decide if the suite is enabled by default or not.
+ */
+CU_BOOL suite_syncobj_timeline_tests_enable(void);
+
+/**
+ * Tests in syncobj timeline test suite
+ */
+extern CU_TestInfo syncobj_timeline_tests[];
+
+
/**
* Helper functions
*/
diff --git a/tests/amdgpu/meson.build b/tests/amdgpu/meson.build
index 4c1237c6..3ceec715 100644
--- a/tests/amdgpu/meson.build
+++ b/tests/amdgpu/meson.build
@@ -24,7 +24,7 @@ if dep_cunit.found()
files(
'amdgpu_test.c', 'basic_tests.c', 'bo_tests.c', 'cs_tests.c',
'vce_tests.c', 'uvd_enc_tests.c', 'vcn_tests.c', 'deadlock_tests.c',
- 'vm_tests.c',
+ 'vm_tests.c', 'syncobj_tests.c',
),
dependencies : [dep_cunit, dep_threads],
include_directories : [inc_root, inc_drm,
include_directories('../../amdgpu')],
diff --git a/tests/amdgpu/syncobj_tests.c b/tests/amdgpu/syncobj_tests.c
new file mode 100644
index ..2a345490
--- /dev/null
+++ b/tests/amdgpu/syncobj_tests.c
@@ -0,0 +1,263 @@
+/*
+ * Copyright 2017 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+*/
+
+#include "CUnit/Basic.h"
+
+#include "amdgpu_test.h"
+#include "amdgpu_drm.h"
+#include "amdgpu_internal.h"
+#include
+
+static amdgpu_device_handle device_handle;
+static uint32_t major_version;
+static uint32_t minor_version;
+
+static void amdgpu_syncobj_timeline_test(void);
+
+CU_BOOL suite_syncobj_timeline_tests_enable(void)
+{
+ return CU_TRUE;
+}
+
+int suite_syncobj_timeline_tests_init(void)
+{
+ int r;
+
+ r = amdgpu
Re: [PATCH 1/5] drm: add support of syncobj timeline point wait v4
Am 02.11.18 um 09:25 schrieb Chunming Zhou:
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.
v3:
userspace can specify two kinds waits::
a. Wait for time point to be completed.
b. and wait for time point to become available
v4:
rebase
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
---
drivers/gpu/drm/drm_internal.h | 2 +
drivers/gpu/drm/drm_ioctl.c| 2 +
drivers/gpu/drm/drm_syncobj.c | 115 -
include/uapi/drm/drm.h | 18 ++
4 files changed, 121 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_private);
int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 94bd872d56c4..a9a17ed35cc4 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -675,6 +675,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 4dca5f7e8c4b..9dc54a345480 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -153,11 +153,12 @@ static void drm_syncobj_add_callback_locked(struct
drm_syncobj *syncobj,
}
static void drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+ uint64_t point,
struct dma_fence **fence,
struct drm_syncobj_cb *cb,
drm_syncobj_func_t func)
{
- u64 pt_value = 0;
+ u64 pt_value = point;
WARN_ON(*fence);
@@ -854,6 +855,7 @@ struct syncobj_wait_entry {
struct dma_fence *fence;
struct dma_fence_cb fence_cb;
struct drm_syncobj_cb syncobj_cb;
+ u64point;
};
static void syncobj_wait_fence_func(struct dma_fence *fence,
@@ -871,12 +873,13 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj
*syncobj,
struct syncobj_wait_entry *wait =
container_of(cb, struct syncobj_wait_entry, syncobj_cb);
- drm_syncobj_search_fence(syncobj, 0, 0, &wait->fence);
+ drm_syncobj_search_fence(syncobj, wait->point, 0, &wait->fence);
wake_up_process(wait->task);
}
static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
+ void __user *user_points,
uint32_t count,
uint32_t flags,
signed long timeout,
@@ -884,12 +887,37 @@ static signed long drm_syncobj_array_wait_timeout(struct
drm_syncobj **syncobjs,
{
struct syncobj_wait_entry *entries;
struct dma_fence *fence;
+ uint64_t *points;
uint32_t signaled_count, i;
- entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
- if (!entries)
+ points = kmalloc_array(count, sizeof(*points), GFP_KERNEL);
+ if (points == NULL)
return -ENOMEM;
+ if (!user_points) {
+ memset(points, 0, count * sizeof(uint64_t));
+ } else if (copy_from_user(points, user_points, sizeof(uint64_t) *
count)) {
+ timeout = -EFAULT;
+ goto err_free_points;
+ }
+
+
Double empty line here.
+ if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE) {
+ struct dma_fence *fence;
+ for (i = 0; i < count; ++i) {
+ timeout = drm_syncobj_search
Re: [PATCH 2/5] drm: add timeline syncobj payload query ioctl v2
Am 02.11.18 um 09:25 schrieb Chunming Zhou:
user mode can query timeline payload.
v2: check return value of copy_to_user
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
---
drivers/gpu/drm/drm_internal.h | 2 ++
drivers/gpu/drm/drm_ioctl.c| 2 ++
drivers/gpu/drm/drm_syncobj.c | 52 ++
include/uapi/drm/drm.h | 11 +++
4 files changed, 67 insertions(+)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 566d44e3c782..9c4826411a3c 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -189,6 +189,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
/* drm_framebuffer.c */
void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a9a17ed35cc4..7578ef6dc1d1 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -681,6 +681,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl,
DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE,
drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl,
DRM_MASTER|DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 9dc54a345480..9ad58d0d21cd 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1291,3 +1291,55 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void
*data,
return ret;
}
+
+static void drm_syncobj_timeline_query_payload(struct drm_syncobj *syncobj,
+ uint64_t *point)
+{
+ if (syncobj->type != DRM_SYNCOBJ_TYPE_TIMELINE) {
+ DRM_ERROR("Normal syncobj cann't be queried!");
+ *point = 0;
+ return;
+ }
+ *point = syncobj->signal_point;
+}
+
+int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private)
+{
+ struct drm_syncobj_timeline_query *args = data;
+ struct drm_syncobj **syncobjs;
+ uint64_t *points;
+ uint32_t i;
+ int ret;
+
+ if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+ return -ENODEV;
+
+ if (args->count_handles == 0)
+ return -EINVAL;
+
+ ret = drm_syncobj_array_find(file_private,
+u64_to_user_ptr(args->handles),
+args->count_handles,
+&syncobjs);
+ if (ret < 0)
+ return ret;
+
+
+ points = kmalloc_array(args->count_handles, sizeof(*points),
+ GFP_KERNEL);
+ if (points == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ for (i = 0; i < args->count_handles; i++)
+ drm_syncobj_timeline_query_payload(syncobjs[i], &points[i]);
+ ret = copy_to_user(u64_to_user_ptr(args->points), points,
+ sizeof(uint64_t) * args->count_handles) ? -EFAULT :
0;
Is it really better to allocate an array for this? Why not just handle
it entry by entry?
Christian.
+
+ kfree(points);
+out:
+ drm_syncobj_array_free(syncobjs, args->count_handles);
+
+ return ret;
+}
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index c8bc1414753d..23c4979d8a1c 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -771,6 +771,15 @@ struct drm_syncobj_array {
__u32 pad;
};
+struct drm_syncobj_timeline_query {
+ __u64 handles;
+ /* points are timeline syncobjs payloads returned by query ioctl */
+ __u64 points;
+ __u32 count_handles;
+ __u32 pad;
+};
+
+
/* Query current scanout sequence number */
struct drm_crtc_get_sequence {
__u32 crtc_id; /* requested crtc_id */
@@ -928,6 +937,8 @@ extern "C" {
#define DRM_IOCTL_MODE_REVOKE_LEASE DRM_IOWR(0xC9, struct
drm_mode_revoke_lease)
#define DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT DRM_IOWR(0xCA, struct drm_syncobj_timeline_wait)
+#define DRM_IO
[Bug 108579] Request new account for IGT
https://bugs.freedesktop.org/show_bug.cgi?id=108579 Daniel Vetter changed: What|Removed |Added Resolution|--- |INVALID Status|NEW |RESOLVED --- Comment #2 from Daniel Vetter --- For igt you need to make sure you have a gitlab.freedesktop.org account (make sure username matches with what you have as a shell account), and then click request access on https://gitlab.freedesktop.org/drm/igt-gpu-tools -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: add timeline support for syncobj export/import
Am 02.11.18 um 09:25 schrieb Chunming Zhou:
user space can specify timeline point fence to export/import.
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
From the coding it looks good to me, but I can't judge if that really
makes sense to export/import individual points.
I would have rather expected that always the whole timeline is
exported/imported. Otherwise the whole thing with waiting for sync
points to appear doesn't really make sense to me.
Christian.
---
drivers/gpu/drm/drm_internal.h | 4 ++
drivers/gpu/drm/drm_ioctl.c| 4 ++
drivers/gpu/drm/drm_syncobj.c | 76 ++
include/uapi/drm/drm.h | 11 +
4 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 9c4826411a3c..5ad6cbdb68ab 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -181,6 +181,10 @@ int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_private);
int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl2(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl2(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7578ef6dc1d1..364d26e949cf 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -673,6 +673,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE,
drm_syncobj_fd_to_handle_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2,
drm_syncobj_handle_to_fd_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2,
drm_syncobj_fd_to_handle_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 9ad58d0d21cd..dffc42ba2f91 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -677,7 +677,7 @@ static int drm_syncobj_fd_to_handle(struct drm_file
*file_private,
}
static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
- int fd, int handle)
+ int fd, int handle, uint64_t
point)
{
struct dma_fence *fence = sync_file_get_fence(fd);
struct drm_syncobj *syncobj;
@@ -691,14 +691,14 @@ static int drm_syncobj_import_sync_file_fence(struct
drm_file *file_private,
return -ENOENT;
}
- drm_syncobj_replace_fence(syncobj, 0, fence);
+ drm_syncobj_replace_fence(syncobj, point, fence);
dma_fence_put(fence);
drm_syncobj_put(syncobj);
return 0;
}
static int drm_syncobj_export_sync_file(struct drm_file *file_private,
- int handle, int *p_fd)
+ int handle, uint64_t point, int *p_fd)
{
int ret;
struct dma_fence *fence;
@@ -708,7 +708,7 @@ static int drm_syncobj_export_sync_file(struct drm_file
*file_private,
if (fd < 0)
return fd;
- ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
+ ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence);
if (ret)
goto err_put_fd;
@@ -817,9 +817,14 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
return -EINVAL;
- if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+ if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) {
+ struct drm_syncobj *syncobj = drm_syncobj_find(file_private,
+ args->handle);
+ if (!syncobj || syncobj->type != DRM_SYNCOBJ_TYPE_BINARY)
+ return -EINVAL;
return drm_syncobj_export_sync_file(file_private, args
[Bug 108496] Non functional -d, --dry-run option for the igt_runner
https://bugs.freedesktop.org/show_bug.cgi?id=108496 Petri Latvala changed: What|Removed |Added Assignee|dri-devel@lists.freedesktop |petri.latv...@intel.com |.org| -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108075] [CI][BAT] igt_runner: abort on TAINT_PAGE, TAINT_DIE, and TAINT_OOPS
https://bugs.freedesktop.org/show_bug.cgi?id=108075 Martin Peres changed: What|Removed |Added Summary|[CI][BAT] |[CI][BAT] igt_runner: abort |igt@amdgpu/amd_prime@(amd-t |on TAINT_PAGE, TAINT_DIE, |o-i915|i915-to-amd) - |and TAINT_OOPS |timeout - last line in | |dmesg "[IGT] amd_prime: | |executing" | -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [v2 1/2] drm: Add colorspace property
Op 31-10-18 om 13:05 schreef Uma Shankar:
> This patch adds a colorspace property enabling
> userspace to switch to various supported colorspaces.
> This will help enable BT2020 along with other colorspaces.
>
> v2: Addressed Maarten and Ville's review comments. Enhanced
> the colorspace enum to incorporate both HDMI and DP supported
> colorspaces. Also, added a default option for colorspace.
>
> Signed-off-by: Uma Shankar
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 4
> drivers/gpu/drm/drm_connector.c | 44
> +++
> include/drm/drm_connector.h | 7 +++
> include/drm/drm_mode_config.h | 6 ++
> include/uapi/drm/drm_mode.h | 24 +
> 5 files changed, 85 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index d5b7f31..9e1d46b 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -721,6 +721,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> state->picture_aspect_ratio = val;
> } else if (property == config->content_type_property) {
> state->content_type = val;
> + } else if (property == config->colorspace_property) {
> + state->colorspace = val;
> } else if (property == connector->scaling_mode_property) {
> state->scaling_mode = val;
> } else if (property == connector->content_protection_property) {
> @@ -795,6 +797,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> *val = state->picture_aspect_ratio;
> } else if (property == config->content_type_property) {
> *val = state->content_type;
> + } else if (property == config->colorspace_property) {
> + *val = state->colorspace;
> } else if (property == connector->scaling_mode_property) {
> *val = state->scaling_mode;
> } else if (property == connector->content_protection_property) {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index aa18b1d..5ad52a0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -826,6 +826,38 @@ int drm_display_info_set_bus_formats(struct
> drm_display_info *info,
> };
> DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
>
> +static const struct drm_prop_enum_list colorspace[] = {
> + /* Standard Definition Colorimetry based on CEA 861 */
> + { COLORIMETRY_ITU_601, "601" },
> + { COLORIMETRY_ITU_709, "709" },
> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> + { COLORIMETRY_XV_YCC_601, "601" },
> + /* High Definition Colorimetry based on IEC 61966-2-4 */
> + { COLORIMETRY_XV_YCC_709, "709" },
2 definitions that share the same name?
All in all, I think the enum strings need to be more descriptive and
self-consistent.
> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
> + { COLORIMETRY_S_YCC_601, "s601" },
> + /* Colorimetry based on IEC 61966-2-5 [33] */
> + { COLORIMETRY_ADOBE_YCC_601, "adobe601" },
> + /* Colorimetry based on IEC 61966-2-5 */
> + { COLORIMETRY_ADOBE_RGB, "adobe_rgb" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { COLORIMETRY_BT2020_RGB, "BT2020_rgb" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { COLORIMETRY_BT2020_YCC, "BT2020_ycc" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { COLORIMETRY_BT2020_CYCC, "BT2020_cycc" },
> + /* DP MSA Colorimetry */
> + { COLORIMETRY_Y_CBCR_ITU_601, "YCBCR_ITU_601" },
> + { COLORIMETRY_Y_CBCR_ITU_709, "YCBCR_ITU_709" },
> + { COLORIMETRY_SRGB, "SRGB" },
sRGB
> + { COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide Gamut" },
> + { COLORIMETRY_SCRGB, "SCRGB" },
scRGB?
> + { COLORIMETRY_DCI_P3, "DCIP3" },
DCI-P3?
> + { COLORIMETRY_CUSTOM_COLOR_PROFILE, "Custom Proflie" },
^Typo
> + /* FOR HD 720p+, Default Colorimetry is based on ITU-R BT.709 */
> + { COLORIMETRY_DEFAULT, "Default: ITU_709" },
This enum together with the code below is broken.
+ COLORIMETRY_DEFAULT = COLORIMETRY_ITU_709,
I would just make it COLORIMETRY_UNSET = "Unset".
To set the default colorimetry for all drivers, just make the default value 0
(preferred),
or add it to __drm_atomic_helper_connector_reset().
> +};
> +
> /**
> * DOC: standard connector properties
> *
> @@ -972,6 +1004,12 @@ int drm_display_info_set_bus_formats(struct
> drm_display_info *info,
> * can also expose this property to external outputs, in which case they
> * must support "None", which should be the default (since external screens
> * have a built-in scaler).
> + *
> + * colorspace:
> + * This property helps select a suitable colorspace based on the sink
> + * capability. Modern sink devices support wider gamut like BT2020.
> + * This helps switch to BT2020 mode if t
Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and enable modeset
Op 31-10-18 om 13:05 schreef Uma Shankar: > This patch attaches the colorspace connector property to the > hdmi connector. Based on colorspace change, modeset will be > triggered to switch to new colorspace. > > Based on colorspace property value create an infoframe > with appropriate colorspace. This can be used to send an > infoframe packet with proper colorspace value set which > will help to enable wider color gamut like BT2020 on sink. > > v2: Merged the changes of creating infoframe as well to this > patch as per Maarten's suggestion. > > Signed-off-by: Uma Shankar > --- > drivers/gpu/drm/i915/intel_atomic.c | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 5 + > 2 files changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c > b/drivers/gpu/drm/i915/intel_atomic.c > index a5a2c8f..35ef70a 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -125,6 +125,7 @@ int intel_digital_connector_atomic_check(struct > drm_connector *conn, >*/ > if (new_conn_state->force_audio != old_conn_state->force_audio || > new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb || > + new_state->colorspace != old_state->colorspace || > new_conn_state->base.picture_aspect_ratio != > old_conn_state->base.picture_aspect_ratio || > new_conn_state->base.content_type != > old_conn_state->base.content_type || > new_conn_state->base.scaling_mode != > old_conn_state->base.scaling_mode) > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 129b880..8a41fb3 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -486,6 +486,8 @@ static void intel_hdmi_set_avi_infoframe(struct > intel_encoder *encoder, > else > frame.avi.colorspace = HDMI_COLORSPACE_RGB; > > + frame.avi.extended_colorimetry = conn_state->colorspace; > + > drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode, > crtc_state->limited_color_range ? > HDMI_QUANTIZATION_RANGE_LIMITED : > @@ -2125,6 +2127,9 @@ static void intel_hdmi_destroy(struct drm_connector > *connector) > intel_attach_broadcast_rgb_property(connector); > intel_attach_aspect_ratio_property(connector); > drm_connector_attach_content_type_property(connector); > + drm_object_attach_property(&connector->base, > + connector->dev->mode_config.colorspace_property, > + COLORIMETRY_ITU_709); Just put 0 here.. If you want to init the default colorspace, put it in the first patch. We should perhaps hide color spaces that are not supported on HDMI? > connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > } > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: add timeline support for syncobj export/import
On 2018年11月02日 16:46, Christian König wrote:
Am 02.11.18 um 09:25 schrieb Chunming Zhou:
user space can specify timeline point fence to export/import.
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
From the coding it looks good to me, but I can't judge if that really
makes sense to export/import individual points.
I would have rather expected that always the whole timeline is
exported/imported. Otherwise the whole thing with waiting for sync
points to appear doesn't really make sense to me.
We can Cpu wait on individual points, why can not we export/import them?
I confirmed with Daniel before, he said the extension need both them,
export/import individual points and export/import the whole timeline
semaphore.
More discussion is
https://gitlab.khronos.org/vulkan/vulkan/merge_requests/2696
Thanks,
David Zhou
Christian.
---
drivers/gpu/drm/drm_internal.h | 4 ++
drivers/gpu/drm/drm_ioctl.c | 4 ++
drivers/gpu/drm/drm_syncobj.c | 76 ++
include/uapi/drm/drm.h | 11 +
4 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h
b/drivers/gpu/drm/drm_internal.h
index 9c4826411a3c..5ad6cbdb68ab 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -181,6 +181,10 @@ int drm_syncobj_handle_to_fd_ioctl(struct
drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl2(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl2(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void
*data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7578ef6dc1d1..364d26e949cf 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -673,6 +673,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE,
drm_syncobj_fd_to_handle_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2,
drm_syncobj_handle_to_fd_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2,
drm_syncobj_fd_to_handle_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c
index 9ad58d0d21cd..dffc42ba2f91 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -677,7 +677,7 @@ static int drm_syncobj_fd_to_handle(struct
drm_file *file_private,
}
static int drm_syncobj_import_sync_file_fence(struct drm_file
*file_private,
- int fd, int handle)
+ int fd, int handle, uint64_t point)
{
struct dma_fence *fence = sync_file_get_fence(fd);
struct drm_syncobj *syncobj;
@@ -691,14 +691,14 @@ static int
drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
return -ENOENT;
}
- drm_syncobj_replace_fence(syncobj, 0, fence);
+ drm_syncobj_replace_fence(syncobj, point, fence);
dma_fence_put(fence);
drm_syncobj_put(syncobj);
return 0;
}
static int drm_syncobj_export_sync_file(struct drm_file
*file_private,
- int handle, int *p_fd)
+ int handle, uint64_t point, int *p_fd)
{
int ret;
struct dma_fence *fence;
@@ -708,7 +708,7 @@ static int drm_syncobj_export_sync_file(struct
drm_file *file_private,
if (fd < 0)
return fd;
- ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence);
+ ret = drm_syncobj_find_fence(file_private, handle, point, 0,
&fence);
if (ret)
goto err_put_fd;
@@ -817,9 +817,14 @@ drm_syncobj_handle_to_fd_ioctl(struct
drm_device *dev, void *data,
args->flags !=
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
return -EINVAL;
- if (args->flags &
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+ if (args->flags &
DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) {
+ struct drm_syncobj *syncobj = drm_syncobj_find(file_private,
+ args->handle);
+ if (!syncobj || syncobj->type != DRM_SYNCOBJ_TYPE_BINAR
Re: [Freedreno] [PATCH] drm/msm: Optimize GPU crashstate capture read path
Thanks for the comments Jordan -
On 11/1/2018 8:34 PM, Jordan Crouse wrote:
On Thu, Nov 01, 2018 at 02:05:41PM +0530, Sharat Masetty wrote:
When the userspace tries to read the crashstate dump, the read side
implementation in the driver currently ascii85 encodes all the binary
buffers and it does this each time the read system call is called.
A userspace tool like cat typically does a page by page read and the
number of read calls depends on the size of the data captured by the
driver. This is certainly not desirable and does not scale well with
large captures.
This patch starts off with moving the encoding part to the capture time,
that is when the GPU tries to recover after a hang. Now we only encode
once per buffer object and not per page read. With this there is an
immediate >10X speed improvement in crashstate save time.
The flip side however is that the GPU recovery time increases and this
negatively impacts the user experience, so this is handled by moving the
encoding part to a worker thread and only register with the dev_coredump
once the encoding of the buffers is complete.
Does your tree have https://patchwork.freedesktop.org/patch/257181/ ? That will
help with a lot of the problems you have described.
The issue is that even with small sized files like ~5MB or so, the
capture time is huge and sometime does not survive the dev_coredump
timeout of 5 minutes. With this change however I am able to dump large
file sizes like 60MB or so in reasonable amount of time (~50 seconds).
As for the patch that you shared, I am thinking that we should drop it
and instead go with this or whatever we agree on. The patch loses stuff
like IB2's which might be important while debugging hang issues.
Signed-off-by: Sharat Masetty
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 35 +++--
drivers/gpu/drm/msm/msm_gpu.c | 93 +++--
drivers/gpu/drm/msm/msm_gpu.h | 1 +
3 files changed, 98 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 141062f..7441571 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -406,7 +406,7 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct
msm_gpu_state *state)
size = j + 1;
if (size) {
- state->ring[i].data = kmalloc(size << 2, GFP_KERNEL);
+ state->ring[i].data = kvmalloc(size << 2, GFP_KERNEL);
if (state->ring[i].data) {
memcpy(state->ring[i].data, gpu->rb[i]->start, size
<< 2);
state->ring[i].data_size = size << 2;
This is a good fix, but unrelated to this change as detailed above. Should be
separate.
@@ -445,7 +445,7 @@ void adreno_gpu_state_destroy(struct msm_gpu_state *state)
int i;
for (i = 0; i < ARRAY_SIZE(state->ring); i++)
- kfree(state->ring[i].data);
+ kvfree(state->ring[i].data);
As above.
for (i = 0; state->bos && i < state->nr_bos; i++)
kvfree(state->bos[i].data);
@@ -475,34 +475,15 @@ int adreno_gpu_state_put(struct msm_gpu_state *state)
#if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
-static void adreno_show_object(struct drm_printer *p, u32 *ptr, int len)
+static void adreno_show_object(struct drm_printer *p, void *ptr)
{
- char out[ASCII85_BUFSZ];
- long l, datalen, i;
-
- if (!ptr || !len)
- return;
-
- /*
-* Only dump the non-zero part of the buffer - rarely will any data
-* completely fill the entire allocated size of the buffer
-*/
- for (datalen = 0, i = 0; i < len >> 2; i++) {
- if (ptr[i])
- datalen = (i << 2) + 1;
- }
-
- /* Skip printing the object if it is empty */
- if (datalen == 0)
+ if (!ptr)
return;
- l = ascii85_encode_len(datalen);
-
drm_puts(p, "data: !!ascii85 |\n");
drm_puts(p, " ");
- for (i = 0; i < l; i++)
- drm_puts(p, ascii85_encode(ptr[i], out));
+ drm_puts(p, ptr);
drm_puts(p, "\n");
}
@@ -534,8 +515,7 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state
*state,
drm_printf(p, "wptr: %d\n", state->ring[i].wptr);
drm_printf(p, "size: %d\n", MSM_GPU_RINGBUFFER_SZ);
- adreno_show_object(p, state->ring[i].data,
- state->ring[i].data_size);
+ adreno_show_object(p, state->ring[i].data);
}
if (state->bos) {
@@ -546,8 +526,7 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state
*state,
state->bos[i].iova);
drm_printf(p, "size: %zd\n", state->bos[i].size);
- adreno_show_object(p, state->bos[i].data,
Re: [PATCH 3/5] drm: add timeline support for syncobj export/import
Am 02.11.18 um 10:42 schrieb zhoucm1:
>
>
> On 2018年11月02日 16:46, Christian König wrote:
>> Am 02.11.18 um 09:25 schrieb Chunming Zhou:
>>> user space can specify timeline point fence to export/import.
>>>
>>> Signed-off-by: Chunming Zhou
>>> Cc: Daniel Rakos
>>> Cc: Jason Ekstrand
>>> Cc: Bas Nieuwenhuizen
>>> Cc: Dave Airlie
>>> Cc: Christian König
>>> Cc: Chris Wilson
>>
>> From the coding it looks good to me, but I can't judge if that really
>> makes sense to export/import individual points.
>>
>> I would have rather expected that always the whole timeline is
>> exported/imported. Otherwise the whole thing with waiting for sync
>> points to appear doesn't really make sense to me.
> We can Cpu wait on individual points, why can not we export/import them?
> I confirmed with Daniel before, he said the extension need both them,
> export/import individual points and export/import the whole timeline
> semaphore.
> More discussion is
> https://gitlab.khronos.org/vulkan/vulkan/merge_requests/2696
Ah! It looked initially to me that this was the only way to
export/import timeline sync objects.
Thinking more about it wouldn't it be better to provide a function to
signal a sync point from another sync point?
That would provide the same functionality, would be cleaner to implement
and more flexible on top.
E.g. if you want to export only a specific sync point you first create a
new syncobj and the move over this sync point into the syncobj.
Otherwise you mix multiple operations into one IOCTL and that is usually
not a good idea.
Regards,
Christian.
>
> Thanks,
> David Zhou
>>
>> Christian.
>>
>>> ---
>>> drivers/gpu/drm/drm_internal.h | 4 ++
>>> drivers/gpu/drm/drm_ioctl.c | 4 ++
>>> drivers/gpu/drm/drm_syncobj.c | 76
>>> ++
>>> include/uapi/drm/drm.h | 11 +
>>> 4 files changed, 88 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_internal.h
>>> b/drivers/gpu/drm/drm_internal.h
>>> index 9c4826411a3c..5ad6cbdb68ab 100644
>>> --- a/drivers/gpu/drm/drm_internal.h
>>> +++ b/drivers/gpu/drm/drm_internal.h
>>> @@ -181,6 +181,10 @@ int drm_syncobj_handle_to_fd_ioctl(struct
>>> drm_device *dev, void *data,
>>> struct drm_file *file_private);
>>> int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void
>>> *data,
>>> struct drm_file *file_private);
>>> +int drm_syncobj_handle_to_fd_ioctl2(struct drm_device *dev, void
>>> *data,
>>> + struct drm_file *file_private);
>>> +int drm_syncobj_fd_to_handle_ioctl2(struct drm_device *dev, void
>>> *data,
>>> + struct drm_file *file_private);
>>> int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
>>> struct drm_file *file_private);
>>> int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void
>>> *data,
>>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>>> index 7578ef6dc1d1..364d26e949cf 100644
>>> --- a/drivers/gpu/drm/drm_ioctl.c
>>> +++ b/drivers/gpu/drm/drm_ioctl.c
>>> @@ -673,6 +673,10 @@ static const struct drm_ioctl_desc drm_ioctls[]
>>> = {
>>> DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE,
>>> drm_syncobj_fd_to_handle_ioctl,
>>> DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>> + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2,
>>> drm_syncobj_handle_to_fd_ioctl2,
>>> + DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>> + DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2,
>>> drm_syncobj_fd_to_handle_ioctl2,
>>> + DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
>>> DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
>>> drm_syncobj_timeline_wait_ioctl,
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index 9ad58d0d21cd..dffc42ba2f91 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -677,7 +677,7 @@ static int drm_syncobj_fd_to_handle(struct
>>> drm_file *file_private,
>>> }
>>> static int drm_syncobj_import_sync_file_fence(struct drm_file
>>> *file_private,
>>> - int fd, int handle)
>>> + int fd, int handle, uint64_t point)
>>> {
>>> struct dma_fence *fence = sync_file_get_fence(fd);
>>> struct drm_syncobj *syncobj;
>>> @@ -691,14 +691,14 @@ static int
>>> drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
>>> return -ENOENT;
>>> }
>>> - drm_syncobj_replace_fence(syncobj, 0, fence);
>>> + drm_syncobj_replace_fence(syncobj, point, fence);
>>> dma_fence_put(fence);
>>> drm_syncobj_put(syncobj);
>>> return 0;
>>> }
>>> static int drm_syncobj_export_sync_file(struct drm_file
>>> *file_private,
>>> - i
[PULL] drm-intel-next for v4.21/v5.1
Hi Dave - I just tagged this minutes ago, but I'm sending this now because I'll be out for about a week. I don't expect you to pull this until some time after -rc1 anyway. I'm asking Joonas and Rodrigo to tell you if this one's a go or a no go. There's a couple of backmerges in there, and I'd probably like to do another one once you've backmerged from Linus. I suck at making myself write proper changelogs. It's a random unordered jumble, I'm afraid. One of these days I'll learn to do it right. Promise! BR, Jani. drm-intel-next-2018-11-02: The first big pile of changes for v4.21/v5.1: - DP Display Stream Compression preliminary work, helpers, etc. (Manasi, Anusha) - Fix flex IO lane count programming (Manasi) - GEM selftest updates (Chris, Matthew) - ICL DSI enabling (Madhav, Jani) - CSR firmware definition cleanup (Jani) - CSR ICL stepping info, DC5/DC6 debugfs info (Jyoti) - intel_display.c cleanups and code movement (Jani, Ville) - PSR fixes and cleanup, enable PSR1 by default on gen9+ (José, Dhinakaran) - Perf updates (Lionel) - DP MST fixes (Lyude) - Improved DP MST support logging (Ville) - ICL workarounds (Oscar, Radhakrishna, Lucas, Anuj) - Workaround cleanups (Rodrigo) - HDCP 2.2 prep work (Ramalingam) - AVI infoframes for LSPCON (Shashank) - CRTC output formats YCBCR 4:2:0 and 4:4:4 (Shashank) - ICL PLL refactoring (Vandita) - Watermark fixes (Paulo) - Master intr fixes (Mika) - Amberlake platform (José, Shawn) - Ensure HDA suspend/resume ordering (Imre) - eDP orientation quirks (Hans) - DP detect and link retrain fixes and cleanups (Dhinakaran) - GuC fixes, cleanups and selftests (Daniele, Michal, Chris) - ICL combophy/TC fixes and cleanups (Mahesh, Lucas, José) - ICL RGB565 90/270 plane rotation (Juha-Pekka) - HDMI 2.0 audio N values (Clint) - Aux channel refactoring, ICL aux power fixes (Imre) - Opregion suspend/resume improvement (Chris) - Sort platform if ladders newest-to-oldest (Rodrigo) - IPC fixes (José) - PCH reset handshake fixes for PCH NOP (José) - Store available engine masks in intel info (Tvrtko) - Fix video DIP register definitions (Dhinakaran) - ICL planar formats, NV12 (Maarten) - Plane alpha blending support (Maarten) - crtc->config usage removal cleanups (Maarten) - Plane init cleanups (Ville) - Use BITS_PER_TYPE (Chris) - Remove i915.enable_ppgtt override (Chris) - Scheduling priority improvements (Chris) - Fix GTT 64-bit computations on 32-bit systems (Chris) - A number of display fixes all around... (Ville) - A number of GEM fixes all around... (Chris) - Tons of other fixes and improvements (Everyone) - Failure to properly credit everyone in the above changelog (Jani) BR, Jani. The following changes since commit f9885ef875e9160454392f85159163674159c51f: Merge tag 'drm-intel-next-fixes-2018-10-25' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2018-11-02 15:17:57 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-next-2018-11-02 for you to fetch changes up to 5468a543409653a94344671371ff784703fdcb26: drm/i915: Update DRIVER_DATE to 20181102 (2018-11-02 12:04:11 +0200) The first big pile of changes for v4.21/v5.1: - DP Display Stream Compression preliminary work, helpers, etc. (Manasi, Anusha) - Fix flex IO lane count programming (Manasi) - GEM selftest updates (Chris, Matthew) - ICL DSI enabling (Madhav, Jani) - CSR firmware definition cleanup (Jani) - CSR ICL stepping info, DC5/DC6 debugfs info (Jyoti) - intel_display.c cleanups and code movement (Jani, Ville) - PSR fixes and cleanup, enable PSR1 by default on gen9+ (José, Dhinakaran) - Perf updates (Lionel) - DP MST fixes (Lyude) - Improved DP MST support logging (Ville) - ICL workarounds (Oscar, Radhakrishna, Lucas, Anuj) - Workaround cleanups (Rodrigo) - HDCP 2.2 prep work (Ramalingam) - AVI infoframes for LSPCON (Shashank) - CRTC output formats YCBCR 4:2:0 and 4:4:4 (Shashank) - ICL PLL refactoring (Vandita) - Watermark fixes (Paulo) - Master intr fixes (Mika) - Amberlake platform (José, Shawn) - Ensure HDA suspend/resume ordering (Imre) - eDP orientation quirks (Hans) - DP detect and link retrain fixes and cleanups (Dhinakaran) - GuC fixes, cleanups and selftests (Daniele, Michal, Chris) - ICL combophy/TC fixes and cleanups (Mahesh, Lucas, José) - ICL RGB565 90/270 plane rotation (Juha-Pekka) - HDMI 2.0 audio N values (Clint) - Aux channel refactoring, ICL aux power fixes (Imre) - Opregion suspend/resume improvement (Chris) - Sort platform if ladders newest-to-oldest (Rodrigo) - IPC fixes (José) - PCH reset handshake fixes for PCH NOP (José) - Store available engine masks in intel info (Tvrtko) - Fix video DIP register definitions (Dhinakaran) - ICL planar formats, NV12 (Maarten) - Plane alpha blending support (Maarten) - crtc->config usage removal cleanups (Maarten) - Plane init cleanups
[PATCH 1/2] drm/scheduler: Set sched->thread to NULL on failure
In cases where the scheduler instance is used as a base object of another
driver object, it's not clear if the driver can call scheduler cleanup on the
fail path. So, Set the sched->thread to NULL, so that the driver can safely
call drm_sched_fini() during cleanup.
Signed-off-by: Sharat Masetty
---
drivers/gpu/drm/scheduler/sched_main.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 44fe587..c993d10 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -594,7 +594,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
long timeout,
const char *name)
{
- int i;
+ int i, ret;
sched->ops = ops;
sched->hw_submission_limit = hw_submission;
sched->name = name;
@@ -615,8 +615,10 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
/* Each scheduler will run on a seperate kernel thread */
sched->thread = kthread_run(drm_sched_main, sched, sched->name);
if (IS_ERR(sched->thread)) {
+ ret = PTR_ERR(sched->thread);
+ sched->thread = NULL;
DRM_ERROR("Failed to create scheduler for %s.\n", name);
- return PTR_ERR(sched->thread);
+ return ret;
}
return 0;
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/scheduler: Add a start_timeout_notify() backend function
Add an optional backend function op which will let the scheduler clients
know when the timeout got scheduled on the scheduler instance. This will
help drivers with multiple schedulers(one per ring) measure time spent on
the ring accurately, eventually helping with better timeout detection.
Signed-off-by: Sharat Masetty
---
Here is an example of how I plan to use this new function callback.
[1] https://patchwork.freedesktop.org/patch/254227/
drivers/gpu/drm/scheduler/sched_main.c | 7 ++-
include/drm/gpu_scheduler.h| 6 ++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index c993d10..afd461e 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -192,8 +192,13 @@ bool drm_sched_dependency_optimized(struct dma_fence*
fence,
static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched)
{
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
- !list_empty(&sched->ring_mirror_list))
+ !list_empty(&sched->ring_mirror_list)) {
+
schedule_delayed_work(&sched->work_tdr, sched->timeout);
+
+ if (sched->ops->start_timeout_notify)
+ sched->ops->start_timeout_notify(sched);
+ }
}
/* job_finish is called after hw fence signaled
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index d87b268..faf28b4 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -239,6 +239,12 @@ struct drm_sched_backend_ops {
* and it's time to clean it up.
*/
void (*free_job)(struct drm_sched_job *sched_job);
+
+ /*
+* (Optional) Called to let the driver know that a timeout detection
+* timer has been started.
+*/
+ void (*start_timeout_notify)(struct drm_gpu_scheduler *sched);
};
/**
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: add timeline support for syncobj export/import
On 2018年11月02日 17:54, Koenig, Christian wrote:
Am 02.11.18 um 10:42 schrieb zhoucm1:
On 2018年11月02日 16:46, Christian König wrote:
Am 02.11.18 um 09:25 schrieb Chunming Zhou:
user space can specify timeline point fence to export/import.
Signed-off-by: Chunming Zhou
Cc: Daniel Rakos
Cc: Jason Ekstrand
Cc: Bas Nieuwenhuizen
Cc: Dave Airlie
Cc: Christian König
Cc: Chris Wilson
From the coding it looks good to me, but I can't judge if that really
makes sense to export/import individual points.
I would have rather expected that always the whole timeline is
exported/imported. Otherwise the whole thing with waiting for sync
points to appear doesn't really make sense to me.
We can Cpu wait on individual points, why can not we export/import them?
I confirmed with Daniel before, he said the extension need both them,
export/import individual points and export/import the whole timeline
semaphore.
More discussion is
https://gitlab.khronos.org/vulkan/vulkan/merge_requests/2696
Ah! It looked initially to me that this was the only way to
export/import timeline sync objects.
Thinking more about it wouldn't it be better to provide a function to
signal a sync point from another sync point?
That would provide the same functionality, would be cleaner to implement
and more flexible on top.
E.g. if you want to export only a specific sync point you first create a
new syncobj and the move over this sync point into the syncobj.
Sorry, I didn't completely get your means. Could you detail a bit?
but I think we need to meet export/import goals:
1. export sepcific sync point or binary syncobj fence to sync file fd or
handle.
2. import fence from fd/handle to timeline sync point or banory syncobj.
3. export/import the whole timeline seamphore.
I'm still not sure how you mentioned can achieve these.
Thanks,
David
Otherwise you mix multiple operations into one IOCTL and that is usually
not a good idea.
Regards,
Christian.
Thanks,
David Zhou
Christian.
---
drivers/gpu/drm/drm_internal.h | 4 ++
drivers/gpu/drm/drm_ioctl.c | 4 ++
drivers/gpu/drm/drm_syncobj.c | 76
++
include/uapi/drm/drm.h | 11 +
4 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_internal.h
b/drivers/gpu/drm/drm_internal.h
index 9c4826411a3c..5ad6cbdb68ab 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -181,6 +181,10 @@ int drm_syncobj_handle_to_fd_ioctl(struct
drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl2(struct drm_device *dev, void
*data,
+ struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl2(struct drm_device *dev, void
*data,
+ struct drm_file *file_private);
int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void
*data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7578ef6dc1d1..364d26e949cf 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -673,6 +673,10 @@ static const struct drm_ioctl_desc drm_ioctls[]
= {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE,
drm_syncobj_fd_to_handle_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD2,
drm_syncobj_handle_to_fd_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE2,
drm_syncobj_fd_to_handle_ioctl2,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c
index 9ad58d0d21cd..dffc42ba2f91 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -677,7 +677,7 @@ static int drm_syncobj_fd_to_handle(struct
drm_file *file_private,
}
static int drm_syncobj_import_sync_file_fence(struct drm_file
*file_private,
- int fd, int handle)
+ int fd, int handle, uint64_t point)
{
struct dma_fence *fence = sync_file_get_fence(fd);
struct drm_syncobj *syncobj;
@@ -691,14 +691,14 @@ static int
drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
return -ENOENT;
}
- drm_syncobj_replace_fence(syncobj, 0, fence);
+ drm_syncobj_replace_fence(syncobj, point, fence);
dma_fence_put(fence);
drm_syncobj_put(syncobj);
return 0;
}
static int drm_syncobj_export_sync_
Re: [PATCH 2/2] drm/scheduler: Add a start_timeout_notify() backend function
Am 02.11.18 um 11:31 schrieb Sharat Masetty:
> Add an optional backend function op which will let the scheduler clients
> know when the timeout got scheduled on the scheduler instance. This will
> help drivers with multiple schedulers(one per ring) measure time spent on
> the ring accurately, eventually helping with better timeout detection.
>
> Signed-off-by: Sharat Masetty
Well, NAK. drm_sched_start_timeout() is called whenever the timer needs
to run, but that doesn't mean that the timer is started (e.g. it can
already be running).
So the callback would be called multiple times and not reflect the
actual job run time.
Additional to that it can be racy, e.g. we can complete multiple jobs at
a time before the timer is started again.
If you want to accurately count how much time you spend on each job/ring
you need to do this by measuring the time inside your driver instead.
E.g. for amdgpu I would get the time first in amdgpu_job_run() and then
again in amdgpu_job_free_cb() and calculate the difference.
Regards,
Christian.
> ---
> Here is an example of how I plan to use this new function callback.
>
> [1] https://patchwork.freedesktop.org/patch/254227/
>
> drivers/gpu/drm/scheduler/sched_main.c | 7 ++-
> include/drm/gpu_scheduler.h| 6 ++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index c993d10..afd461e 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -192,8 +192,13 @@ bool drm_sched_dependency_optimized(struct dma_fence*
> fence,
> static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched)
> {
> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> - !list_empty(&sched->ring_mirror_list))
> + !list_empty(&sched->ring_mirror_list)) {
> +
> schedule_delayed_work(&sched->work_tdr, sched->timeout);
> +
> + if (sched->ops->start_timeout_notify)
> + sched->ops->start_timeout_notify(sched);
> + }
> }
>
> /* job_finish is called after hw fence signaled
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index d87b268..faf28b4 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -239,6 +239,12 @@ struct drm_sched_backend_ops {
>* and it's time to clean it up.
>*/
> void (*free_job)(struct drm_sched_job *sched_job);
> +
> + /*
> + * (Optional) Called to let the driver know that a timeout detection
> + * timer has been started.
> + */
> + void (*start_timeout_notify)(struct drm_gpu_scheduler *sched);
> };
>
> /**
> --
> 1.9.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 106175] amdgpu.dc=1 shows performance issues with Xorg compositors when moving windows
https://bugs.freedesktop.org/show_bug.cgi?id=106175 Michel Dänzer changed: What|Removed |Added CC||nicholas.kazlaus...@amd.com --- Comment #40 from Michel Dänzer --- For the DC guys: We've now confirmed that the problem is due to some bad interaction between page flips and HW cursor updates. (In reply to tempel.julian from comment #37) > I think software cursor would also be unusable even if it left pageflipping > on. It causes nasty issues like flickering cursor or other visual corruption. Yeah, that's why xf86-video-amdgpu disables DRI page flipping while there's an SW cursor, as I said in comment 31. Note that the modesetting driver doesn't do this, allowing users to run into those issues. (In reply to grmat from comment #39) > (In reply to Michel Dänzer from comment #34) > > > > Right, you'd have to disable TearFree as well. > > Then I think the logs should represent that, even when the manpage tells me > that tearfree is using page flipping. If i set explicitly to off, and the > log says so, I expect it to be off. Patches or at least specific suggestions welcome, but I'm afraid it's tricky to describe all possible interactions concisely and clearly. DRI page flipping and TearFree are mostly separate things, but they use the same kernel page flipping mechanism, which is what matters for this issue. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [v2 1/2] drm: Add colorspace property
On Fri, Nov 02, 2018 at 10:19:10AM +0100, Maarten Lankhorst wrote:
> Op 31-10-18 om 13:05 schreef Uma Shankar:
> > This patch adds a colorspace property enabling
> > userspace to switch to various supported colorspaces.
> > This will help enable BT2020 along with other colorspaces.
> >
> > v2: Addressed Maarten and Ville's review comments. Enhanced
> > the colorspace enum to incorporate both HDMI and DP supported
> > colorspaces. Also, added a default option for colorspace.
> >
> > Signed-off-by: Uma Shankar
> > ---
> > drivers/gpu/drm/drm_atomic_uapi.c | 4
> > drivers/gpu/drm/drm_connector.c | 44
> > +++
> > include/drm/drm_connector.h | 7 +++
> > include/drm/drm_mode_config.h | 6 ++
> > include/uapi/drm/drm_mode.h | 24 +
> > 5 files changed, 85 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index d5b7f31..9e1d46b 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -721,6 +721,8 @@ static int drm_atomic_connector_set_property(struct
> > drm_connector *connector,
> > state->picture_aspect_ratio = val;
> > } else if (property == config->content_type_property) {
> > state->content_type = val;
> > + } else if (property == config->colorspace_property) {
> > + state->colorspace = val;
> > } else if (property == connector->scaling_mode_property) {
> > state->scaling_mode = val;
> > } else if (property == connector->content_protection_property) {
> > @@ -795,6 +797,8 @@ static int drm_atomic_connector_set_property(struct
> > drm_connector *connector,
> > *val = state->picture_aspect_ratio;
> > } else if (property == config->content_type_property) {
> > *val = state->content_type;
> > + } else if (property == config->colorspace_property) {
> > + *val = state->colorspace;
> > } else if (property == connector->scaling_mode_property) {
> > *val = state->scaling_mode;
> > } else if (property == connector->content_protection_property) {
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index aa18b1d..5ad52a0 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -826,6 +826,38 @@ int drm_display_info_set_bus_formats(struct
> > drm_display_info *info,
> > };
> > DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
> >
> > +static const struct drm_prop_enum_list colorspace[] = {
> > + /* Standard Definition Colorimetry based on CEA 861 */
> > + { COLORIMETRY_ITU_601, "601" },
> > + { COLORIMETRY_ITU_709, "709" },
> > + /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> > + { COLORIMETRY_XV_YCC_601, "601" },
> > + /* High Definition Colorimetry based on IEC 61966-2-4 */
> > + { COLORIMETRY_XV_YCC_709, "709" },
> 2 definitions that share the same name?
> All in all, I think the enum strings need to be more descriptive and
> self-consistent.
+1
> > + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
> > + { COLORIMETRY_S_YCC_601, "s601" },
> > + /* Colorimetry based on IEC 61966-2-5 [33] */
> > + { COLORIMETRY_ADOBE_YCC_601, "adobe601" },
Hans (cc:d) recetly noted that these things aren't called
Adobe anymore in the CTA-861 spec. There is some new name for
them, which I now forget.
> > + /* Colorimetry based on IEC 61966-2-5 */
> > + { COLORIMETRY_ADOBE_RGB, "adobe_rgb" },
> > + /* Colorimetry based on ITU-R BT.2020 */
> > + { COLORIMETRY_BT2020_RGB, "BT2020_rgb" },
> > + /* Colorimetry based on ITU-R BT.2020 */
> > + { COLORIMETRY_BT2020_YCC, "BT2020_ycc" },
> > + /* Colorimetry based on ITU-R BT.2020 */
> > + { COLORIMETRY_BT2020_CYCC, "BT2020_cycc" },
> > + /* DP MSA Colorimetry */
> > + { COLORIMETRY_Y_CBCR_ITU_601, "YCBCR_ITU_601" },
> > + { COLORIMETRY_Y_CBCR_ITU_709, "YCBCR_ITU_709" },
> > + { COLORIMETRY_SRGB, "SRGB" },
> sRGB
> > + { COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide Gamut" },
> > + { COLORIMETRY_SCRGB, "SCRGB" },
> scRGB?
> > + { COLORIMETRY_DCI_P3, "DCIP3" },
> DCI-P3?
> > + { COLORIMETRY_CUSTOM_COLOR_PROFILE, "Custom Proflie" },
> ^Typo
> > + /* FOR HD 720p+, Default Colorimetry is based on ITU-R BT.709 */
> > + { COLORIMETRY_DEFAULT, "Default: ITU_709" },
> This enum together with the code below is broken.
>
> + COLORIMETRY_DEFAULT = COLORIMETRY_ITU_709,
>
> I would just make it COLORIMETRY_UNSET = "Unset".
"Unset" might work. Though feels a bit strange to me. Other ideas
would be "Default", "Automatic", "Undefined" or something along those lines.
Ideally it should convey the meaning of "the driver will pick this for you",
and for that I'd lean towards "Default" or "Automatic".
>
> To set the default colorimetry for all drivers, just make the default value 0
> (preferred),
> or add it to
[PATCH v2] drm/msm: Move fence put to where failure occurs
If dma_fence_wait fails to wait for a supplied in-fence in
msm_ioctl_gem_submit, make sure we release that in-fence.
Also remove this dma_fence_put() from the 'out' label.
Signed-off-by: Robert Foss
---
Changes since v1:
- Chris Wilson: Make sure that dma_fence_put() is always executed
drivers/gpu/drm/msm/msm_gem_submit.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index a90aedd6883a..d5e6665a4c8f 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -411,7 +411,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
struct msm_file_private *ctx = file->driver_priv;
struct msm_gem_submit *submit;
struct msm_gpu *gpu = priv->gpu;
- struct dma_fence *in_fence = NULL;
struct sync_file *sync_file = NULL;
struct msm_gpu_submitqueue *queue;
struct msm_ringbuffer *ring;
@@ -444,6 +443,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
ring = gpu->rb[queue->prio];
if (args->flags & MSM_SUBMIT_FENCE_FD_IN) {
+ struct dma_fence *in_fence;
+
in_fence = sync_file_get_fence(args->fence_fd);
if (!in_fence)
@@ -453,11 +454,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
* Wait if the fence is from a foreign context, or if the fence
* array contains any fence from a foreign context.
*/
- if (!dma_fence_match_context(in_fence, ring->fctx->context)) {
+ ret = 0;
+ if (!dma_fence_match_context(in_fence, ring->fctx->context))
ret = dma_fence_wait(in_fence, true);
- if (ret)
- return ret;
- }
+
+ dma_fence_put(in_fence);
+ if (ret)
+ return ret;
}
ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -583,8 +586,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
}
out:
- if (in_fence)
- dma_fence_put(in_fence);
submit_cleanup(submit);
if (ret)
msm_gem_submit_free(submit);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108625] AMDGPU - Can't even get Xorg to start - Kernel driver hangs with ring buffer timeout on ARM64
https://bugs.freedesktop.org/show_bug.cgi?id=108625 --- Comment #2 from Carsten Haitzler --- Created attachment 142337 --> https://bugs.freedesktop.org/attachment.cgi?id=142337&action=edit log - dmesg -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108625] AMDGPU - Can't even get Xorg to start - Kernel driver hangs with ring buffer timeout on ARM64
https://bugs.freedesktop.org/show_bug.cgi?id=108625 --- Comment #3 from Carsten Haitzler --- Created attachment 142338 --> https://bugs.freedesktop.org/attachment.cgi?id=142338&action=edit log - xorg -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108625] AMDGPU - Can't even get Xorg to start - Kernel driver hangs with ring buffer timeout on ARM64
https://bugs.freedesktop.org/show_bug.cgi?id=108625 --- Comment #4 from Carsten Haitzler --- Created attachment 142339 --> https://bugs.freedesktop.org/attachment.cgi?id=142339&action=edit log - xorg - gdb attach + bt -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108625] AMDGPU - Can't even get Xorg to start - Kernel driver hangs with ring buffer timeout on ARM64
https://bugs.freedesktop.org/show_bug.cgi?id=108625 --- Comment #5 from Carsten Haitzler --- Attached them (too big to put inline as comments). -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/msm: Move fence put to where failure occurs
Quoting Robert Foss (2018-11-02 12:13:13) > If dma_fence_wait fails to wait for a supplied in-fence in > msm_ioctl_gem_submit, make sure we release that in-fence. > > Also remove this dma_fence_put() from the 'out' label. > > Signed-off-by: Robert Foss Reviewed-by: Chris Wilson Rob, this probably merits a cc:stable tag -- if the wait was interrupted by a signal, the fence would be leaked. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/msm: Move fence put to where failure occurs
+stable
On 2018-11-02 13:13, Robert Foss wrote:
If dma_fence_wait fails to wait for a supplied in-fence in
msm_ioctl_gem_submit, make sure we release that in-fence.
Also remove this dma_fence_put() from the 'out' label.
Signed-off-by: Robert Foss
---
Changes since v1:
- Chris Wilson: Make sure that dma_fence_put() is always executed
drivers/gpu/drm/msm/msm_gem_submit.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index a90aedd6883a..d5e6665a4c8f 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -411,7 +411,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
struct msm_file_private *ctx = file->driver_priv;
struct msm_gem_submit *submit;
struct msm_gpu *gpu = priv->gpu;
- struct dma_fence *in_fence = NULL;
struct sync_file *sync_file = NULL;
struct msm_gpu_submitqueue *queue;
struct msm_ringbuffer *ring;
@@ -444,6 +443,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
ring = gpu->rb[queue->prio];
if (args->flags & MSM_SUBMIT_FENCE_FD_IN) {
+ struct dma_fence *in_fence;
+
in_fence = sync_file_get_fence(args->fence_fd);
if (!in_fence)
@@ -453,11 +454,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
* Wait if the fence is from a foreign context, or if the fence
* array contains any fence from a foreign context.
*/
- if (!dma_fence_match_context(in_fence, ring->fctx->context)) {
+ ret = 0;
+ if (!dma_fence_match_context(in_fence, ring->fctx->context))
ret = dma_fence_wait(in_fence, true);
- if (ret)
- return ret;
- }
+
+ dma_fence_put(in_fence);
+ if (ret)
+ return ret;
}
ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -583,8 +586,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
}
out:
- if (in_fence)
- dma_fence_put(in_fence);
submit_cleanup(submit);
if (ret)
msm_gem_submit_free(submit);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/msm: Move fence put to where failure occurs
Hey Chris, On 2018-11-02 13:16, Chris Wilson wrote: Quoting Robert Foss (2018-11-02 12:13:13) If dma_fence_wait fails to wait for a supplied in-fence in msm_ioctl_gem_submit, make sure we release that in-fence. Also remove this dma_fence_put() from the 'out' label. Signed-off-by: Robert Foss Reviewed-by: Chris Wilson Danke! Rob, this probably merits a cc:stable tag -- if the wait was interrupted by a signal, the fence would be leaked. Ack, CC-ed the v2 submission. Rob. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/selftests: Fix build warning -Wframe-larger-than
It seems for some random configuration drm_device is bigger than 2048
bytes.
The fix is to make the mock objects static variables.
Bug reported by 0-DAY Kernel test infrastructure here:
https://lists.01.org/pipermail/kbuild-all/2018-November/054431.html
Fixes: 6ff3d9ffdcbb ("drm/selftests: Add tests for
drm_internal_framebuffer_create")
Signed-off-by: Alexandru Gheorghe
---
.../gpu/drm/selftests/test-drm_framebuffer.c | 30 ++-
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/selftests/test-drm_framebuffer.c
b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
index 3098435678af..a04d02dacce2 100644
--- a/drivers/gpu/drm/selftests/test-drm_framebuffer.c
+++ b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
@@ -307,25 +307,27 @@ static struct drm_framebuffer *fb_create_mock(struct
drm_device *dev,
return ERR_PTR(-EINVAL);
}
+static struct drm_mode_config_funcs mock_config_funcs = {
+ .fb_create = fb_create_mock,
+};
+
+static struct drm_device mock_drm_device = {
+ .mode_config = {
+ .min_width = MIN_WIDTH,
+ .max_width = MAX_WIDTH,
+ .min_height = MIN_HEIGHT,
+ .max_height = MAX_HEIGHT,
+ .allow_fb_modifiers = true,
+ .funcs = &mock_config_funcs,
+ },
+};
+
static int execute_drm_mode_fb_cmd2(struct drm_mode_fb_cmd2 *r)
{
int buffer_created = 0;
struct drm_framebuffer *fb;
- struct drm_mode_config_funcs mock_config_funcs = {
- .fb_create = fb_create_mock,
- };
- struct drm_device mock_drm_device = {
- .mode_config = {
- .min_width = MIN_WIDTH,
- .max_width = MAX_WIDTH,
- .min_height = MIN_HEIGHT,
- .max_height = MAX_HEIGHT,
- .allow_fb_modifiers = true,
- .funcs = &mock_config_funcs,
- },
- .dev_private = &buffer_created
- };
+ mock_drm_device.dev_private = &buffer_created;
fb = drm_internal_framebuffer_create(&mock_drm_device, r, NULL);
return buffer_created;
}
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/msm: Move fence put to where failure occurs
On Fri, Nov 02, 2018 at 01:29:25PM +0100, Robert Foss wrote: > +stable This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/msm/dpu: Correct dpu destroy and disable order
On Fri, Nov 02, 2018 at 06:19:17PM +0530, Jayant Shekhar wrote: > In case of msm drm bind failure, dpu_mdss_destroy is triggered. > In this function, resources are freed and pm runtime disable is > called, which triggers dpu_mdss_disable. Now in dpu_mdss_disable, > driver tries to access a memory which is already freed. This > results in kernel panic. Fix this by ensuring proper sequence > of dpu destroy and disable calls. > > Changes in v2: >- Removed double spacings [Jeykumar] > > Signed-off-by: Jayant Shekhar Reviewed-by: Sean Paul > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > index fd9c893..902bb4c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c > @@ -156,18 +156,15 @@ static void dpu_mdss_destroy(struct drm_device *dev) > struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); > struct dss_module_power *mp = &dpu_mdss->mp; > > + pm_runtime_disable(dev->dev); > _dpu_mdss_irq_domain_fini(dpu_mdss); > - > free_irq(platform_get_irq(pdev, 0), dpu_mdss); > - > msm_dss_put_clk(mp->clk_config, mp->num_clk); > devm_kfree(&pdev->dev, mp->clk_config); > > if (dpu_mdss->mmio) > devm_iounmap(&pdev->dev, dpu_mdss->mmio); > dpu_mdss->mmio = NULL; > - > - pm_runtime_disable(dev->dev); > priv->mdss = NULL; > } > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 2/2] drm/scheduler: Add a start_timeout_notify() backend function
On 11/2/2018 4:09 PM, Koenig, Christian wrote:
Am 02.11.18 um 11:31 schrieb Sharat Masetty:
Add an optional backend function op which will let the scheduler clients
know when the timeout got scheduled on the scheduler instance. This will
help drivers with multiple schedulers(one per ring) measure time spent on
the ring accurately, eventually helping with better timeout detection.
Signed-off-by: Sharat Masetty
Well, NAK. drm_sched_start_timeout() is called whenever the timer needs
to run, but that doesn't mean that the timer is started (e.g. it can
already be running).
So the callback would be called multiple times and not reflect the
actual job run time.
Additional to that it can be racy, e.g. we can complete multiple jobs at
a time before the timer is started again.
If you want to accurately count how much time you spend on each job/ring
you need to do this by measuring the time inside your driver instead.
E.g. for amdgpu I would get the time first in amdgpu_job_run() and then
again in amdgpu_job_free_cb() and calculate the difference.
Hi Christian,
Thank you for the comments and apologies if this was confusing. All I
want to determine(more accurately) is that when the scheduler instance
timer of say 500 ms goes off, is if the ring(associated with the
scheduler instance) actually spent 500 ms on the hardware - and for this
I need to know in the driver space when the timer actually started.
In msm hardware we have ring preemption support enabled and the kernel
driver triggers a preemption switch to a higher priority ring if there
is work available on that ring for the GPU to work on. So in the
presence of preemption it is possible that a lower priority ring did not
actually get to spend the full 500 ms and this is what I am trying to
catch with this callback.
I am *not* trying to profile per job time consumption with this.
> Well, NAK. drm_sched_start_timeout() is called whenever the timer needs
> to run, but that doesn't mean that the timer is started (e.g. it can
> already be running).
Regarding the case where the timer may already be running - good point,
but it should be easy to address the scenario. I will check the output
of schedule_delayed_work() and only call the callback(new proposed) if
the timer was really scheduled.
In summary, when this timedout_job() callback is called, it is assumed
that the job actually did time out from the POV of the scheduler, but
this will not hold true with preemption switching and that is what I am
trying to better address with this patch.
Sharat
Regards,
Christian.
---
Here is an example of how I plan to use this new function callback.
[1] https://patchwork.freedesktop.org/patch/254227/
drivers/gpu/drm/scheduler/sched_main.c | 7 ++-
include/drm/gpu_scheduler.h| 6 ++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index c993d10..afd461e 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -192,8 +192,13 @@ bool drm_sched_dependency_optimized(struct dma_fence*
fence,
static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched)
{
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
- !list_empty(&sched->ring_mirror_list))
+ !list_empty(&sched->ring_mirror_list)) {
+
schedule_delayed_work(&sched->work_tdr, sched->timeout);
+
+ if (sched->ops->start_timeout_notify)
+ sched->ops->start_timeout_notify(sched);
+ }
}
/* job_finish is called after hw fence signaled
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index d87b268..faf28b4 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -239,6 +239,12 @@ struct drm_sched_backend_ops {
* and it's time to clean it up.
*/
void (*free_job)(struct drm_sched_job *sched_job);
+
+ /*
+* (Optional) Called to let the driver know that a timeout detection
+* timer has been started.
+*/
+ void (*start_timeout_notify)(struct drm_gpu_scheduler *sched);
};
/**
--
1.9.1
___
Freedreno mailing list
freedr...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/file: Uncompact the feature flags
This essentially undoes
commit 39868bd7668bd47308b1dfd97c212757caee764f
Author: Chris Wilson
Date: Tue Oct 29 08:55:58 2013 +
drm: Compact booleans within struct drm_file
We do lockless access to these flags everywhere, and it's kinda not a
great idea to mix lockless and bitfields. Aside from that gcc isn't
generating great code for these.
If this ever becomes an issue size-wise, I think we need atomic_t here
and atomic bitflag ops.
Cc: Chris Wilson
Cc: David Herrmann
Cc: Dave Airlie
Signed-off-by: Daniel Vetter
---
include/drm/drm_file.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 26485acc51d7..84ac79219e4c 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -164,14 +164,14 @@ struct drm_file {
* See also the :ref:`section on primary nodes and authentication
* `.
*/
- unsigned authenticated :1;
+ bool authenticated;
/**
* @stereo_allowed:
*
* True when the client has asked us to expose stereo 3D mode flags.
*/
- unsigned stereo_allowed :1;
+ bool stereo_allowed;
/**
* @universal_planes:
@@ -179,10 +179,10 @@ struct drm_file {
* True if client understands CRTC primary planes and cursor planes
* in the plane list. Automatically set when @atomic is set.
*/
- unsigned universal_planes:1;
+ bool universal_planes;
/** @atomic: True if client understands atomic properties. */
- unsigned atomic:1;
+ bool atomic;
/**
* @aspect_ratio_allowed:
@@ -190,14 +190,14 @@ struct drm_file {
* True, if client can handle picture aspect ratios, and has requested
* to pass this information along with the mode.
*/
- unsigned aspect_ratio_allowed:1;
+ bool aspect_ratio_allowed;
/**
* @writeback_connectors:
*
* True if client understands writeback connectors
*/
- unsigned writeback_connectors:1;
+ bool writeback_connectors;
/**
* @is_master:
@@ -208,7 +208,7 @@ struct drm_file {
* See also the :ref:`section on primary nodes and authentication
* `.
*/
- unsigned is_master:1;
+ bool is_master;
/**
* @master:
--
2.14.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/lease: look at ->universal_planes only once
It's lockless, and userspace might chance it underneath us. That's not
really a problem, all userspace gets is a slightly dysfunctional
lease with the current code. But this might change, and gcc might
decide to reload a few too many times, and then boom. So better safe
than sorry.
Cc: Keith Packard
Cc: Dave Airlie
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_lease.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 46408278c5b1..ce49d1919214 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -359,7 +359,8 @@ void drm_lease_revoke(struct drm_master *top)
static int validate_lease(struct drm_device *dev,
struct drm_file *lessor_priv,
int object_count,
- struct drm_mode_object **objects)
+ struct drm_mode_object **objects,
+ bool universal_planes)
{
int o;
int has_crtc = -1;
@@ -376,14 +377,14 @@ static int validate_lease(struct drm_device *dev,
if (objects[o]->type == DRM_MODE_OBJECT_CONNECTOR &&
has_connector == -1)
has_connector = o;
- if (lessor_priv->universal_planes) {
+ if (universal_planes) {
if (objects[o]->type == DRM_MODE_OBJECT_PLANE &&
has_plane == -1)
has_plane = o;
}
}
if (has_crtc == -1 || has_connector == -1)
return -EINVAL;
- if (lessor_priv->universal_planes && has_plane == -1)
+ if (universal_planes && has_plane == -1)
return -EINVAL;
return 0;
}
@@ -397,6 +398,8 @@ static int fill_object_idr(struct drm_device *dev,
struct drm_mode_object **objects;
u32 o;
int ret;
+ bool universal_planes = READ_ONCE(lessor_priv->universal_planes);
+
objects = kcalloc(object_count, sizeof(struct drm_mode_object *),
GFP_KERNEL);
if (!objects)
@@ -425,7 +428,8 @@ static int fill_object_idr(struct drm_device *dev,
}
}
- ret = validate_lease(dev, lessor_priv, object_count, objects);
+ ret = validate_lease(dev, lessor_priv, object_count, objects,
+universal_planes);
if (ret) {
DRM_DEBUG_LEASE("lease validation failed\n");
goto out_free_objects;
@@ -452,7 +456,7 @@ static int fill_object_idr(struct drm_device *dev,
object_id, ret);
goto out_free_objects;
}
- if (obj->type == DRM_MODE_OBJECT_CRTC &&
!lessor_priv->universal_planes) {
+ if (obj->type == DRM_MODE_OBJECT_CRTC && !universal_planes) {
struct drm_crtc *crtc = obj_to_crtc(obj);
ret = idr_alloc(leases, &drm_lease_idr_object,
crtc->primary->base.id, crtc->primary->base.id + 1, GFP_KERNEL);
if (ret < 0) {
--
2.14.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/lease: debug output for lease creation
I spent a bit of time scratching heads and figuring out why the igts
don't work. Probably useful to keep this work.
Cc: Keith Packard
Cc: Dave Airlie
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_lease.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 9ab88db8fad0..46408278c5b1 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -419,14 +419,17 @@ static int fill_object_idr(struct drm_device *dev,
}
if (!drm_mode_object_lease_required(objects[o]->type)) {
+ DRM_DEBUG_KMS("invalid object for lease\n");
ret = -EINVAL;
goto out_free_objects;
}
}
ret = validate_lease(dev, lessor_priv, object_count, objects);
- if (ret)
+ if (ret) {
+ DRM_DEBUG_LEASE("lease validation failed\n");
goto out_free_objects;
+ }
/* add their IDs to the lease request - taking into account
universal planes */
@@ -509,15 +512,21 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
return -EOPNOTSUPP;
/* Do not allow sub-leases */
- if (lessor->lessor)
+ if (lessor->lessor) {
+ DRM_DEBUG_LEASE("recursive leasing not allowed\n");
return -EINVAL;
+ }
/* need some objects */
- if (cl->object_count == 0)
+ if (cl->object_count == 0) {
+ DRM_DEBUG_LEASE("no objects in lease\n");
return -EINVAL;
+ }
- if (cl->flags && (cl->flags & ~(O_CLOEXEC | O_NONBLOCK)))
+ if (cl->flags && (cl->flags & ~(O_CLOEXEC | O_NONBLOCK))) {
+ DRM_DEBUG_LEASE("invalid flags\n");
return -EINVAL;
+ }
object_count = cl->object_count;
@@ -532,6 +541,7 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
object_count, object_ids);
kfree(object_ids);
if (ret) {
+ DRM_DEBUG_LEASE("lease object lookup failed: %i\n", ret);
idr_destroy(&leases);
return ret;
}
--
2.14.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/selftests: Fix build warning -Wframe-larger-than
Op 02-11-18 om 14:01 schreef Alexandru-Cosmin Gheorghe:
> It seems for some random configuration drm_device is bigger than 2048
> bytes.
> The fix is to make the mock objects static variables.
>
> Bug reported by 0-DAY Kernel test infrastructure here:
> https://lists.01.org/pipermail/kbuild-all/2018-November/054431.html
>
> Fixes: 6ff3d9ffdcbb ("drm/selftests: Add tests for
> drm_internal_framebuffer_create")
> Signed-off-by: Alexandru Gheorghe
> ---
> .../gpu/drm/selftests/test-drm_framebuffer.c | 30 ++-
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/selftests/test-drm_framebuffer.c
> b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
> index 3098435678af..a04d02dacce2 100644
> --- a/drivers/gpu/drm/selftests/test-drm_framebuffer.c
> +++ b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
> @@ -307,25 +307,27 @@ static struct drm_framebuffer *fb_create_mock(struct
> drm_device *dev,
> return ERR_PTR(-EINVAL);
> }
>
> +static struct drm_mode_config_funcs mock_config_funcs = {
> + .fb_create = fb_create_mock,
> +};
> +
> +static struct drm_device mock_drm_device = {
> + .mode_config = {
> + .min_width = MIN_WIDTH,
> + .max_width = MAX_WIDTH,
> + .min_height = MIN_HEIGHT,
> + .max_height = MAX_HEIGHT,
> + .allow_fb_modifiers = true,
> + .funcs = &mock_config_funcs,
> + },
> +};
> +
> static int execute_drm_mode_fb_cmd2(struct drm_mode_fb_cmd2 *r)
> {
> int buffer_created = 0;
> struct drm_framebuffer *fb;
> - struct drm_mode_config_funcs mock_config_funcs = {
> - .fb_create = fb_create_mock,
> - };
> - struct drm_device mock_drm_device = {
> - .mode_config = {
> - .min_width = MIN_WIDTH,
> - .max_width = MAX_WIDTH,
> - .min_height = MIN_HEIGHT,
> - .max_height = MAX_HEIGHT,
> - .allow_fb_modifiers = true,
> - .funcs = &mock_config_funcs,
> - },
> - .dev_private = &buffer_created
> - };
>
> + mock_drm_device.dev_private = &buffer_created;
> fb = drm_internal_framebuffer_create(&mock_drm_device, r, NULL);
> return buffer_created;
> }
Thanks, pushed. :)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences
On Thu, 1 Nov 2018 at 12:56, Robert Foss wrote: > On 2018-10-31 10:38, Emil Velikov wrote: > > Hi Rob, > > > > On Thu, 25 Oct 2018 at 19:38, Robert Foss wrote: > >> > >> Add a new field called fence_fd that will be used by userspace to send > >> in-fences to the kernel and receive out-fences created by the kernel. > >> > >> This uapi enables virtio to take advantage of explicit synchronization of > >> dma-bufs. > >> > >> There are two new flags: > >> > >> * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd. > >> * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd > >> > >> The execbuffer IOCTL is now read-write to allow the userspace to read the > >> out-fence. > >> > >> On error -1 should be returned in the fence_fd field. > >> > >> Signed-off-by: Gustavo Padovan > >> Signed-off-by: Robert Foss > >> --- > >> Changes since v2: > >> - Since exbuf-flags is a new flag, check that unsupported > >> flags aren't set. > >> > >> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 + > >> include/uapi/drm/virtgpu_drm.h | 13 ++--- > >> 2 files changed, 15 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > >> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > >> index d01a9ed100d1..1af289b28fc4 100644 > >> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > >> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > >> @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct > >> drm_device *dev, void *data, > >> struct ww_acquire_ctx ticket; > >> void *buf; > >> > >> + exbuf->fence_fd = -1; > >> + > > Move this after the sanity checking. > > Agreed. Fixed in v4 > > > > >> if (vgdev->has_virgl_3d == false) > >> return -ENOSYS; > >> > >> + if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) > >> + return -EINVAL; > >> + > > I assume this did this trigger when using old userspace? > > No, not as far as I'm aware. This check is there to prevent userspace from > polluting the bitspace of flag, so that all free bits can be used for new > flags. > > As far as I understand this is pointed out by a drm driver development > document > written by danvet, which I unfortunately can't seem to find the link for at > the > moment. > Yes that is correct. What I was asking is: Does a kernel with this patch, work with mesa lacking the corresponding updates? I'd imagine things work just fine. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 2/2] drm/scheduler: Add a start_timeout_notify() backend function
Am 02.11.18 um 14:25 schrieb Sharat Masetty:
>
>
> On 11/2/2018 4:09 PM, Koenig, Christian wrote:
>> Am 02.11.18 um 11:31 schrieb Sharat Masetty:
>>> Add an optional backend function op which will let the scheduler
>>> clients
>>> know when the timeout got scheduled on the scheduler instance. This
>>> will
>>> help drivers with multiple schedulers(one per ring) measure time
>>> spent on
>>> the ring accurately, eventually helping with better timeout detection.
>>>
>>> Signed-off-by: Sharat Masetty
>>
>> Well, NAK. drm_sched_start_timeout() is called whenever the timer needs
>> to run, but that doesn't mean that the timer is started (e.g. it can
>> already be running).
>>
>> So the callback would be called multiple times and not reflect the
>> actual job run time.
>>
>> Additional to that it can be racy, e.g. we can complete multiple jobs at
>> a time before the timer is started again.
>>
>> If you want to accurately count how much time you spend on each job/ring
>> you need to do this by measuring the time inside your driver instead.
>>
>> E.g. for amdgpu I would get the time first in amdgpu_job_run() and then
>> again in amdgpu_job_free_cb() and calculate the difference.
> Hi Christian,
>
> Thank you for the comments and apologies if this was confusing. All I
> want to determine(more accurately) is that when the scheduler instance
> timer of say 500 ms goes off, is if the ring(associated with the
> scheduler instance) actually spent 500 ms on the hardware - and for
> this I need to know in the driver space when the timer actually started.
>
> In msm hardware we have ring preemption support enabled and the kernel
> driver triggers a preemption switch to a higher priority ring if there
> is work available on that ring for the GPU to work on. So in the
> presence of preemption it is possible that a lower priority ring did
> not actually get to spend the full 500 ms and this is what I am trying
> to catch with this callback.
>
> I am *not* trying to profile per job time consumption with this.
>
> > Well, NAK. drm_sched_start_timeout() is called whenever the timer needs
> > to run, but that doesn't mean that the timer is started (e.g. it can
> > already be running).
>
> Regarding the case where the timer may already be running - good
> point, but it should be easy to address the scenario. I will check the
> output
> of schedule_delayed_work() and only call the callback(new proposed) if
> the timer was really scheduled.
Yeah, that should work.
>
> In summary, when this timedout_job() callback is called, it is assumed
> that the job actually did time out from the POV of the scheduler, but
> this will not hold true with preemption switching and that is what I
> am trying to better address with this patch.
Mhm, so what you actually need is to suspend the timeout when the lower
priority ring is preempted and resume it when it is started again? I
wonder if that wouldn't be simpler.
We have support for ring preemption as well, but not implemented yet. So
it would be nice to have something that works for everybody.
But on the other hand a callback to notify the driver that the timer
started isn't so bad either.
Regards,
Christian.
>
> Sharat
>>
>> Regards,
>> Christian.
>>
>>> ---
>>> Here is an example of how I plan to use this new function callback.
>>>
>>> [1] https://patchwork.freedesktop.org/patch/254227/
>>>
>>> drivers/gpu/drm/scheduler/sched_main.c | 7 ++-
>>> include/drm/gpu_scheduler.h | 6 ++
>>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
>>> b/drivers/gpu/drm/scheduler/sched_main.c
>>> index c993d10..afd461e 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>> @@ -192,8 +192,13 @@ bool drm_sched_dependency_optimized(struct
>>> dma_fence* fence,
>>> static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched)
>>> {
>>> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>>> - !list_empty(&sched->ring_mirror_list))
>>> + !list_empty(&sched->ring_mirror_list)) {
>>> +
>>> schedule_delayed_work(&sched->work_tdr, sched->timeout);
>>> +
>>> + if (sched->ops->start_timeout_notify)
>>> + sched->ops->start_timeout_notify(sched);
>>> + }
>>> }
>>>
>>> /* job_finish is called after hw fence signaled
>>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>>> index d87b268..faf28b4 100644
>>> --- a/include/drm/gpu_scheduler.h
>>> +++ b/include/drm/gpu_scheduler.h
>>> @@ -239,6 +239,12 @@ struct drm_sched_backend_ops {
>>> * and it's time to clean it up.
>>> */
>>> void (*free_job)(struct drm_sched_job *sched_job);
>>> +
>>> + /*
>>> + * (Optional) Called to let the driver know that a timeout
>>> detection
>>> + * timer has been started.
>>> + */
>>> + void (*start_timeout_notify)(struct drm_gpu_scheduler *sched
[Bug 107141] Manual setting of pp_dpm_sclk resets after monitor off/on (rx 480)
https://bugs.freedesktop.org/show_bug.cgi?id=107141 Michel Dänzer changed: What|Removed |Added CC||tempel.jul...@gmail.com --- Comment #3 from Michel Dänzer --- *** Bug 108613 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108613] amdgpu.dc=1 + xf86-video-amdgpu: changing to a GPU upscaling resolution resets pp_dpm_mclk
https://bugs.freedesktop.org/show_bug.cgi?id=108613 Michel Dänzer changed: What|Removed |Added Resolution|--- |DUPLICATE Status|NEW |RESOLVED --- Comment #6 from Michel Dänzer --- *** This bug has been marked as a duplicate of bug 107141 *** -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH] drm/msm: Optimize GPU crashstate capture read path
On Fri, Nov 02, 2018 at 03:12:51PM +0530, Sharat Masetty wrote: > Thanks for the comments Jordan - > > On 11/1/2018 8:34 PM, Jordan Crouse wrote: > >On Thu, Nov 01, 2018 at 02:05:41PM +0530, Sharat Masetty wrote: > >>When the userspace tries to read the crashstate dump, the read side > >>implementation in the driver currently ascii85 encodes all the binary > >>buffers and it does this each time the read system call is called. > >>A userspace tool like cat typically does a page by page read and the > >>number of read calls depends on the size of the data captured by the > >>driver. This is certainly not desirable and does not scale well with > >>large captures. > >> > >>This patch starts off with moving the encoding part to the capture time, > >>that is when the GPU tries to recover after a hang. Now we only encode > >>once per buffer object and not per page read. With this there is an > >>immediate >10X speed improvement in crashstate save time. > >> > >>The flip side however is that the GPU recovery time increases and this > >>negatively impacts the user experience, so this is handled by moving the > >>encoding part to a worker thread and only register with the dev_coredump > >>once the encoding of the buffers is complete. > > > >Does your tree have https://patchwork.freedesktop.org/patch/257181/ ? That > >will > >help with a lot of the problems you have described. > The issue is that even with small sized files like ~5MB or so, the > capture time is huge and sometime does not survive the dev_coredump > timeout of 5 minutes. With this change however I am able to dump > large file sizes like 60MB or so in reasonable amount of time (~50 > seconds). > > As for the patch that you shared, I am thinking that we should drop > it and instead go with this or whatever we agree on. The patch loses > stuff like IB2's which might be important while debugging hang > issues. You won't lose IB2s if you mark them as MSM_SUBMIT_CMD_IB_TARGET_BUF and Rob is working on some new changes for userspace to identify buffers as "dumpable" which will coincide with the patch I posted. Thats not to say that your patch isn't appropriate - there is good stuff here but even with it in place we probably shouldn't be generating 60MB dumps unless we're going for some sort of replay scenario (which we could enabled with an appropriate debugfs flag). Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx] [v2 1/2] drm: Add colorspace property
>-Original Message-
>From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
>Sent: Friday, November 2, 2018 5:00 PM
>To: Maarten Lankhorst
>Cc: Shankar, Uma ; dri-devel@lists.freedesktop.org;
>intel-...@lists.freedesktop.org; Syrjala, Ville ;
>Lankhorst, Maarten ; Hans Verkuil
>
>Subject: Re: [Intel-gfx] [v2 1/2] drm: Add colorspace property
>
>On Fri, Nov 02, 2018 at 10:19:10AM +0100, Maarten Lankhorst wrote:
>> Op 31-10-18 om 13:05 schreef Uma Shankar:
>> > This patch adds a colorspace property enabling userspace to switch
>> > to various supported colorspaces.
>> > This will help enable BT2020 along with other colorspaces.
>> >
>> > v2: Addressed Maarten and Ville's review comments. Enhanced the
>> > colorspace enum to incorporate both HDMI and DP supported
>> > colorspaces. Also, added a default option for colorspace.
>> >
>> > Signed-off-by: Uma Shankar
>> > ---
>> > drivers/gpu/drm/drm_atomic_uapi.c | 4
>> > drivers/gpu/drm/drm_connector.c | 44
>+++
>> > include/drm/drm_connector.h | 7 +++
>> > include/drm/drm_mode_config.h | 6 ++
>> > include/uapi/drm/drm_mode.h | 24 +
>> > 5 files changed, 85 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
>> > b/drivers/gpu/drm/drm_atomic_uapi.c
>> > index d5b7f31..9e1d46b 100644
>> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> > @@ -721,6 +721,8 @@ static int drm_atomic_connector_set_property(struct
>drm_connector *connector,
>> >state->picture_aspect_ratio = val;
>> >} else if (property == config->content_type_property) {
>> >state->content_type = val;
>> > + } else if (property == config->colorspace_property) {
>> > + state->colorspace = val;
>> >} else if (property == connector->scaling_mode_property) {
>> >state->scaling_mode = val;
>> >} else if (property == connector->content_protection_property) {
>> > @@ -795,6 +797,8 @@ static int drm_atomic_connector_set_property(struct
>drm_connector *connector,
>> >*val = state->picture_aspect_ratio;
>> >} else if (property == config->content_type_property) {
>> >*val = state->content_type;
>> > + } else if (property == config->colorspace_property) {
>> > + *val = state->colorspace;
>> >} else if (property == connector->scaling_mode_property) {
>> >*val = state->scaling_mode;
>> >} else if (property == connector->content_protection_property) {
>> > diff --git a/drivers/gpu/drm/drm_connector.c
>> > b/drivers/gpu/drm/drm_connector.c index aa18b1d..5ad52a0 100644
>> > --- a/drivers/gpu/drm/drm_connector.c
>> > +++ b/drivers/gpu/drm/drm_connector.c
>> > @@ -826,6 +826,38 @@ int drm_display_info_set_bus_formats(struct
>> > drm_display_info *info, };
>> > DRM_ENUM_NAME_FN(drm_get_content_protection_name,
>drm_cp_enum_list)
>> >
>> > +static const struct drm_prop_enum_list colorspace[] = {
>> > + /* Standard Definition Colorimetry based on CEA 861 */
>> > + { COLORIMETRY_ITU_601, "601" },
>> > + { COLORIMETRY_ITU_709, "709" },
>> > + /* Standard Definition Colorimetry based on IEC 61966-2-4 */
>> > + { COLORIMETRY_XV_YCC_601, "601" },
>> > + /* High Definition Colorimetry based on IEC 61966-2-4 */
>> > + { COLORIMETRY_XV_YCC_709, "709" },
>> 2 definitions that share the same name?
>> All in all, I think the enum strings need to be more descriptive and self-
>consistent.
>+1
Sure, will modify this.
>> > + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
>> > + { COLORIMETRY_S_YCC_601, "s601" },
>> > + /* Colorimetry based on IEC 61966-2-5 [33] */
>> > + { COLORIMETRY_ADOBE_YCC_601, "adobe601" },
>
>Hans (cc:d) recetly noted that these things aren't called Adobe
>anymore in the CTA-861 spec. There is some new name for them, which I now
>forget.
EC2 EC1 EC0 Extended Colorimetry
0 1 1 AdobeYCC601
This is the bit format mentioned in CEA861.F while defining the AVI infoframe,
so looks
like they are still keeping it that way. Not sure if its updated as part of any
latest spec
update.
>> > + /* Colorimetry based on IEC 61966-2-5 */
>> > + { COLORIMETRY_ADOBE_RGB, "adobe_rgb" },
>> > + /* Colorimetry based on ITU-R BT.2020 */
>> > + { COLORIMETRY_BT2020_RGB, "BT2020_rgb" },
>> > + /* Colorimetry based on ITU-R BT.2020 */
>> > + { COLORIMETRY_BT2020_YCC, "BT2020_ycc" },
>> > + /* Colorimetry based on ITU-R BT.2020 */
>> > + { COLORIMETRY_BT2020_CYCC, "BT2020_cycc" },
>> > + /* DP MSA Colorimetry */
>> > + { COLORIMETRY_Y_CBCR_ITU_601, "YCBCR_ITU_601" },
>> > + { COLORIMETRY_Y_CBCR_ITU_709, "YCBCR_ITU_709" },
>> > + { COLORIMETRY_SRGB, "SRGB" },
>> sRGB
Was trying to avoid camelcase, but for these names, I guess we can keep how
spec defines them. Will change this.
>> > + { COLORIMETRY_RGB_WIDE_GAMUT, "RGB Wide Gamut" },
>> > + { COLORIMETRY_SCRGB, "SCRGB" },
>> scRGB?
Will update this.
>>
RE: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and enable modeset
>-Original Message- >From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] >Sent: Friday, November 2, 2018 2:53 PM >To: Shankar, Uma ; dri-devel@lists.freedesktop.org; >intel-...@lists.freedesktop.org >Cc: Syrjala, Ville ; a...@redhat.com; Lankhorst, >Maarten > >Subject: Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and >enable >modeset > >Op 31-10-18 om 13:05 schreef Uma Shankar: >> This patch attaches the colorspace connector property to the hdmi >> connector. Based on colorspace change, modeset will be triggered to >> switch to new colorspace. >> >> Based on colorspace property value create an infoframe with >> appropriate colorspace. This can be used to send an infoframe packet >> with proper colorspace value set which will help to enable wider color >> gamut like BT2020 on sink. >> >> v2: Merged the changes of creating infoframe as well to this patch as >> per Maarten's suggestion. >> >> Signed-off-by: Uma Shankar >> --- >> drivers/gpu/drm/i915/intel_atomic.c | 1 + >> drivers/gpu/drm/i915/intel_hdmi.c | 5 + >> 2 files changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c >> b/drivers/gpu/drm/i915/intel_atomic.c >> index a5a2c8f..35ef70a 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic.c >> +++ b/drivers/gpu/drm/i915/intel_atomic.c >> @@ -125,6 +125,7 @@ int intel_digital_connector_atomic_check(struct >drm_connector *conn, >> */ >> if (new_conn_state->force_audio != old_conn_state->force_audio || >> new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb >> || >> +new_state->colorspace != old_state->colorspace || >> new_conn_state->base.picture_aspect_ratio != old_conn_state- >>base.picture_aspect_ratio || >> new_conn_state->base.content_type != old_conn_state- >>base.content_type || >> new_conn_state->base.scaling_mode != >> old_conn_state->base.scaling_mode) >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c >> b/drivers/gpu/drm/i915/intel_hdmi.c >> index 129b880..8a41fb3 100644 >> --- a/drivers/gpu/drm/i915/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >> @@ -486,6 +486,8 @@ static void intel_hdmi_set_avi_infoframe(struct >intel_encoder *encoder, >> else >> frame.avi.colorspace = HDMI_COLORSPACE_RGB; >> >> +frame.avi.extended_colorimetry = conn_state->colorspace; >> + >> drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode, >> crtc_state->limited_color_range ? >> >HDMI_QUANTIZATION_RANGE_LIMITED : >> @@ -2125,6 +2127,9 @@ static void intel_hdmi_destroy(struct drm_connector >*connector) >> intel_attach_broadcast_rgb_property(connector); >> intel_attach_aspect_ratio_property(connector); >> drm_connector_attach_content_type_property(connector); >> +drm_object_attach_property(&connector->base, >> +connector->dev->mode_config.colorspace_property, >> +COLORIMETRY_ITU_709); >Just put 0 here.. >If you want to init the default colorspace, put it in the first patch. Ok, will update this. >We should perhaps hide color spaces that are not supported on HDMI? Currently the supported colorspaces will be picked from edid by userspace and they should use the current property interface to set the one which is supported. Even on HDMI, some connectors may not support certain colorspace, so keeping it on userspace to set the one which is supported by the particular connector. Hope this approach is fine ? Regards, Uma Shankar >> connector->state->picture_aspect_ratio = >HDMI_PICTURE_ASPECT_NONE; >> } >> > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] amdgpu/test: Add illegal register and memory access test.
On 2018-10-31 7:33 p.m., Andrey Grodzovsky wrote:
> Illegal access will cause CP hang followed by job timeout and
> recovery kicking in.
> Also, disable the suite for all APU ASICs until GPU
> reset issues for them will be resolved and GPU reset recovery
> will be enabled by default.
>
> Signed-off-by: Andrey Grodzovsky
>
> [...]
>
> @@ -94,7 +119,9 @@ CU_BOOL suite_deadlock_tests_enable(void)
>&minor_version, &device_handle))
> return CU_FALSE;
>
> - if (device_handle->info.family_id == AMDGPU_FAMILY_SI) {
> + if (device_handle->info.family_id == AMDGPU_FAMILY_SI ||
> + device_handle->info.family_id == AMDGPU_FAMILY_CZ ||
> + device_handle->info.family_id == AMDGPU_FAMILY_RV) {
> printf("\n\nCurrently hangs the CP on this ASIC, deadlock suite
> disabled\n");
> enable = CU_FALSE;
> }
Indentation is wrong here and in other places. The libdrm tree contains
configuration files for EditorConfig (https://editorconfig.org/); since
you're using Eclipse, https://github.com/ncjones/editorconfig-eclipse
should help.
I run amdgpu_test as part of my daily build/test script during lunch
break; when I came back today, I was greeted by a GFX hang of the
Bonaire in my development box due to this test. Please disable it for
all pre-GFX8 ASICs. Ideally, it should also check at runtime that GPU
recovery is actually enabled, as that still isn't the case by default
except with bleeding edge amdgpu kernel code.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [v2 1/2] drm: Add colorspace property
On 2018-11-02 15:13, Shankar, Uma wrote:
>
>> -Original Message-
>> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
>> Sent: Friday, November 2, 2018 5:00 PM
>> To: Maarten Lankhorst
>> Cc: Shankar, Uma ; dri-devel@lists.freedesktop.org;
>> intel-...@lists.freedesktop.org; Syrjala, Ville ;
>> Lankhorst, Maarten ; Hans Verkuil
>>
>> Subject: Re: [Intel-gfx] [v2 1/2] drm: Add colorspace property
>>
>> On Fri, Nov 02, 2018 at 10:19:10AM +0100, Maarten Lankhorst wrote:
>>> Op 31-10-18 om 13:05 schreef Uma Shankar:
This patch adds a colorspace property enabling userspace to switch
to various supported colorspaces.
This will help enable BT2020 along with other colorspaces.
v2: Addressed Maarten and Ville's review comments. Enhanced the
colorspace enum to incorporate both HDMI and DP supported
colorspaces. Also, added a default option for colorspace.
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/drm_atomic_uapi.c | 4
drivers/gpu/drm/drm_connector.c | 44
>> +++
include/drm/drm_connector.h | 7 +++
include/drm/drm_mode_config.h | 6 ++
include/uapi/drm/drm_mode.h | 24 +
5 files changed, 85 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index d5b7f31..9e1d46b 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -721,6 +721,8 @@ static int drm_atomic_connector_set_property(struct
>> drm_connector *connector,
state->picture_aspect_ratio = val;
} else if (property == config->content_type_property) {
state->content_type = val;
+ } else if (property == config->colorspace_property) {
+ state->colorspace = val;
} else if (property == connector->scaling_mode_property) {
state->scaling_mode = val;
} else if (property == connector->content_protection_property) {
@@ -795,6 +797,8 @@ static int drm_atomic_connector_set_property(struct
>> drm_connector *connector,
*val = state->picture_aspect_ratio;
} else if (property == config->content_type_property) {
*val = state->content_type;
+ } else if (property == config->colorspace_property) {
+ *val = state->colorspace;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
} else if (property == connector->content_protection_property) {
diff --git a/drivers/gpu/drm/drm_connector.c
b/drivers/gpu/drm/drm_connector.c index aa18b1d..5ad52a0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -826,6 +826,38 @@ int drm_display_info_set_bus_formats(struct
drm_display_info *info, };
DRM_ENUM_NAME_FN(drm_get_content_protection_name,
>> drm_cp_enum_list)
+static const struct drm_prop_enum_list colorspace[] = {
+ /* Standard Definition Colorimetry based on CEA 861 */
+ { COLORIMETRY_ITU_601, "601" },
+ { COLORIMETRY_ITU_709, "709" },
+ /* Standard Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_601, "601" },
+ /* High Definition Colorimetry based on IEC 61966-2-4 */
+ { COLORIMETRY_XV_YCC_709, "709" },
>>> 2 definitions that share the same name?
>>> All in all, I think the enum strings need to be more descriptive and self-
>> consistent.
>> +1
> Sure, will modify this.
>
+ /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
+ { COLORIMETRY_S_YCC_601, "s601" },
+ /* Colorimetry based on IEC 61966-2-5 [33] */
+ { COLORIMETRY_ADOBE_YCC_601, "adobe601" },
>> Hans (cc:d) recetly noted that these things aren't called Adobe
>> anymore in the CTA-861 spec. There is some new name for them, which I now
>> forget.
> EC2 EC1 EC0 Extended Colorimetry
> 0 1 1 AdobeYCC601
> This is the bit format mentioned in CEA861.F while defining the AVI
> infoframe, so looks
> like they are still keeping it that way. Not sure if its updated as part of
> any latest spec
> update.
New names is opRGB and opYCC601 according to the notice on the first page of
CTA-861-G [1]
Updated CTA-861-E/F/G can be found at
https://standards.cta.tech/kwspub/published_docs/
[1]
https://standards.cta.tech/kwspub/published_docs/CTA-861-G_FINAL_revised_2017.pdf
>
+ /* Colorimetry based on IEC 61966-2-5 */
+ { COLORIMETRY_ADOBE_RGB, "adobe_rgb" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_RGB, "BT2020_rgb" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_YCC, "BT2020_ycc" },
+ /* Colorimetry based on ITU-R BT.2020 */
+ { COLORIMETRY_BT2020_CYCC, "BT2020_cycc" },
+ /* DP MSA Colorimetry */
+ { COLORIMETRY_Y_CBCR_ITU_601, "Y
[PATCH] drm/msm/dpu: Don't use devm for component devices
Devices that are bound as components should not use devm since
device managed memory is not freed when the component is
unbound.
In particular this is an issue if the component bind fails
due to an -EPROBE_DEFER. In this case the bind would try again
later and any devm managed memory allocated during the former
aborted attempt would be leaked until the device itself was
destroyed. Since all the memory allocated during a bind
should be freed during an unbind (or bind error case) there isn't
any reason to use devm for resources that have a explicit
teardown step.
This doesn't remove devm for all resources - in particular
msm_ioremap() still uses devm_ioremap() but thats a generic
issue that can easily be addressed as a cleanup later and the
unbind code already does the requisite devm calls to unmap it.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c | 4 +---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c| 8 +---
4 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82c55efb500f..287d4c3e58c3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2220,14 +2220,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device
*dev,
struct dpu_encoder_virt *dpu_enc = NULL;
int rc = 0;
- dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
+ dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);
if (!dpu_enc)
return ERR_PTR(ENOMEM);
rc = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
drm_enc_mode, NULL);
if (rc) {
- devm_kfree(dev->dev, dpu_enc);
+ kfree(dpu_enc);
return ERR_PTR(rc);
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
index 89ee4b36beff..90b53e9508f2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
@@ -155,9 +155,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
return 0;
}
- mp->clk_config = devm_kzalloc(&pdev->dev,
- sizeof(struct dss_clk) * num_clk,
- GFP_KERNEL);
+ mp->clk_config = kcalloc(num_clk, sizeof(struct dss_clk), GFP_KERNEL);
if (!mp->clk_config)
return -ENOMEM;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 985c855796ae..5ac3c3f3b08d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1086,13 +1086,14 @@ static int dpu_bind(struct device *dev, struct device
*master, void *data)
struct dss_module_power *mp;
int ret = 0;
- dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL);
+ dpu_kms = kzalloc(sizeof(*dpu_kms), GFP_KERNEL);
if (!dpu_kms)
return -ENOMEM;
mp = &dpu_kms->mp;
ret = msm_dss_parse_clock(pdev, mp);
if (ret) {
+ kfree(dpu_kms);
DPU_ERROR("failed to parse clocks, ret=%d\n", ret);
return ret;
}
@@ -1109,7 +1110,7 @@ static int dpu_bind(struct device *dev, struct device
*master, void *data)
dpu_kms->rpm_enabled = true;
priv->kms = &dpu_kms->base;
- return ret;
+ return 0;
}
static void dpu_unbind(struct device *dev, struct device *master, void *data)
@@ -1120,11 +1121,12 @@ static void dpu_unbind(struct device *dev, struct
device *master, void *data)
dpu_power_resource_deinit(pdev, &dpu_kms->phandle);
msm_dss_put_clk(mp->clk_config, mp->num_clk);
- devm_kfree(&pdev->dev, mp->clk_config);
- mp->num_clk = 0;
+ kfree(mp->clk_config);
if (dpu_kms->rpm_enabled)
pm_runtime_disable(&pdev->dev);
+
+ kfree(dpu_kms);
}
static const struct component_ops dpu_ops = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 2235ef8129f4..34ab489b1a5b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -161,7 +161,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
free_irq(platform_get_irq(pdev, 0), dpu_mdss);
msm_dss_put_clk(mp->clk_config, mp->num_clk);
- devm_kfree(&pdev->dev, mp->clk_config);
+ kfree(mp->clk_config);
if (dpu_mdss->mmio)
devm_iounmap(&pdev->dev, dpu_mdss->mmio);
@@ -169,6 +169,8 @@ static void dpu_mdss_destroy(struct drm_device *dev)
pm_runtime_disable(dev->dev);
priv->mdss = NULL;
+
+ kfree(dpu_mdss);
}
s
[PATCH] drm/lease: drop EXPORT_SYMBOL
Leases are entirely implemented within drm.ko, no need to even tempt drivers into doing nasty things. And if there's really a need, we can always re-export these again. Cc: Keith Packard Cc: Dave Airlie Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_lease.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index ce49d1919214..7e02fdd7d3d1 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -39,7 +39,6 @@ struct drm_master *drm_lease_owner(struct drm_master *master) master = master->lessor; return master; } -EXPORT_SYMBOL(drm_lease_owner); /** * _drm_find_lessee - find lessee by id (idr_mutex held) @@ -117,7 +116,6 @@ bool _drm_lease_held(struct drm_file *file_priv, int id) return _drm_lease_held_master(file_priv->master, id); } -EXPORT_SYMBOL(_drm_lease_held); /** * drm_lease_held - check drm_mode_object lease status (idr_mutex not held) @@ -144,7 +142,6 @@ bool drm_lease_held(struct drm_file *file_priv, int id) mutex_unlock(&master->dev->mode_config.idr_mutex); return ret; } -EXPORT_SYMBOL(drm_lease_held); /** * drm_lease_filter_crtcs - restricted crtc set to leased values (idr_mutex not held) @@ -184,7 +181,6 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs_in) mutex_unlock(&master->dev->mode_config.idr_mutex); return crtcs_out; } -EXPORT_SYMBOL(drm_lease_filter_crtcs); /* * drm_lease_create - create a new drm_master with leased objects (idr_mutex not held) -- 2.14.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v2] drm/msm/dpu: Correct dpu destroy and disable order
On Fri, Nov 02, 2018 at 06:19:17PM +0530, Jayant Shekhar wrote: > In case of msm drm bind failure, dpu_mdss_destroy is triggered. > In this function, resources are freed and pm runtime disable is > called, which triggers dpu_mdss_disable. Now in dpu_mdss_disable, > driver tries to access a memory which is already freed. This > results in kernel panic. Fix this by ensuring proper sequence > of dpu destroy and disable calls. Since you are in this specific area, I noticed we also had some memory leaks on bind "failure" and I just sent out a patch for that. Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH] drm/msm/dpu: Don't use devm for component devices
On Fri, Nov 02, 2018 at 08:30:08AM -0600, Jordan Crouse wrote:
> Devices that are bound as components should not use devm since
> device managed memory is not freed when the component is
> unbound.
>
> In particular this is an issue if the component bind fails
> due to an -EPROBE_DEFER. In this case the bind would try again
> later and any devm managed memory allocated during the former
> aborted attempt would be leaked until the device itself was
> destroyed. Since all the memory allocated during a bind
> should be freed during an unbind (or bind error case) there isn't
> any reason to use devm for resources that have a explicit
> teardown step.
>
> This doesn't remove devm for all resources - in particular
> msm_ioremap() still uses devm_ioremap() but thats a generic
> issue that can easily be addressed as a cleanup later and the
> unbind code already does the requisite devm calls to unmap it.
>
> Signed-off-by: Jordan Crouse
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c | 4 +---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c| 8 +---
> 4 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 82c55efb500f..287d4c3e58c3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2220,14 +2220,14 @@ struct drm_encoder *dpu_encoder_init(struct
> drm_device *dev,
> struct dpu_encoder_virt *dpu_enc = NULL;
> int rc = 0;
>
> - dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
> + dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);
> if (!dpu_enc)
> return ERR_PTR(ENOMEM);
>
> rc = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
> drm_enc_mode, NULL);
> if (rc) {
> - devm_kfree(dev->dev, dpu_enc);
> + kfree(dpu_enc);
> return ERR_PTR(rc);
> }
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> index 89ee4b36beff..90b53e9508f2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
> @@ -155,9 +155,7 @@ int msm_dss_parse_clock(struct platform_device *pdev,
> return 0;
> }
>
> - mp->clk_config = devm_kzalloc(&pdev->dev,
> - sizeof(struct dss_clk) * num_clk,
> - GFP_KERNEL);
> + mp->clk_config = kcalloc(num_clk, sizeof(struct dss_clk), GFP_KERNEL);
This should be freed in the err label instead of relying on the caller to clean
it up on failure.
> if (!mp->clk_config)
> return -ENOMEM;
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 985c855796ae..5ac3c3f3b08d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1086,13 +1086,14 @@ static int dpu_bind(struct device *dev, struct device
> *master, void *data)
> struct dss_module_power *mp;
> int ret = 0;
>
> - dpu_kms = devm_kzalloc(&pdev->dev, sizeof(*dpu_kms), GFP_KERNEL);
> + dpu_kms = kzalloc(sizeof(*dpu_kms), GFP_KERNEL);
> if (!dpu_kms)
> return -ENOMEM;
>
> mp = &dpu_kms->mp;
> ret = msm_dss_parse_clock(pdev, mp);
> if (ret) {
> + kfree(dpu_kms);
> DPU_ERROR("failed to parse clocks, ret=%d\n", ret);
> return ret;
> }
> @@ -1109,7 +1110,7 @@ static int dpu_bind(struct device *dev, struct device
> *master, void *data)
> dpu_kms->rpm_enabled = true;
>
> priv->kms = &dpu_kms->base;
> - return ret;
> + return 0;
> }
>
> static void dpu_unbind(struct device *dev, struct device *master, void *data)
> @@ -1120,11 +1121,12 @@ static void dpu_unbind(struct device *dev, struct
> device *master, void *data)
>
> dpu_power_resource_deinit(pdev, &dpu_kms->phandle);
> msm_dss_put_clk(mp->clk_config, mp->num_clk);
> - devm_kfree(&pdev->dev, mp->clk_config);
> - mp->num_clk = 0;
> + kfree(mp->clk_config);
>
> if (dpu_kms->rpm_enabled)
> pm_runtime_disable(&pdev->dev);
> +
> + kfree(dpu_kms);
> }
>
> static const struct component_ops dpu_ops = {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> index 2235ef8129f4..34ab489b1a5b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
> @@ -161,7 +161,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
> free_irq(platform_get_irq(pdev, 0), dpu_mdss);
>
> msm_dss_put_clk(mp->clk_config, mp->num_clk);
> - devm_kfree(&pdev->dev,
Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences
Hey Emil, On 2018-11-02 14:34, Emil Velikov wrote: On Thu, 1 Nov 2018 at 12:56, Robert Foss wrote: On 2018-10-31 10:38, Emil Velikov wrote: Hi Rob, On Thu, 25 Oct 2018 at 19:38, Robert Foss wrote: Add a new field called fence_fd that will be used by userspace to send in-fences to the kernel and receive out-fences created by the kernel. This uapi enables virtio to take advantage of explicit synchronization of dma-bufs. There are two new flags: * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd. * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd The execbuffer IOCTL is now read-write to allow the userspace to read the out-fence. On error -1 should be returned in the fence_fd field. Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- Changes since v2: - Since exbuf-flags is a new flag, check that unsupported flags aren't set. drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 + include/uapi/drm/virtgpu_drm.h | 13 ++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index d01a9ed100d1..1af289b28fc4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, struct ww_acquire_ctx ticket; void *buf; + exbuf->fence_fd = -1; + Move this after the sanity checking. Agreed. Fixed in v4 if (vgdev->has_virgl_3d == false) return -ENOSYS; + if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) + return -EINVAL; + I assume this did this trigger when using old userspace? No, not as far as I'm aware. This check is there to prevent userspace from polluting the bitspace of flag, so that all free bits can be used for new flags. As far as I understand this is pointed out by a drm driver development document written by danvet, which I unfortunately can't seem to find the link for at the moment. Yes that is correct. What I was asking is: Does a kernel with this patch, work with mesa lacking the corresponding updates? I'd imagine things work just fine. Yes it does! -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108637] kernel 4.19.0-arch1-1-ARCH - raven ridge
https://bugs.freedesktop.org/show_bug.cgi?id=108637 Bug ID: 108637 Summary: kernel 4.19.0-arch1-1-ARCH - raven ridge Product: DRI Version: XOrg git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: bibitocar...@gmail.com Created attachment 142342 --> https://bugs.freedesktop.org/attachment.cgi?id=142342&action=edit raven ridge 2400 Hi, My computer, Raven ridge 2400G with Asrock X470 Gaming-ITX/ac, BIOS P1.30 07/05/2018. With 4.18, everything worked except failures at boot and suspend to ram. With 4.19.0-arch1-1-ARCH, the failures at boot are gone but i get a lot of warnings in dmesg and failures at use. See my dmesg. Please tell me if you need something else. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108637] kernel 4.19.0-arch1-1-ARCH - raven ridge
https://bugs.freedesktop.org/show_bug.cgi?id=108637 --- Comment #1 from bibitocarlos --- Created attachment 142343 --> https://bugs.freedesktop.org/attachment.cgi?id=142343&action=edit xorg.log -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108637] kernel 4.19.0-arch1-1-ARCH - raven ridge
https://bugs.freedesktop.org/show_bug.cgi?id=108637 --- Comment #2 from bibitocarlos --- Created attachment 142344 --> https://bugs.freedesktop.org/attachment.cgi?id=142344&action=edit and i cant watch video with kodi -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108637] kernel 4.19.0-arch1-1-ARCH - raven ridge
https://bugs.freedesktop.org/show_bug.cgi?id=108637 --- Comment #3 from bibitocarlos --- Created attachment 142346 --> https://bugs.freedesktop.org/attachment.cgi?id=142346&action=edit journalctl with kodi bug -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/lease: debug output for lease creation
Daniel Vetter writes:
> I spent a bit of time scratching heads and figuring out why the igts
> don't work. Probably useful to keep this work.
Acked-by: Keith Packard
> /* Do not allow sub-leases */
> - if (lessor->lessor)
> + if (lessor->lessor) {
> + DRM_DEBUG_LEASE("recursive leasing not allowed\n");
> return -EINVAL;
> + }
(not review about this patch, just noticing that the relevant code is here:-)
We may want to revisit this restriction as it looks like multi-seat may
end up using leases from a manager to multiple X servers and letting
those X servers support leases would be a good thing.
--
-keith
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/lease: look at ->universal_planes only once
Daniel Vetter writes: > @@ -359,7 +359,8 @@ void drm_lease_revoke(struct drm_master *top) > static int validate_lease(struct drm_device *dev, > struct drm_file *lessor_priv, > int object_count, > - struct drm_mode_object **objects) > + struct drm_mode_object **objects, > + bool universal_planes) It looks like you can remove the lessor_priv argument here, which would ensure that we didn't reference any fields in it in the future. -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/lease: drop EXPORT_SYMBOL
Daniel Vetter writes: > Leases are entirely implemented within drm.ko, no need to even tempt > drivers into doing nasty things. And if there's really a need, we can > always re-export these again. > > Cc: Keith Packard > Cc: Dave Airlie > Signed-off-by: Daniel Vetter Acked-by: Keith Packard -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/10] drm/msm/gpu: Allocate the correct size for the GPU memptrs
Allocate the correct buffer size for the GPU memptrs. The incorrect size hasn't affected us thus far since the incorrect size was larger than the intended size and we're still stuck on page sized granularity anyway but technically correct is the best kind of correct. Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index ff958486819b..b7b4539c1731 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -920,7 +920,8 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, goto fail; } - memptrs = msm_gem_kernel_new(drm, sizeof(*gpu->memptrs_bo), + memptrs = msm_gem_kernel_new(drm, + sizeof(struct msm_rbmemptrs) * nr_rings, MSM_BO_UNCACHED, gpu->aspace, &gpu->memptrs_bo, &memptrs_iova); -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/10] drm/msm/gpu: Only store local command buffers in the GPU state
Instead of trying to store all the tagged buffers from a hanging
submit only store the command buffers that were not imported.
This cuts down on the amount of data stored in the GPU state to
the base minimum of useful information.
The downside is that this will make it more difficult to
successfully replay a hang with just the GPU state but there
isn't any reason why that functionality can't be added back
in later once we've figured out how to better communicate
such massive amounts of data.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/msm_gpu.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 4dcc759746df..f0985a75f441 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -316,28 +316,28 @@ static void msm_gpu_crashstate_get_bo(struct
msm_gpu_state *state,
struct msm_gpu_state_bo *state_bo = &state->bos[state->nr_bos];
/* Don't record write only objects */
-
state_bo->size = obj->base.size;
state_bo->iova = iova;
- /* Only store the data for buffer objects marked for read */
- if ((flags & MSM_SUBMIT_BO_READ)) {
+ /* Only store data for non imported buffer objects marked for read */
+ if ((flags & MSM_SUBMIT_BO_READ) && !obj->base.import_attach) {
void *ptr;
state_bo->data = kvmalloc(obj->base.size, GFP_KERNEL);
if (!state_bo->data)
- return;
+ goto out;
ptr = msm_gem_get_vaddr_active(&obj->base);
if (IS_ERR(ptr)) {
kvfree(state_bo->data);
- return;
+ state_bo->data = NULL;
+ goto out;
}
memcpy(state_bo->data, ptr, obj->base.size);
msm_gem_put_vaddr(&obj->base);
}
-
+out:
state->nr_bos++;
}
@@ -365,12 +365,15 @@ static void msm_gpu_crashstate_capture(struct msm_gpu
*gpu,
if (submit) {
int i;
- state->bos = kcalloc(submit->nr_bos,
+ state->bos = kcalloc(submit->nr_cmds,
sizeof(struct msm_gpu_state_bo), GFP_KERNEL);
- for (i = 0; state->bos && i < submit->nr_bos; i++)
- msm_gpu_crashstate_get_bo(state, submit->bos[i].obj,
- submit->bos[i].iova, submit->bos[i].flags);
+ for (i = 0; state->bos && i < submit->nr_cmds; i++) {
+ int idx = submit->cmd[i].idx;
+
+ msm_gpu_crashstate_get_bo(state, submit->bos[idx].obj,
+ submit->bos[idx].iova, submit->bos[idx].flags);
+ }
}
/* Set the active crash state to be dumped on failure */
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/10] v2: drm/msm/gpu updates for 4.21
Resend of [1] with the order rearranged to put a6xx gpu state at the end and attach an add-on patch to better manage a6xx state memory. I think everything here is good to go for 4.21 (maybe even some 4.20 fixes) but I can understand if we want to let the gpu state soak for another cycle. Sharat has been doing an excellent job of finding the bugs in it. Jordan Crouse (10): drm/msm: Update generated headers drm/msm/gpu: Allocate the correct size for the GPU memptrs drm/msm: Gracefully handle failure in _msm_gem_kernel_new drm/msm/gpu: Add per-submission statistics drm/msm/gpu: Add trace events for tracking GPU submissions drm/msm/gpu: Only store local command buffers in the GPU state drm/msm/gpu: Move gpu_poll_timeout() to adreno_gpu.h drm/msm/adreno: Don't capture register values if target doesn't define them drm/msm/a6xx: Add a6xx gpu state drm/msm/a6xx: Track and manage a6xx state memory drivers/gpu/drm/msm/Makefile|4 +- drivers/gpu/drm/msm/adreno/a5xx_gpu.c |5 - drivers/gpu/drm/msm/adreno/a6xx.xml.h | 54 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 45 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.h |3 + drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 80 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.h |8 + drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 1171 +++ drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h | 430 +++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 19 +- drivers/gpu/drm/msm/adreno/adreno_gpu.h |6 + drivers/gpu/drm/msm/msm_gem.c | 18 +- drivers/gpu/drm/msm/msm_gem.h |1 + drivers/gpu/drm/msm/msm_gem_submit.c| 15 +- drivers/gpu/drm/msm/msm_gpu.c | 49 +- drivers/gpu/drm/msm/msm_gpu_trace.h | 90 ++ drivers/gpu/drm/msm/msm_gpu_tracepoints.c |6 + drivers/gpu/drm/msm/msm_ringbuffer.h| 16 + 18 files changed, 1921 insertions(+), 99 deletions(-) create mode 100644 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c create mode 100644 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h create mode 100644 drivers/gpu/drm/msm/msm_gpu_trace.h create mode 100644 drivers/gpu/drm/msm/msm_gpu_tracepoints.c -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/10] drm/msm: Update generated headers
Resync the a6xx generated headers from the database.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/a6xx.xml.h | 54 +--
1 file changed, 42 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx.xml.h
b/drivers/gpu/drm/msm/adreno/a6xx.xml.h
index a6f7c40454a6..3b04110308ec 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx.xml.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx.xml.h
@@ -8,17 +8,17 @@ This file was generated by the rules-ng-ng headergen tool in
this git repository
git clone https://github.com/freedreno/envytools.git
The rules-ng-ng source files this header was generated from are:
-- /home/robclark/src/envytools/rnndb/adreno.xml (501 bytes,
from 2018-07-03 19:37:13)
-- /home/robclark/src/envytools/rnndb/freedreno_copyright.xml ( 1572 bytes,
from 2018-07-03 19:37:13)
-- /home/robclark/src/envytools/rnndb/adreno/a2xx.xml ( 36805 bytes,
from 2018-07-03 19:37:13)
-- /home/robclark/src/envytools/rnndb/adreno/adreno_common.xml ( 13634 bytes,
from 2018-07-03 19:37:13)
-- /home/robclark/src/envytools/rnndb/adreno/adreno_pm4.xml( 42585 bytes,
from 2018-10-04 19:06:37)
-- /home/robclark/src/envytools/rnndb/adreno/a3xx.xml ( 83840 bytes,
from 2018-07-03 19:37:13)
-- /home/robclark/src/envytools/rnndb/adreno/a4xx.xml ( 112086 bytes,
from 2018-07-03 19:37:13)
-- /home/robclark/src/envytools/rnndb/adreno/a5xx.xml ( 147240 bytes,
from 2018-10-04 19:06:37)
-- /home/robclark/src/envytools/rnndb/adreno/a6xx.xml ( 139581 bytes,
from 2018-10-04 19:06:42)
-- /home/robclark/src/envytools/rnndb/adreno/a6xx_gmu.xml ( 10431 bytes,
from 2018-09-14 13:03:07)
-- /home/robclark/src/envytools/rnndb/adreno/ocmem.xml ( 1773 bytes,
from 2018-07-03 19:37:13)
+- ./adreno.xml (501 bytes, from 2018-10-10 15:35:49)
+- ./freedreno_copyright.xml ( 1572 bytes, from 2016-10-24 21:12:27)
+- ./adreno/a2xx.xml ( 37936 bytes, from 2018-10-10 15:35:49)
+- ./adreno/adreno_common.xml ( 14201 bytes, from 2018-10-10 15:35:49)
+- ./adreno/adreno_pm4.xml( 42864 bytes, from 2018-10-10 15:35:49)
+- ./adreno/a3xx.xml ( 83840 bytes, from 2017-12-05 18:20:27)
+- ./adreno/a4xx.xml ( 112086 bytes, from 2018-10-10 15:35:49)
+- ./adreno/a5xx.xml ( 147240 bytes, from 2018-10-10 15:35:49)
+- ./adreno/a6xx.xml ( 140514 bytes, from 2018-10-10 15:35:49)
+- ./adreno/a6xx_gmu.xml ( 10431 bytes, from 2018-10-10 15:35:49)
+- ./adreno/ocmem.xml ( 1773 bytes, from 2016-10-24 21:12:27)
Copyright (C) 2013-2018 by the following authors:
- Rob Clark (robclark)
@@ -501,7 +501,7 @@ enum a6xx_vfd_perfcounter_select {
PERF_VFDP_VS_STAGE_WAVES = 22,
};
-enum a6xx_hslq_perfcounter_select {
+enum a6xx_hlsq_perfcounter_select {
PERF_HLSQ_BUSY_CYCLES = 0,
PERF_HLSQ_STALL_CYCLES_UCHE = 1,
PERF_HLSQ_STALL_CYCLES_SP_STATE = 2,
@@ -2959,6 +2959,8 @@ static inline uint32_t
A6XX_GRAS_SC_WINDOW_SCISSOR_BR_Y(uint32_t val)
#define A6XX_GRAS_LRZ_CNTL_ENABLE 0x0001
#define A6XX_GRAS_LRZ_CNTL_LRZ_WRITE 0x0002
#define A6XX_GRAS_LRZ_CNTL_GREATER 0x0004
+#define A6XX_GRAS_LRZ_CNTL_UNK3
0x0008
+#define A6XX_GRAS_LRZ_CNTL_UNK4
0x0010
#define REG_A6XX_GRAS_UNKNOWN_8101 0x8101
@@ -2997,6 +2999,13 @@ static inline uint32_t
A6XX_GRAS_LRZ_BUFFER_PITCH_ARRAY_PITCH(uint32_t val)
#define REG_A6XX_GRAS_UNKNOWN_8110 0x8110
#define REG_A6XX_GRAS_2D_BLIT_CNTL 0x8400
+#define A6XX_GRAS_2D_BLIT_CNTL_COLOR_FORMAT__MASK 0xff00
+#define A6XX_GRAS_2D_BLIT_CNTL_COLOR_FORMAT__SHIFT 8
+static inline uint32_t A6XX_GRAS_2D_BLIT_CNTL_COLOR_FORMAT(enum a6xx_color_fmt
val)
+{
+ return ((val) << A6XX_GRAS_2D_BLIT_CNTL_COLOR_FORMAT__SHIFT) &
A6XX_GRAS_2D_BLIT_CNTL_COLOR_FORMAT__MASK;
+}
+#define A6XX_GRAS_2D_BLIT_CNTL_SCISSOR 0x0001
#define REG_A6XX_GRAS_2D_SRC_TL_X 0x8401
#define A6XX_GRAS_2D_SRC_TL_X_X__MASK 0x0000
@@ -3642,6 +3651,9 @@ static inline uint32_t A6XX_RB_WINDOW_OFFSET_Y(uint32_t
val)
#define REG_A6XX_RB_SAMPLE_COUNT_CONTROL 0x8891
#define A6XX_RB_SAMPLE_COUNT_CONTROL_COPY 0x0002
+#define REG_A6XX_RB_LRZ_CNTL 0x8898
+#define A6XX_RB_LRZ_CNTL_ENABLE
0x0001
+
#define REG_A6XX_RB_UNKNOWN_88D0 0x88d0
#define REG_A6XX_RB_BLIT_SCISSOR_TL0x88d1
@@ -3780,6 +3792,9 @@ static inline uint32_t
A6XX_RB_2D_BLIT_CNTL_COLOR_
[PATCH 03/10] drm/msm: Gracefully handle failure in _msm_gem_kernel_new
If any of the function calls in _msm_gem_kernel_new fail we need
to make sure to dereference the GEM object with the appropriate
function for the current locking state.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/msm_gem.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 00c795ced02c..4646e9e45fc2 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -1041,23 +1041,29 @@ static void *_msm_gem_kernel_new(struct drm_device
*dev, uint32_t size,
if (iova) {
ret = msm_gem_get_iova(obj, aspace, iova);
- if (ret) {
- drm_gem_object_put(obj);
- return ERR_PTR(ret);
- }
+ if (ret)
+ goto err;
}
vaddr = msm_gem_get_vaddr(obj);
if (IS_ERR(vaddr)) {
msm_gem_put_iova(obj, aspace);
- drm_gem_object_put(obj);
- return ERR_CAST(vaddr);
+ ret = PTR_ERR(vaddr);
+ goto err;
}
if (bo)
*bo = obj;
return vaddr;
+err:
+ if (locked)
+ drm_gem_object_put(obj);
+ else
+ drm_gem_object_put_unlocked(obj);
+
+ return ERR_PTR(ret);
+
}
void *msm_gem_kernel_new(struct drm_device *dev, uint32_t size,
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/10] drm/msm/gpu: Add per-submission statistics
Add infrastructure to track statistics for GPU submissions
by sampling certain perfcounters before and after a submission.
To store the statistics, the per-ring memptrs region is
expanded to include room for up to 64 entries - this should
cover a reasonable amount of inflight submissions without
worrying about losing data. The target specific code inserts
PM4 commands to sample the counters before and after
submission and store them in the data region. The CPU can
access the data after the submission retires to make sense
of the statistics and communicate them to the user.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 +---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 34 ---
drivers/gpu/drm/msm/msm_ringbuffer.h | 16 +
3 files changed, 54 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 0a0ceb76e2ba..e816947ac7d8 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -346,16 +346,20 @@ static int a6xx_rpmh_start(struct a6xx_gmu *gmu)
ret = gmu_poll_timeout(gmu, REG_A6XX_RSCC_SEQ_BUSY_DRV0, val,
!val, 100, 1);
- if (!ret) {
- gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, 0);
-
- /* Re-enable the power counter */
- gmu_write(gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_ENABLE, 1);
- return 0;
+ if (ret) {
+ DRM_DEV_ERROR(gmu->dev, "GPU RSC sequence stuck while waking up
the GPU\n");
+ return ret;
}
- DRM_DEV_ERROR(gmu->dev, "GPU RSC sequence stuck while waking up the
GPU\n");
- return ret;
+ gmu_write(gmu, REG_A6XX_GMU_RSCC_CONTROL_REQ, 0);
+
+ /* Set up CX GMU counter 0 to count busy ticks */
+ gmu_write(gmu, REG_A6XX_GPU_GMU_AO_GPU_CX_BUSY_MASK, 0xff00);
+ gmu_rmw(gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_SELECT_0, 0xff, 0x20);
+
+ /* Enable the power counter */
+ gmu_write(gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_ENABLE, 1);
+ return 0;
}
static void a6xx_rpmh_stop(struct a6xx_gmu *gmu)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 38b7a5a92bfb..cf66edfb5246 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -67,13 +67,34 @@ static void a6xx_flush(struct msm_gpu *gpu, struct
msm_ringbuffer *ring)
gpu_write(gpu, REG_A6XX_CP_RB_WPTR, wptr);
}
+static void get_stats_counter(struct msm_ringbuffer *ring, u32 counter,
+ u64 iova)
+{
+ OUT_PKT7(ring, CP_REG_TO_MEM, 3);
+ OUT_RING(ring, counter | (1 << 30) | (2 << 18));
+ OUT_RING(ring, lower_32_bits(iova));
+ OUT_RING(ring, upper_32_bits(iova));
+}
+
static void a6xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
struct msm_file_private *ctx)
{
+ unsigned int index = submit->seqno % MSM_GPU_SUBMIT_STATS_COUNT;
struct msm_drm_private *priv = gpu->dev->dev_private;
struct msm_ringbuffer *ring = submit->ring;
unsigned int i;
+ get_stats_counter(ring, REG_A6XX_RBBM_PERFCTR_CP_0_LO,
+ rbmemptr_stats(ring, index, cpcycles_start));
+
+ /*
+* For PM4 the GMU register offsets are calculated from the base of the
+* GPU registers so we need to add 0x1a800 to the register value on A630
+* to get the right value from PM4.
+*/
+ get_stats_counter(ring, REG_A6XX_GMU_ALWAYS_ON_COUNTER_L + 0x1a800,
+ rbmemptr_stats(ring, index, alwayson_start));
+
/* Invalidate CCU depth and color */
OUT_PKT7(ring, CP_EVENT_WRITE, 1);
OUT_RING(ring, PC_CCU_INVALIDATE_DEPTH);
@@ -98,6 +119,11 @@ static void a6xx_submit(struct msm_gpu *gpu, struct
msm_gem_submit *submit,
}
}
+ get_stats_counter(ring, REG_A6XX_RBBM_PERFCTR_CP_0_LO,
+ rbmemptr_stats(ring, index, cpcycles_end));
+ get_stats_counter(ring, REG_A6XX_GMU_ALWAYS_ON_COUNTER_L + 0x1a800,
+ rbmemptr_stats(ring, index, alwayson_end));
+
/* Write the fence to the scratch register */
OUT_PKT4(ring, REG_A6XX_CP_SCRATCH_REG(2), 1);
OUT_RING(ring, submit->seqno);
@@ -387,14 +413,6 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
/* Select CP0 to always count cycles */
gpu_write(gpu, REG_A6XX_CP_PERFCTR_CP_SEL_0, PERF_CP_ALWAYS_COUNT);
- /* FIXME: not sure if this should live here or in a6xx_gmu.c */
- gmu_write(&a6xx_gpu->gmu, REG_A6XX_GPU_GMU_AO_GPU_CX_BUSY_MASK,
- 0xff00);
- gmu_rmw(&a6xx_gpu->gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_SELECT_0,
- 0xff, 0x20);
- gmu_write(&a6xx_gpu->gmu, REG_A6XX_GMU_CX_GMU_POWER_COUNTER_ENABLE,
- 0x01);
-
gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2
[PATCH 09/10] drm/msm/a6xx: Add a6xx gpu state
Add support for gathering and dumping the a6xx GPU state including
registers, GMU registers, indexed registers, shader blocks,
context clusters and debugbus.
v2: Fix bugs discovered by Sharat Masetty
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/Makefile|1 +
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 25 +-
drivers/gpu/drm/msm/adreno/a6xx_gmu.h |3 +
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 39 +-
drivers/gpu/drm/msm/adreno/a6xx_gpu.h |8 +
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 1159 +++
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h | 430 +++
7 files changed, 1627 insertions(+), 38 deletions(-)
create mode 100644 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
create mode 100644 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 5de2d8f0a7e5..fabc17bf1a58 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -14,6 +14,7 @@ msm-y := \
adreno/a6xx_gpu.o \
adreno/a6xx_gmu.o \
adreno/a6xx_hfi.o \
+ adreno/a6xx_gpu_state.o \
hdmi/hdmi.o \
hdmi/hdmi_audio.o \
hdmi/hdmi_bridge.o \
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index e816947ac7d8..c58e953fefa3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -51,10 +51,31 @@ static irqreturn_t a6xx_hfi_irq(int irq, void *data)
return IRQ_HANDLED;
}
+bool a6xx_gmu_sptprac_is_on(struct a6xx_gmu *gmu)
+{
+ u32 val;
+
+ /* This can be called from gpu state code so make sure GMU is valid */
+ if (IS_ERR_OR_NULL(gmu->mmio))
+ return false;
+
+ val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS);
+
+ return !(val &
+ (A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_SPTPRAC_GDSC_POWER_OFF |
+ A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_SP_CLOCK_OFF));
+}
+
/* Check to see if the GX rail is still powered */
-static bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu)
+bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu)
{
- u32 val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS);
+ u32 val;
+
+ /* This can be called from gpu state code so make sure GMU is valid */
+ if (IS_ERR_OR_NULL(gmu->mmio))
+ return false;
+
+ val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS);
return !(val &
(A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_GDSC_POWER_OFF |
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index 35f765afae45..c721d9165d8e 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -164,4 +164,7 @@ void a6xx_hfi_init(struct a6xx_gmu *gmu);
int a6xx_hfi_start(struct a6xx_gmu *gmu, int boot_state);
void a6xx_hfi_stop(struct a6xx_gmu *gmu);
+bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu);
+bool a6xx_gmu_sptprac_is_on(struct a6xx_gmu *gmu);
+
#endif
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index e0a918e8e969..11f0b99f94c8 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -670,33 +670,6 @@ static const u32
a6xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_CNTL, REG_A6XX_CP_RB_CNTL),
};
-static const u32 a6xx_registers[] = {
- 0x, 0x0002, 0x0010, 0x0010, 0x0012, 0x0012, 0x0018, 0x001b,
- 0x001e, 0x0032, 0x0038, 0x003c, 0x0042, 0x0042, 0x0044, 0x0044,
- 0x0047, 0x0047, 0x0056, 0x0056, 0x00ad, 0x00ae, 0x00b0, 0x00fb,
- 0x0100, 0x011d, 0x0200, 0x020d, 0x0210, 0x0213, 0x0218, 0x023d,
- 0x0400, 0x04f9, 0x0500, 0x0500, 0x0505, 0x050b, 0x050e, 0x0511,
- 0x0533, 0x0533, 0x0540, 0x0555, 0x0800, 0x0808, 0x0810, 0x0813,
- 0x0820, 0x0821, 0x0823, 0x0827, 0x0830, 0x0833, 0x0840, 0x0843,
- 0x084f, 0x086f, 0x0880, 0x088a, 0x08a0, 0x08ab, 0x08c0, 0x08c4,
- 0x08d0, 0x08dd, 0x08f0, 0x08f3, 0x0900, 0x0903, 0x0908, 0x0911,
- 0x0928, 0x093e, 0x0942, 0x094d, 0x0980, 0x0984, 0x098d, 0x0996,
- 0x0998, 0x099e, 0x09a0, 0x09a6, 0x09a8, 0x09ae, 0x09b0, 0x09b1,
- 0x09c2, 0x09c8, 0x0a00, 0x0a03, 0x0c00, 0x0c04, 0x0c06, 0x0c06,
- 0x0c10, 0x0cd9, 0x0e00, 0x0e0e, 0x0e10, 0x0e13, 0x0e17, 0x0e19,
- 0x0e1c, 0x0e2b, 0x0e30, 0x0e32, 0x0e38, 0x0e39, 0x8600, 0x8601,
- 0x8610, 0x861b, 0x8620, 0x8620, 0x8628, 0x862b, 0x8630, 0x8637,
- 0x8e01, 0x8e01, 0x8e04, 0x8e05, 0x8e07, 0x8e08, 0x8e0c, 0x8e0c,
- 0x8e10, 0x8e1c, 0x8e20, 0x8e25, 0x8e28, 0x8e28, 0x8e2c, 0x8e2f,
- 0x8e3b, 0x8e3e, 0x8e40, 0x8e43, 0x8e50, 0x8e5e, 0x8e70, 0x8e77,
- 0x9600, 0x9604, 0x9624, 0x9637, 0x9e00, 0x9e01, 0x9e03, 0x9e0e,
- 0x9e11, 0x9e16, 0x9e19, 0x9e19, 0x9e1c, 0x9e1c, 0x9e20, 0x9e23,
- 0x9e30, 0x9e31, 0x9e34, 0x9e34, 0x9e70, 0x9e72, 0x9e78, 0x9
[PATCH 08/10] drm/msm/adreno: Don't capture register values if target doesn't define them
If the GPU target doesn't define a list of registers then gracefully skip
capturing and/or printing them. This is used by more complex targets like
6xx that have other means of capturing register values.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 141062fbb4c8..2c9bbad9c943 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -414,6 +414,10 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct
msm_gpu_state *state)
}
}
+ /* Some targets prefer to collect their own registers */
+ if (!adreno_gpu->registers)
+ return 0;
+
/* Count the number of registers */
for (i = 0; adreno_gpu->registers[i] != ~0; i += 2)
count += adreno_gpu->registers[i + 1] -
@@ -551,12 +555,14 @@ void adreno_show(struct msm_gpu *gpu, struct
msm_gpu_state *state,
}
}
- drm_puts(p, "registers:\n");
+ if (state->nr_registers) {
+ drm_puts(p, "registers:\n");
- for (i = 0; i < state->nr_registers; i++) {
- drm_printf(p, " - { offset: 0x%04x, value: 0x%08x }\n",
- state->registers[i * 2] << 2,
- state->registers[(i * 2) + 1]);
+ for (i = 0; i < state->nr_registers; i++) {
+ drm_printf(p, " - { offset: 0x%04x, value: 0x%08x }\n",
+ state->registers[i * 2] << 2,
+ state->registers[(i * 2) + 1]);
+ }
}
}
#endif
@@ -595,6 +601,9 @@ void adreno_dump(struct msm_gpu *gpu)
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
int i;
+ if (!adreno_gpu->registers)
+ return;
+
/* dump these out in a form that can be parsed by demsm: */
printk("IO:region %s 0002\n", gpu->name);
for (i = 0; adreno_gpu->registers[i] != ~0; i += 2) {
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/10] drm/msm/gpu: Add trace events for tracking GPU submissions
Add trace events to track the progress of a GPU submission
msm_gpu_submit occurs at the beginning of the submissions,
msm_gpu_submit_flush happens when the submission is put on
the ringbuffer and msm_submit_flush_retired is sent when
the operation is retired.
To make it easier to track the operations a unique sequence
number is assigned to each submission and displayed in each
event output so a human or a script can easily associate
the events related to a specific submission.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/Makefile | 3 +-
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 7 ++
drivers/gpu/drm/msm/msm_gem.h | 1 +
drivers/gpu/drm/msm/msm_gem_submit.c | 15 +++-
drivers/gpu/drm/msm/msm_gpu.c | 23 +-
drivers/gpu/drm/msm/msm_gpu_trace.h | 90 +++
drivers/gpu/drm/msm/msm_gpu_tracepoints.c | 6 ++
7 files changed, 139 insertions(+), 6 deletions(-)
create mode 100644 drivers/gpu/drm/msm/msm_gpu_trace.h
create mode 100644 drivers/gpu/drm/msm/msm_gpu_tracepoints.c
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 19ab521d4c3a..5de2d8f0a7e5 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -90,7 +90,8 @@ msm-y := \
msm_perf.o \
msm_rd.o \
msm_ringbuffer.o \
- msm_submitqueue.o
+ msm_submitqueue.o \
+ msm_gpu_tracepoints.o
msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \
disp/dpu1/dpu_dbg.o
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index cf66edfb5246..e0a918e8e969 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -4,6 +4,7 @@
#include "msm_gem.h"
#include "msm_mmu.h"
+#include "msm_gpu_trace.h"
#include "a6xx_gpu.h"
#include "a6xx_gmu.xml.h"
@@ -81,6 +82,8 @@ static void a6xx_submit(struct msm_gpu *gpu, struct
msm_gem_submit *submit,
{
unsigned int index = submit->seqno % MSM_GPU_SUBMIT_STATS_COUNT;
struct msm_drm_private *priv = gpu->dev->dev_private;
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
struct msm_ringbuffer *ring = submit->ring;
unsigned int i;
@@ -138,6 +141,10 @@ static void a6xx_submit(struct msm_gpu *gpu, struct
msm_gem_submit *submit,
OUT_RING(ring, upper_32_bits(rbmemptr(ring, fence)));
OUT_RING(ring, submit->seqno);
+ trace_msm_gpu_submit_flush(submit,
+ gmu_read64(&a6xx_gpu->gmu, REG_A6XX_GMU_ALWAYS_ON_COUNTER_L,
+ REG_A6XX_GMU_ALWAYS_ON_COUNTER_H));
+
a6xx_flush(gpu, ring);
}
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index c5d9bd3e47a8..ddaf8663dc95 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -150,6 +150,7 @@ struct msm_gem_submit {
struct msm_ringbuffer *ring;
unsigned int nr_cmds;
unsigned int nr_bos;
+ u32 ident; /* A "identifier" for the submit for logging */
struct {
uint32_t type;
uint32_t size; /* in dwords */
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 66673ea9bf6f..1b05b8481c31 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -20,6 +20,7 @@
#include "msm_drv.h"
#include "msm_gpu.h"
#include "msm_gem.h"
+#include "msm_gpu_trace.h"
/*
* Cmdstream submission:
@@ -48,7 +49,6 @@ static struct msm_gem_submit *submit_create(struct drm_device
*dev,
submit->dev = dev;
submit->gpu = gpu;
submit->fence = NULL;
- submit->pid = get_pid(task_pid(current));
submit->cmd = (void *)&submit->bos[nr_bos];
submit->queue = queue;
submit->ring = gpu->rb[queue->prio];
@@ -408,6 +408,7 @@ static void submit_cleanup(struct msm_gem_submit *submit)
int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
struct drm_file *file)
{
+ static atomic_t ident = ATOMIC_INIT(0);
struct msm_drm_private *priv = dev->dev_private;
struct drm_msm_gem_submit *args = data;
struct msm_file_private *ctx = file->driver_priv;
@@ -418,9 +419,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
struct msm_gpu_submitqueue *queue;
struct msm_ringbuffer *ring;
int out_fence_fd = -1;
+ struct pid *pid = get_pid(task_pid(current));
unsigned i;
- int ret;
-
+ int ret, submitid;
if (!gpu)
return -ENXIO;
@@ -443,7 +444,12 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
*data,
if (!queue)
return -ENOENT;
+ /* Get a unique identifier for the submission for logging purposes */
+ submitid = atomic_inc_return(&ident) - 1;
+
ring = gpu->rb
[PATCH 07/10] drm/msm/gpu: Move gpu_poll_timeout() to adreno_gpu.h
The gpu_poll_timeout() function can be useful to multiple targets so
mvoe it into adreno_gpu.h from the a5xx code.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 5 -
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 6 ++
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 2d34643ef2e2..fcb8e99563bc 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -20,7 +20,6 @@
#include
#include
#include
-#include
#include
#include "msm_gem.h"
#include "msm_mmu.h"
@@ -1211,10 +1210,6 @@ struct a5xx_gpu_state {
u32 *hlsqregs;
};
-#define gpu_poll_timeout(gpu, addr, val, cond, interval, timeout) \
- readl_poll_timeout((gpu)->mmio + ((addr) << 2), val, cond, \
- interval, timeout)
-
static int a5xx_crashdumper_init(struct msm_gpu *gpu,
struct a5xx_crashdumper *dumper)
{
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index de6e6ee42fba..7e5f1120ce7a 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -21,6 +21,7 @@
#define __ADRENO_GPU_H__
#include
+#include
#include "msm_gpu.h"
@@ -375,4 +376,9 @@ static inline uint32_t get_wptr(struct msm_ringbuffer *ring)
((1 << 29) \
((ilog2((_len)) & 0x1F) << 24) | (((_reg) << 2) & 0xF))
+
+#define gpu_poll_timeout(gpu, addr, val, cond, interval, timeout) \
+ readl_poll_timeout((gpu)->mmio + ((addr) << 2), val, cond, \
+ interval, timeout)
+
#endif /* __ADRENO_GPU_H__ */
--
2.18.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/10] drm/msm/a6xx: Track and manage a6xx state memory
The a6xx GPU state allocates a LOT of memory. Add a bit of
infrastructure to track the memory allocations in the GPU structure
and delete them when the state is destroyed much the same way
that devm works with the device model as a whole. This protects
against the developer accidentally forgetting to add a kfree() to
an ever growing list.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 192 +++-
1 file changed, 102 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
index 20f5b914c6fb..ec57ddeb8c77 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
@@ -41,6 +41,8 @@ struct a6xx_gpu_state {
struct a6xx_gpu_state_obj *cx_debugbus;
int nr_cx_debugbus;
+
+ struct list_head objs;
};
static inline int CRASHDUMP_WRITE(u64 *in, u32 reg, u32 val)
@@ -73,6 +75,33 @@ struct a6xx_crashdumper {
u64 iova;
};
+struct a6xx_state_memobj {
+ struct list_head node;
+ unsigned long long data[];
+};
+
+void *state_kcalloc(struct a6xx_gpu_state *a6xx_state, int nr, size_t objsize)
+{
+ struct a6xx_state_memobj *obj =
+ kzalloc((nr * objsize) + sizeof(*obj), GFP_KERNEL);
+
+ if (!obj)
+ return NULL;
+
+ list_add_tail(&obj->node, &a6xx_state->objs);
+ return &obj->data;
+}
+
+void *state_kmemdup(struct a6xx_gpu_state *a6xx_state, void *src,
+ size_t size)
+{
+ void *dst = state_kcalloc(a6xx_state, 1, size);
+
+ if (dst)
+ memcpy(dst, src, size);
+ return dst;
+}
+
/*
* Allocate 1MB for the crashdumper scratch region - 8k for the script and
* the rest for the data
@@ -203,12 +232,17 @@ static int vbif_debugbus_read(struct msm_gpu *gpu, u32
ctrl0, u32 ctrl1,
(12 * XIN_CORE_BLOCKS))
static void a6xx_get_vbif_debugbus_block(struct msm_gpu *gpu,
+ struct a6xx_gpu_state *a6xx_state,
struct a6xx_gpu_state_obj *obj)
{
u32 clk, *ptr;
int i;
- obj->data = kcalloc(VBIF_DEBUGBUS_BLOCK_SIZE, sizeof(u32), GFP_KERNEL);
+ obj->data = state_kcalloc(a6xx_state, VBIF_DEBUGBUS_BLOCK_SIZE,
+ sizeof(u32));
+ if (!obj->data)
+ return;
+
obj->handle = NULL;
/* Get the current clock setting */
@@ -252,13 +286,14 @@ static void a6xx_get_vbif_debugbus_block(struct msm_gpu
*gpu,
}
static void a6xx_get_debugbus_block(struct msm_gpu *gpu,
+ struct a6xx_gpu_state *a6xx_state,
const struct a6xx_debugbus_block *block,
struct a6xx_gpu_state_obj *obj)
{
int i;
u32 *ptr;
- obj->data = kcalloc(block->count, sizeof(u64), GFP_KERNEL);
+ obj->data = state_kcalloc(a6xx_state, block->count, sizeof(u64));
if (!obj->data)
return;
@@ -269,13 +304,14 @@ static void a6xx_get_debugbus_block(struct msm_gpu *gpu,
}
static void a6xx_get_cx_debugbus_block(void __iomem *cxdbg,
+ struct a6xx_gpu_state *a6xx_state,
const struct a6xx_debugbus_block *block,
struct a6xx_gpu_state_obj *obj)
{
int i;
u32 *ptr;
- obj->data = kcalloc(block->count, sizeof(u64), GFP_KERNEL);
+ obj->data = state_kcalloc(a6xx_state, block->count, sizeof(u64));
if (!obj->data)
return;
@@ -344,36 +380,42 @@ static void a6xx_get_debugbus(struct msm_gpu *gpu,
cxdbg_write(cxdbg, REG_A6XX_DBGC_CFG_DBGBUS_MASKL_3, 0);
}
- a6xx_state->debugbus = kcalloc(ARRAY_SIZE(a6xx_debugbus_blocks),
- sizeof(*a6xx_state->debugbus), GFP_KERNEL);
+ a6xx_state->debugbus = state_kcalloc(a6xx_state,
+ ARRAY_SIZE(a6xx_debugbus_blocks),
+ sizeof(*a6xx_state->debugbus));
if (a6xx_state->debugbus) {
int i;
for (i = 0; i < ARRAY_SIZE(a6xx_debugbus_blocks); i++)
a6xx_get_debugbus_block(gpu,
+ a6xx_state,
&a6xx_debugbus_blocks[i],
&a6xx_state->debugbus[i]);
a6xx_state->nr_debugbus = ARRAY_SIZE(a6xx_debugbus_blocks);
}
- a6xx_state->vbif_debugbus = kzalloc(sizeof(*a6xx_state->vbif_debugbus),
- GFP_KERNEL);
+ a6xx_state->vbif_debugbus =
+ state_kcalloc(a6xx_state, 1,
+ sizeof(*a6xx_state->vbif_debugbus));
if (a6xx_state->vbif_debugbus)
- a6xx_get_vbif_debugbus_block(gpu, a6xx_state->vbif_debugbus);
+ a6xx_get_vbif_debugbus_block(gpu, a6xx_state,
+ a6xx_state->vbif_debugbus);
if (cxdbg) {
a6xx_state->cx_debugbus =
- kcalloc(ARRAY_SIZE(a6xx_cx
Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and enable modeset
On Fri, Nov 02, 2018 at 02:18:42PM +, Shankar, Uma wrote: > > > >-Original Message- > >From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] > >Sent: Friday, November 2, 2018 2:53 PM > >To: Shankar, Uma ; dri-devel@lists.freedesktop.org; > >intel-...@lists.freedesktop.org > >Cc: Syrjala, Ville ; a...@redhat.com; Lankhorst, > >Maarten > > > >Subject: Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and > >enable > >modeset > > > >Op 31-10-18 om 13:05 schreef Uma Shankar: > >> This patch attaches the colorspace connector property to the hdmi > >> connector. Based on colorspace change, modeset will be triggered to > >> switch to new colorspace. > >> > >> Based on colorspace property value create an infoframe with > >> appropriate colorspace. This can be used to send an infoframe packet > >> with proper colorspace value set which will help to enable wider color > >> gamut like BT2020 on sink. > >> > >> v2: Merged the changes of creating infoframe as well to this patch as > >> per Maarten's suggestion. > >> > >> Signed-off-by: Uma Shankar > >> --- > >> drivers/gpu/drm/i915/intel_atomic.c | 1 + > >> drivers/gpu/drm/i915/intel_hdmi.c | 5 + > >> 2 files changed, 6 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c > >> b/drivers/gpu/drm/i915/intel_atomic.c > >> index a5a2c8f..35ef70a 100644 > >> --- a/drivers/gpu/drm/i915/intel_atomic.c > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c > >> @@ -125,6 +125,7 @@ int intel_digital_connector_atomic_check(struct > >drm_connector *conn, > >> */ > >>if (new_conn_state->force_audio != old_conn_state->force_audio || > >>new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb > >> || > >> + new_state->colorspace != old_state->colorspace || > >>new_conn_state->base.picture_aspect_ratio != old_conn_state- > >>base.picture_aspect_ratio || > >>new_conn_state->base.content_type != old_conn_state- > >>base.content_type || > >>new_conn_state->base.scaling_mode != > >> old_conn_state->base.scaling_mode) > >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > >> b/drivers/gpu/drm/i915/intel_hdmi.c > >> index 129b880..8a41fb3 100644 > >> --- a/drivers/gpu/drm/i915/intel_hdmi.c > >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c > >> @@ -486,6 +486,8 @@ static void intel_hdmi_set_avi_infoframe(struct > >intel_encoder *encoder, > >>else > >>frame.avi.colorspace = HDMI_COLORSPACE_RGB; > >> > >> + frame.avi.extended_colorimetry = conn_state->colorspace; > >> + > >>drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode, > >> crtc_state->limited_color_range ? > >> > >HDMI_QUANTIZATION_RANGE_LIMITED : > >> @@ -2125,6 +2127,9 @@ static void intel_hdmi_destroy(struct drm_connector > >*connector) > >>intel_attach_broadcast_rgb_property(connector); > >>intel_attach_aspect_ratio_property(connector); > >>drm_connector_attach_content_type_property(connector); > >> + drm_object_attach_property(&connector->base, > >> + connector->dev->mode_config.colorspace_property, > >> + COLORIMETRY_ITU_709); > >Just put 0 here.. > >If you want to init the default colorspace, put it in the first patch. > > Ok, will update this. > > >We should perhaps hide color spaces that are not supported on HDMI? > > Currently the supported colorspaces will be picked from edid by userspace and > they should use the current property interface to set the one which is > supported. > Even on HDMI, some connectors may not support certain colorspace, so keeping > it on userspace to set the one which is supported by the particular > connector. Hope > this approach is fine ? I think we want to trim the list to whatever the infoframe vs. MSA/VSC SDP can carry. So HDMI will have one list, DP another. And I guess for lspcon we want to go with the HDMI definition since we populate the infoframe by hand. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and enable modeset
Op 02-11-18 om 16:41 schreef Ville Syrjälä: > On Fri, Nov 02, 2018 at 02:18:42PM +, Shankar, Uma wrote: >> >>> -Original Message- >>> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] >>> Sent: Friday, November 2, 2018 2:53 PM >>> To: Shankar, Uma ; dri-devel@lists.freedesktop.org; >>> intel-...@lists.freedesktop.org >>> Cc: Syrjala, Ville ; a...@redhat.com; Lankhorst, >>> Maarten >>> >>> Subject: Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and >>> enable >>> modeset >>> >>> Op 31-10-18 om 13:05 schreef Uma Shankar: This patch attaches the colorspace connector property to the hdmi connector. Based on colorspace change, modeset will be triggered to switch to new colorspace. Based on colorspace property value create an infoframe with appropriate colorspace. This can be used to send an infoframe packet with proper colorspace value set which will help to enable wider color gamut like BT2020 on sink. v2: Merged the changes of creating infoframe as well to this patch as per Maarten's suggestion. Signed-off-by: Uma Shankar --- drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_hdmi.c | 5 + 2 files changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index a5a2c8f..35ef70a 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -125,6 +125,7 @@ int intel_digital_connector_atomic_check(struct >>> drm_connector *conn, */ if (new_conn_state->force_audio != old_conn_state->force_audio || new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb || + new_state->colorspace != old_state->colorspace || new_conn_state->base.picture_aspect_ratio != old_conn_state- base.picture_aspect_ratio || new_conn_state->base.content_type != old_conn_state- base.content_type || new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode) diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 129b880..8a41fb3 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -486,6 +486,8 @@ static void intel_hdmi_set_avi_infoframe(struct >>> intel_encoder *encoder, else frame.avi.colorspace = HDMI_COLORSPACE_RGB; + frame.avi.extended_colorimetry = conn_state->colorspace; + drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode, crtc_state->limited_color_range ? >>> HDMI_QUANTIZATION_RANGE_LIMITED : @@ -2125,6 +2127,9 @@ static void intel_hdmi_destroy(struct drm_connector >>> *connector) intel_attach_broadcast_rgb_property(connector); intel_attach_aspect_ratio_property(connector); drm_connector_attach_content_type_property(connector); + drm_object_attach_property(&connector->base, + connector->dev->mode_config.colorspace_property, + COLORIMETRY_ITU_709); >>> Just put 0 here.. >>> If you want to init the default colorspace, put it in the first patch. >> Ok, will update this. >> >>> We should perhaps hide color spaces that are not supported on HDMI? >> Currently the supported colorspaces will be picked from edid by userspace and >> they should use the current property interface to set the one which is >> supported. >> Even on HDMI, some connectors may not support certain colorspace, so keeping >> it on userspace to set the one which is supported by the particular >> connector. Hope >> this approach is fine ? > I think we want to trim the list to whatever the infoframe vs. MSA/VSC > SDP can carry. So HDMI will have one list, DP another. And I guess for > lspcon we want to go with the HDMI definition since we populate the > infoframe by hand. > What about passive DP to HDMI convertors? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property and enable modeset
On Fri, Nov 02, 2018 at 04:44:10PM +0100, Maarten Lankhorst wrote: > Op 02-11-18 om 16:41 schreef Ville Syrjälä: > > On Fri, Nov 02, 2018 at 02:18:42PM +, Shankar, Uma wrote: > >> > >>> -Original Message- > >>> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] > >>> Sent: Friday, November 2, 2018 2:53 PM > >>> To: Shankar, Uma ; dri-devel@lists.freedesktop.org; > >>> intel-...@lists.freedesktop.org > >>> Cc: Syrjala, Ville ; a...@redhat.com; Lankhorst, > >>> Maarten > >>> > >>> Subject: Re: [Intel-gfx] [v2 2/2] drm/i915: Attach colorspace property > >>> and enable > >>> modeset > >>> > >>> Op 31-10-18 om 13:05 schreef Uma Shankar: > This patch attaches the colorspace connector property to the hdmi > connector. Based on colorspace change, modeset will be triggered to > switch to new colorspace. > > Based on colorspace property value create an infoframe with > appropriate colorspace. This can be used to send an infoframe packet > with proper colorspace value set which will help to enable wider color > gamut like BT2020 on sink. > > v2: Merged the changes of creating infoframe as well to this patch as > per Maarten's suggestion. > > Signed-off-by: Uma Shankar > --- > drivers/gpu/drm/i915/intel_atomic.c | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 5 + > 2 files changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c > b/drivers/gpu/drm/i915/intel_atomic.c > index a5a2c8f..35ef70a 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -125,6 +125,7 @@ int intel_digital_connector_atomic_check(struct > >>> drm_connector *conn, > */ > if (new_conn_state->force_audio != old_conn_state->force_audio > || > new_conn_state->broadcast_rgb != > old_conn_state->broadcast_rgb > || > +new_state->colorspace != old_state->colorspace || > new_conn_state->base.picture_aspect_ratio != old_conn_state- > base.picture_aspect_ratio || > new_conn_state->base.content_type != old_conn_state- > base.content_type || > new_conn_state->base.scaling_mode != > old_conn_state->base.scaling_mode) > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c > b/drivers/gpu/drm/i915/intel_hdmi.c > index 129b880..8a41fb3 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -486,6 +486,8 @@ static void intel_hdmi_set_avi_infoframe(struct > >>> intel_encoder *encoder, > else > frame.avi.colorspace = HDMI_COLORSPACE_RGB; > > +frame.avi.extended_colorimetry = conn_state->colorspace; > + > drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode, > > crtc_state->limited_color_range ? > > >>> HDMI_QUANTIZATION_RANGE_LIMITED : > @@ -2125,6 +2127,9 @@ static void intel_hdmi_destroy(struct drm_connector > >>> *connector) > intel_attach_broadcast_rgb_property(connector); > intel_attach_aspect_ratio_property(connector); > drm_connector_attach_content_type_property(connector); > +drm_object_attach_property(&connector->base, > +connector->dev->mode_config.colorspace_property, > +COLORIMETRY_ITU_709); > >>> Just put 0 here.. > >>> If you want to init the default colorspace, put it in the first patch. > >> Ok, will update this. > >> > >>> We should perhaps hide color spaces that are not supported on HDMI? > >> Currently the supported colorspaces will be picked from edid by userspace > >> and > >> they should use the current property interface to set the one which is > >> supported. > >> Even on HDMI, some connectors may not support certain colorspace, so > >> keeping > >> it on userspace to set the one which is supported by the particular > >> connector. Hope > >> this approach is fine ? > > I think we want to trim the list to whatever the infoframe vs. MSA/VSC > > SDP can carry. So HDMI will have one list, DP another. And I guess for > > lspcon we want to go with the HDMI definition since we populate the > > infoframe by hand. > > > What about passive DP to HDMI convertors? By passive you mean DP++? Those are HDMI for us. External DP->HDMI protocol converters are perhaps more tricky, but since we treat those as pure DP now and don't send any infoframes I think we should do the same when it comes to the exposed valeus for the property. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108577] Black X laptop screen with cursor if HDMI not plugged in, bisected
https://bugs.freedesktop.org/show_bug.cgi?id=108577 --- Comment #20 from Alex Deucher --- Created attachment 142348 --> https://bugs.freedesktop.org/attachment.cgi?id=142348&action=edit possible fix 2/2 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108577] Black X laptop screen with cursor if HDMI not plugged in, bisected
https://bugs.freedesktop.org/show_bug.cgi?id=108577 --- Comment #19 from Alex Deucher --- Created attachment 142347 --> https://bugs.freedesktop.org/attachment.cgi?id=142347&action=edit possible fix 1/2 Do these patches fix the issue? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] amdgpu/test: Add illegal register and memory access test.
On 11/02/2018 10:24 AM, Michel Dänzer wrote:
> On 2018-10-31 7:33 p.m., Andrey Grodzovsky wrote:
>> Illegal access will cause CP hang followed by job timeout and
>> recovery kicking in.
>> Also, disable the suite for all APU ASICs until GPU
>> reset issues for them will be resolved and GPU reset recovery
>> will be enabled by default.
>>
>> Signed-off-by: Andrey Grodzovsky
>>
>> [...]
>>
>> @@ -94,7 +119,9 @@ CU_BOOL suite_deadlock_tests_enable(void)
>> &minor_version, &device_handle))
>> return CU_FALSE;
>>
>> -if (device_handle->info.family_id == AMDGPU_FAMILY_SI) {
>> +if (device_handle->info.family_id == AMDGPU_FAMILY_SI ||
>> +device_handle->info.family_id == AMDGPU_FAMILY_CZ ||
>> +device_handle->info.family_id == AMDGPU_FAMILY_RV) {
>> printf("\n\nCurrently hangs the CP on this ASIC, deadlock suite
>> disabled\n");
>> enable = CU_FALSE;
>> }
> Indentation is wrong here and in other places. The libdrm tree contains
> configuration files for EditorConfig (https://editorconfig.org/); since
> you're using Eclipse, https://github.com/ncjones/editorconfig-eclipse
> should help.
I installed the eclipse plugin.
>
>
> I run amdgpu_test as part of my daily build/test script during lunch
> break; when I came back today, I was greeted by a GFX hang of the
> Bonaire in my development box due to this test. Please disable it for
> all pre-GFX8 ASICs. Ideally, it should also check at runtime that GPU
> recovery is actually enabled, as that still isn't the case by default
> except with bleeding edge amdgpu kernel code.
Thanks for testing - I will send a fix.
Andrey
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH v2] android: ion: How to properly clean caches for uncached allocations
Based on the suggestions from Laura I created a first draft for a change
which will attempt to ensure that uncached mappings are only applied to
ION memory who's cache lines have been cleaned.
It does this by providing cached mappings (for uncached ION allocations)
until the ION buffer is dma mapped and successfully cleaned, then it drops
the userspace mappings and when pages are accessed they are faulted back
in and uncached mappings are created.
This change has the following potential disadvantages:
- It assumes that userpace clients won't attempt to access the buffer
while it is being mapped as we are removing the userpspace mappings at
this point (though it is okay for them to have it mapped)
- It assumes that kernel clients won't hold a kernel mapping to the buffer
(ie dma_buf_kmap) while it is being dma-mapped. What should we do if there
is a kernel mapping at the time of dma mapping, fail the mapping, warn?
- There may be a performance penalty as a result of having to fault in the
pages after removing the userspace mappings.
It passes basic testing involving reading writing and reading from
uncached system heap allocations before and after dma mapping.
Please let me know if this is heading in the right direction and if there
are any concerns.
Signed-off-by: Liam Mark
---
drivers/staging/android/ion/ion.c | 146 +-
drivers/staging/android/ion/ion.h | 9 +++
2 files changed, 152 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 99073325b0c0..3dc0f5a265bf 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -96,6 +96,7 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
}
INIT_LIST_HEAD(&buffer->attachments);
+ INIT_LIST_HEAD(&buffer->vmas);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
ion_buffer_add(dev, buffer);
@@ -117,6 +118,7 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+ vfree(buffer->pages);
kfree(buffer);
}
@@ -245,11 +247,29 @@ static void ion_dma_buf_detatch(struct dma_buf *dmabuf,
kfree(a);
}
+static bool ion_buffer_uncached_clean(struct ion_buffer *buffer)
+{
+ return buffer->uncached_clean;
+}
+
+/* expect buffer->lock to be already taken */
+static void ion_buffer_zap_mappings(struct ion_buffer *buffer)
+{
+ struct ion_vma_list *vma_list;
+
+ list_for_each_entry(vma_list, &buffer->vmas, list) {
+ struct vm_area_struct *vma = vma_list->vma;
+
+ zap_page_range(vma, vma->vm_start, vma->vm_end - vma->vm_start);
+ }
+}
+
static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)
{
struct ion_dma_buf_attachment *a = attachment->priv;
struct sg_table *table;
+ struct ion_buffer *buffer = attachment->dmabuf->priv;
table = a->table;
@@ -257,6 +277,19 @@ static struct sg_table *ion_map_dma_buf(struct
dma_buf_attachment *attachment,
direction))
return ERR_PTR(-ENOMEM);
+ if (!ion_buffer_cached(buffer)) {
+ mutex_lock(&buffer->lock);
+ if (!ion_buffer_uncached_clean(buffer)) {
+ ion_buffer_zap_mappings(buffer);
+ if (buffer->kmap_cnt > 0) {
+ pr_warn_once("%s: buffer still mapped in the
kernel\n",
+__func__);
+ }
+ buffer->uncached_clean = true;
+ }
+ mutex_unlock(&buffer->lock);
+ }
+
return table;
}
@@ -267,6 +300,94 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment
*attachment,
dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
}
+static void __ion_vm_open(struct vm_area_struct *vma, bool lock)
+{
+ struct ion_buffer *buffer = vma->vm_private_data;
+ struct ion_vma_list *vma_list;
+
+ vma_list = kmalloc(sizeof(*vma_list), GFP_KERNEL);
+ if (!vma_list)
+ return;
+ vma_list->vma = vma;
+
+ if (lock)
+ mutex_lock(&buffer->lock);
+ list_add(&vma_list->list, &buffer->vmas);
+ if (lock)
+ mutex_unlock(&buffer->lock);
+}
+
+static void ion_vm_open(struct vm_area_struct *vma)
+{
+ __ion_vm_open(vma, true);
+}
+
+static void ion_vm_close(struct vm_area_struct *vma)
+{
+ struct ion_buffer *buffer = vma->vm_private_data;
+ struct ion_vma_list *vma_list, *tmp;
+
+ mutex_lock(&buffer->lock);
+ list_for_each_entry_safe(vma_list, tmp, &buffer->vmas, list) {
+ if (vma_list->vma != vma)
+
RE: [RFC] drm/bridge/sii902x: Fix EDID readback
Hello Peter, Thank you for your feedback! > > The "mux-locked" implementation was the one I first tried and I discovered > > it doesn't work for me, as other traffic on the parent adapter can get in > > the > > way. What we need for this device is no other traffic on the bus during the > > "select transaction deselect" procedure, that's why I went with the parent > > locking. Also this device needs a delay between stop and start conditions > > while addressing the monitor. > > Ok, I thought the problem was that a delay was needed between the STOP > of the command opening the gate and the START of the edid eeprom xfer, and > that everything else worked normally. Too bad this wasn't the actual problem. > > Hmm. If it is indeed true that other xfers must never creep into the "select > xfer deselect" procedure then you are indeed stuck with a parent-locking. > But is that really the case? Could it be that the extra delay between > STOP-START is only needed after the absolutely last xfer before the > command closing the gate? > > If that problem description is correct, it should be possible to go back to > a mux-locked gate, but then in your deselect implementation grab the i2c > adapter > lock manually - with i2c_lock_bus(adapter, I2C_LOCK_ROOT_ADAPTER) - before the > 30 us delay, then open code the command to close the gate with an unlocked i2c > access, and finally release the i2c bus lock. That way you have ensured > silence > on the bus for the required time before closing the gate. You would still need > to bypass regmap, but only in this one place (but maybe you should bypass > regmap for opening the gate too, for symmetry). To further detail the problem, the system is vulnerable from before the last write performed by sii902x_i2c_bypass_select to after we confirm we need the switch to be closed within sii902x_i2c_bypass_deselect, that's the minimum amount of time we could keep the parent adapter locked for, I guess I am stuck with a parent-locking architecture, aren't I? But I guess I could release it when it's not actually needed, or is this going to be a pain to maintain? Shall I just keep going with the parent-locking but using bare i2c transactions only within select and deselect and leave regmap to deal with everything else? snip > > > > Ah, do you think the following is going to cause any issue in the future? > > -edid = drm_get_edid(connector, sii902x->i2c->adapter); > > +edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); > > No, that was a critical part of the idea to use a gate to introduce the > needed delay. Again, thank you very much for spending your time looking into this, your feedback is very much appreciated. Fab > > Cheers, > Peter Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Performance regression in ast drm driver
Hi David, The following commit: commit 7cf321d118a825c1541b43ca45294126fd474efa Author: Dave Airlie Date: Mon Oct 24 15:37:48 2016 +1000 drm/drivers: add support for using the arch wc mapping API. is causing a huge performance regression for the ast drm driver. In a text console, if I call "cat" on a large text file, it takes almost twice as much time to be displayed and scrolled completely. Can you please check that the ast driver portion of that commit is both correct and complete? Thanks, -- Jean Delvare SUSE L3 Support ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] uapi: fix linux/kfd_ioctl.h userspace compilation errors
Consistently use types provided by via
to fix the following linux/kfd_ioctl.h userspace compilation errors:
/usr/include/linux/kfd_ioctl.h:250:2: error: unknown type name 'uint32_t'
uint32_t reset_type;
/usr/include/linux/kfd_ioctl.h:251:2: error: unknown type name 'uint32_t'
uint32_t reset_cause;
/usr/include/linux/kfd_ioctl.h:252:2: error: unknown type name 'uint32_t'
uint32_t memory_lost;
/usr/include/linux/kfd_ioctl.h:253:2: error: unknown type name 'uint32_t'
uint32_t gpu_id;
Fixes: 0c119abad7f0d ("drm/amd: Add kfd ioctl defines for hw_exception event")
Cc: # v4.19
Signed-off-by: Dmitry V. Levin
---
include/uapi/linux/kfd_ioctl.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index f5ff8a76e208..dae897f38e59 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -255,10 +255,10 @@ struct kfd_hsa_memory_exception_data {
/* hw exception data */
struct kfd_hsa_hw_exception_data {
- uint32_t reset_type;
- uint32_t reset_cause;
- uint32_t memory_lost;
- uint32_t gpu_id;
+ __u32 reset_type;
+ __u32 reset_cause;
+ __u32 memory_lost;
+ __u32 gpu_id;
};
/* Event data */
--
ldv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RFC] drm/bridge/sii902x: Fix EDID readback
Hello Mark, Thank you for your feedback! > Subject: Re: [RFC] drm/bridge/sii902x: Fix EDID readback > > On Wed, Oct 31, 2018 at 04:55:53PM +, Fabrizio Castro wrote: > > > Having the option of using "unlocked" flavours of reads and writes > > is what we need here, but looking at drivers/base/regmap/regmap-i2c.c > > I couldn't find anything suitable for my case, maybe Mark could advise > > on this one? I am sure I overlooked something here, is there a better > > way to address this? > > As Linus said you can go as far as writing your own read and write > functions if you want to. That seems to be the easiest thing, though I > am suspicous that you're having to use the I2C framework in this way - > it doesn't sound terribly clever. You could I guess also just have a > custom function for whatever register is doing all this stuff that just > ignores regmap and bodge things that way while using regmap as normal > for everything else. I agree, I have prototyped it and it seems to be working just fine. Will send a patch addressing all of the comments, including this one. Fab Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/msm/dpu: Correct dpu destroy and disable order
In case of msm drm bind failure, dpu_mdss_destroy is triggered. In this function, resources are freed and pm runtime disable is called, which triggers dpu_mdss_disable. Now in dpu_mdss_disable, driver tries to access a memory which is already freed. This results in kernel panic. Fix this by ensuring proper sequence of dpu destroy and disable calls. Changes in v2: - Removed double spacings [Jeykumar] Signed-off-by: Jayant Shekhar --- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c index fd9c893..902bb4c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c @@ -156,18 +156,15 @@ static void dpu_mdss_destroy(struct drm_device *dev) struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); struct dss_module_power *mp = &dpu_mdss->mp; + pm_runtime_disable(dev->dev); _dpu_mdss_irq_domain_fini(dpu_mdss); - free_irq(platform_get_irq(pdev, 0), dpu_mdss); - msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(&pdev->dev, mp->clk_config); if (dpu_mdss->mmio) devm_iounmap(&pdev->dev, dpu_mdss->mmio); dpu_mdss->mmio = NULL; - - pm_runtime_disable(dev->dev); priv->mdss = NULL; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] drm/bridge/sii902x: Fix EDID readback
On 2018-11-01 12:04, Fabrizio Castro wrote:
> Hello Peter,
>
> Thank you for your feedback!
>
> snip
>
>>> Yeah, there is some issue with locking here, and that's coming down
>>> from the fact that when we call into drm_get_edid, we implicitly call
>>> i2c_transfer which ultimately locks the i2c adapter, and then calls
>>> into our sii902x_i2c_bypass_select, which in turn calls into the regmap
>>> functions and therefore we try and lock the same i2c adapter again,
>>> driving the system into a deadlock.
>>> Having the option of using "unlocked" flavours of reads and writes
>>> is what we need here, but looking at drivers/base/regmap/regmap-i2c.c
>>> I couldn't find anything suitable for my case, maybe Mark could advise
>>> on this one? I am sure I overlooked something here, is there a better
>>> way to address this?
>>
>> This recursive locking problem surfaces when an i2c mux is operated
>> by writing commands on the same i2c bus that is muxed. When this
>> happens for a typical i2c mux chip like nxp,pca9548 this can be kept
>> local to that driver. However, when there are interactions with other
>> parts of the kernel (e.g. if the i2c-mux is operated by gpio pins,
>> and those gpio pins in turn are operated by accesses to the i2c bus
>> that is muxed) this locked vs. unlocked thing would spread like
>> wildfire.
>>
>> Since I did not like that wildfire, I came up with the "mux-locked"
>> scheme. It is not appropriate everywhere, but in this case I think it
>> is a perfect fit. By using it, you should be able to dodge all locking
>> issues and it should be possible to keep the regmap usage as-is and the
>> patch would in all likelihood be much less intrusive.
>>
>> For some background on "mux-locked" vs. "parent-locked" (which is what
>> you have used in this RFC patch), see Documentation/i2c/i2c-topology.
>
> The "mux-locked" implementation was the one I first tried and I discovered
> it doesn't work for me, as other traffic on the parent adapter can get in the
> way. What we need for this device is no other traffic on the bus during the
> "select transaction deselect" procedure, that's why I went with the parent
> locking. Also this device needs a delay between stop and start conditions
> while addressing the monitor.
Ok, I thought the problem was that a delay was needed between the STOP
of the command opening the gate and the START of the edid eeprom xfer, and
that everything else worked normally. Too bad this wasn't the actual problem.
Hmm. If it is indeed true that other xfers must never creep into the "select
xfer deselect" procedure then you are indeed stuck with a parent-locking.
But is that really the case? Could it be that the extra delay between
STOP-START is only needed after the absolutely last xfer before the
command closing the gate?
If that problem description is correct, it should be possible to go back to
a mux-locked gate, but then in your deselect implementation grab the i2c adapter
lock manually - with i2c_lock_bus(adapter, I2C_LOCK_ROOT_ADAPTER) - before the
30 us delay, then open code the command to close the gate with an unlocked i2c
access, and finally release the i2c bus lock. That way you have ensured silence
on the bus for the required time before closing the gate. You would still need
to bypass regmap, but only in this one place (but maybe you should bypass
regmap for opening the gate too, for symmetry).
Cheers,
Peter
>>
>> There are a couple of more comments below, inline.
>>
>
> snip
>
>
> +static int sii902x_i2c_bypass_select(struct i2c_mux_core *mux, u32
> chan_id)
> +{
> + struct sii902x *sii902x = i2c_mux_priv(mux);
> + struct device *dev = &sii902x->i2c->dev;
> + unsigned long timeout;
> + u8 status;
> + int ret;
> +
> + ret = __sii902x_update_bits(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> + SII902X_SYS_CTRL_DDC_BUS_REQ,
> + SII902X_SYS_CTRL_DDC_BUS_REQ);
> +
> + timeout = jiffies +
> +
> msecs_to_jiffies(SII902X_I2C_BUS_ACQUISITION_TIMEOUT_MS);
> + do {
> + ret = __sii902x_read(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> +&status);
> + if (ret)
> + return ret;
> + } while (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD) &&
> +time_before(jiffies, timeout));
> +
> + if (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD)) {
> + dev_err(dev, "Failed to acquire the i2c bus\n");
> + return -ETIMEDOUT;
> + }
> +
> + ret = __sii902x_write(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> status);
> + if (ret)
> + return ret;
>>
>> Why do I not see a delay here? I thought the whole point of adding the i2c
>> gate
>> was that it enabled the introduction of a dela
Re: [PATCH v3 1/1] drm/panel: Add support for Olimex LCD-OLinuXino panel
On 7/16/18 6:08 PM, Rob Herring wrote: On Thu, Jul 12, 2018 at 11:21:53AM +0300, Stefan Mavrodiev wrote: This patch adds Olimex Ltd. LCD-OLinuXino bridge panel driver. The panel is used with different LCDs (currently from 480x272 to 1280x800). Small EEPROM chip is used for identification, which holds some factory data and timing requirements. Signed-off-by: Stefan Mavrodiev --- Changes for v3: - Change module license from "GPL v2" to "GPL" - Make use of backlight_enable()/backlight_disable() helpers - Rework backlight device request - Remove drm_panel_detach() - Use module_i2c_driver() for initialization - Make the first mode preferred - If num_modes value is invalid, overwrite it with 4, which is the maximum - Some variable types optimizations Changes for v2: - Changed lcd_olinuxino_funcs to static const .../display/panel/olimex,lcd-olinuxino.txt | 42 +++ Reviewed-by: Rob Herring MAINTAINERS| 6 + drivers/gpu/drm/panel/Kconfig | 10 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c | 331 + 5 files changed, 390 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/olimex,lcd-olinuxino.txt create mode 100644 drivers/gpu/drm/panel/panel-olimex-lcd-olinuxino.c I would like to ask if there will be any progress on this patch? Best regards, Stefan Mavrodiev ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: intel_pipe_config_compare: Don't compare DSI PLL regs when adjusting
The GOP sometimes initializes the DSI pclk at a (slightly) different freq
then the pclk which we pick. intel_pipe_config_compare() allows for this
by doing a fuzzy compare on the port_clock.
But the pclk difference not only results in the port_clock and
base.adjusted_mode.crtc_clock clocks being a bit different, but also in
us picking different dsi_pll register values matching the different pclk.
This commit makes us only do the dsi_pll register compare when the adjust
parameter is false, so only from verify_crtc_state(), so that we correctly
do a fast modeset at boot avoiding the screen going black for about 1 sec.
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/intel_display.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b219d5858160..82fa85df0fba 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11759,8 +11759,10 @@ intel_pipe_config_compare(struct drm_i915_private
*dev_priv,
PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_bias);
PIPE_CONF_CHECK_X(dpll_hw_state.mg_pll_tdc_coldst_bias);
- PIPE_CONF_CHECK_X(dsi_pll.ctrl);
- PIPE_CONF_CHECK_X(dsi_pll.div);
+ if (!adjust) {
+ PIPE_CONF_CHECK_X(dsi_pll.ctrl);
+ PIPE_CONF_CHECK_X(dsi_pll.div);
+ }
if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5)
PIPE_CONF_CHECK_I(pipe_bpp);
--
2.19.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge/sii902x: Fix EDID readback
While adding SiI9022A support to the iwg23s board, it came
up that when the HDMI transmitter is in pass through mode the
device is not compliant with the I2C specification anymore,
as it requires a far bigger tbuf, due to a delay the HDMI
transmitter is adding when relaying the STOP condition on the
monitor i2c side of things.
When not providing an appropriate delay after the STOP condition
the i2c bus would get stuck. Also, any other traffic on the bus
while talking to the monitor may cause the transaction to fail
or even cause issues with the i2c bus as well.
I2c-gates seemed to reach consent as a possible way to address
these issues, and as such this patch is implementing a solution
based on that. Since others are clearly relying on the current
implementation of the driver, this patch won't require any DT
changes.
Since we don't want any interference during the DDC Bus
Request/Grant procedure and while talking to the monitor, we
have to use the adapter locking primitives rather than the
i2c-mux locking primitives.
Signed-off-by: Fabrizio Castro
---
RFC->PATCH:
* Incorporated Linus Walleij, Peter Rosin, and Mark Brown comments
Thank you guys
drivers/gpu/drm/bridge/sii902x.c | 264 +--
1 file changed, 196 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index e59a135..4f9d9c7 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -1,4 +1,6 @@
/*
+ * Copyright (C) 2018 Renesas Electronics
+ *
* Copyright (C) 2016 Atmel
* Bo Shen
*
@@ -21,6 +23,7 @@
*/
#include
+#include
#include
#include
#include
@@ -86,8 +89,68 @@ struct sii902x {
struct drm_bridge bridge;
struct drm_connector connector;
struct gpio_desc *reset_gpio;
+ struct i2c_mux_core *i2cmux;
};
+static int sii902x_read_unlocked(struct i2c_client *i2c, u8 reg, u8 *val)
+{
+ struct i2c_msg xfer[] = {
+ {
+ .addr = i2c->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = ®,
+ }, {
+ .addr = i2c->addr,
+ .flags = I2C_M_RD,
+ .len = 1,
+ .buf = val,
+ }
+ };
+ unsigned char xfers = ARRAY_SIZE(xfer);
+ int ret, retries = 5;
+
+ do {
+ ret = __i2c_transfer(i2c->adapter, xfer, xfers);
+ if (ret < 0)
+ return ret;
+ } while (ret != xfers && --retries);
+ return ret == xfers ? 0 : -1;
+}
+
+static int sii902x_write_unlocked(struct i2c_client *i2c, u8 reg, u8 val)
+{
+ u8 data[2] = {reg, val};
+ struct i2c_msg xfer = {
+ .addr = i2c->addr,
+ .flags = 0,
+ .len = sizeof(data),
+ .buf = data,
+ };
+ int ret, retries = 5;
+
+ do {
+ ret = __i2c_transfer(i2c->adapter, &xfer, 1);
+ if (ret < 0)
+ return ret;
+ } while (!ret && --retries);
+ return !ret ? -1 : 0;
+}
+
+static int sii902x_update_bits_unlocked(struct i2c_client *i2c, u8 reg, u8
mask,
+ u8 val)
+{
+ int ret;
+ u8 status;
+
+ ret = sii902x_read_unlocked(i2c, reg, &status);
+ if (ret)
+ return ret;
+ status &= ~mask;
+ status |= val & mask;
+ return sii902x_write_unlocked(i2c, reg, status);
+}
+
static inline struct sii902x *bridge_to_sii902x(struct drm_bridge *bridge)
{
return container_of(bridge, struct sii902x, bridge);
@@ -135,41 +198,11 @@ static const struct drm_connector_funcs
sii902x_connector_funcs = {
static int sii902x_get_modes(struct drm_connector *connector)
{
struct sii902x *sii902x = connector_to_sii902x(connector);
- struct regmap *regmap = sii902x->regmap;
u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
- struct device *dev = &sii902x->i2c->dev;
- unsigned long timeout;
- unsigned int retries;
- unsigned int status;
struct edid *edid;
- int num = 0;
- int ret;
-
- ret = regmap_update_bits(regmap, SII902X_SYS_CTRL_DATA,
-SII902X_SYS_CTRL_DDC_BUS_REQ,
-SII902X_SYS_CTRL_DDC_BUS_REQ);
- if (ret)
- return ret;
-
- timeout = jiffies +
- msecs_to_jiffies(SII902X_I2C_BUS_ACQUISITION_TIMEOUT_MS);
- do {
- ret = regmap_read(regmap, SII902X_SYS_CTRL_DATA, &status);
- if (ret)
- return ret;
- } while (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD) &&
-time_before(jiffies, timeout));
-
- if (!(status & SII902X_SYS_CTRL_DDC_BUS_GRTD)) {
- dev_err(dev, "failed to acquire the i2c bus\n")
[PATCH] backlight/arcxcnn support newer chips in arcxcnn family and fix vendor prefix
Support for ArcticSand arc1 and arc3 ships is added. Some ranges
and control paths are modified based on the chip id probed via
i2c. Also updates vendor prefix to arctic from arc which was a
mistake in the original driver submission
Signed-off-by: Brian Dodge
---
.../bindings/leds/backlight/arcxcnn_bl.txt | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
index dcaa239..230abde 100644
--- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
@@ -1,8 +1,8 @@
-Binding for ArcticSand arc family LED drivers
+Binding for ArcticSand arc2c0608 LED driver
Required properties:
-- compatible: "arctic,arc1c0608", "arctic,arc2c0608", "arctic,arc3c0845"
-- reg: slave address
+- compatible: should be "arc,arc2c0608"
+- reg: slave address
Optional properties:
- default-brightness: brightness value on boot, value from: 0-4095
@@ -11,19 +11,19 @@ Optional properties:
- led-sources: List of enabled channels from 0 to 5.
See Documentation/devicetree/bindings/leds/common.txt
-- arctic,led-config-0: setting for register ILED_CONFIG_0
-- arctic,led-config-1: setting for register ILED_CONFIG_1
-- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used)
-- arctic,comp-config: setting for register CONFIG_COMP
-- arctic,filter-config:setting for register FILTER_CONFIG
-- arctic,trim-config: setting for register IMAXTUNE
+- arc,led-config-0:setting for register ILED_CONFIG_0
+- arc,led-config-1:setting for register ILED_CONFIG_1
+- arc,dim-freq:PWM mode frequence setting (bits [3:0] used)
+- arc,comp-config: setting for register CONFIG_COMP
+- arc,filter-config: setting for register FILTER_CONFIG
+- arc,trim-config: setting for register IMAXTUNE
Note: Optional properties not specified will default to values in IC EPROM
Example:
arc2c0608@30 {
- compatible = "arctic,arc2c0608";
+ compatible = "arc,arc2c0608";
reg = <0x30>;
default-brightness = <500>;
label = "lcd-backlight";
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RFC] drm/bridge/sii902x: Fix EDID readback
Hello Peter, Again, thank you very much for your precious comments. I'll send a patch out shortly addressing all of the comments I have received so far, including yours. Cheers, Fab > Subject: Re: [RFC] drm/bridge/sii902x: Fix EDID readback > > On 2018-11-01 18:32, Fabrizio Castro wrote: > > Hello Peter, > > > > Thank you for your feedback! > > > >> Subject: Re: [RFC] drm/bridge/sii902x: Fix EDID readback > > > > snip > > > >>> > >>> To further detail the problem, the system is vulnerable from before the > >>> last write > >>> performed by sii902x_i2c_bypass_select to after we confirm we need the > >>> switch > >>> to be closed within sii902x_i2c_bypass_deselect, that's the minimum > >>> amount of time > >>> we could keep the parent adapter locked for, I guess I am stuck with a > >>> parent-locking > >>> architecture, aren't I? > >> > >> If that problem description is correct, then yes, I think the *only* > >> solution > >> is to combine the three parts "open bypass mode", "edid xfer" and "close > >> bypass > >> mode", and to keep the i2c adapter locked during the procedure so that > >> other > >> xfers do not creep in and crap thing up from the side. And one way to > >> combine > >> the three parts is to use a parent-locked i2c gate. And since you need to > >> keep > >> the i2c adapter locked over the whole procedure, you need to use unlocked > >> xfers > >> (as you have already discovered). But how do you know that this problem > >> description is accurate? > > > > I basically observed what was going on on the bus (with a logic analyser) > > while generating > > traffic on the parent adapter > > > >> Why is it not ok for unrelated xfers to creep in > >> between opening the bypass mode and the edid xfer, and how do you know that > >> this is not ok? > > > > Because those transfers would come with no extra delay between STOP and > > START > > conditions while the HDMI transmitter is in passthrough mode > > Yeah yeah, now I get it. It's not the edid eeprom that's bad, it's the > passthrough mode in the hdmi transmitter that's broken. Your problem > description follows naturally. > > >> > >>> But I guess I could release it when it's not actually needed, > >> > >> How would you figure out when it's not needed? > > > > The moment you tell the HDMI transmitter you are going to talk to the > > monitor nothing else can > > flow on the bus, up until you gracefully close the pass through session, > > which means I wouldn't > > really need to hold the parent lock during the entire duration of the > > select callback, I would need > > to hold the parent lock only from before the last write as that's when we > > tell the HDMI transmitter > > to activate the passthrough mode, but I guess it would make the driver hard > > to maintain (as in > > others would need to understand this properly before making any changes), > > wouldn't it? > > Yes, that would just complicate things and would probably not have all that > much benefit. I don't think I'd go there... > > >> > >>> or is this going to be a pain to maintain? Shall I just keep going with > >>> the parent-locking > >>> but using bare i2c transactions only within select and deselect and leave > >>> regmap > >>> to deal with everything else? > >> > >> That's a possibility. Take care to not mess up any cached state in regmap > >> though. > > > > The original version of the driver wasn't using any caching, so I guess I > > would need to fallback > > exactly to the same implementation. > > > > So, what should I do? Should I keep the parent-locking, the unlocked > > flavours of rd/wr/rmw from > > within select/deselect, and put back regmap based rd/wr/rmw with no caching > > for everything else? > > I think that sounds like a reasonable compromise, but be careful since you > still > might need the two implementations to interact, e.g. the two rmw variants > might > still need a common lock so that they don't trample on each others toes. At > least if there are accesses to the same register (SII902X_SYS_CTRL_DATA in > this > case if I read it right). > > Cheers, > Peter Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/bridge/sii902x: Fix EDID readback
Hello Peter,
Thank you for your feedback!
> > +static int sii902x_i2c_bypass_select(struct i2c_mux_core *mux, u32 chan_id)
> > +{
> > +struct sii902x *sii902x = i2c_mux_priv(mux);
> > +struct device *dev = &sii902x->i2c->dev;
> > +unsigned long timeout;
> > +u8 status;
> > +int ret;
> > +
> > +ret = sii902x_update_bits_unlocked(sii902x->i2c, SII902X_SYS_CTRL_DATA,
> > + SII902X_SYS_CTRL_DDC_BUS_REQ,
> > + SII902X_SYS_CTRL_DDC_BUS_REQ);
>
> Here (and also in sii902x_i2c_bypass_deselect) you do a rmw access to the
> SII902X_SYS_CTRL_DATA register without coordinating with regmap. Regmap is
> also doing rmw accesses to that register in other parts of the driver. I
> think you need to either add comment as to why that is safe (maybe other
> things make it impossible for the two rmw accesses to cross?), or add the
> missing coordination.
>
The other two places where SII902X_SYS_CTRL_DATA is being handled are
sii902x_bridge_disable and sii902x_bridge_enable, I didn’t think there is
any chance of the modes being probed while the bridge gets enabled/disabled,
but now that you make me think about it user space may trigger the readback
of the EDID at the most inconvenient times. Anyway, this is a good point, and
since I don't think I am supposed to access regmap's lock from outside the APIs,
I could switch to the unlocked version of update_bits from within
sii902x_bridge_disable
and sii902x_bridge_enable, and manually grab the i2c adapter lock, what do you
think?
Fab
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End,
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered
No. 04586709.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[[DPU PATCH]] drm/msm/dpu: Correct dpu destroy and disable order
In case of msm drm bind failure, dpu_mdss_destroy is triggered. In this function, resources are freed and pm runtime disable is called, which triggers dpu_mdss_disable. Now in dpu_mdss_disable, driver tries to access a memory which is already freed. This results in kernel panic. Fix this by ensuring proper sequence of dpu destroy and disable calls. Change-Id: Id6e01a537ae9c40789c5752dc28c397391ab7dfe Signed-off-by: Jayant Shekhar --- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c index fd9c893..cd9a6bd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c @@ -156,6 +156,8 @@ static void dpu_mdss_destroy(struct drm_device *dev) struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); struct dss_module_power *mp = &dpu_mdss->mp; + pm_runtime_disable(dev->dev); + _dpu_mdss_irq_domain_fini(dpu_mdss); free_irq(platform_get_irq(pdev, 0), dpu_mdss); @@ -167,7 +169,6 @@ static void dpu_mdss_destroy(struct drm_device *dev) devm_iounmap(&pdev->dev, dpu_mdss->mmio); dpu_mdss->mmio = NULL; - pm_runtime_disable(dev->dev); priv->mdss = NULL; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] backlight/arcxcnn user proper vendor prefix in device tree bindings
The vendor-prefixes.txt file properly refers to ArcticSand
as "arctic" while the actual backlight driver and the
device tree bindings for it improperly use "arc". The
bindings are also modified to support newer backlight chips
in the arcxcnn family
Signed-off-by: Brian Dodge
---
.../bindings/leds/backlight/arcxcnn_bl.txt | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
index 230abde..dcaa239 100644
--- a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
@@ -1,8 +1,8 @@
-Binding for ArcticSand arc2c0608 LED driver
+Binding for ArcticSand arc family LED drivers
Required properties:
-- compatible: should be "arc,arc2c0608"
-- reg: slave address
+- compatible: "arctic,arc1c0608", "arctic,arc2c0608", "arctic,arc3c0845"
+- reg: slave address
Optional properties:
- default-brightness: brightness value on boot, value from: 0-4095
@@ -11,19 +11,19 @@ Optional properties:
- led-sources: List of enabled channels from 0 to 5.
See Documentation/devicetree/bindings/leds/common.txt
-- arc,led-config-0:setting for register ILED_CONFIG_0
-- arc,led-config-1:setting for register ILED_CONFIG_1
-- arc,dim-freq:PWM mode frequence setting (bits [3:0] used)
-- arc,comp-config: setting for register CONFIG_COMP
-- arc,filter-config: setting for register FILTER_CONFIG
-- arc,trim-config: setting for register IMAXTUNE
+- arctic,led-config-0: setting for register ILED_CONFIG_0
+- arctic,led-config-1: setting for register ILED_CONFIG_1
+- arctic,dim-freq: PWM mode frequence setting (bits [3:0] used)
+- arctic,comp-config: setting for register CONFIG_COMP
+- arctic,filter-config:setting for register FILTER_CONFIG
+- arctic,trim-config: setting for register IMAXTUNE
Note: Optional properties not specified will default to values in IC EPROM
Example:
arc2c0608@30 {
- compatible = "arc,arc2c0608";
+ compatible = "arctic,arc2c0608";
reg = <0x30>;
default-brightness = <500>;
label = "lcd-backlight";
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] uapi: fix more linux/kfd_ioctl.h userspace compilation errors
Consistently use types provided by via
to fix struct kfd_ioctl_get_queue_wave_state_args userspace compilation errors.
Fixes: 5df099e8bc83f ("drm/amdkfd: Add wavefront context save state retrieval
ioctl")
Signed-off-by: Dmitry V. Levin
---
include/uapi/linux/kfd_ioctl.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index dae897f38e59..b01eb502d49c 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -83,11 +83,11 @@ struct kfd_ioctl_set_cu_mask_args {
};
struct kfd_ioctl_get_queue_wave_state_args {
- uint64_t ctl_stack_address; /* to KFD */
- uint32_t ctl_stack_used_size; /* from KFD */
- uint32_t save_area_used_size; /* from KFD */
- uint32_t queue_id; /* to KFD */
- uint32_t pad;
+ __u64 ctl_stack_address;/* to KFD */
+ __u32 ctl_stack_used_size; /* from KFD */
+ __u32 save_area_used_size; /* from KFD */
+ __u32 queue_id; /* to KFD */
+ __u32 pad;
};
/* For kfd_ioctl_set_memory_policy_args.default_policy and alternate_policy */
--
ldv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RFC] drm/bridge/sii902x: Fix EDID readback
Hello Peter, Thank you for your feedback! > Subject: Re: [RFC] drm/bridge/sii902x: Fix EDID readback snip > > > > To further detail the problem, the system is vulnerable from before the > > last write > > performed by sii902x_i2c_bypass_select to after we confirm we need the > > switch > > to be closed within sii902x_i2c_bypass_deselect, that's the minimum amount > > of time > > we could keep the parent adapter locked for, I guess I am stuck with a > > parent-locking > > architecture, aren't I? > > If that problem description is correct, then yes, I think the *only* solution > is to combine the three parts "open bypass mode", "edid xfer" and "close > bypass > mode", and to keep the i2c adapter locked during the procedure so that other > xfers do not creep in and crap thing up from the side. And one way to combine > the three parts is to use a parent-locked i2c gate. And since you need to keep > the i2c adapter locked over the whole procedure, you need to use unlocked > xfers > (as you have already discovered). But how do you know that this problem > description is accurate? I basically observed what was going on on the bus (with a logic analyser) while generating traffic on the parent adapter > Why is it not ok for unrelated xfers to creep in > between opening the bypass mode and the edid xfer, and how do you know that > this is not ok? Because those transfers would come with no extra delay between STOP and START conditions while the HDMI transmitter is in passthrough mode > > > But I guess I could release it when it's not actually needed, > > How would you figure out when it's not needed? The moment you tell the HDMI transmitter you are going to talk to the monitor nothing else can flow on the bus, up until you gracefully close the pass through session, which means I wouldn't really need to hold the parent lock during the entire duration of the select callback, I would need to hold the parent lock only from before the last write as that's when we tell the HDMI transmitter to activate the passthrough mode, but I guess it would make the driver hard to maintain (as in others would need to understand this properly before making any changes), wouldn't it? > > > or is this going to be a pain to maintain? Shall I just keep going with the > > parent-locking > > but using bare i2c transactions only within select and deselect and leave > > regmap > > to deal with everything else? > > That's a possibility. Take care to not mess up any cached state in regmap > though. The original version of the driver wasn't using any caching, so I guess I would need to fallback exactly to the same implementation. So, what should I do? Should I keep the parent-locking, the unlocked flavours of rd/wr/rmw from within select/deselect, and put back regmap based rd/wr/rmw with no caching for everything else? Thank you! Fab > The registers you touch from select/deselect should probably be volatile or > something like that? > > Cheers, > Peter Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
