[PATCH 4/4] drm/amdgpu: add timeline support in amdgpu CS

2018-09-18 Thread Chunming Zhou
Signed-off-by: Chunming Zhou 
---
 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, 100 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 447c4c7a36d6..6e4a3db56833 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -975,6 +975,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;
@@ -1003,9 +1008,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 d9d2ede96490..aa829c6ef08e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -204,6 +204,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:
@@ -783,7 +785,7 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser 
*parser, int error,
   >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);
@@ -1098,11 +1100,11 @@ 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)
 {
int r;
struct dma_fence *fence;
-   r = drm_syncobj_find_fence(p->filp, handle, 0, );
+   r = drm_syncobj_find_fence(p->filp, handle, point, );
if (r)
return r;
 
@@ -1113,48 +1115,90 @@ 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 amdgpu_cs_chunk *chunk,
+   bool timeline)
 {
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);
+   if (!timeline) {
+   struct drm_amdgpu_cs_chunk_sem *deps;
 
-   for (i = 0; i < num_deps; ++i) {
-   r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle);
+   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);
if (r)
return r;
+   }
+   } else {
+   struct drm_amdgpu_cs_chunk_syncobj *syncobj_deps;
+
+   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, 
syncobj_deps[i].handle,
+ 
syncobj_deps[i].point);
+   if (r)
+   return r;
+   }
}
+
return 0;
 }
 
 static int amdgpu_cs_process_syncobj_out_dep(struct amdgpu_cs_parser *p,
-struct amdgpu_cs_chunk *chunk)
+struct amdgpu_cs_chunk *chunk,
+bool timeline)
 {

[PATCH 3/4] drm: add timeline syncobj payload query ioctl

2018-09-18 Thread Chunming Zhou
user mode can query timeline payload.

Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/drm_internal.h |  2 ++
 drivers/gpu/drm/drm_ioctl.c|  2 ++
 drivers/gpu/drm/drm_syncobj.c  | 53 ++
 include/uapi/drm/drm.h | 11 +++
 4 files changed, 68 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 c0891614f516..c3c0617e6372 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_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 311ea7ba6f57..51aec09cb18a 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1305,3 +1305,56 @@ 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,
+);
+   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], [i]);
+   copy_to_user(u64_to_user_ptr(args->points), points,
+sizeof(uint64_t) * args->count_handles);
+   }
+
+   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 501e86d81f47..188b63f1975b 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -767,6 +767,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 */
@@ -924,6 +933,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
  * The device specific ioctl range is from 0x40 to 0x9f.
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

[PATCH 1/4] [RFC]drm: add syncobj timeline support v6

2018-09-18 Thread Chunming Zhou
This patch is for VK_KHR_timeline_semaphore extension, semaphore is called 
syncobj in kernel side:
This extension introduces a new type of syncobj that has an integer payload
identifying a point in a timeline. Such timeline syncobjs support the
following operations:
   * CPU query - A host operation that allows querying the payload of the
 timeline syncobj.
   * CPU wait - A host operation that allows a blocking wait for a
 timeline syncobj to reach a specified value.
   * Device wait - A device operation that allows waiting for a
 timeline syncobj to reach a specified value.
   * Device signal - A device operation that allows advancing the
 timeline syncobj to a specified value.

v1:
Since it's a timeline, that means the front time point(PT) always is signaled 
before the late PT.
a. signal PT design:
Signal PT fence N depends on PT[N-1] fence and signal opertion fence, when 
PT[N] fence is signaled,
the timeline will increase to value of PT[N].
b. wait PT design:
Wait PT fence is signaled by reaching timeline point value, when timeline is 
increasing, will compare
wait PTs value with new timeline value, if PT value is lower than timeline 
value, then wait PT will be
signaled, otherwise keep in list. syncobj wait operation can wait on any point 
of timeline,
so need a RB tree to order them. And wait PT could ahead of signal PT, we need 
a sumission fence to
perform that.

v2:
1. remove unused DRM_SYNCOBJ_CREATE_TYPE_NORMAL. (Christian)
2. move unexposed denitions to .c file. (Daniel Vetter)
3. split up the change to drm_syncobj_find_fence() in a separate patch. 
(Christian)
4. split up the change to drm_syncobj_replace_fence() in a separate patch.
5. drop the submission_fence implementation and instead use wait_event() for 
that. (Christian)
6. WARN_ON(point != 0) for NORMAL type syncobj case. (Daniel Vetter)

v3:
1. replace normal syncobj with timeline implemenation. (Vetter and Christian)
a. normal syncobj signal op will create a signal PT to tail of signal pt 
list.
b. normal syncobj wait op will create a wait pt with last signal point, and 
this wait PT is only signaled by related signal point PT.
2. many bug fix and clean up
3. stub fence moving is moved to other patch.

v4:
1. fix RB tree loop with while(node=rb_first(...)). (Christian)
2. fix syncobj lifecycle. (Christian)
3. only enable_signaling when there is wait_pt. (Christian)
4. fix timeline path issues.
5. write a timeline test in libdrm

v5: (Christian)
1. semaphore is called syncobj in kernel side.
2. don't need 'timeline' characters in some function name.
3. keep syncobj cb

v6: (Christian)
1. merge syncobj_timeline to syncobj structure
2. simplify some check sentences.
3. some misc change.
4. fix CTS failed issue

normal syncobj is tested by ./deqp-vk -n dEQP-VK*semaphore*
timeline syncobj is tested by ./amdgpu_test -s 9

Signed-off-by: Chunming Zhou 
Cc: Christian Konig 
Cc: Dave Airlie 
Cc: Daniel Rakos 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/drm_syncobj.c  | 307 ++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   2 +-
 include/drm/drm_syncobj.h  |  65 ++---
 include/uapi/drm/drm.h |   1 +
 4 files changed, 299 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index e9ce623d049e..987b5a120b65 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,6 +56,9 @@
 #include "drm_internal.h"
 #include 
 
+/* merge normal syncobj to timeline syncobj, the point interval is 1 */
+#define DRM_SYNCOBJ_INDIVIDUAL_POINT 1
+
 struct drm_syncobj_stub_fence {
struct dma_fence base;
spinlock_t lock;
@@ -82,6 +85,11 @@ static const struct dma_fence_ops drm_syncobj_stub_fence_ops 
= {
.release = drm_syncobj_stub_fence_release,
 };
 
+struct drm_syncobj_signal_pt {
+   struct dma_fence_array *base;
+   u64value;
+   struct list_head list;
+};
 
 /**
  * drm_syncobj_find - lookup and reference a sync object.
@@ -117,6 +125,9 @@ static void drm_syncobj_add_callback_locked(struct 
drm_syncobj *syncobj,
list_add_tail(>node, >cb_list);
 }
 
+static int drm_syncobj_search_fence(struct drm_syncobj *syncobj, u64 point,
+   u64 flags, struct dma_fence **fence);
+
 static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
 struct dma_fence **fence,
 struct drm_syncobj_cb *cb,
@@ -124,8 +135,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
 {
int ret;
 
-   *fence = drm_syncobj_fence_get(syncobj);
-   if (*fence)
+   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   if (!ret)
return 1;
 
spin_lock(>lock);
@@ -133,10 +144,12 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
 

[PATCH 2/4] drm: add support of syncobj timeline point wait v2

2018-09-18 Thread Chunming Zhou
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.

Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/drm_internal.h |  2 +
 drivers/gpu/drm/drm_ioctl.c|  2 +
 drivers/gpu/drm/drm_syncobj.c  | 99 +-
 include/uapi/drm/drm.h | 14 +
 4 files changed, 103 insertions(+), 14 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 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,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 987b5a120b65..311ea7ba6f57 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -129,13 +129,14 @@ static int drm_syncobj_search_fence(struct drm_syncobj 
*syncobj, u64 point,
u64 flags, struct dma_fence **fence);
 
 static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+u64 point,
 struct dma_fence **fence,
 struct drm_syncobj_cb *cb,
 drm_syncobj_func_t func)
 {
int ret;
 
-   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
if (!ret)
return 1;
 
@@ -146,7 +147,7 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
 */
if (fence && !list_empty(>signal_pt_list)) {
spin_unlock(>lock);
-   drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   drm_syncobj_search_fence(syncobj, point, 0, fence);
if (*fence)
ret = 1;
spin_lock(>lock);
@@ -355,7 +356,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
spin_lock(>lock);
list_for_each_entry_safe(cur, tmp, >cb_list, node) {
list_del_init(>node);
+   spin_unlock(>lock);
cur->func(syncobj, cur);
+   spin_lock(>lock);
}
spin_unlock(>lock);
}
@@ -866,6 +869,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,
@@ -883,12 +887,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, >fence);
+   drm_syncobj_search_fence(syncobj, wait->point, 0, >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,
@@ -896,13 +901,27 @@ static signed long drm_syncobj_array_wait_timeout(struct 
drm_syncobj **syncobjs,
 {
struct syncobj_wait_entry *entries;
struct dma_fence *fence;
+   uint64_t *points;
signed long ret;

Re: [PATCH] [RFC]drm: add syncobj timeline support v5

2018-09-18 Thread zhoucm1



On 2018年09月18日 16:32, Christian König wrote:

+    for (i = 0; i < args->count_handles; i++) {
+    if (syncobjs[i]->type == DRM_SYNCOBJ_TYPE_TIMELINE) {
+    DRM_ERROR("timeline syncobj cannot reset!\n");


Why not? I mean that should still work or do I miss anything?
timeline semaphore spec doesn't require reset interface, it says the 
timeline value only can be changed by signal operations.


Yeah, but we don't care about the timeline spec in the kernel.

Question is rather if that still makes sense to support that and as 
far as I can see it should be trivial to reinitialize the object. 

Hi Daniel Rakos,

Could you give a comment on this question? Is it necessary to support 
timeline reset interface?  I only see the timeline value can be changed 
by signal operations in Spec.



Thanks,
David Zhou
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 5/6] drm/i915: Fix intel_dp_mst_best_encoder()

2018-09-18 Thread Lyude Paul
Currently, i915 appears to rely on blocking modesets on
no-longer-present MSTB ports by simply returning NULL for
->best_encoder(), which in turn causes any new atomic commits that don't
disable the CRTC to fail. This is wrong however, since we still want to
allow userspace to disable CRTCs on no-longer-present MSTB ports by
changing the DPMS state to off and this still requires that we retrieve
an encoder.

So, fix this by always returning a valid encoder regardless of the state
of the MST port. Additionally, make intel_dp_mst_atomic_check() simply
rely on drm_dp_mst_connector_atomic_check() to prevent new modesets on
no-longer-present MSTB ports.

Signed-off-by: Lyude Paul 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index a366f32b048a..2b798d4592f0 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -106,14 +106,21 @@ static bool intel_dp_mst_compute_config(struct 
intel_encoder *encoder,
 }
 
 static int intel_dp_mst_atomic_check(struct drm_connector *connector,
-   struct drm_connector_state *new_conn_state)
+struct drm_connector_state *new_conn_state)
 {
struct drm_atomic_state *state = new_conn_state->state;
struct drm_connector_state *old_conn_state;
struct drm_crtc *old_crtc;
struct drm_crtc_state *crtc_state;
+   struct drm_dp_mst_topology_mgr *mgr =
+   _intel_connector(connector)->mst_port->mst_mgr;
int slots, ret = 0;
 
+   ret = drm_dp_mst_connector_atomic_check(connector, new_conn_state,
+   mgr);
+   if (ret)
+   return ret;
+
old_conn_state = drm_atomic_get_old_connector_state(state, connector);
old_crtc = old_conn_state->crtc;
if (!old_crtc)
@@ -122,12 +129,6 @@ static int intel_dp_mst_atomic_check(struct drm_connector 
*connector,
crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc);
slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu;
if (drm_atomic_crtc_needs_modeset(crtc_state) && slots > 0) {
-   struct drm_dp_mst_topology_mgr *mgr;
-   struct drm_encoder *old_encoder;
-
-   old_encoder = old_conn_state->best_encoder;
-   mgr = _to_mst(old_encoder)->primary->dp.mst_mgr;
-
ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots);
if (ret)
DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", 
slots, ret);
@@ -407,8 +408,6 @@ static struct drm_encoder 
*intel_mst_atomic_best_encoder(struct drm_connector *c
struct intel_dp *intel_dp = intel_connector->mst_port;
struct intel_crtc *crtc = to_intel_crtc(state->crtc);
 
-   if (intel_connector->mst_port_gone)
-   return NULL;
return _dp->mst_encoders[crtc->pipe]->base.base;
 }
 
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 6/6] drm/amdgpu/dm/mst: Use drm_dp_mst_connector_atomic_check()

2018-09-18 Thread Lyude Paul
Hook this into amdgpu's atomic check for their connectors so they never
get modesets on no-longer-present MST connectors. We'll also expand on
this later once we add DP MST fallback retraining support.

As well, turns out that the only atomic DRM driver without the
->best_encoder() bug is amdgpu. Congrats AMD!

Signed-off-by: Lyude Paul 
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c  | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 9a300732ba37..d011a39f17b2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -294,10 +294,22 @@ static struct drm_encoder *dm_mst_best_encoder(struct 
drm_connector *connector)
return _dm_connector->mst_encoder->base;
 }
 
+static int
+amdgpu_dm_mst_connector_atomic_check(struct drm_connector *connector,
+struct drm_connector_state *new_cstate)
+{
+   struct amdgpu_dm_connector *aconnector =
+   to_amdgpu_dm_connector(connector);
+
+   return drm_dp_mst_connector_atomic_check(connector, new_cstate,
+>mst_mgr);
+}
+
 static const struct drm_connector_helper_funcs 
dm_dp_mst_connector_helper_funcs = {
.get_modes = dm_dp_mst_get_modes,
.mode_valid = amdgpu_dm_connector_mode_valid,
.best_encoder = dm_mst_best_encoder,
+   .atomic_check = amdgpu_dm_mst_connector_atomic_check,
 };
 
 static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder)
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 4/6] drm/i915: Skip vcpi allocation for MSTB ports that are gone

2018-09-18 Thread Lyude Paul
Since we need to be able to allow DPMS on->off prop changes after an MST
port has disappeared from the system, we need to be able to make sure we
can compute a config for the resulting atomic commit. Currently this is
impossible when the port has disappeared, since the VCPI slot searching
we try to do in intel_dp_mst_compute_config() will fail with -EINVAL.

Since the only commits we want to allow on no-longer-present MST ports
are ones that shut off display hardware, we already know that no VCPI
allocations are needed. So, hardcode the VCPI slot count to 0 when
intel_dp_mst_compute_config() is called on an MST port that's gone.

Signed-off-by: Lyude Paul 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index fcb9b87b9339..a366f32b048a 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -42,7 +42,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder 
*encoder,
to_intel_connector(conn_state->connector);
struct drm_atomic_state *state = pipe_config->base.state;
int bpp;
-   int lane_count, slots;
+   int lane_count, slots = 0;
const struct drm_display_mode *adjusted_mode = 
_config->base.adjusted_mode;
int mst_pbn;
bool reduce_m_n = drm_dp_has_quirk(_dp->desc,
@@ -76,11 +76,16 @@ static bool intel_dp_mst_compute_config(struct 
intel_encoder *encoder,
mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
pipe_config->pbn = mst_pbn;
 
-   slots = drm_dp_atomic_find_vcpi_slots(state, _dp->mst_mgr,
- connector->port, mst_pbn);
-   if (slots < 0) {
-   DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);
-   return false;
+   if (!connector->mst_port_gone) {
+   slots = drm_dp_atomic_find_vcpi_slots(state,
+ _dp->mst_mgr,
+ connector->port,
+ mst_pbn);
+   if (slots < 0) {
+   DRM_DEBUG_KMS("failed finding vcpi slots:%d\n",
+ slots);
+   return false;
+   }
}
 
intel_link_compute_m_n(bpp, lane_count,
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 2/6] drm/nouveau: Unbreak nv50_mstc->best_encoder()

2018-09-18 Thread Lyude Paul
As mentioned in the previous commit, we currently prevent new modesets
on recently-removed MST connectors by returning no encoder from our
->best_encoder() callback once the MST port has disappeared. This is
wrong however, because it prevents legacy modesetting users from being
able to disable CRTCs on MST connectors after the connector's respective
topology has disappeared.

So, fix this by instead using the new
drm_dp_mst_connector_atomic_check() helper instead while always
returning a valid encoder from ->best_encoder().

Signed-off-by: Lyude Paul 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 9da0bdfe1e1c..8d6f6ee9bc75 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -881,22 +881,16 @@ nv50_mstc_atomic_best_encoder(struct drm_connector 
*connector,
 {
struct nv50_head *head = nv50_head(connector_state->crtc);
struct nv50_mstc *mstc = nv50_mstc(connector);
-   if (mstc->port) {
-   struct nv50_mstm *mstm = mstc->mstm;
-   return >msto[head->base.index]->encoder;
-   }
-   return NULL;
+
+   return >mstm->msto[head->base.index]->encoder;
 }
 
 static struct drm_encoder *
 nv50_mstc_best_encoder(struct drm_connector *connector)
 {
struct nv50_mstc *mstc = nv50_mstc(connector);
-   if (mstc->port) {
-   struct nv50_mstm *mstm = mstc->mstm;
-   return >msto[0]->encoder;
-   }
-   return NULL;
+
+   return >mstm->msto[0]->encoder;
 }
 
 static enum drm_mode_status
@@ -926,10 +920,21 @@ nv50_mstc_get_modes(struct drm_connector *connector)
return ret;
 }
 
+static int
+nv50_mstc_atomic_check(struct drm_connector *connector,
+  struct drm_connector_state *state)
+{
+   struct nv50_mstc *mstc = nv50_mstc(connector);
+
+   return drm_dp_mst_connector_atomic_check(connector, state,
+>mstm->mgr);
+}
+
 static const struct drm_connector_helper_funcs
 nv50_mstc_help = {
.get_modes = nv50_mstc_get_modes,
.mode_valid = nv50_mstc_mode_valid,
+   .atomic_check = nv50_mstc_atomic_check,
.best_encoder = nv50_mstc_best_encoder,
.atomic_best_encoder = nv50_mstc_atomic_best_encoder,
 };
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 1/6] drm/dp_mst: Introduce drm_dp_mst_connector_atomic_check()

2018-09-18 Thread Lyude Paul
Currently the way that we prevent userspace from performing new modesets
on MST connectors that have just been destroyed is rather broken.
There's nothing in the actual DRM DP MST topology helpers that checks
whether or not a connector still exists, instead each DRM driver does
this on it's own, usually by returning NULL from the best_encoder
callback which in turn, causes the atomic commit to fail.

However, this is wrong in a rather subtle way. If ->best_encoder()
returns NULL, this makes ALL modesets involving the connector fail. This
includes modesets from userspace that would shut off the CRTCs being
used by the connector. Since this results in blocking any changes to a
connector's DPMS prop, it has the sideaffect of preventing legacy
modesetting users from ever disabling a CRTC that was previously enabled
for use in an MST topology. An example of this, where X tries to
change the DPMS property of an MST connector that was just detached from
the system:

[ 2908.320131] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
[CONNECTOR:82:DP-6]
[ 2908.320148] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
[CONNECTOR:82:DP-6] status updated from connected to disconnected
[ 2908.320166] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
[CONNECTOR:82:DP-6] disconnected
[ 2908.320193] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 111 (1)
[ 2908.320230] [drm:drm_sysfs_hotplug_event [drm]] generating hotplug event
...
[ 2908.638539] [drm:drm_ioctl [drm]] pid=12928, dev=0xe201, auth=1, 
DRM_IOCTL_MODE_SETPROPERTY
[ 2908.638546] [drm:drm_atomic_state_init [drm]] Allocated atomic state 
7155ba49
[ 2908.638553] [drm:drm_mode_object_get [drm]] OBJ ID: 114 (1)
[ 2908.638560] [drm:drm_mode_object_get [drm]] OBJ ID: 108 (1)
[ 2908.638568] [drm:drm_atomic_get_crtc_state [drm]] Added [CRTC:41:head-0] 
97a6396e state to 7155ba49
[ 2908.638575] [drm:drm_atomic_add_affected_connectors [drm]] Adding all 
current connectors for [CRTC:41:head-0] to 7155ba49
[ 2908.638582] [drm:drm_mode_object_get [drm]] OBJ ID: 82 (3)
[ 2908.638589] [drm:drm_mode_object_get [drm]] OBJ ID: 82 (4)
[ 2908.638596] [drm:drm_atomic_get_connector_state [drm]] Added 
[CONNECTOR:82:DP-6] 87427144 state to 7155ba49
[ 2908.638603] [drm:drm_atomic_check_only [drm]] checking 7155ba49
[ 2908.638609] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] 
[CRTC:41:head-0] active changed
[ 2908.638613] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] Updating 
routing for [CONNECTOR:82:DP-6]
[ 2908.638616] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] No 
suitable encoder found for [CONNECTOR:82:DP-6]
[ 2908.638623] [drm:drm_atomic_check_only [drm]] atomic driver check for 
7155ba49 failed: -22
[ 2908.638630] [drm:drm_atomic_state_default_clear [drm]] Clearing atomic state 
7155ba49
[ 2908.638637] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (4)
[ 2908.638643] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (3)
[ 2908.638650] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 114 (2)
[ 2908.638656] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 108 (2)
[ 2908.638663] [drm:__drm_atomic_state_free [drm]] Freeing atomic state 
7155ba49
[ 2908.638669] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (2)
[ 2908.638676] [drm:drm_ioctl [drm]] pid=12928, ret = -22

While this doesn't usually result in any errors that would be obvious to
the user, it does result in us leaving display resources on. This in
turn leads to unwanted sideaffects like inactive GPUs being left on
(usually from the resulting leaked runtime PM ref).

So, provide an easier way of doing this that doesn't require breaking
->best_encoder(): add a common drm_dp_mst_connector_atomic_check()
function that DRM drivers can call in order to have CRTC enabling
commits fail automatically if the MST port driving the connector no
longer exists. We'll also be able to expand upon this later as well once
we add MST fallback retraining support.

Signed-off-by: Lyude Paul 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 76 +++
 include/drm/drm_dp_mst_helper.h   |  3 ++
 2 files changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 7780567aa669..0162d4bf2549 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3129,6 +3129,82 @@ static const struct drm_private_state_funcs 
mst_state_funcs = {
.atomic_destroy_state = drm_dp_mst_destroy_state,
 };
 
+static bool
+drm_dp_mst_connector_still_exists(struct drm_connector *connector,
+ struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_branch *mstb)
+{
+   struct drm_dp_mst_port *port;
+   bool exists = false;
+
+   mstb = drm_dp_get_validated_mstb_ref(mgr, mstb);
+   if 

[PATCH 3/6] drm/i915: Leave intel_conn->mst_port set, use mst_port_gone instead

2018-09-18 Thread Lyude Paul
Currently we set intel_connector->mst_port to NULL to signify that the
MST port has been removed from the system so that we can prevent further
action on the port such as connector probes, mode probing, etc.
However, we're going to need access to intel_connector->mst_port in
order to fixup ->best_encoder() so that it can always return the correct
encoder for an MST port to prevent legacy DPMS prop changes from
failing. This should be safe, so instead keep intel_connector->mst_port
always set and instead add intel_connector->mst_port_gone in order to
signify whether or not the connector has disappeared from the system.

Signed-off-by: Lyude Paul 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 14 +++---
 drivers/gpu/drm/i915/intel_drv.h|  1 +
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 4ecd65375603..fcb9b87b9339 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -311,9 +311,8 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector 
*connector)
struct edid *edid;
int ret;
 
-   if (!intel_dp) {
+   if (intel_connector->mst_port_gone)
return intel_connector_update_modes(connector, NULL);
-   }
 
edid = drm_dp_mst_get_edid(connector, _dp->mst_mgr, 
intel_connector->port);
ret = intel_connector_update_modes(connector, edid);
@@ -328,9 +327,10 @@ intel_dp_mst_detect(struct drm_connector *connector, bool 
force)
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_connector->mst_port;
 
-   if (!intel_dp)
+   if (intel_connector->mst_port_gone)
return connector_status_disconnected;
-   return drm_dp_mst_detect_port(connector, _dp->mst_mgr, 
intel_connector->port);
+   return drm_dp_mst_detect_port(connector, _dp->mst_mgr,
+ intel_connector->port);
 }
 
 static void
@@ -370,7 +370,7 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
int bpp = 24; /* MST uses fixed bpp */
int max_rate, mode_rate, max_lanes, max_link_clock;
 
-   if (!intel_dp)
+   if (intel_connector->mst_port_gone)
return MODE_ERROR;
 
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
@@ -402,7 +402,7 @@ static struct drm_encoder 
*intel_mst_atomic_best_encoder(struct drm_connector *c
struct intel_dp *intel_dp = intel_connector->mst_port;
struct intel_crtc *crtc = to_intel_crtc(state->crtc);
 
-   if (!intel_dp)
+   if (intel_connector->mst_port_gone)
return NULL;
return _dp->mst_encoders[crtc->pipe]->base.base;
 }
@@ -514,7 +514,7 @@ static void intel_dp_destroy_mst_connector(struct 
drm_dp_mst_topology_mgr *mgr,
   connector);
/* prevent race with the check in ->detect */
drm_modeset_lock(>dev->mode_config.connection_mutex, NULL);
-   intel_connector->mst_port = NULL;
+   intel_connector->mst_port_gone = true;
drm_modeset_unlock(>dev->mode_config.connection_mutex);
 
drm_connector_put(connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8fc61e96754f..87ce772ae7f8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -409,6 +409,7 @@ struct intel_connector {
void *port; /* store this opaque as its illegal to dereference it */
 
struct intel_dp *mst_port;
+   bool mst_port_gone;
 
/* Work struct to schedule a uevent on link train failure */
struct work_struct modeset_retry_work;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 0/6] Fix legacy DPMS changes with MST

2018-09-18 Thread Lyude Paul
There's two major things this patchset does:
 - Add drm_dp_mst_connector_atomic_check() so drivers don't need to use
   ->best_encoder() to prevent modesets on zombie MST connectors. We'll
   use this later for implementing MST fallback retraining as well.
 - Fix DPMS on->off changes failing with legacy modesetting users after
   an MST connector's topology has disappeared, which resulted in CRTCs
   being left on when they shouldn't have been

Lyude Paul (6):
  drm/dp_mst: Introduce drm_dp_mst_connector_atomic_check()
  drm/nouveau: Unbreak nv50_mstc->best_encoder()
  drm/i915: Leave intel_conn->mst_port set, use mst_port_gone instead
  drm/i915: Skip vcpi allocation for MSTB ports that are gone
  drm/i915: Fix intel_dp_mst_best_encoder()
  drm/amdgpu/dm/mst: Use drm_dp_mst_connector_atomic_check()

 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 12 +++
 drivers/gpu/drm/drm_dp_mst_topology.c | 76 +++
 drivers/gpu/drm/i915/intel_dp_mst.c   | 46 ++-
 drivers/gpu/drm/i915/intel_drv.h  |  1 +
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 25 +++---
 include/drm/drm_dp_mst_helper.h   |  3 +
 6 files changed, 132 insertions(+), 31 deletions(-)

-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

2018-09-18 Thread Jason Gunthorpe
On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote:
> On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote:
> > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote:
> >  
> > > Acked-by: Darren Hart (VMware) 
> > > 
> > > As for a longer term solution, would it be possible to init fops in such
> > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
> > > so we don't have to duplicate this boilerplate for every ioctl fops
> > > structure?
> > 
> > Bad idea, that...  Because several years down the road somebody will add
> > an ioctl that takes an unsigned int for argument.  Without so much as 
> > looking
> > at your magical mystery macro being used to initialize file_operations.
> 
> Fair, being explicit in the declaration as it is currently may be
> preferable then.

It would be much cleaner and safer if you could arrange things to add
something like this to struct file_operations:

  long (*ptr_ioctl) (struct file *, unsigned int, void __user *);

Where the core code automatically converts the unsigned long to the
void __user * as appropriate.

Then it just works right always and the compiler will help address
Al's concern down the road.

Cheers,
Jason
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

2018-09-18 Thread Darren Hart
On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote:
> On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote:
>  
> > Acked-by: Darren Hart (VMware) 
> > 
> > As for a longer term solution, would it be possible to init fops in such
> > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
> > so we don't have to duplicate this boilerplate for every ioctl fops
> > structure?
> 
>   Bad idea, that...  Because several years down the road somebody will add
> an ioctl that takes an unsigned int for argument.  Without so much as looking
> at your magical mystery macro being used to initialize file_operations.

Fair, being explicit in the declaration as it is currently may be
preferable then.

-- 
Darren Hart
VMware Open Source Technology Center
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] headers: Sync with drm-next

2018-09-18 Thread Ayan Kumar Halder
Generated using make headers_install from the drm-next
tree - git://anongit.freedesktop.org/drm/drm
branch - drm-next
commit - 2dc7bad71cd310dc94d1c9907909324dd2b0618f

The changes were as follows :-

  core: (drm.h, drm_fourcc.h, drm_mode.h)
- Added client capabilities for ASPECT_RATIO and WRITEBACK_CONNECTORS
- Added Arm AFBC modifiers
- Added BROADCOM's SAND and UIF modifiers
- Added Qualcomm's modifiers
- Added some picture aspect ratio and content type options
- Added some drm mode flags
- Added writeback connector id

  amdgpu:
- Added GEM domain mask
- Added some GEM flags
- Added some hardware ip flags
- Added chunk id and IB fence.
- Added some query ids

  i915:
-Added an IOCTL (I915_PARAM_MMAP_GTT_COHERENT)

  qxl:
- Minor changes

  radeon:
- Removed RADEON_TILING_R600_NO_SCANOUT

  sis:
- Changed the data type of some structure members from 'int' to 'long'.

  tegra:
- Added some comments about struct drm_tegra* members
- Modified DRM_IOCTL_TEGRA_CLOSE_CHANNEL

  vc4:
- Added some members for 'struct drm_vc4_submit_cl'

  via:
- Removed inclusion of 'via_drmclient.h'.

  vmwgfx:
- Added some DRM_VMW_* macros
- Renamed some structures like 'drm_vmw_dmabuf_rep' to 'drm_vmw_bo_rep', etc
- Added some new DRM_VMW_GB_SURFACE related structures and unions

Signed-off-by: Ayan Kumar halder 

---
Dropped changes to nouveau_drm.h as it causes compilation failure

 include/drm/amdgpu_drm.h |  47 -
 include/drm/drm.h|  16 ++
 include/drm/drm_fourcc.h | 215 +
 include/drm/drm_mode.h   |  35 +++-
 include/drm/i915_drm.h   |  22 +++
 include/drm/qxl_drm.h|   2 -
 include/drm/radeon_drm.h |   1 -
 include/drm/sis_drm.h|   8 +-
 include/drm/tegra_drm.h  | 492 ++-
 include/drm/vc4_drm.h|  13 +-
 include/drm/via_drm.h|   1 -
 include/drm/vmwgfx_drm.h | 166 
 12 files changed, 957 insertions(+), 61 deletions(-)

diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
index c363b67..1ceec56 100644
--- a/include/drm/amdgpu_drm.h
+++ b/include/drm/amdgpu_drm.h
@@ -72,12 +72,41 @@ extern "C" {
 #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
 #define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE + 
DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
 
+/**
+ * DOC: memory domains
+ *
+ * %AMDGPU_GEM_DOMAIN_CPU  System memory that is not GPU accessible.
+ * Memory in this pool could be swapped out to disk if there is pressure.
+ *
+ * %AMDGPU_GEM_DOMAIN_GTT  GPU accessible system memory, mapped into the
+ * GPU's virtual address space via gart. Gart memory linearizes non-contiguous
+ * pages of system memory, allows GPU access system memory in a linezrized
+ * fashion.
+ *
+ * %AMDGPU_GEM_DOMAIN_VRAM Local video memory. For APUs, it is memory
+ * carved out by the BIOS.
+ *
+ * %AMDGPU_GEM_DOMAIN_GDS  Global on-chip data storage used to share data
+ * across shader threads.
+ *
+ * %AMDGPU_GEM_DOMAIN_GWS  Global wave sync, used to synchronize the
+ * execution of all the waves on a device.
+ *
+ * %AMDGPU_GEM_DOMAIN_OA   Ordered append, used by 3D or Compute engines
+ * for appending data.
+ */
 #define AMDGPU_GEM_DOMAIN_CPU  0x1
 #define AMDGPU_GEM_DOMAIN_GTT  0x2
 #define AMDGPU_GEM_DOMAIN_VRAM 0x4
 #define AMDGPU_GEM_DOMAIN_GDS  0x8
 #define AMDGPU_GEM_DOMAIN_GWS  0x10
 #define AMDGPU_GEM_DOMAIN_OA   0x20
+#define AMDGPU_GEM_DOMAIN_MASK (AMDGPU_GEM_DOMAIN_CPU | \
+AMDGPU_GEM_DOMAIN_GTT | \
+AMDGPU_GEM_DOMAIN_VRAM | \
+AMDGPU_GEM_DOMAIN_GDS | \
+AMDGPU_GEM_DOMAIN_GWS | \
+AMDGPU_GEM_DOMAIN_OA)
 
 /* Flag that CPU access will be required for the case of VRAM domain */
 #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED  (1 << 0)
@@ -95,6 +124,10 @@ extern "C" {
 #define AMDGPU_GEM_CREATE_VM_ALWAYS_VALID  (1 << 6)
 /* Flag that BO sharing will be explicitly synchronized */
 #define AMDGPU_GEM_CREATE_EXPLICIT_SYNC(1 << 7)
+/* Flag that indicates allocating MQD gart on GFX9, where the mtype
+ * for the second page onward should be set to NC.
+ */
+#define AMDGPU_GEM_CREATE_MQD_GFX9 (1 << 8)
 
 struct drm_amdgpu_gem_create_in  {
/** the requested memory size */
@@ -473,7 +506,8 @@ struct drm_amdgpu_gem_va {
 #define AMDGPU_HW_IP_UVD_ENC  5
 #define AMDGPU_HW_IP_VCN_DEC  6
 #define AMDGPU_HW_IP_VCN_ENC  7
-#define AMDGPU_HW_IP_NUM  8
+#define AMDGPU_HW_IP_VCN_JPEG 8
+#define AMDGPU_HW_IP_NUM  9
 
 #define AMDGPU_HW_IP_INSTANCE_MAX_COUNT 1
 
@@ -482,6 +516,7 @@ struct drm_amdgpu_gem_va {
 

[PATCH 11/11] drm/amd/display: fix gamma not being applied

2018-09-18 Thread Bhawanpreet Lakha
From: SivapiriyanKumarasamy 

[WHY]
Previously night light forced a full update by
applying a  transfer function update regardless of if it was changed.
This logic was removed,

Now gamma surface updates are only applied when there is also a plane
info update, this does not work in cases such as using the night light
slider.

[HOW]
When moving the night light slider we will perform a full update if
the gamma has changed and there is a surface, even when the surface
has not changed. Also get stream updates in setgamma prior to
update planes and stream.

Change-Id: I6d960892fb981b7b94919c3060b891f7ce1d091c
Signed-off-by: SivapiriyanKumarasamy 
Reviewed-by: Anthony Koo 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 61bb3d52641c..76fe5a9af3bf 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1176,9 +1176,6 @@ static enum surface_update_type 
get_plane_info_update_type(const struct dc_surfa
 */
update_flags->bits.bpp_change = 1;
 
-   if (u->gamma && dce_use_lut(u->plane_info->format))
-   update_flags->bits.gamma_change = 1;
-
if (memcmp(>plane_info->tiling_info, >surface->tiling_info,
sizeof(union dc_tiling_info)) != 0) {
update_flags->bits.swizzle_change = 1;
@@ -1195,7 +1192,6 @@ static enum surface_update_type 
get_plane_info_update_type(const struct dc_surfa
if (update_flags->bits.rotation_change
|| update_flags->bits.stereo_format_change
|| update_flags->bits.pixel_format_change
-   || update_flags->bits.gamma_change
|| update_flags->bits.bpp_change
|| update_flags->bits.bandwidth_change
|| update_flags->bits.output_tf_change)
@@ -1285,13 +1281,26 @@ static enum surface_update_type 
det_surface_update(const struct dc *dc,
if (u->coeff_reduction_factor)
update_flags->bits.coeff_reduction_change = 1;
 
+   if (u->gamma) {
+   enum surface_pixel_format format = 
SURFACE_PIXEL_FORMAT_GRPH_BEGIN;
+
+   if (u->plane_info)
+   format = u->plane_info->format;
+   else if (u->surface)
+   format = u->surface->format;
+
+   if (dce_use_lut(format))
+   update_flags->bits.gamma_change = 1;
+   }
+
if (update_flags->bits.in_transfer_func_change) {
type = UPDATE_TYPE_MED;
elevate_update_type(_type, type);
}
 
if (update_flags->bits.input_csc_change
-   || update_flags->bits.coeff_reduction_change) {
+   || update_flags->bits.coeff_reduction_change
+   || update_flags->bits.gamma_change) {
type = UPDATE_TYPE_FULL;
elevate_update_type(_type, type);
}
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 10/11] drm/amd/display: Remove mst_hotplug_work

2018-09-18 Thread Bhawanpreet Lakha
From: Leo Li 

[Why]
The work struct's schedule call was removed a while ago, making this
useless.

[How]
Remove it.

Change-Id: I22139bcba4275ff679b2e054847e6241f660c8fc
Signed-off-by: Leo Li 
Reviewed-by: David Francis 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 --
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 --
 2 files changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 8f2c61f2ecb7..0f10d920a785 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -338,14 +338,6 @@ static int dm_set_powergating_state(void *handle,
 /* Prototypes of private functions */
 static int dm_early_init(void* handle);
 
-static void hotplug_notify_work_func(struct work_struct *work)
-{
-   struct amdgpu_display_manager *dm = container_of(work, struct 
amdgpu_display_manager, mst_hotplug_work);
-   struct drm_device *dev = dm->ddev;
-
-   drm_kms_helper_hotplug_event(dev);
-}
-
 /* Allocate memory for FBC compressed data  */
 static void amdgpu_dm_fbc_init(struct drm_connector *connector)
 {
@@ -447,8 +439,6 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
goto error;
}
 
-   INIT_WORK(>dm.mst_hotplug_work, hotplug_notify_work_func);
-
adev->dm.freesync_module = mod_freesync_create(adev->dm.dc);
if (!adev->dm.freesync_module) {
DRM_ERROR(
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 7519f9ad77dd..c584a36bf47e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -108,8 +108,6 @@ struct amdgpu_display_manager {
 
const struct dc_link *backlight_link;
 
-   struct work_struct mst_hotplug_work;
-
struct mod_freesync *freesync_module;
 
/**
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 09/11] drm/amd/display: Guard against null stream dereference in do flip

2018-09-18 Thread Bhawanpreet Lakha
From: Nicholas Kazlauskas 

[Why]

During suspend under some hardware configurations can result in a
series of atomic commits with a NULL stream status - which
causes a NULL pointer dereference. This should be guarded.

[How]

Exit early from the function - if we can't access the stream then
there isn't anything that can be done here.

Change-Id: Ie846cce9eba7098b77cac87122c41a2ae69bf905
Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Harry Wentland 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6e9b4b8c9992..8f2c61f2ecb7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4094,6 +4094,7 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc,
/* TODO eliminate or rename surface_update */
struct dc_surface_update surface_updates[1] = { {0} };
struct dm_crtc_state *acrtc_state = to_dm_crtc_state(crtc->state);
+   struct dc_stream_status *stream_status;
 
 
/* Prepare wait for target vblank early - before the fence-waits */
@@ -4149,7 +4150,19 @@ static void amdgpu_dm_do_flip(struct drm_crtc *crtc,
 
spin_unlock_irqrestore(>dev->event_lock, flags);
 
-   surface_updates->surface = 
dc_stream_get_status(acrtc_state->stream)->plane_states[0];
+   stream_status = dc_stream_get_status(acrtc_state->stream);
+   if (!stream_status) {
+   DRM_ERROR("No stream status for CRTC: id=%d\n",
+   acrtc->crtc_id);
+   return;
+   }
+
+   surface_updates->surface = stream_status->plane_states[0];
+   if (!surface_updates->surface) {
+   DRM_ERROR("No surface for CRTC: id=%d\n",
+   acrtc->crtc_id);
+   return;
+   }
surface_updates->flip_addr = 
 
dc_commit_updates_for_stream(adev->dm.dc,
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 08/11] drm/amd/display: Stereo 3D support in VSC

2018-09-18 Thread Bhawanpreet Lakha
From: Eric Bernstein 

[Why]
Need to add strere 3D information in VSC

[How]
Update mod_build_vsc_infopacket with stereo info

Change-Id: Ie99f9f3259e10dcf37a92192cd08e2a65299cf9d
Signed-off-by: Eric Bernstein 
Reviewed-by: Charlene Liu 
Acked-by: Bhawanpreet Lakha 
---
 .../amd/display/modules/info_packet/info_packet.c  | 58 --
 1 file changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c 
b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
index 52378fc69079..ff8bfb9b43b0 100644
--- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
+++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
@@ -48,9 +48,12 @@ static void mod_build_vsc_infopacket(const struct 
dc_stream_state *stream,
unsigned int i;
unsigned int pixelEncoding = 0;
unsigned int colorimetryFormat = 0;
+   bool stereo3dSupport = false;
 
-   if (stream->timing.timing_3d_format != TIMING_3D_FORMAT_NONE && 
stream->view_format != VIEW_3D_FORMAT_NONE)
+   if (stream->timing.timing_3d_format != TIMING_3D_FORMAT_NONE && 
stream->view_format != VIEW_3D_FORMAT_NONE) {
vscPacketRevision = 1;
+   stereo3dSupport = true;
+   }
 
/*VSC packet set to 2 when DP revision >= 1.2*/
if (stream->psr_version != 0)
@@ -94,12 +97,59 @@ static void mod_build_vsc_infopacket(const struct 
dc_stream_state *stream,
info_packet->hb2 = 0x01;// 01h = Revision number. VSC 
SDP supporting 3D stereo only
info_packet->hb3 = 0x01;// 01h = VSC SDP supporting 3D 
stereo only (HB2 = 01h).
 
-   if (stream->timing.timing_3d_format == 
TIMING_3D_FORMAT_INBAND_FA)
-   info_packet->sb[0] = 0x1;
-
info_packet->valid = true;
}
 
+   if (stereo3dSupport) {
+   /* 
==|
+* A. STEREO 3D
+* 
==|
+* VSC Payload (1 byte) From DP1.2 spec
+*
+* Bits 3:0 (Stereo Interface Method Code)  |  Bits 7:4 (Stereo 
Interface Method Specific Parameter)
+* 
-
+* 0 = Non Stereo Video |  Must be set to 
0x0
+* 
-
+* 1 = Frame/Field Sequential   |  0x0: L + R view 
indication based on MISC1 bit 2:1
+*  |  0x1: Right when 
Stereo Signal = 1
+*  |  0x2: Left when 
Stereo Signal = 1
+*  |  (others reserved)
+* 
-
+* 2 = Stacked Frame|  0x0: Left view 
is on top and right view on bottom
+*  |  (others reserved)
+* 
-
+* 3 = Pixel Interleaved|  0x0: horiz 
interleaved, right view pixels on even lines
+*  |  0x1: horiz 
interleaved, right view pixels on odd lines
+*  |  0x2: checker 
board, start with left view pixel
+*  |  0x3: vertical 
interleaved, start with left view pixels
+*  |  0x4: vertical 
interleaved, start with right view pixels
+*  |  (others reserved)
+* 
-
+* 4 = Side-by-side |  0x0: left half 
represents left eye view
+*  |  0x1: left half 
represents right eye view
+*/
+   switch (stream->timing.timing_3d_format) {
+   case TIMING_3D_FORMAT_HW_FRAME_PACKING:
+   case TIMING_3D_FORMAT_SW_FRAME_PACKING:
+   case TIMING_3D_FORMAT_TOP_AND_BOTTOM:
+   case TIMING_3D_FORMAT_TB_SW_PACKED:
+   info_packet->sb[0] = 0x02; // Stacked Frame, Left view 
is on top and right view on bottom.
+   

[PATCH 07/11] drm/amd/display: dc 3.1.67

2018-09-18 Thread Bhawanpreet Lakha
From: Tony Cheng 

Change-Id: Ib8a760749e950259214db6bd11167768058e5975
Signed-off-by: Tony Cheng 
Reviewed-by: Steven Chiu 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/dc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 7691139363a9..11ea2a226952 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -38,7 +38,7 @@
 #include "inc/compressor.h"
 #include "dml/display_mode_lib.h"
 
-#define DC_VER "3.1.66"
+#define DC_VER "3.1.67"
 
 #define MAX_SURFACES 3
 #define MAX_STREAMS 6
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 06/11] drm/amd/display: program v_update and v_ready with proper field

2018-09-18 Thread Bhawanpreet Lakha
From: Su Sung Chung 

[WHY]
There are two different variables used to calculate v_update and v_ready,
one for validation and the other for performance parameter calculation.
Before the variable for validation was used which caused underflow on
1080edp with vsr enabled

[HOW]
program v_update and v_ready with the variables for performance parameter
calculation

Change-Id: I23aaa71289fa6739422fb7c789cf63f2cdbdc6b2
Signed-off-by: Su Sung Chung 
Reviewed-by: Dmytro Laktyushkin 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c |  8 
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 12 ++--
 drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h   |  6 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c 
b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c
index 5e2ea12fbb73..d0fc54f8fb1c 100644
--- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c
+++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c
@@ -1625,11 +1625,11 @@ void 
dispclkdppclkdcfclk_deep_sleep_prefetch_parameters_watermarks_and_performan
else {
v->dsty_after_scaler = 0.0;
}
-   v->v_update_offset_pix 
=dcn_bw_ceil2(v->htotal[k] / 4.0, 1.0);
+   v->v_update_offset_pix[k] = 
dcn_bw_ceil2(v->htotal[k] / 4.0, 1.0);
v->total_repeater_delay_time = 
v->max_inter_dcn_tile_repeaters * (2.0 / v->dppclk + 3.0 / v->dispclk);
-   v->v_update_width_pix = (14.0 / 
v->dcf_clk_deep_sleep + 12.0 / v->dppclk + v->total_repeater_delay_time) * 
v->pixel_clock[k];
-   v->v_ready_offset_pix =dcn_bw_max2(150.0 / 
v->dppclk, v->total_repeater_delay_time + 20.0 / v->dcf_clk_deep_sleep + 10.0 / 
v->dppclk) * v->pixel_clock[k];
-   v->t_setup = (v->v_update_offset_pix + 
v->v_update_width_pix + v->v_ready_offset_pix) / v->pixel_clock[k];
+   v->v_update_width_pix[k] = (14.0 / 
v->dcf_clk_deep_sleep + 12.0 / v->dppclk + v->total_repeater_delay_time) * 
v->pixel_clock[k];
+   v->v_ready_offset_pix[k] = dcn_bw_max2(150.0 / 
v->dppclk, v->total_repeater_delay_time + 20.0 / v->dcf_clk_deep_sleep + 10.0 / 
v->dppclk) * v->pixel_clock[k];
+   v->t_setup = (v->v_update_offset_pix[k] + 
v->v_update_width_pix[k] + v->v_ready_offset_pix[k]) / v->pixel_clock[k];
v->v_startup[k] 
=dcn_bw_min2(v->v_startup_lines, v->max_vstartup_lines[k]);
if (v->prefetch_mode == 0.0) {
v->t_wait 
=dcn_bw_max3(v->dram_clock_change_latency + v->urgent_latency, 
v->sr_enter_plus_exit_time, v->urgent_latency);
diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c 
b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
index 80ec09eef44f..3208188b7ed4 100644
--- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
@@ -1096,9 +1096,9 @@ bool dcn_validate_bandwidth(
if (pipe->top_pipe && pipe->top_pipe->plane_state == 
pipe->plane_state)
continue;
 
-   pipe->pipe_dlg_param.vupdate_width = 
v->v_update_width[input_idx][v->dpp_per_plane[input_idx] == 2 ? 1 : 0];
-   pipe->pipe_dlg_param.vupdate_offset = 
v->v_update_offset[input_idx][v->dpp_per_plane[input_idx] == 2 ? 1 : 0];
-   pipe->pipe_dlg_param.vready_offset = 
v->v_ready_offset[input_idx][v->dpp_per_plane[input_idx] == 2 ? 1 : 0];
+   pipe->pipe_dlg_param.vupdate_width = 
v->v_update_width_pix[input_idx];
+   pipe->pipe_dlg_param.vupdate_offset = 
v->v_update_offset_pix[input_idx];
+   pipe->pipe_dlg_param.vready_offset = 
v->v_ready_offset_pix[input_idx];
pipe->pipe_dlg_param.vstartup_start = 
v->v_startup[input_idx];
 
pipe->pipe_dlg_param.htotal = 
pipe->stream->timing.h_total;
@@ -1137,9 +1137,9 @@ bool dcn_validate_bandwidth(
 TIMING_3D_FORMAT_SIDE_BY_SIDE))) {
if (hsplit_pipe && 
hsplit_pipe->plane_state == pipe->plane_state) {
/* update previously split pipe 
*/
-   
hsplit_pipe->pipe_dlg_param.vupdate_width = 
v->v_update_width[input_idx][v->dpp_per_plane[input_idx] == 2 ? 1 : 0];
-   
hsplit_pipe->pipe_dlg_param.vupdate_offset = 
v->v_update_offset[input_idx][v->dpp_per_plane[input_idx] == 2 ? 1 : 0];
-   

[PATCH 05/11] drm/amd/display: Add color bit info to freesync infoframe

2018-09-18 Thread Bhawanpreet Lakha
From: SivapiriyanKumarasamy 

Parse the native color bit and send it to freesync module for future
use

Change-Id: I75e35194dd1aca45349b6274f85e9cd7472c2363
Signed-off-by: SivapiriyanKumarasamy 
Reviewed-by: Anthony Koo 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |   2 +
 .../drm/amd/display/modules/freesync/freesync.c| 164 ++---
 .../gpu/drm/amd/display/modules/inc/mod_freesync.h |   4 +-
 .../gpu/drm/amd/display/modules/inc/mod_shared.h   |  49 ++
 4 files changed, 198 insertions(+), 21 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/display/modules/inc/mod_shared.h

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index bcc5bf7e9857..6e9b4b8c9992 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4812,6 +4812,8 @@ void set_freesync_on_stream(struct amdgpu_display_manager 
*dm,
mod_freesync_build_vrr_infopacket(dm->freesync_module,
  new_stream,
  ,
+ packet_type_fs1,
+ NULL,
  _infopacket);
 
new_crtc_state->adjust = vrr.adjust;
diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c 
b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index e1688902a1b0..4018c7180d00 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -480,22 +480,11 @@ bool mod_freesync_get_v_position(struct mod_freesync 
*mod_freesync,
return false;
 }
 
-void mod_freesync_build_vrr_infopacket(struct mod_freesync *mod_freesync,
-   const struct dc_stream_state *stream,
-   const struct mod_vrr_params *vrr,
-   struct dc_info_packet *infopacket)
+static void build_vrr_infopacket_header_v1(enum signal_type signal,
+   struct dc_info_packet *infopacket,
+   unsigned int *payload_size)
 {
-   /* SPD info packet for FreeSync */
-   unsigned char checksum = 0;
-   unsigned int idx, payload_size = 0;
-
-   /* Check if Freesync is supported. Return if false. If true,
-* set the corresponding bit in the info packet
-*/
-   if (!vrr->supported || !vrr->send_vsif)
-   return;
-
-   if (dc_is_hdmi_signal(stream->signal)) {
+   if (dc_is_hdmi_signal(signal)) {
 
/* HEADER */
 
@@ -510,9 +499,9 @@ void mod_freesync_build_vrr_infopacket(struct mod_freesync 
*mod_freesync,
/* HB2  = [Bits 7:5 = 0] [Bits 4:0 = Length = 0x08] */
infopacket->hb2 = 0x08;
 
-   payload_size = 0x08;
+   *payload_size = 0x08;
 
-   } else if (dc_is_dp_signal(stream->signal)) {
+   } else if (dc_is_dp_signal(signal)) {
 
/* HEADER */
 
@@ -536,9 +525,62 @@ void mod_freesync_build_vrr_infopacket(struct mod_freesync 
*mod_freesync,
 */
infopacket->hb3 = 0x04;
 
-   payload_size = 0x1B;
+   *payload_size = 0x1B;
}
+}
+
+static void build_vrr_infopacket_header_v2(enum signal_type signal,
+   struct dc_info_packet *infopacket,
+   unsigned int *payload_size)
+{
+   if (dc_is_hdmi_signal(signal)) {
+
+   /* HEADER */
+
+   /* HB0  = Packet Type = 0x83 (Source Product
+*Descriptor InfoFrame)
+*/
+   infopacket->hb0 = DC_HDMI_INFOFRAME_TYPE_SPD;
+
+   /* HB1  = Version = 0x02 */
+   infopacket->hb1 = 0x02;
+
+   /* HB2  = [Bits 7:5 = 0] [Bits 4:0 = Length = 0x09] */
+   infopacket->hb2 = 0x09;
+
+   *payload_size = 0x0A;
 
+   } else if (dc_is_dp_signal(signal)) {
+
+   /* HEADER */
+
+   /* HB0  = Secondary-data Packet ID = 0 - Only non-zero
+*when used to associate audio related info packets
+*/
+   infopacket->hb0 = 0x00;
+
+   /* HB1  = Packet Type = 0x83 (Source Product
+*Descriptor InfoFrame)
+*/
+   infopacket->hb1 = DC_HDMI_INFOFRAME_TYPE_SPD;
+
+   /* HB2  = [Bits 7:0 = Least significant eight bits -
+*For INFOFRAME, the value must be 1Bh]
+*/
+   infopacket->hb2 = 0x1B;
+
+   /* HB3  = [Bits 7:2 = INFOFRAME SDP Version Number = 0x2]
+*[Bits 1:0 = Most significant two bits = 0x00]
+*/
+   infopacket->hb3 = 0x08;
+
+   *payload_size = 0x1B;
+   }
+}
+
+static void build_vrr_infopacket_data(const struct mod_vrr_params *vrr,
+   

[PATCH 04/11] drm/amd/display: add pp_smu NULL pointer check

2018-09-18 Thread Bhawanpreet Lakha
From: Charlene Liu 

add pp_smu NULL ptr check

Change-Id: Ib810078b1f4ea29d48e197539f3c24bfe7085b75
Signed-off-by: Charlene Liu 
Reviewed-by: Dmytro Laktyushkin 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 7d9be87140cc..61bb3d52641c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1395,7 +1395,7 @@ static void notify_display_count_to_smu(
 * sent as part of pplib_apply_display_requirements.
 * So just return.
 */
-   if (!pp_smu->set_display_count)
+   if (!pp_smu || !pp_smu->set_display_count)
return;
 
display_count = 0;
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 02/11] drm/amd/display: Refactor FPGA-specific link setup

2018-09-18 Thread Bhawanpreet Lakha
From: Nikola Cornij 

FPGA doesn't program backend, so we don't need certain link settings
(audio stream for example).

Change-Id: I12ebdbefb23a0c53a6c7edc749a5e47fbdbf68a6
Signed-off-by: Nikola Cornij 
Reviewed-by: Tony Cheng 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c  | 56 --
 .../amd/display/dc/dce110/dce110_hw_sequencer.c| 15 +-
 .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c  |  1 +
 3 files changed, 32 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index bd58dbae7d3e..9f9503a9b9aa 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -2559,23 +2559,24 @@ void core_link_enable_stream(
pipe_ctx->stream_res.stream_enc,
>timing);
 
-   resource_build_info_frame(pipe_ctx);
-   core_dc->hwss.update_info_frame(pipe_ctx);
-
-   /* eDP lit up by bios already, no need to enable again. */
-   if (pipe_ctx->stream->signal == SIGNAL_TYPE_EDP &&
-   pipe_ctx->stream->apply_edp_fast_boot_optimization) {
-   pipe_ctx->stream->apply_edp_fast_boot_optimization = false;
-   pipe_ctx->stream->dpms_off = false;
-   return;
-   }
+   if (!IS_FPGA_MAXIMUS_DC(core_dc->ctx->dce_environment)) {
+   resource_build_info_frame(pipe_ctx);
+   core_dc->hwss.update_info_frame(pipe_ctx);
+
+   /* eDP lit up by bios already, no need to enable again. */
+   if (pipe_ctx->stream->signal == SIGNAL_TYPE_EDP &&
+   
pipe_ctx->stream->apply_edp_fast_boot_optimization) {
+   pipe_ctx->stream->apply_edp_fast_boot_optimization = 
false;
+   pipe_ctx->stream->dpms_off = false;
+   return;
+   }
 
-   if (pipe_ctx->stream->dpms_off)
-   return;
+   if (pipe_ctx->stream->dpms_off)
+   return;
 
-   status = enable_link(state, pipe_ctx);
+   status = enable_link(state, pipe_ctx);
 
-   if (status != DC_OK) {
+   if (status != DC_OK) {
DC_LOG_WARNING("enabling link %u failed: %d\n",
pipe_ctx->stream->sink->link->link_index,
status);
@@ -2590,23 +2591,26 @@ void core_link_enable_stream(
BREAK_TO_DEBUGGER();
return;
}
-   }
+   }
 
-   core_dc->hwss.enable_audio_stream(pipe_ctx);
+   core_dc->hwss.enable_audio_stream(pipe_ctx);
 
-   /* turn off otg test pattern if enable */
-   if (pipe_ctx->stream_res.tg->funcs->set_test_pattern)
-   
pipe_ctx->stream_res.tg->funcs->set_test_pattern(pipe_ctx->stream_res.tg,
-   CONTROLLER_DP_TEST_PATTERN_VIDEOMODE,
-   COLOR_DEPTH_UNDEFINED);
+   /* turn off otg test pattern if enable */
+   if (pipe_ctx->stream_res.tg->funcs->set_test_pattern)
+   
pipe_ctx->stream_res.tg->funcs->set_test_pattern(pipe_ctx->stream_res.tg,
+   CONTROLLER_DP_TEST_PATTERN_VIDEOMODE,
+   COLOR_DEPTH_UNDEFINED);
 
-   core_dc->hwss.enable_stream(pipe_ctx);
+   core_dc->hwss.enable_stream(pipe_ctx);
 
-   if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
-   allocate_mst_payload(pipe_ctx);
+   if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
+   allocate_mst_payload(pipe_ctx);
+
+   core_dc->hwss.unblank_stream(pipe_ctx,
+   _ctx->stream->sink->link->cur_link_settings);
+
+   }
 
-   core_dc->hwss.unblank_stream(pipe_ctx,
-   _ctx->stream->sink->link->cur_link_settings);
 }
 
 void core_link_disable_stream(struct pipe_ctx *pipe_ctx, int option)
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index dc1eed5ba996..6b7486d8 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1377,26 +1377,13 @@ static enum dc_status apply_single_controller_ctx_to_hw(
/*  */
dc->hwss.enable_stream_timing(pipe_ctx, context, dc);
 
-   /* FPGA does not program backend */
-   if (IS_FPGA_MAXIMUS_DC(dc->ctx->dce_environment)) {
-   pipe_ctx->stream_res.opp->funcs->opp_set_dyn_expansion(
-   pipe_ctx->stream_res.opp,
-   COLOR_SPACE_YCBCR601,
-   stream->timing.display_color_depth,
-   pipe_ctx->stream->signal);
-

[PATCH 03/11] drm/amd/display: use proper pipe_ctx index

2018-09-18 Thread Bhawanpreet Lakha
From: Samson Tam 

Use link->link_index as index to pipe_ctx[] to get proper link
information instead of using index 0 to avoid potential miss matches.

Change-Id: If3f8c5f1e02396949d0a0a0d2e14400ecd52af87
Signed-off-by: Samson Tam 
Reviewed-by: Anthony Koo 
Acked-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 1c438eedf77a..7d9be87140cc 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -460,9 +460,25 @@ void dc_link_set_preferred_link_settings(struct dc *dc,
 struct dc_link_settings *link_setting,
 struct dc_link *link)
 {
+   int i;
+   struct pipe_ctx *pipe;
+   struct dc_stream_state *link_stream;
struct dc_link_settings store_settings = *link_setting;
-   struct dc_stream_state *link_stream =
-   link->dc->current_state->res_ctx.pipe_ctx[0].stream;
+
+   for (i = 0; i < MAX_PIPES; i++) {
+   pipe = >current_state->res_ctx.pipe_ctx[i];
+   if (pipe->stream && pipe->stream->sink
+   && pipe->stream->sink->link) {
+   if (pipe->stream->sink->link == link)
+   break;
+   }
+   }
+
+   /* Stream not found */
+   if (i == MAX_PIPES)
+   return;
+
+   link_stream = link->dc->current_state->res_ctx.pipe_ctx[i].stream;
 
link->preferred_link_setting = store_settings;
if (link_stream)
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 01/11] drm/amdgpu: move reserving GDS/GWS/OA into common code

2018-09-18 Thread Bhawanpreet Lakha
From: Christian König 

We don't need that in the per ASIC code.

Signed-off-by: Christian König 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 18 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   | 19 ---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 19 ---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 19 ---
 4 files changed, 18 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d83f4e265c5c..a44fc12ae1f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1852,6 +1852,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
return r;
}
 
+   r = amdgpu_bo_create_kernel(adev, adev->gds.mem.gfx_partition_size,
+   PAGE_SIZE, AMDGPU_GEM_DOMAIN_GDS,
+   >gds.gds_gfx_bo, NULL, NULL);
+   if (r)
+   return r;
+
r = ttm_bo_init_mm(>mman.bdev, AMDGPU_PL_GWS,
   adev->gds.gws.total_size);
if (r) {
@@ -1859,6 +1865,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
return r;
}
 
+   r = amdgpu_bo_create_kernel(adev, adev->gds.gws.gfx_partition_size,
+   PAGE_SIZE, AMDGPU_GEM_DOMAIN_GWS,
+   >gds.gws_gfx_bo, NULL, NULL);
+   if (r)
+   return r;
+
r = ttm_bo_init_mm(>mman.bdev, AMDGPU_PL_OA,
   adev->gds.oa.total_size);
if (r) {
@@ -1866,6 +1878,12 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
return r;
}
 
+   r = amdgpu_bo_create_kernel(adev, adev->gds.oa.gfx_partition_size,
+   PAGE_SIZE, AMDGPU_GEM_DOMAIN_OA,
+   >gds.oa_gfx_bo, NULL, NULL);
+   if (r)
+   return r;
+
/* Register debugfs entries for amdgpu_ttm */
r = amdgpu_ttm_debugfs_init(adev);
if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index c0f9732cbaf7..fc39ebbc9d9f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -4582,25 +4582,6 @@ static int gfx_v7_0_sw_init(void *handle)
}
}
 
-   /* reserve GDS, GWS and OA resource for gfx */
-   r = amdgpu_bo_create_kernel(adev, adev->gds.mem.gfx_partition_size,
-   PAGE_SIZE, AMDGPU_GEM_DOMAIN_GDS,
-   >gds.gds_gfx_bo, NULL, NULL);
-   if (r)
-   return r;
-
-   r = amdgpu_bo_create_kernel(adev, adev->gds.gws.gfx_partition_size,
-   PAGE_SIZE, AMDGPU_GEM_DOMAIN_GWS,
-   >gds.gws_gfx_bo, NULL, NULL);
-   if (r)
-   return r;
-
-   r = amdgpu_bo_create_kernel(adev, adev->gds.oa.gfx_partition_size,
-   PAGE_SIZE, AMDGPU_GEM_DOMAIN_OA,
-   >gds.oa_gfx_bo, NULL, NULL);
-   if (r)
-   return r;
-
adev->gfx.ce_ram_size = 0x8000;
 
gfx_v7_0_gpu_early_init(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 96df23c99cfb..470dc80f4fe7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -2161,25 +2161,6 @@ static int gfx_v8_0_sw_init(void *handle)
if (r)
return r;
 
-   /* reserve GDS, GWS and OA resource for gfx */
-   r = amdgpu_bo_create_kernel(adev, adev->gds.mem.gfx_partition_size,
-   PAGE_SIZE, AMDGPU_GEM_DOMAIN_GDS,
-   >gds.gds_gfx_bo, NULL, NULL);
-   if (r)
-   return r;
-
-   r = amdgpu_bo_create_kernel(adev, adev->gds.gws.gfx_partition_size,
-   PAGE_SIZE, AMDGPU_GEM_DOMAIN_GWS,
-   >gds.gws_gfx_bo, NULL, NULL);
-   if (r)
-   return r;
-
-   r = amdgpu_bo_create_kernel(adev, adev->gds.oa.gfx_partition_size,
-   PAGE_SIZE, AMDGPU_GEM_DOMAIN_OA,
-   >gds.oa_gfx_bo, NULL, NULL);
-   if (r)
-   return r;
-
adev->gfx.ce_ram_size = 0x8000;
 
r = gfx_v8_0_gpu_early_init(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 528a8a567633..f369d9603435 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1700,25 +1700,6 @@ static int gfx_v9_0_sw_init(void *handle)
if (r)
return r;
 
-   /* reserve GDS, GWS and OA resource for gfx */
-   r = amdgpu_bo_create_kernel(adev, 

[PATCH 00/11] DC Patches Sep 18, 2018

2018-09-18 Thread Bhawanpreet Lakha
*Summary of Changes
*Add Stereo 3D support in VSC
*Add Null pointers in flip and pp_smu  
*Fix gamma not being applied


Charlene Liu (1):
  drm/amd/display: add pp_smu NULL pointer check

Christian König (1):
  drm/amdgpu: move reserving GDS/GWS/OA into common code

Eric Bernstein (1):
  drm/amd/display: Stereo 3D support in VSC

Leo Li (1):
  drm/amd/display: Remove mst_hotplug_work

Nicholas Kazlauskas (1):
  drm/amd/display: Guard against null stream dereference in do flip

Nikola Cornij (1):
  drm/amd/display: Refactor FPGA-specific link setup

Samson Tam (1):
  drm/amd/display: use proper pipe_ctx index

SivapiriyanKumarasamy (2):
  drm/amd/display: Add color bit info to freesync infoframe
  drm/amd/display: fix gamma not being applied

Su Sung Chung (1):
  drm/amd/display: program v_update and v_ready with proper field

Tony Cheng (1):
  drm/amd/display: dc 3.1.67

 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  18 +++
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  19 ---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  |  19 ---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  |  19 ---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  27 ++--
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |   2 -
 .../gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c   |   8 +-
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c   |  12 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c   |  41 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c  |  56 +++
 drivers/gpu/drm/amd/display/dc/dc.h|   2 +-
 .../amd/display/dc/dce110/dce110_hw_sequencer.c|  15 +-
 .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c  |   1 +
 drivers/gpu/drm/amd/display/dc/inc/dcn_calcs.h |   6 +-
 .../drm/amd/display/modules/freesync/freesync.c| 164 ++---
 .../gpu/drm/amd/display/modules/inc/mod_freesync.h |   4 +-
 .../gpu/drm/amd/display/modules/inc/mod_shared.h   |  49 ++
 .../amd/display/modules/info_packet/info_packet.c  |  58 +++-
 18 files changed, 363 insertions(+), 157 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/display/modules/inc/mod_shared.h

-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Christian König

Tom,

can you try if the following makes it working again?

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c

index b6160de70d12..d65f5ba92fc5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -937,6 +937,10 @@ static int gfx_v8_0_ring_test_ib(struct amdgpu_ring 
*ring, long timeout)

    return r;
 }

+static int gfx_v8_0_kiq_ring_test_ib(struct amdgpu_ring *ring, long 
timeout)

+{
+   return 0;
+}

 static void gfx_v8_0_free_microcode(struct amdgpu_device *adev)
 {
@@ -7174,7 +7178,7 @@ static const struct amdgpu_ring_funcs 
gfx_v8_0_ring_funcs_kiq = {

    .emit_ib = gfx_v8_0_ring_emit_ib_compute,
    .emit_fence = gfx_v8_0_ring_emit_fence_kiq,
    .test_ring = gfx_v8_0_ring_test_ring,
-   .test_ib = gfx_v8_0_ring_test_ib,
+   .test_ib = gfx_v8_0_kiq_ring_test_ib,
    .insert_nop = amdgpu_ring_insert_nop,
    .pad_ib = amdgpu_ring_generic_pad_ib,
    .emit_rreg = gfx_v8_0_ring_emit_rreg,


Thanks,
Christian.

Am 18.09.2018 um 16:41 schrieb Christian König:

CRTC and GFX interrupts seem to be working perfectly fine.

The problem here looks like only EOP interrupts from the Compute queue 
are not correctly handled.


Most likely a bug somewhere in gfx_v8_0_eop_irq().

Christian.

Am 18.09.2018 um 16:36 schrieb Deucher, Alexander:


FWIW, a number of consumer Raven boards have bad IVRS tables (windows 
doesn't use interrupt remapping so they are sometimes wrong and 
probably not validated.  There are a number of workaround to manually 
override the IVRS tables to make interrupts work. I think specifying 
pci=noacpi is also a possible workaround.



Alex


*From:* amd-gfx  on behalf of 
Christian König 

*Sent:* Tuesday, September 18, 2018 10:31:16 AM
*To:* StDenis, Tom; amd-gfx mailing list; Zhou, David(ChunMing)
*Subject:* Re: Regression on gfx8 with ring init
Well looks like interrupt processing is working perfectly fine.

But looking at the error message once more I see that this actually
affects ring number 9 and not the GFX ring.

Can you fix amdgpu_ib_ring_tests() to print ring->name instead of the
number?

That must be some of the compute rings.

Thanks,
Christian.

Am 18.09.2018 um 16:20 schrieb Tom St Denis:
> On 2018-09-18 10:13 a.m., Christian König wrote:
>> Mhm, there is no more failed IB-test in there isn't it?
>
> oh sorry I thought you wanted to test HEAD~ ... Attached is a log from
> the tip of drm-next
>
> Tom
>
>>
>> Christian.
>>
>> Am 18.09.2018 um 16:09 schrieb Tom St Denis:
>>> Disabling IOMMU in the BIOS resulted in a correct boot up...
>>>
>>> Here's the log.
>>>
>>> Tom
>>>
>>> On 2018-09-18 9:58 a.m., Tom St Denis wrote:
 Odd I couldn't even boot my system with the dGPU as primary after
 rebuilding the kernel.  It got hung up in the IOMMU driver (loads
 of AMD-Vi IOMMU errors) which I wasn't able to capture because it
 panic'ed before loading the network stack.

 Bizarre.

 I'll keep trying.

 Tom

 On 2018-09-18 9:35 a.m., Christian König wrote:
> Am 18.09.2018 um 15:32 schrieb Tom St Denis:
>> On 2018-09-18 9:30 a.m., Christian König wrote:
>>> Great, not sure if that is a good or a bad news.
>>>
>>> Anyway going to revert the change for now. Does anybody
>>> volunteer to figure out why interrupts sometimes doesn't work
>>> correctly on Raven?
>>
>> What does "doesn't work correctly?"  My workstation is a Raven1
>> (Ryzen 2400G) and other than the TTM bulk move issue has been
>> perfectly stable (through suspend/resumes too I might add).
>>
>> Anything I could test with my devel raven?
>
> The problem seems to be that on some boards IH handling doesn't
> work as it should.
>
> Can you try to disable the onboard graphics and try again?
>
> If that still doesn't work there is a DRM_DEBUG in
> amdgpu_ih_process(), make that a DRM_ERROR and send me the
> resulting dmesg of loading amdgpu (but don't start any UMD).
>
> Thanks,
> Christian.
>
>>
>>
>> Tom
>>
>>>
>>> Christian.
>>>
>>> Am 18.09.2018 um 15:27 schrieb Tom St Denis:
 This commit:

 [root@raven linux]# git bisect good
 9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
 commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
 Author: Christian König 
 Date:   Tue Sep 18 10:38:09 2018 +0200

     drm/amdgpu: remove fence fallback

     DC doesn't seem to have a fallback path either.

     So when interrupts doesn't work any more we are pretty much
 busted no
     matter what.

     Signed-off-by: Christian König 
     Reviewed-by: Chunming Zhou 

 Results in this:


Re: Regression on gfx8 with ring init

2018-09-18 Thread Christian König

CRTC and GFX interrupts seem to be working perfectly fine.

The problem here looks like only EOP interrupts from the Compute queue 
are not correctly handled.


Most likely a bug somewhere in gfx_v8_0_eop_irq().

Christian.

Am 18.09.2018 um 16:36 schrieb Deucher, Alexander:


FWIW, a number of consumer Raven boards have bad IVRS tables (windows 
doesn't use interrupt remapping so they are sometimes wrong and 
probably not validated.  There are a number of workaround to manually 
override the IVRS tables to make interrupts work.  I think specifying 
pci=noacpi is also a possible workaround.



Alex


*From:* amd-gfx  on behalf of 
Christian König 

*Sent:* Tuesday, September 18, 2018 10:31:16 AM
*To:* StDenis, Tom; amd-gfx mailing list; Zhou, David(ChunMing)
*Subject:* Re: Regression on gfx8 with ring init
Well looks like interrupt processing is working perfectly fine.

But looking at the error message once more I see that this actually
affects ring number 9 and not the GFX ring.

Can you fix amdgpu_ib_ring_tests() to print ring->name instead of the
number?

That must be some of the compute rings.

Thanks,
Christian.

Am 18.09.2018 um 16:20 schrieb Tom St Denis:
> On 2018-09-18 10:13 a.m., Christian König wrote:
>> Mhm, there is no more failed IB-test in there isn't it?
>
> oh sorry I thought you wanted to test HEAD~ ... Attached is a log from
> the tip of drm-next
>
> Tom
>
>>
>> Christian.
>>
>> Am 18.09.2018 um 16:09 schrieb Tom St Denis:
>>> Disabling IOMMU in the BIOS resulted in a correct boot up...
>>>
>>> Here's the log.
>>>
>>> Tom
>>>
>>> On 2018-09-18 9:58 a.m., Tom St Denis wrote:
 Odd I couldn't even boot my system with the dGPU as primary after
 rebuilding the kernel.  It got hung up in the IOMMU driver (loads
 of AMD-Vi IOMMU errors) which I wasn't able to capture because it
 panic'ed before loading the network stack.

 Bizarre.

 I'll keep trying.

 Tom

 On 2018-09-18 9:35 a.m., Christian König wrote:
> Am 18.09.2018 um 15:32 schrieb Tom St Denis:
>> On 2018-09-18 9:30 a.m., Christian König wrote:
>>> Great, not sure if that is a good or a bad news.
>>>
>>> Anyway going to revert the change for now. Does anybody
>>> volunteer to figure out why interrupts sometimes doesn't work
>>> correctly on Raven?
>>
>> What does "doesn't work correctly?"  My workstation is a Raven1
>> (Ryzen 2400G) and other than the TTM bulk move issue has been
>> perfectly stable (through suspend/resumes too I might add).
>>
>> Anything I could test with my devel raven?
>
> The problem seems to be that on some boards IH handling doesn't
> work as it should.
>
> Can you try to disable the onboard graphics and try again?
>
> If that still doesn't work there is a DRM_DEBUG in
> amdgpu_ih_process(), make that a DRM_ERROR and send me the
> resulting dmesg of loading amdgpu (but don't start any UMD).
>
> Thanks,
> Christian.
>
>>
>>
>> Tom
>>
>>>
>>> Christian.
>>>
>>> Am 18.09.2018 um 15:27 schrieb Tom St Denis:
 This commit:

 [root@raven linux]# git bisect good
 9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
 commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
 Author: Christian König 
 Date:   Tue Sep 18 10:38:09 2018 +0200

     drm/amdgpu: remove fence fallback

     DC doesn't seem to have a fallback path either.

     So when interrupts doesn't work any more we are pretty much
 busted no
     matter what.

     Signed-off-by: Christian König 
     Reviewed-by: Chunming Zhou 

 Results in this:

 [   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for
 :07:00.0 on minor 1
 [   24.335674] modprobe (3895) used greatest stack depth: 12600
 bytes left
 [   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR*
 amdgpu: IB test timed out.
 [   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR*
 amdgpu: failed testing IB on ring 9 (-110).
 [   26.407885] [drm:process_one_work] *ERROR* ib ring test
 failed (-110).
 [   28.506708] fuse init (API version 7.27)

 On init with my polaris/raven1 system.

 Cheers,
 Tom
 ___
 amd-gfx mailing list
 amd-gfx@lists.freedesktop.org
 https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>
>

>>>
>>
>

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing 

Re: Regression on gfx8 with ring init

2018-09-18 Thread Tom St Denis

On 2018-09-18 10:31 a.m., Christian König wrote:

Well looks like interrupt processing is working perfectly fine.

But looking at the error message once more I see that this actually 
affects ring number 9 and not the GFX ring.


Can you fix amdgpu_ib_ring_tests() to print ring->name instead of the 
number?


That must be some of the compute rings.


That's a bingo.

[   32.231734] [drm] Initialized amdgpu 3.27.0 20150101 for :01:00.0 
on minor 0

[   32.233803] modprobe (3816) used greatest stack depth: 12464 bytes left
[   35.266007] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* amdgpu: IB 
test timed out.
[   35.266373] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* amdgpu: 
failed testing IB on ring (kiq_2.1.0) 9 (-110).

[   35.403034] [drm:process_one_work] *ERROR* ib ring test failed (-110).

Should point out that kfd still has the old fence logic:

[root@raven amd]# git grep enable_signaling
amdgpu/amdgpu_amdkfd_fence.c: *  nofity when the BO is free to move. 
fence_add_callback --> enable_signaling

amdgpu/amdgpu_amdkfd_fence.c: *  --> amdgpu_amdkfd_fence.enable_signaling
amdgpu/amdgpu_amdkfd_fence.c: * amdgpu_amdkfd_fence.enable_signaling - 
Start a work item that will quiesce
amdgpu/amdgpu_amdkfd_fence.c: * amdkfd_fence_enable_signaling - This 
gets called when TTM wants to evict
amdgpu/amdgpu_amdkfd_fence.c:static bool 
amdkfd_fence_enable_signaling(struct dma_fence *f)
amdgpu/amdgpu_amdkfd_fence.c:   .enable_signaling = 
amdkfd_fence_enable_signaling,



Tom



Thanks,
Christian.

Am 18.09.2018 um 16:20 schrieb Tom St Denis:

On 2018-09-18 10:13 a.m., Christian König wrote:

Mhm, there is no more failed IB-test in there isn't it?


oh sorry I thought you wanted to test HEAD~ ... Attached is a log from 
the tip of drm-next


Tom



Christian.

Am 18.09.2018 um 16:09 schrieb Tom St Denis:

Disabling IOMMU in the BIOS resulted in a correct boot up...

Here's the log.

Tom

On 2018-09-18 9:58 a.m., Tom St Denis wrote:
Odd I couldn't even boot my system with the dGPU as primary after 
rebuilding the kernel.  It got hung up in the IOMMU driver (loads 
of AMD-Vi IOMMU errors) which I wasn't able to capture because it 
panic'ed before loading the network stack.


Bizarre.

I'll keep trying.

Tom

On 2018-09-18 9:35 a.m., Christian König wrote:

Am 18.09.2018 um 15:32 schrieb Tom St Denis:

On 2018-09-18 9:30 a.m., Christian König wrote:

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody 
volunteer to figure out why interrupts sometimes doesn't work 
correctly on Raven?


What does "doesn't work correctly?"  My workstation is a Raven1 
(Ryzen 2400G) and other than the TTM bulk move issue has been 
perfectly stable (through suspend/resumes too I might add).


Anything I could test with my devel raven?


The problem seems to be that on some boards IH handling doesn't 
work as it should.


Can you try to disable the onboard graphics and try again?

If that still doesn't work there is a DRM_DEBUG in 
amdgpu_ih_process(), make that a DRM_ERROR and send me the 
resulting dmesg of loading amdgpu (but don't start any UMD).


Thanks,
Christian.




Tom



Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much 
busted no

    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 
bytes left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* 
amdgpu: IB test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* 
amdgpu: failed testing IB on ring 9 (-110).
[   26.407885] [drm:process_one_work] *ERROR* ib ring test 
failed (-110).

[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


















___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Deucher, Alexander
FWIW, a number of consumer Raven boards have bad IVRS tables (windows doesn't 
use interrupt remapping so they are sometimes wrong and probably not validated. 
 There are a number of workaround to manually override the IVRS tables to make 
interrupts work.  I think specifying pci=noacpi is also a possible workaround.


Alex


From: amd-gfx  on behalf of Christian 
König 
Sent: Tuesday, September 18, 2018 10:31:16 AM
To: StDenis, Tom; amd-gfx mailing list; Zhou, David(ChunMing)
Subject: Re: Regression on gfx8 with ring init

Well looks like interrupt processing is working perfectly fine.

But looking at the error message once more I see that this actually
affects ring number 9 and not the GFX ring.

Can you fix amdgpu_ib_ring_tests() to print ring->name instead of the
number?

That must be some of the compute rings.

Thanks,
Christian.

Am 18.09.2018 um 16:20 schrieb Tom St Denis:
> On 2018-09-18 10:13 a.m., Christian König wrote:
>> Mhm, there is no more failed IB-test in there isn't it?
>
> oh sorry I thought you wanted to test HEAD~ ... Attached is a log from
> the tip of drm-next
>
> Tom
>
>>
>> Christian.
>>
>> Am 18.09.2018 um 16:09 schrieb Tom St Denis:
>>> Disabling IOMMU in the BIOS resulted in a correct boot up...
>>>
>>> Here's the log.
>>>
>>> Tom
>>>
>>> On 2018-09-18 9:58 a.m., Tom St Denis wrote:
 Odd I couldn't even boot my system with the dGPU as primary after
 rebuilding the kernel.  It got hung up in the IOMMU driver (loads
 of AMD-Vi IOMMU errors) which I wasn't able to capture because it
 panic'ed before loading the network stack.

 Bizarre.

 I'll keep trying.

 Tom

 On 2018-09-18 9:35 a.m., Christian König wrote:
> Am 18.09.2018 um 15:32 schrieb Tom St Denis:
>> On 2018-09-18 9:30 a.m., Christian König wrote:
>>> Great, not sure if that is a good or a bad news.
>>>
>>> Anyway going to revert the change for now. Does anybody
>>> volunteer to figure out why interrupts sometimes doesn't work
>>> correctly on Raven?
>>
>> What does "doesn't work correctly?"  My workstation is a Raven1
>> (Ryzen 2400G) and other than the TTM bulk move issue has been
>> perfectly stable (through suspend/resumes too I might add).
>>
>> Anything I could test with my devel raven?
>
> The problem seems to be that on some boards IH handling doesn't
> work as it should.
>
> Can you try to disable the onboard graphics and try again?
>
> If that still doesn't work there is a DRM_DEBUG in
> amdgpu_ih_process(), make that a DRM_ERROR and send me the
> resulting dmesg of loading amdgpu (but don't start any UMD).
>
> Thanks,
> Christian.
>
>>
>>
>> Tom
>>
>>>
>>> Christian.
>>>
>>> Am 18.09.2018 um 15:27 schrieb Tom St Denis:
 This commit:

 [root@raven linux]# git bisect good
 9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
 commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
 Author: Christian König 
 Date:   Tue Sep 18 10:38:09 2018 +0200

 drm/amdgpu: remove fence fallback

 DC doesn't seem to have a fallback path either.

 So when interrupts doesn't work any more we are pretty much
 busted no
 matter what.

 Signed-off-by: Christian König 
 Reviewed-by: Chunming Zhou 

 Results in this:

 [   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for
 :07:00.0 on minor 1
 [   24.335674] modprobe (3895) used greatest stack depth: 12600
 bytes left
 [   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR*
 amdgpu: IB test timed out.
 [   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR*
 amdgpu: failed testing IB on ring 9 (-110).
 [   26.407885] [drm:process_one_work] *ERROR* ib ring test
 failed (-110).
 [   28.506708] fuse init (API version 7.27)

 On init with my polaris/raven1 system.

 Cheers,
 Tom
 ___
 amd-gfx mailing list
 amd-gfx@lists.freedesktop.org
 https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>
>

>>>
>>
>

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Christian König

Well looks like interrupt processing is working perfectly fine.

But looking at the error message once more I see that this actually 
affects ring number 9 and not the GFX ring.


Can you fix amdgpu_ib_ring_tests() to print ring->name instead of the 
number?


That must be some of the compute rings.

Thanks,
Christian.

Am 18.09.2018 um 16:20 schrieb Tom St Denis:

On 2018-09-18 10:13 a.m., Christian König wrote:

Mhm, there is no more failed IB-test in there isn't it?


oh sorry I thought you wanted to test HEAD~ ... Attached is a log from 
the tip of drm-next


Tom



Christian.

Am 18.09.2018 um 16:09 schrieb Tom St Denis:

Disabling IOMMU in the BIOS resulted in a correct boot up...

Here's the log.

Tom

On 2018-09-18 9:58 a.m., Tom St Denis wrote:
Odd I couldn't even boot my system with the dGPU as primary after 
rebuilding the kernel.  It got hung up in the IOMMU driver (loads 
of AMD-Vi IOMMU errors) which I wasn't able to capture because it 
panic'ed before loading the network stack.


Bizarre.

I'll keep trying.

Tom

On 2018-09-18 9:35 a.m., Christian König wrote:

Am 18.09.2018 um 15:32 schrieb Tom St Denis:

On 2018-09-18 9:30 a.m., Christian König wrote:

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody 
volunteer to figure out why interrupts sometimes doesn't work 
correctly on Raven?


What does "doesn't work correctly?"  My workstation is a Raven1 
(Ryzen 2400G) and other than the TTM bulk move issue has been 
perfectly stable (through suspend/resumes too I might add).


Anything I could test with my devel raven?


The problem seems to be that on some boards IH handling doesn't 
work as it should.


Can you try to disable the onboard graphics and try again?

If that still doesn't work there is a DRM_DEBUG in 
amdgpu_ih_process(), make that a DRM_ERROR and send me the 
resulting dmesg of loading amdgpu (but don't start any UMD).


Thanks,
Christian.




Tom



Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much 
busted no

    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 
bytes left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* 
amdgpu: IB test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* 
amdgpu: failed testing IB on ring 9 (-110).
[   26.407885] [drm:process_one_work] *ERROR* ib ring test 
failed (-110).

[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
















___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 1/2] drm/amd/pp: Honour DC's clock limits on Rv

2018-09-18 Thread Deucher, Alexander
Series is:

Reviewed-by: Alex Deucher 


From: amd-gfx  on behalf of Rex Zhu 

Sent: Tuesday, September 18, 2018 9:11:13 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhu, Rex
Subject: [PATCH 1/2] drm/amd/pp: Honour DC's clock limits on Rv

Honour display's request for min engine clock/memory clock.

Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 25 +++
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
index 9808bd4..5d1dae2 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
@@ -552,6 +552,8 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
 {
 struct smu10_hwmgr *data = hwmgr->backend;
 struct amdgpu_device *adev = hwmgr->adev;
+   uint32_t min_sclk = hwmgr->display_config->min_core_set_clock;
+   uint32_t min_mclk = hwmgr->display_config->min_mem_set_clock/100;

 if (hwmgr->smu_version < 0x1E3700) {
 pr_info("smu firmware version too old, can not set dpm 
level\n");
@@ -563,6 +565,13 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
 (adev->rev_id >= 8))
 return 0;

+   if (min_sclk < data->gfx_min_freq_limit)
+   min_sclk = data->gfx_min_freq_limit;
+
+   min_sclk /= 100; /* transfer 10KHz to MHz */
+   if (min_mclk < data->clock_table.FClocks[0].Freq)
+   min_mclk = data->clock_table.FClocks[0].Freq;
+
 switch (level) {
 case AMD_DPM_FORCED_LEVEL_HIGH:
 case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK:
@@ -595,18 +604,18 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
 case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK:
 smum_send_msg_to_smc_with_parameter(hwmgr,
 PPSMC_MSG_SetHardMinGfxClk,
-   data->gfx_min_freq_limit/100);
+   min_sclk);
 smum_send_msg_to_smc_with_parameter(hwmgr,
 PPSMC_MSG_SetSoftMaxGfxClk,
-   data->gfx_min_freq_limit/100);
+   min_sclk);
 break;
 case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK:
 smum_send_msg_to_smc_with_parameter(hwmgr,
 PPSMC_MSG_SetHardMinFclkByFreq,
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
 smum_send_msg_to_smc_with_parameter(hwmgr,
 PPSMC_MSG_SetSoftMaxFclkByFreq,
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
 break;
 case AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD:
 smum_send_msg_to_smc_with_parameter(hwmgr,
@@ -638,12 +647,12 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
 case AMD_DPM_FORCED_LEVEL_AUTO:
 smum_send_msg_to_smc_with_parameter(hwmgr,
 PPSMC_MSG_SetHardMinGfxClk,
-   data->gfx_min_freq_limit/100);
+   min_sclk);
 smum_send_msg_to_smc_with_parameter(hwmgr,
 PPSMC_MSG_SetHardMinFclkByFreq,
 
hwmgr->display_config->num_display > 3 ?
 SMU10_UMD_PSTATE_PEAK_FCLK :
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);

 smum_send_msg_to_smc_with_parameter(hwmgr,
 
PPSMC_MSG_SetHardMinSocclkByFreq,
@@ -674,10 +683,10 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
 data->gfx_min_freq_limit/100);
 smum_send_msg_to_smc_with_parameter(hwmgr,
 PPSMC_MSG_SetHardMinFclkByFreq,
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
 smum_send_msg_to_smc_with_parameter(hwmgr,
 PPSMC_MSG_SetSoftMaxFclkByFreq,
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
 break;
 case AMD_DPM_FORCED_LEVEL_MANUAL:
 case 

Re: [PATCH 1/2] drm/amdgpu: add vega20 sriov capability detection

2018-09-18 Thread Deucher, Alexander
Series is:

Reviewed-by: Alex Deucher 


From: amd-gfx  on behalf of Xiangliang 
Yu 
Sent: Tuesday, September 18, 2018 5:56:03 AM
To: amd-gfx@lists.freedesktop.org
Cc: Yu, Xiangliang; Min, Frank
Subject: [PATCH 1/2] drm/amdgpu: add vega20 sriov capability detection

From: Frank Min 

Add sriov capability detection for vega20, then can check if device is
virtual device.

Signed-off-by: Frank Min 
Signed-off-by: Xiangliang Yu 
---
 drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c 
b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
index 2e65447..f8cee95 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
@@ -205,8 +205,19 @@ static const struct nbio_hdp_flush_reg 
nbio_v7_4_hdp_flush_reg = {

 static void nbio_v7_4_detect_hw_virt(struct amdgpu_device *adev)
 {
-   if (is_virtual_machine())   /* passthrough mode exclus sriov mod */
-   adev->virt.caps |= AMDGPU_PASSTHROUGH_MODE;
+   uint32_t reg;
+
+   reg = RREG32_SOC15(NBIO, 0, mmRCC_IOV_FUNC_IDENTIFIER);
+   if (reg & 1)
+   adev->virt.caps |= AMDGPU_SRIOV_CAPS_IS_VF;
+
+   if (reg & 0x8000)
+   adev->virt.caps |= AMDGPU_SRIOV_CAPS_ENABLE_IOV;
+
+   if (!reg) {
+   if (is_virtual_machine())   /* passthrough mode exclus 
sriov mod */
+   adev->virt.caps |= AMDGPU_PASSTHROUGH_MODE;
+   }
 }

 static void nbio_v7_4_init_registers(struct amdgpu_device *adev)
--
2.7.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - 
freedesktop.org
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx 
Archives.. Using amd-gfx: To post a message to all the list members, send email 
to amd-gfx@lists.freedesktop.org. You can subscribe to the list, or change your 
existing subscription, in the sections below.


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Tom St Denis

On 2018-09-18 10:13 a.m., Christian König wrote:

Mhm, there is no more failed IB-test in there isn't it?


oh sorry I thought you wanted to test HEAD~ ... Attached is a log from 
the tip of drm-next


Tom



Christian.

Am 18.09.2018 um 16:09 schrieb Tom St Denis:

Disabling IOMMU in the BIOS resulted in a correct boot up...

Here's the log.

Tom

On 2018-09-18 9:58 a.m., Tom St Denis wrote:
Odd I couldn't even boot my system with the dGPU as primary after 
rebuilding the kernel.  It got hung up in the IOMMU driver (loads of 
AMD-Vi IOMMU errors) which I wasn't able to capture because it 
panic'ed before loading the network stack.


Bizarre.

I'll keep trying.

Tom

On 2018-09-18 9:35 a.m., Christian König wrote:

Am 18.09.2018 um 15:32 schrieb Tom St Denis:

On 2018-09-18 9:30 a.m., Christian König wrote:

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody volunteer 
to figure out why interrupts sometimes doesn't work correctly on 
Raven?


What does "doesn't work correctly?"  My workstation is a Raven1 
(Ryzen 2400G) and other than the TTM bulk move issue has been 
perfectly stable (through suspend/resumes too I might add).


Anything I could test with my devel raven?


The problem seems to be that on some boards IH handling doesn't work 
as it should.


Can you try to disable the onboard graphics and try again?

If that still doesn't work there is a DRM_DEBUG in 
amdgpu_ih_process(), make that a DRM_ERROR and send me the resulting 
dmesg of loading amdgpu (but don't start any UMD).


Thanks,
Christian.




Tom



Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much 
busted no

    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 
bytes left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* 
amdgpu: IB test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* 
amdgpu: failed testing IB on ring 9 (-110).
[   26.407885] [drm:process_one_work] *ERROR* ib ring test failed 
(-110).

[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
















amdgpu_ih_process2.log.gz
Description: application/gzip
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Christian König

Mhm, there is no more failed IB-test in there isn't it?

Christian.

Am 18.09.2018 um 16:09 schrieb Tom St Denis:

Disabling IOMMU in the BIOS resulted in a correct boot up...

Here's the log.

Tom

On 2018-09-18 9:58 a.m., Tom St Denis wrote:
Odd I couldn't even boot my system with the dGPU as primary after 
rebuilding the kernel.  It got hung up in the IOMMU driver (loads of 
AMD-Vi IOMMU errors) which I wasn't able to capture because it 
panic'ed before loading the network stack.


Bizarre.

I'll keep trying.

Tom

On 2018-09-18 9:35 a.m., Christian König wrote:

Am 18.09.2018 um 15:32 schrieb Tom St Denis:

On 2018-09-18 9:30 a.m., Christian König wrote:

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody volunteer 
to figure out why interrupts sometimes doesn't work correctly on 
Raven?


What does "doesn't work correctly?"  My workstation is a Raven1 
(Ryzen 2400G) and other than the TTM bulk move issue has been 
perfectly stable (through suspend/resumes too I might add).


Anything I could test with my devel raven?


The problem seems to be that on some boards IH handling doesn't work 
as it should.


Can you try to disable the onboard graphics and try again?

If that still doesn't work there is a DRM_DEBUG in 
amdgpu_ih_process(), make that a DRM_ERROR and send me the resulting 
dmesg of loading amdgpu (but don't start any UMD).


Thanks,
Christian.




Tom



Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much 
busted no

    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 
bytes left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* 
amdgpu: IB test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* 
amdgpu: failed testing IB on ring 9 (-110).
[   26.407885] [drm:process_one_work] *ERROR* ib ring test failed 
(-110).

[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx












___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Tom St Denis

Disabling IOMMU in the BIOS resulted in a correct boot up...

Here's the log.

Tom

On 2018-09-18 9:58 a.m., Tom St Denis wrote:
Odd I couldn't even boot my system with the dGPU as primary after 
rebuilding the kernel.  It got hung up in the IOMMU driver (loads of 
AMD-Vi IOMMU errors) which I wasn't able to capture because it panic'ed 
before loading the network stack.


Bizarre.

I'll keep trying.

Tom

On 2018-09-18 9:35 a.m., Christian König wrote:

Am 18.09.2018 um 15:32 schrieb Tom St Denis:

On 2018-09-18 9:30 a.m., Christian König wrote:

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody volunteer to 
figure out why interrupts sometimes doesn't work correctly on Raven?


What does "doesn't work correctly?"  My workstation is a Raven1 
(Ryzen 2400G) and other than the TTM bulk move issue has been 
perfectly stable (through suspend/resumes too I might add).


Anything I could test with my devel raven?


The problem seems to be that on some boards IH handling doesn't work 
as it should.


Can you try to disable the onboard graphics and try again?

If that still doesn't work there is a DRM_DEBUG in 
amdgpu_ih_process(), make that a DRM_ERROR and send me the resulting 
dmesg of loading amdgpu (but don't start any UMD).


Thanks,
Christian.




Tom



Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much 
busted no

    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 
bytes left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* amdgpu: 
IB test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* amdgpu: 
failed testing IB on ring 9 (-110).
[   26.407885] [drm:process_one_work] *ERROR* ib ring test failed 
(-110).

[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx












amdgpu_ih_process.log.gz
Description: application/gzip
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Tom St Denis
Odd I couldn't even boot my system with the dGPU as primary after 
rebuilding the kernel.  It got hung up in the IOMMU driver (loads of 
AMD-Vi IOMMU errors) which I wasn't able to capture because it panic'ed 
before loading the network stack.


Bizarre.

I'll keep trying.

Tom

On 2018-09-18 9:35 a.m., Christian König wrote:

Am 18.09.2018 um 15:32 schrieb Tom St Denis:

On 2018-09-18 9:30 a.m., Christian König wrote:

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody volunteer to 
figure out why interrupts sometimes doesn't work correctly on Raven?


What does "doesn't work correctly?"  My workstation is a Raven1 (Ryzen 
2400G) and other than the TTM bulk move issue has been perfectly 
stable (through suspend/resumes too I might add).


Anything I could test with my devel raven?


The problem seems to be that on some boards IH handling doesn't work as 
it should.


Can you try to disable the onboard graphics and try again?

If that still doesn't work there is a DRM_DEBUG in amdgpu_ih_process(), 
make that a DRM_ERROR and send me the resulting dmesg of loading amdgpu 
(but don't start any UMD).


Thanks,
Christian.




Tom



Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much 
busted no

    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 
bytes left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* amdgpu: 
IB test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* amdgpu: 
failed testing IB on ring 9 (-110).
[   26.407885] [drm:process_one_work] *ERROR* ib ring test failed 
(-110).

[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx








___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Christian König

Am 18.09.2018 um 15:32 schrieb Tom St Denis:

On 2018-09-18 9:30 a.m., Christian König wrote:

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody volunteer to 
figure out why interrupts sometimes doesn't work correctly on Raven?


What does "doesn't work correctly?"  My workstation is a Raven1 (Ryzen 
2400G) and other than the TTM bulk move issue has been perfectly 
stable (through suspend/resumes too I might add).


Anything I could test with my devel raven?


The problem seems to be that on some boards IH handling doesn't work as 
it should.


Can you try to disable the onboard graphics and try again?

If that still doesn't work there is a DRM_DEBUG in amdgpu_ih_process(), 
make that a DRM_ERROR and send me the resulting dmesg of loading amdgpu 
(but don't start any UMD).


Thanks,
Christian.




Tom



Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much 
busted no

    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 
bytes left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* amdgpu: 
IB test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* amdgpu: 
failed testing IB on ring 9 (-110).
[   26.407885] [drm:process_one_work] *ERROR* ib ring test failed 
(-110).

[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx






___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Tom St Denis

On 2018-09-18 9:30 a.m., Christian König wrote:

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody volunteer to 
figure out why interrupts sometimes doesn't work correctly on Raven?


What does "doesn't work correctly?"  My workstation is a Raven1 (Ryzen 
2400G) and other than the TTM bulk move issue has been perfectly stable 
(through suspend/resumes too I might add).


Anything I could test with my devel raven?

Tom



Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much busted no
    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 bytes 
left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* amdgpu: IB 
test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* amdgpu: 
failed testing IB on ring 9 (-110).

[   26.407885] [drm:process_one_work] *ERROR* ib ring test failed (-110).
[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: Regression on gfx8 with ring init

2018-09-18 Thread Christian König

Great, not sure if that is a good or a bad news.

Anyway going to revert the change for now. Does anybody volunteer to 
figure out why interrupts sometimes doesn't work correctly on Raven?


Christian.

Am 18.09.2018 um 15:27 schrieb Tom St Denis:

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

    drm/amdgpu: remove fence fallback

    DC doesn't seem to have a fallback path either.

    So when interrupts doesn't work any more we are pretty much busted no
    matter what.

    Signed-off-by: Christian König 
    Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for 
:07:00.0 on minor 1
[   24.335674] modprobe (3895) used greatest stack depth: 12600 bytes 
left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* amdgpu: IB 
test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* amdgpu: 
failed testing IB on ring 9 (-110).

[   26.407885] [drm:process_one_work] *ERROR* ib ring test failed (-110).
[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Regression on gfx8 with ring init

2018-09-18 Thread Tom St Denis

This commit:

[root@raven linux]# git bisect good
9b0df0937a852d299fbe42a5939c9a8a4cc83c55 is the first bad commit
commit 9b0df0937a852d299fbe42a5939c9a8a4cc83c55
Author: Christian König 
Date:   Tue Sep 18 10:38:09 2018 +0200

drm/amdgpu: remove fence fallback

DC doesn't seem to have a fallback path either.

So when interrupts doesn't work any more we are pretty much busted no
matter what.

Signed-off-by: Christian König 
Reviewed-by: Chunming Zhou 

Results in this:

[   24.334025] [drm] Initialized amdgpu 3.27.0 20150101 for :07:00.0 
on minor 1

[   24.335674] modprobe (3895) used greatest stack depth: 12600 bytes left
[   26.272358] [drm:gfx_v8_0_ring_test_ib [amdgpu]] *ERROR* amdgpu: IB 
test timed out.
[   26.272460] [drm:amdgpu_ib_ring_tests [amdgpu]] *ERROR* amdgpu: 
failed testing IB on ring 9 (-110).

[   26.407885] [drm:process_one_work] *ERROR* ib ring test failed (-110).
[   28.506708] fuse init (API version 7.27)

On init with my polaris/raven1 system.

Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 1/2] drm/amd/pp: Honour DC's clock limits on Rv

2018-09-18 Thread Rex Zhu
Honour display's request for min engine clock/memory clock.

Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 25 +++
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
index 9808bd4..5d1dae2 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
@@ -552,6 +552,8 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr *hwmgr,
 {
struct smu10_hwmgr *data = hwmgr->backend;
struct amdgpu_device *adev = hwmgr->adev;
+   uint32_t min_sclk = hwmgr->display_config->min_core_set_clock;
+   uint32_t min_mclk = hwmgr->display_config->min_mem_set_clock/100;
 
if (hwmgr->smu_version < 0x1E3700) {
pr_info("smu firmware version too old, can not set dpm 
level\n");
@@ -563,6 +565,13 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
(adev->rev_id >= 8))
return 0;
 
+   if (min_sclk < data->gfx_min_freq_limit)
+   min_sclk = data->gfx_min_freq_limit;
+
+   min_sclk /= 100; /* transfer 10KHz to MHz */
+   if (min_mclk < data->clock_table.FClocks[0].Freq)
+   min_mclk = data->clock_table.FClocks[0].Freq;
+
switch (level) {
case AMD_DPM_FORCED_LEVEL_HIGH:
case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK:
@@ -595,18 +604,18 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK:
smum_send_msg_to_smc_with_parameter(hwmgr,
PPSMC_MSG_SetHardMinGfxClk,
-   data->gfx_min_freq_limit/100);
+   min_sclk);
smum_send_msg_to_smc_with_parameter(hwmgr,
PPSMC_MSG_SetSoftMaxGfxClk,
-   data->gfx_min_freq_limit/100);
+   min_sclk);
break;
case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK:
smum_send_msg_to_smc_with_parameter(hwmgr,
PPSMC_MSG_SetHardMinFclkByFreq,
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
smum_send_msg_to_smc_with_parameter(hwmgr,
PPSMC_MSG_SetSoftMaxFclkByFreq,
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
break;
case AMD_DPM_FORCED_LEVEL_PROFILE_STANDARD:
smum_send_msg_to_smc_with_parameter(hwmgr,
@@ -638,12 +647,12 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
case AMD_DPM_FORCED_LEVEL_AUTO:
smum_send_msg_to_smc_with_parameter(hwmgr,
PPSMC_MSG_SetHardMinGfxClk,
-   data->gfx_min_freq_limit/100);
+   min_sclk);
smum_send_msg_to_smc_with_parameter(hwmgr,
PPSMC_MSG_SetHardMinFclkByFreq,

hwmgr->display_config->num_display > 3 ?
SMU10_UMD_PSTATE_PEAK_FCLK :
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
 
smum_send_msg_to_smc_with_parameter(hwmgr,

PPSMC_MSG_SetHardMinSocclkByFreq,
@@ -674,10 +683,10 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
data->gfx_min_freq_limit/100);
smum_send_msg_to_smc_with_parameter(hwmgr,
PPSMC_MSG_SetHardMinFclkByFreq,
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
smum_send_msg_to_smc_with_parameter(hwmgr,
PPSMC_MSG_SetSoftMaxFclkByFreq,
-   SMU10_UMD_PSTATE_MIN_FCLK);
+   min_mclk);
break;
case AMD_DPM_FORCED_LEVEL_MANUAL:
case AMD_DPM_FORCED_LEVEL_PROFILE_EXIT:
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 2/2] drm/amd/pp: Return error immediately if load firmware failed

2018-09-18 Thread Rex Zhu
this can avoid hard hang and be useful for debug.

Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
index f7e3bc2..a74c5be 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
@@ -724,11 +724,13 @@ static int smu8_start_smu(struct pp_hwmgr *hwmgr)
if (hwmgr->chip_id == CHIP_STONEY)
fw_to_check &= ~(UCODE_ID_SDMA1_MASK | 
UCODE_ID_CP_MEC_JT2_MASK);
 
-   ret = smu8_request_smu_load_fw(hwmgr);
-   if (ret)
-   pr_err("SMU firmware load failed\n");
+   smu8_request_smu_load_fw(hwmgr);
 
-   smu8_check_fw_load_finish(hwmgr, fw_to_check);
+   ret = smu8_check_fw_load_finish(hwmgr, fw_to_check);
+   if (ret) {
+   pr_err("SMU firmware load failed\n");
+   return ret;
+   }
 
ret = smu8_load_mec_firmware(hwmgr);
if (ret)
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 2/2] drm/amdgpu: Exclude MM engines for vega20 virtual device

2018-09-18 Thread Xiangliang Yu
From: Frank Min 

Temporary disable UVD/VCE block if is virtual device

Signed-off-by: Frank Min 
Signed-off-by: Xiangliang Yu 
---
 drivers/gpu/drm/amd/amdgpu/soc15.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index fc0cb7d3..0f40dd4 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -534,8 +534,10 @@ int soc15_set_ip_blocks(struct amdgpu_device *adev)
 #endif
amdgpu_device_ip_block_add(adev, _v9_0_ip_block);
amdgpu_device_ip_block_add(adev, _v4_0_ip_block);
-   amdgpu_device_ip_block_add(adev, _v7_0_ip_block);
-   amdgpu_device_ip_block_add(adev, _v4_0_ip_block);
+   if (!(adev->asic_type == CHIP_VEGA20 && amdgpu_sriov_vf(adev))) 
{
+   amdgpu_device_ip_block_add(adev, _v7_0_ip_block);
+   amdgpu_device_ip_block_add(adev, _v4_0_ip_block);
+   }
break;
case CHIP_RAVEN:
amdgpu_device_ip_block_add(adev, _common_ip_block);
-- 
2.7.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 1/2] drm/amdgpu: add vega20 sriov capability detection

2018-09-18 Thread Xiangliang Yu
From: Frank Min 

Add sriov capability detection for vega20, then can check if device is
virtual device.

Signed-off-by: Frank Min 
Signed-off-by: Xiangliang Yu 
---
 drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c 
b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
index 2e65447..f8cee95 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
@@ -205,8 +205,19 @@ static const struct nbio_hdp_flush_reg 
nbio_v7_4_hdp_flush_reg = {
 
 static void nbio_v7_4_detect_hw_virt(struct amdgpu_device *adev)
 {
-   if (is_virtual_machine())   /* passthrough mode exclus sriov mod */
-   adev->virt.caps |= AMDGPU_PASSTHROUGH_MODE;
+   uint32_t reg;
+
+   reg = RREG32_SOC15(NBIO, 0, mmRCC_IOV_FUNC_IDENTIFIER);
+   if (reg & 1)
+   adev->virt.caps |= AMDGPU_SRIOV_CAPS_IS_VF;
+
+   if (reg & 0x8000)
+   adev->virt.caps |= AMDGPU_SRIOV_CAPS_ENABLE_IOV;
+
+   if (!reg) {
+   if (is_virtual_machine())   /* passthrough mode exclus 
sriov mod */
+   adev->virt.caps |= AMDGPU_PASSTHROUGH_MODE;
+   }
 }
 
 static void nbio_v7_4_init_registers(struct amdgpu_device *adev)
-- 
2.7.4

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

2018-09-18 Thread Zhu, Rex
Thanks. 
So we still need to check the bo valid before use for the case that if we don't 
know the size when allocate.

Best Regards
Rex

> -Original Message-
> From: Koenig, Christian
> Sent: Tuesday, September 18, 2018 5:34 PM
> To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> 
> Am 18.09.2018 um 11:27 schrieb Zhu, Rex:
> >
> >> -Original Message-
> >> From: Koenig, Christian
> >> Sent: Tuesday, September 18, 2018 4:41 PM
> >> To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
> >> Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> >>
> >> Am 18.09.2018 um 10:16 schrieb Zhu, Rex:
>  -Original Message-
>  From: Christian König 
>  Sent: Tuesday, September 18, 2018 3:14 PM
>  To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
>  Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL
>  pointer
> 
>  Am 18.09.2018 um 08:16 schrieb Zhu, Rex:
> >> -Original Message-
> >> From: amd-gfx  On Behalf
> >> Of Christian König
> >> Sent: Tuesday, September 18, 2018 2:07 AM
> >> To: amd-gfx@lists.freedesktop.org
> >> Subject: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> >>
> >> Don't try to unreserve a BO we doesn't allocated.
> >>
> >> Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel
> >> BOs
> >>
> >> Signed-off-by: Christian König 
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> index 84d82d5382f9..c1387efc0c91 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> @@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct
>  amdgpu_device
> >> *adev,
> >>if (r)
> >>return r;
> >> -  amdgpu_bo_unreserve(*bo_ptr);
> >> +  if (*bo_ptr)
> >> +  amdgpu_bo_unreserve(*bo_ptr);
> >>
> >>return 0;
> >> }
> > It is weird.
> > If we return true for allocate bo with size  0.
> > Does that mean we need to check all the bo_ptr before we use them.
>  No, allocating a BO with zero size doesn't make much sense and was
>  essentially undefined behavior previously.
> 
>  So now we get a defined behavior, but not necessary the one you
> >> expected.
>  Is that only a rhetorical question or really a problem somewhere?
> >>> Logically, the code is trick.
> >> Yeah, that is not a bad argument.
> >>
> >> But it also makes the function more useful, e.g. we don't need size
> >> checks any more whenever an optional BO is allocated.
> > Yes, the problem is user don't need size check. But user have no way to
> check whether the bo is allocated successfully.
> >
> > Because in size 0 case, the create function also return true.
> > And as you said below, check bo_ptr is not safe either(the *bo_ptr  may be
> not modified at all).
> 
> That is not correct. When size==0 we call amdgpu_bo_unref(bo_ptr), and
> that one sets bo_ptr to NULL.
> 
> When size==0 was possible before, the calling code would have needed to
> check bo_ptr later on anyway.
> 
> In other words what we had before:
> 
> calling_function()
> {
>      if (size) {
>          r = amdgpu_bo_create_kernel(..., size, );
>          if (r)
>              goto error_handling;
>      }
>      
>      if (bo)
>          
> }
> 
> 
> But now that looks like:
> calling_function()
> {
>      r = amdgpu_bo_create_kernel(..., size, );
>      if (r)
>          goto error_handling;
>      
>      if (bo) {
>          
> }
> 
> So we just removed the extra size check of the calling function. I think that 
> is
> a valid usage.
> 
> Christian.
> 
> 
> >
> > Regards
> > Rex
> >
> >
> >
> >
> >>>It also make the  code
> >>> if (r)
> >>>   return r;
> >>> redundant.
> >> Actually that is not correct.
> >>
> >> When an error happens the *bo_ptr is not modified at all. So we could
> >> then try to unreserve a BO which was never reserved.
> > Yes, You ae right.
> >
> >
> >> Christian.
> >>
> >>> Regards
> >>> Rex
> >>>
>  Regards,
>  Christian.
> 
> > Best Regards
> > Rex
> >
> >> --
> >> 2.14.1
> >>
> >> ___
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

2018-09-18 Thread Christian König

Am 18.09.2018 um 11:27 schrieb Zhu, Rex:



-Original Message-
From: Koenig, Christian
Sent: Tuesday, September 18, 2018 4:41 PM
To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

Am 18.09.2018 um 10:16 schrieb Zhu, Rex:

-Original Message-
From: Christian König 
Sent: Tuesday, September 18, 2018 3:14 PM
To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

Am 18.09.2018 um 08:16 schrieb Zhu, Rex:

-Original Message-
From: amd-gfx  On Behalf Of
Christian König
Sent: Tuesday, September 18, 2018 2:07 AM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

Don't try to unreserve a BO we doesn't allocated.

Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel
BOs

Signed-off-by: Christian König 
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct

amdgpu_device

*adev,
if (r)
return r;
-   amdgpu_bo_unreserve(*bo_ptr);
+   if (*bo_ptr)
+   amdgpu_bo_unreserve(*bo_ptr);

return 0;
}

It is weird.
If we return true for allocate bo with size  0.
Does that mean we need to check all the bo_ptr before we use them.

No, allocating a BO with zero size doesn't make much sense and was
essentially undefined behavior previously.

So now we get a defined behavior, but not necessary the one you

expected.

Is that only a rhetorical question or really a problem somewhere?

Logically, the code is trick.

Yeah, that is not a bad argument.

But it also makes the function more useful, e.g. we don't need size checks
any more whenever an optional BO is allocated.

Yes, the problem is user don't need size check. But user have no way to check 
whether the bo is allocated successfully.

Because in size 0 case, the create function also return true.
And as you said below, check bo_ptr is not safe either(the *bo_ptr  may be not 
modified at all).


That is not correct. When size==0 we call amdgpu_bo_unref(bo_ptr), and 
that one sets bo_ptr to NULL.


When size==0 was possible before, the calling code would have needed to 
check bo_ptr later on anyway.


In other words what we had before:

calling_function()
{
    if (size) {
        r = amdgpu_bo_create_kernel(..., size, );
        if (r)
            goto error_handling;
    }
    
    if (bo)
        
}


But now that looks like:
calling_function()
{
    r = amdgpu_bo_create_kernel(..., size, );
    if (r)
        goto error_handling;
    
    if (bo) {
        
}

So we just removed the extra size check of the calling function. I think 
that is a valid usage.


Christian.




Regards
Rex





   It also make the  code
if (r)
return r;
redundant.

Actually that is not correct.

When an error happens the *bo_ptr is not modified at all. So we could then
try to unreserve a BO which was never reserved.

Yes, You ae right.



Christian.


Regards
Rex


Regards,
Christian.


Best Regards
Rex


--
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

2018-09-18 Thread Zhu, Rex


> -Original Message-
> From: Koenig, Christian
> Sent: Tuesday, September 18, 2018 4:41 PM
> To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> 
> Am 18.09.2018 um 10:16 schrieb Zhu, Rex:
> >
> >> -Original Message-
> >> From: Christian König 
> >> Sent: Tuesday, September 18, 2018 3:14 PM
> >> To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
> >> Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> >>
> >> Am 18.09.2018 um 08:16 schrieb Zhu, Rex:
>  -Original Message-
>  From: amd-gfx  On Behalf Of
>  Christian König
>  Sent: Tuesday, September 18, 2018 2:07 AM
>  To: amd-gfx@lists.freedesktop.org
>  Subject: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> 
>  Don't try to unreserve a BO we doesn't allocated.
> 
>  Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel
>  BOs
> 
>  Signed-off-by: Christian König 
>  ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
>  diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>  b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>  index 84d82d5382f9..c1387efc0c91 100644
>  --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>  +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>  @@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct
> >> amdgpu_device
>  *adev,
>   if (r)
>   return r;
>  -amdgpu_bo_unreserve(*bo_ptr);
>  +if (*bo_ptr)
>  +amdgpu_bo_unreserve(*bo_ptr);
> 
>   return 0;
> }
> >>> It is weird.
> >>> If we return true for allocate bo with size  0.
> >>> Does that mean we need to check all the bo_ptr before we use them.
> >> No, allocating a BO with zero size doesn't make much sense and was
> >> essentially undefined behavior previously.
> >>
> >> So now we get a defined behavior, but not necessary the one you
> expected.
> >>
> >> Is that only a rhetorical question or really a problem somewhere?
> > Logically, the code is trick.
> 
> Yeah, that is not a bad argument.
> 
> But it also makes the function more useful, e.g. we don't need size checks
> any more whenever an optional BO is allocated.

Yes, the problem is user don't need size check. But user have no way to check 
whether the bo is allocated successfully.

Because in size 0 case, the create function also return true.
And as you said below, check bo_ptr is not safe either(the *bo_ptr  may be not 
modified at all).

Regards
Rex




> >   It also make the  code
> > if (r)
> > return r;
> > redundant.
> 
> Actually that is not correct.
> 
> When an error happens the *bo_ptr is not modified at all. So we could then
> try to unreserve a BO which was never reserved.

Yes, You ae right.


> Christian.
> 
> >
> > Regards
> > Rex
> >
> >> Regards,
> >> Christian.
> >>
> >>> Best Regards
> >>> Rex
> >>>
>  --
>  2.14.1
> 
>  ___
>  amd-gfx mailing list
>  amd-gfx@lists.freedesktop.org
>  https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amdgpu: remove fence fallback

2018-09-18 Thread Zhou, David(ChunMing)


> -Original Message-
> From: amd-gfx  On Behalf Of
> Christian K?nig
> Sent: Tuesday, September 18, 2018 4:43 PM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH] drm/amdgpu: remove fence fallback
> 
> DC doesn't seem to have a fallback path either.
> 
> So when interrupts doesn't work any more we are pretty much busted no
> matter what.
> 
> Signed-off-by: Christian König 

Reviewed-by: Chunming Zhou 


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 56 ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h  |  1 -
>  3 files changed, 58 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 27382767e15a..c18d68575462 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -146,7 +146,6 @@ extern int amdgpu_cik_support;
>  #define AMDGPU_DEFAULT_GTT_SIZE_MB   3072ULL /* 3GB by
> default */
>  #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS   3000
>  #define AMDGPU_MAX_USEC_TIMEOUT  10  /* 100
> ms */
> -#define AMDGPU_FENCE_JIFFIES_TIMEOUT (HZ / 2)
>  /* AMDGPU_IB_POOL_SIZE must be a power of 2 */
>  #define AMDGPU_IB_POOL_SIZE  16
>  #define AMDGPU_DEBUGFS_MAX_COMPONENTS32
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index da36731460b5..176f28777f5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -195,19 +195,6 @@ int amdgpu_fence_emit_polling(struct amdgpu_ring
> *ring, uint32_t *s)
>   return 0;
>  }
> 
> -/**
> - * amdgpu_fence_schedule_fallback - schedule fallback check
> - *
> - * @ring: pointer to struct amdgpu_ring
> - *
> - * Start a timer as fallback to our interrupts.
> - */
> -static void amdgpu_fence_schedule_fallback(struct amdgpu_ring *ring) -{
> - mod_timer(>fence_drv.fallback_timer,
> -   jiffies + AMDGPU_FENCE_JIFFIES_TIMEOUT);
> -}
> -
>  /**
>   * amdgpu_fence_process - check for fence activity
>   *
> @@ -229,9 +216,6 @@ void amdgpu_fence_process(struct amdgpu_ring
> *ring)
> 
>   } while (atomic_cmpxchg(>last_seq, last_seq, seq) != last_seq);
> 
> - if (seq != ring->fence_drv.sync_seq)
> - amdgpu_fence_schedule_fallback(ring);
> -
>   if (unlikely(seq == last_seq))
>   return;
> 
> @@ -262,21 +246,6 @@ void amdgpu_fence_process(struct amdgpu_ring
> *ring)
>   } while (last_seq != seq);
>  }
> 
> -/**
> - * amdgpu_fence_fallback - fallback for hardware interrupts
> - *
> - * @work: delayed work item
> - *
> - * Checks for fence activity.
> - */
> -static void amdgpu_fence_fallback(struct timer_list *t) -{
> - struct amdgpu_ring *ring = from_timer(ring, t,
> -   fence_drv.fallback_timer);
> -
> - amdgpu_fence_process(ring);
> -}
> -
>  /**
>   * amdgpu_fence_wait_empty - wait for all fences to signal
>   *
> @@ -424,8 +393,6 @@ int amdgpu_fence_driver_init_ring(struct
> amdgpu_ring *ring,
>   atomic_set(>fence_drv.last_seq, 0);
>   ring->fence_drv.initialized = false;
> 
> - timer_setup(>fence_drv.fallback_timer,
> amdgpu_fence_fallback, 0);
> -
>   ring->fence_drv.num_fences_mask = num_hw_submission * 2 - 1;
>   spin_lock_init(>fence_drv.lock);
>   ring->fence_drv.fences = kcalloc(num_hw_submission * 2,
> sizeof(void *), @@ -501,7 +468,6 @@ void amdgpu_fence_driver_fini(struct
> amdgpu_device *adev)
>   amdgpu_irq_put(adev, ring->fence_drv.irq_src,
>  ring->fence_drv.irq_type);
>   drm_sched_fini(>sched);
> - del_timer_sync(>fence_drv.fallback_timer);
>   for (j = 0; j <= ring->fence_drv.num_fences_mask; ++j)
>   dma_fence_put(ring->fence_drv.fences[j]);
>   kfree(ring->fence_drv.fences);
> @@ -594,27 +560,6 @@ static const char
> *amdgpu_fence_get_timeline_name(struct dma_fence *f)
>   return (const char *)fence->ring->name;  }
> 
> -/**
> - * amdgpu_fence_enable_signaling - enable signalling on fence
> - * @fence: fence
> - *
> - * This function is called with fence_queue lock held, and adds a callback
> - * to fence_queue that checks if this fence is signaled, and if so it
> - * signals the fence and removes itself.
> - */
> -static bool amdgpu_fence_enable_signaling(struct dma_fence *f) -{
> - struct amdgpu_fence *fence = to_amdgpu_fence(f);
> - struct amdgpu_ring *ring = fence->ring;
> -
> - if (!timer_pending(>fence_drv.fallback_timer))
> - amdgpu_fence_schedule_fallback(ring);
> -
> - DMA_FENCE_TRACE(>base, "armed on ring %i!\n", ring-
> >idx);
> -
> - return true;
> -}
> -
>  /**
>   * amdgpu_fence_free - free up the fence memory
>   *
> @@ -645,7 +590,6 @@ static void amdgpu_fence_release(struct dma_fence
> *f)  

[PATCH] drm/amdgpu: remove fence fallback

2018-09-18 Thread Christian König
DC doesn't seem to have a fallback path either.

So when interrupts doesn't work any more we are pretty much busted no
matter what.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 56 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h  |  1 -
 3 files changed, 58 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 27382767e15a..c18d68575462 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -146,7 +146,6 @@ extern int amdgpu_cik_support;
 #define AMDGPU_DEFAULT_GTT_SIZE_MB 3072ULL /* 3GB by default */
 #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
 #define AMDGPU_MAX_USEC_TIMEOUT10  /* 100 ms */
-#define AMDGPU_FENCE_JIFFIES_TIMEOUT   (HZ / 2)
 /* AMDGPU_IB_POOL_SIZE must be a power of 2 */
 #define AMDGPU_IB_POOL_SIZE16
 #define AMDGPU_DEBUGFS_MAX_COMPONENTS  32
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index da36731460b5..176f28777f5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -195,19 +195,6 @@ int amdgpu_fence_emit_polling(struct amdgpu_ring *ring, 
uint32_t *s)
return 0;
 }
 
-/**
- * amdgpu_fence_schedule_fallback - schedule fallback check
- *
- * @ring: pointer to struct amdgpu_ring
- *
- * Start a timer as fallback to our interrupts.
- */
-static void amdgpu_fence_schedule_fallback(struct amdgpu_ring *ring)
-{
-   mod_timer(>fence_drv.fallback_timer,
- jiffies + AMDGPU_FENCE_JIFFIES_TIMEOUT);
-}
-
 /**
  * amdgpu_fence_process - check for fence activity
  *
@@ -229,9 +216,6 @@ void amdgpu_fence_process(struct amdgpu_ring *ring)
 
} while (atomic_cmpxchg(>last_seq, last_seq, seq) != last_seq);
 
-   if (seq != ring->fence_drv.sync_seq)
-   amdgpu_fence_schedule_fallback(ring);
-
if (unlikely(seq == last_seq))
return;
 
@@ -262,21 +246,6 @@ void amdgpu_fence_process(struct amdgpu_ring *ring)
} while (last_seq != seq);
 }
 
-/**
- * amdgpu_fence_fallback - fallback for hardware interrupts
- *
- * @work: delayed work item
- *
- * Checks for fence activity.
- */
-static void amdgpu_fence_fallback(struct timer_list *t)
-{
-   struct amdgpu_ring *ring = from_timer(ring, t,
- fence_drv.fallback_timer);
-
-   amdgpu_fence_process(ring);
-}
-
 /**
  * amdgpu_fence_wait_empty - wait for all fences to signal
  *
@@ -424,8 +393,6 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring,
atomic_set(>fence_drv.last_seq, 0);
ring->fence_drv.initialized = false;
 
-   timer_setup(>fence_drv.fallback_timer, amdgpu_fence_fallback, 0);
-
ring->fence_drv.num_fences_mask = num_hw_submission * 2 - 1;
spin_lock_init(>fence_drv.lock);
ring->fence_drv.fences = kcalloc(num_hw_submission * 2, sizeof(void *),
@@ -501,7 +468,6 @@ void amdgpu_fence_driver_fini(struct amdgpu_device *adev)
amdgpu_irq_put(adev, ring->fence_drv.irq_src,
   ring->fence_drv.irq_type);
drm_sched_fini(>sched);
-   del_timer_sync(>fence_drv.fallback_timer);
for (j = 0; j <= ring->fence_drv.num_fences_mask; ++j)
dma_fence_put(ring->fence_drv.fences[j]);
kfree(ring->fence_drv.fences);
@@ -594,27 +560,6 @@ static const char *amdgpu_fence_get_timeline_name(struct 
dma_fence *f)
return (const char *)fence->ring->name;
 }
 
-/**
- * amdgpu_fence_enable_signaling - enable signalling on fence
- * @fence: fence
- *
- * This function is called with fence_queue lock held, and adds a callback
- * to fence_queue that checks if this fence is signaled, and if so it
- * signals the fence and removes itself.
- */
-static bool amdgpu_fence_enable_signaling(struct dma_fence *f)
-{
-   struct amdgpu_fence *fence = to_amdgpu_fence(f);
-   struct amdgpu_ring *ring = fence->ring;
-
-   if (!timer_pending(>fence_drv.fallback_timer))
-   amdgpu_fence_schedule_fallback(ring);
-
-   DMA_FENCE_TRACE(>base, "armed on ring %i!\n", ring->idx);
-
-   return true;
-}
-
 /**
  * amdgpu_fence_free - free up the fence memory
  *
@@ -645,7 +590,6 @@ static void amdgpu_fence_release(struct dma_fence *f)
 static const struct dma_fence_ops amdgpu_fence_ops = {
.get_driver_name = amdgpu_fence_get_driver_name,
.get_timeline_name = amdgpu_fence_get_timeline_name,
-   .enable_signaling = amdgpu_fence_enable_signaling,
.release = amdgpu_fence_release,
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 9cc239968e40..44fc665e4577 100644
--- 

Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

2018-09-18 Thread Christian König

Am 18.09.2018 um 10:16 schrieb Zhu, Rex:



-Original Message-
From: Christian König 
Sent: Tuesday, September 18, 2018 3:14 PM
To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

Am 18.09.2018 um 08:16 schrieb Zhu, Rex:

-Original Message-
From: amd-gfx  On Behalf Of
Christian König
Sent: Tuesday, September 18, 2018 2:07 AM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

Don't try to unreserve a BO we doesn't allocated.

Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs

Signed-off-by: Christian König 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct

amdgpu_device

*adev,
if (r)
return r;
-   amdgpu_bo_unreserve(*bo_ptr);
+   if (*bo_ptr)
+   amdgpu_bo_unreserve(*bo_ptr);

return 0;
   }

It is weird.
If we return true for allocate bo with size  0.
Does that mean we need to check all the bo_ptr before we use them.

No, allocating a BO with zero size doesn't make much sense and was
essentially undefined behavior previously.

So now we get a defined behavior, but not necessary the one you expected.

Is that only a rhetorical question or really a problem somewhere?

Logically, the code is trick.


Yeah, that is not a bad argument.

But it also makes the function more useful, e.g. we don't need size 
checks any more whenever an optional BO is allocated.



  It also make the  code
if (r)
return r;
redundant.


Actually that is not correct.

When an error happens the *bo_ptr is not modified at all. So we could 
then try to unreserve a BO which was never reserved.


Christian.



Regards
Rex


Regards,
Christian.


Best Regards
Rex


--
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 5/5] drm/amdgpu: fix shadow BO restoring

2018-09-18 Thread Huang Rui
On Mon, Sep 17, 2018 at 07:42:38PM +0800, Christian König wrote:
> Am 17.09.2018 um 11:10 schrieb Huang Rui:
> > On Fri, Sep 14, 2018 at 03:42:57PM +0200, Christian König wrote:
> >> Don't grab the reservation lock any more and simplify the handling quite
> >> a bit.
> >>
> >> Signed-off-by: Christian König 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 109 
> >> -
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  46 
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |   8 +--
> >>   3 files changed, 43 insertions(+), 120 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index 899342c6dfad..1cbc372964f8 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -2954,54 +2954,6 @@ static int amdgpu_device_ip_post_soft_reset(struct 
> >> amdgpu_device *adev)
> >>return 0;
> >>   }
> >>   
> >> -/**
> >> - * amdgpu_device_recover_vram_from_shadow - restore shadowed VRAM buffers
> >> - *
> >> - * @adev: amdgpu_device pointer
> >> - * @ring: amdgpu_ring for the engine handling the buffer operations
> >> - * @bo: amdgpu_bo buffer whose shadow is being restored
> >> - * @fence: dma_fence associated with the operation
> >> - *
> >> - * Restores the VRAM buffer contents from the shadow in GTT.  Used to
> >> - * restore things like GPUVM page tables after a GPU reset where
> >> - * the contents of VRAM might be lost.
> >> - * Returns 0 on success, negative error code on failure.
> >> - */
> >> -static int amdgpu_device_recover_vram_from_shadow(struct amdgpu_device 
> >> *adev,
> >> -struct amdgpu_ring *ring,
> >> -struct amdgpu_bo *bo,
> >> -struct dma_fence **fence)
> >> -{
> >> -  uint32_t domain;
> >> -  int r;
> >> -
> >> -  if (!bo->shadow)
> >> -  return 0;
> >> -
> >> -  r = amdgpu_bo_reserve(bo, true);
> >> -  if (r)
> >> -  return r;
> >> -  domain = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
> >> -  /* if bo has been evicted, then no need to recover */
> >> -  if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
> >> -  r = amdgpu_bo_validate(bo->shadow);
> >> -  if (r) {
> >> -  DRM_ERROR("bo validate failed!\n");
> >> -  goto err;
> >> -  }
> >> -
> >> -  r = amdgpu_bo_restore_from_shadow(adev, ring, bo,
> >> -   NULL, fence, true);
> >> -  if (r) {
> >> -  DRM_ERROR("recover page table failed!\n");
> >> -  goto err;
> >> -  }
> >> -  }
> >> -err:
> >> -  amdgpu_bo_unreserve(bo);
> >> -  return r;
> >> -}
> >> -
> >>   /**
> >>* amdgpu_device_recover_vram - Recover some VRAM contents
> >>*
> >> @@ -3010,16 +2962,15 @@ static int 
> >> amdgpu_device_recover_vram_from_shadow(struct amdgpu_device *adev,
> >>* Restores the contents of VRAM buffers from the shadows in GTT.  Used 
> >> to
> >>* restore things like GPUVM page tables after a GPU reset where
> >>* the contents of VRAM might be lost.
> >> - * Returns 0 on success, 1 on failure.
> >> + *
> >> + * Returns:
> >> + * 0 on success, negative error code on failure.
> >>*/
> >>   static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
> >>   {
> >> -  struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
> >> -  struct amdgpu_bo *bo, *tmp;
> >>struct dma_fence *fence = NULL, *next = NULL;
> >> -  long r = 1;
> >> -  int i = 0;
> >> -  long tmo;
> >> +  struct amdgpu_bo *shadow;
> >> +  long r = 1, tmo;
> >>   
> >>if (amdgpu_sriov_runtime(adev))
> >>tmo = msecs_to_jiffies(8000);
> >> @@ -3028,44 +2979,40 @@ static int amdgpu_device_recover_vram(struct 
> >> amdgpu_device *adev)
> >>   
> >>DRM_INFO("recover vram bo from shadow start\n");
> >>mutex_lock(>shadow_list_lock);
> >> -  list_for_each_entry_safe(bo, tmp, >shadow_list, shadow_list) {
> >> -  next = NULL;
> >> -  amdgpu_device_recover_vram_from_shadow(adev, ring, bo, );
> >> +  list_for_each_entry(shadow, >shadow_list, shadow_list) {
> >> +
> >> +  /* No need to recover an evicted BO */
> >> +  if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
> >> +  shadow->parent->tbo.mem.mem_type != TTM_PL_VRAM)
> >> +  continue;
> >> +
> >> +  r = amdgpu_bo_restore_shadow(shadow, );
> >> +  if (r)
> >> +  break;
> >> +
> >>if (fence) {
> >>r = dma_fence_wait_timeout(fence, false, tmo);
> >> -  if (r == 0)
> >> -  pr_err("wait fence %p[%d] timeout\n", fence, i);
> >> -  else if (r < 0)
> >> -  pr_err("wait fence %p[%d] interrupted\n", 
> >> fence, i);
> >> 

Re: [PATCH] [RFC]drm: add syncobj timeline support v5

2018-09-18 Thread Christian König

Am 17.09.2018 um 11:33 schrieb zhoucm1:

[SNIP]

  +static struct dma_fence *
+drm_syncobj_point_get(struct drm_syncobj *syncobj, u64 point, u64 
flags)

+{
+    struct dma_fence *fence;
+    int ret = 0;
+
+    if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
+    ret = wait_event_timeout(syncobj->syncobj_timeline.wq,
+    point <= syncobj->syncobj_timeline.signal_point,
+    msecs_to_jiffies(1)); /* wait 10s */


Either don't use a timeout or provide the timeout explicitely, but 
don't hard code it here.


Additional to that we probably need an incorruptible wait.
Initially, I used interruptible wait, I found it sometimes often 
return -ERESTARTSYS without waiting any second when I wrote unit test.

Any ideas for that?


Yeah, that is perfectly normal. The application just received a signal 
and we need to abort the IOCTL ASAP.



still need to interruptible wait?


Yes, that should certainly be an interruptible wait.

Otherwise an application could block on this forever when somebody 
forgets to supply a signal point.



  -    for (i = 0; i < args->count_handles; i++)
-    drm_syncobj_replace_fence(syncobjs[i], 0, NULL);
-
+    for (i = 0; i < args->count_handles; i++) {
+    if (syncobjs[i]->type == DRM_SYNCOBJ_TYPE_TIMELINE) {
+    DRM_ERROR("timeline syncobj cannot reset!\n");


Why not? I mean that should still work or do I miss anything?
timeline semaphore spec doesn't require reset interface, it says the 
timeline value only can be changed by signal operations.


Yeah, but we don't care about the timeline spec in the kernel.

Question is rather if that still makes sense to support that and as far 
as I can see it should be trivial to reinitialize the object.



+    ret = -EINVAL;
+    goto out;
+    }
+    drm_syncobj_timeline_fini(syncobjs[i],
+  [i]->syncobj_timeline);
+    drm_syncobj_timeline_init(syncobjs[i],
+  [i]->syncobj_timeline);
+    }
+out:
  drm_syncobj_array_free(syncobjs, args->count_handles);
  -    return 0;
+    return ret;
  }
    int
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c

index 0a8d2d64f380..579e91a5858b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -2137,7 +2137,9 @@ await_fence_array(struct i915_execbuffer *eb,
  if (!(flags & I915_EXEC_FENCE_WAIT))
  continue;
  -    fence = drm_syncobj_fence_get(syncobj);
+    drm_syncobj_search_fence(syncobj, 0,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,


Not sure if we can make DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT the 
default here.


Maybe better if drivers explicitly need to ask for it?
if you don't like the flag used in specific driver, I can wrap it to a 
new function.


Yeah, that is probably a good idea.




+ );
  if (!fence)
  return -EINVAL;
  diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 425432b85a87..9535521e6623 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -30,6 +30,25 @@
    struct drm_syncobj_cb;
  +enum drm_syncobj_type {
+    DRM_SYNCOBJ_TYPE_NORMAL,


Does anybody have a better suggestion for _TYPE_NORMAL? That sounds 
like timeline would be special.

How about _TYPE_INDIVIDUAL?


Oh, really good idea.

btw: I encounter a problem after removing advanced wait pt, I debug it 
two days, but still don't find reason, from log, it's timeount when 
wait_event_timeout, that means it don't receive signal operation.


Good question. Maybe a race condition? Have you tried to trace both 
threads to the trace buffer, e.g. use trace_printk?


"./deqp-vk -n dEQP-VK*semaphore*" will fail on 
'dEQP-VK.synchronization.basic.semaphore.chain' case, but can pass if 
only run that one case like "./deqp-vk -n 
dEQP-VK.synchronization.basic.semaphore.chain".


Both old and my previous implementation can pass and have no this 
problem.


Can you reproduce that as well with the unit tests in libdrm?

Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

2018-09-18 Thread Zhu, Rex


> -Original Message-
> From: Christian König 
> Sent: Tuesday, September 18, 2018 3:14 PM
> To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> 
> Am 18.09.2018 um 08:16 schrieb Zhu, Rex:
> >
> >> -Original Message-
> >> From: amd-gfx  On Behalf Of
> >> Christian König
> >> Sent: Tuesday, September 18, 2018 2:07 AM
> >> To: amd-gfx@lists.freedesktop.org
> >> Subject: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> >>
> >> Don't try to unreserve a BO we doesn't allocated.
> >>
> >> Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs
> >>
> >> Signed-off-by: Christian König 
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> index 84d82d5382f9..c1387efc0c91 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> @@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct
> amdgpu_device
> >> *adev,
> >>if (r)
> >>return r;
> >> -  amdgpu_bo_unreserve(*bo_ptr);
> >> +  if (*bo_ptr)
> >> +  amdgpu_bo_unreserve(*bo_ptr);
> >>
> >>return 0;
> >>   }
> > It is weird.
> > If we return true for allocate bo with size  0.
> > Does that mean we need to check all the bo_ptr before we use them.
> 
> No, allocating a BO with zero size doesn't make much sense and was
> essentially undefined behavior previously.
> 
> So now we get a defined behavior, but not necessary the one you expected.
> 
> Is that only a rhetorical question or really a problem somewhere?

Logically, the code is trick. It also make the  code
if (r)
return r; 
redundant.

Regards
Rex

> Regards,
> Christian.
> 
> >
> > Best Regards
> > Rex
> >
> >> --
> >> 2.14.1
> >>
> >> ___
> >> amd-gfx mailing list
> >> amd-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

2018-09-18 Thread Christian König

Am 17.09.2018 um 20:41 schrieb James Zhu:



On 2018-09-17 02:07 PM, Christian König wrote:

Don't try to unreserve a BO we doesn't allocated.

Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device 
*adev,

  if (r)
  return r;
  -    amdgpu_bo_unreserve(*bo_ptr);
+    if (*bo_ptr)

Can we put this check inside ttm_bo_unreserve?


No, calling amdgpu_bo_unreserve() with a NULL pointer is certainly a bug.

Christian.


James Zhu

+    amdgpu_bo_unreserve(*bo_ptr);
    return 0;
  }


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

2018-09-18 Thread Christian König

Am 18.09.2018 um 08:16 schrieb Zhu, Rex:



-Original Message-
From: amd-gfx  On Behalf Of
Christian König
Sent: Tuesday, September 18, 2018 2:07 AM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

Don't try to unreserve a BO we doesn't allocated.

Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 84d82d5382f9..c1387efc0c91 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device
*adev,
if (r)
return r;

-   amdgpu_bo_unreserve(*bo_ptr);
+   if (*bo_ptr)
+   amdgpu_bo_unreserve(*bo_ptr);

return 0;
  }

It is weird.
If we return true for allocate bo with size  0.
Does that mean we need to check all the bo_ptr before we use them.


No, allocating a BO with zero size doesn't make much sense and was 
essentially undefined behavior previously.


So now we get a defined behavior, but not necessary the one you expected.

Is that only a rhetorical question or really a problem somewhere?

Regards,
Christian.



Best Regards
Rex
  

--
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer

2018-09-18 Thread Zhu, Rex


> -Original Message-
> From: amd-gfx  On Behalf Of
> Christian König
> Sent: Tuesday, September 18, 2018 2:07 AM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH] drm/amdgpu: don't try to unreserve NULL pointer
> 
> Don't try to unreserve a BO we doesn't allocated.
> 
> Fixes: 07012fdd497e drm/amdgpu: don't allocate zero sized kernel BOs
> 
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 84d82d5382f9..c1387efc0c91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -348,7 +348,8 @@ int amdgpu_bo_create_kernel(struct amdgpu_device
> *adev,
>   if (r)
>   return r;
> 
> - amdgpu_bo_unreserve(*bo_ptr);
> + if (*bo_ptr)
> + amdgpu_bo_unreserve(*bo_ptr);
> 
>   return 0;
>  }
It is weird. 
If we return true for allocate bo with size  0.
Does that mean we need to check all the bo_ptr before we use them.

Best Regards
Rex
 
> --
> 2.14.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 5/5] drm/amdgpu: fix shadow BO restoring

2018-09-18 Thread Zhang, Jerry(Junwei)

On 09/14/2018 07:54 PM, Christian König wrote:

Am 13.09.2018 um 11:29 schrieb Zhang, Jerry(Junwei):

On 09/11/2018 05:56 PM, Christian König wrote:
Don't grab the reservation lock any more and simplify the handling 
quite

a bit.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 109 
-

  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  46 
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |   8 +--
  3 files changed, 43 insertions(+), 120 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

index 5eba66ecf668..20bb702f5c7f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2940,54 +2940,6 @@ static int 
amdgpu_device_ip_post_soft_reset(struct amdgpu_device *adev)

  return 0;
  }
  -/**
- * amdgpu_device_recover_vram_from_shadow - restore shadowed VRAM 
buffers

- *
- * @adev: amdgpu_device pointer
- * @ring: amdgpu_ring for the engine handling the buffer operations
- * @bo: amdgpu_bo buffer whose shadow is being restored
- * @fence: dma_fence associated with the operation
- *
- * Restores the VRAM buffer contents from the shadow in GTT. Used to
- * restore things like GPUVM page tables after a GPU reset where
- * the contents of VRAM might be lost.
- * Returns 0 on success, negative error code on failure.
- */
-static int amdgpu_device_recover_vram_from_shadow(struct 
amdgpu_device *adev,

-  struct amdgpu_ring *ring,
-  struct amdgpu_bo *bo,
-  struct dma_fence **fence)
-{
-    uint32_t domain;
-    int r;
-
-    if (!bo->shadow)
-    return 0;
-
-    r = amdgpu_bo_reserve(bo, true);
-    if (r)
-    return r;
-    domain = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
-    /* if bo has been evicted, then no need to recover */
-    if (domain == AMDGPU_GEM_DOMAIN_VRAM) {
-    r = amdgpu_bo_validate(bo->shadow);
-    if (r) {
-    DRM_ERROR("bo validate failed!\n");
-    goto err;
-    }
-
-    r = amdgpu_bo_restore_from_shadow(adev, ring, bo,
- NULL, fence, true);
-    if (r) {
-    DRM_ERROR("recover page table failed!\n");
-    goto err;
-    }
-    }
-err:
-    amdgpu_bo_unreserve(bo);
-    return r;
-}
-
  /**
   * amdgpu_device_recover_vram - Recover some VRAM contents
   *
@@ -2996,16 +2948,15 @@ static int 
amdgpu_device_recover_vram_from_shadow(struct amdgpu_device *adev,
   * Restores the contents of VRAM buffers from the shadows in GTT.  
Used to

   * restore things like GPUVM page tables after a GPU reset where
   * the contents of VRAM might be lost.
- * Returns 0 on success, 1 on failure.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
   */
  static int amdgpu_device_recover_vram(struct amdgpu_device *adev)
  {
-    struct amdgpu_ring *ring = adev->mman.buffer_funcs_ring;
-    struct amdgpu_bo *bo, *tmp;
  struct dma_fence *fence = NULL, *next = NULL;
-    long r = 1;
-    int i = 0;
-    long tmo;
+    struct amdgpu_bo *shadow;
+    long r = 1, tmo;
    if (amdgpu_sriov_runtime(adev))
  tmo = msecs_to_jiffies(8000);
@@ -3014,44 +2965,40 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)

    DRM_INFO("recover vram bo from shadow start\n");
  mutex_lock(>shadow_list_lock);
-    list_for_each_entry_safe(bo, tmp, >shadow_list, 
shadow_list) {

-    next = NULL;
-    amdgpu_device_recover_vram_from_shadow(adev, ring, bo, );
+    list_for_each_entry(shadow, >shadow_list, shadow_list) {
+
+    /* No need to recover an evicted BO */
+    if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
+    shadow->parent->tbo.mem.mem_type != TTM_PL_VRAM)

is there a change that shadow bo evicted to other domain?
like SYSTEM?


Yes, that's why I test "!= TTM_PL_TT" here.

What can happen is that either the shadow or the page table or page 
directory is evicted.


But in this case we don't need to restore anything because of patch #1 
in this series.


Thanks, then it's
Acked-by: Junwei Zhang 

Regards,
Jerry



Regards,
Christian.



Regards,
Jerry

+    continue;
+
+    r = amdgpu_bo_restore_shadow(shadow, );
+    if (r)
+    break;
+
  if (fence) {
  r = dma_fence_wait_timeout(fence, false, tmo);
-    if (r == 0)
-    pr_err("wait fence %p[%d] timeout\n", fence, i);
-    else if (r < 0)
-    pr_err("wait fence %p[%d] interrupted\n", fence, i);
-    if (r < 1) {
-    dma_fence_put(fence);
-    fence = next;
+    dma_fence_put(fence);
+    fence = next;
+    if (r <= 0)
  break;
-    }
-    i++;
+    } else {
+    fence = next;
  }
-
-    dma_fence_put(fence);
-    fence = next;
 

Re: [PATCH libdrm 3/3] test/amdgpu: add GDS, GWS and OA tests

2018-09-18 Thread Zhang, Jerry(Junwei)

On 09/14/2018 09:09 PM, Christian König wrote:

Add allocation tests for GDW, GWS and OA.

Signed-off-by: Christian König 
---
  tests/amdgpu/amdgpu_test.h | 48 +-
  tests/amdgpu/bo_tests.c| 21 
  2 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h
index d1e14e23..af3041e5 100644
--- a/tests/amdgpu/amdgpu_test.h
+++ b/tests/amdgpu/amdgpu_test.h
@@ -207,11 +207,9 @@ static inline amdgpu_bo_handle gpu_mem_alloc(
amdgpu_va_handle *va_handle)
  {
struct amdgpu_bo_alloc_request req = {0};
-   amdgpu_bo_handle buf_handle;
+   amdgpu_bo_handle buf_handle = NULL;
int r;
  
-	CU_ASSERT_NOT_EQUAL(vmc_addr, NULL);

-
req.alloc_size = size;
req.phys_alignment = alignment;
req.preferred_heap = type;
@@ -222,16 +220,19 @@ static inline amdgpu_bo_handle gpu_mem_alloc(
if (r)
return NULL;
  
-	r = amdgpu_va_range_alloc(device_handle,

- amdgpu_gpu_va_range_general,
- size, alignment, 0, vmc_addr,
- va_handle, 0);
-   CU_ASSERT_EQUAL(r, 0);
-   if (r)
-   goto error_free_bo;
-
-   r = amdgpu_bo_va_op(buf_handle, 0, size, *vmc_addr, 0, 
AMDGPU_VA_OP_MAP);
-   CU_ASSERT_EQUAL(r, 0);
+   if (vmc_addr && va_handle) {
+   r = amdgpu_va_range_alloc(device_handle,
+ amdgpu_gpu_va_range_general,
+ size, alignment, 0, vmc_addr,
+ va_handle, 0);
+   CU_ASSERT_EQUAL(r, 0);
+   if (r)
+   goto error_free_bo;
+
+   r = amdgpu_bo_va_op(buf_handle, 0, size, *vmc_addr, 0,
+   AMDGPU_VA_OP_MAP);
+   CU_ASSERT_EQUAL(r, 0);


Error check for bo map here as well.

Regards,
Jerry

+   }
  
  	return buf_handle;
  
@@ -256,15 +257,18 @@ static inline int gpu_mem_free(amdgpu_bo_handle bo,

if (!bo)
return 0;
  
-	r = amdgpu_bo_va_op(bo, 0, size, vmc_addr, 0, AMDGPU_VA_OP_UNMAP);

-   CU_ASSERT_EQUAL(r, 0);
-   if (r)
-   return r;
-
-   r = amdgpu_va_range_free(va_handle);
-   CU_ASSERT_EQUAL(r, 0);
-   if (r)
-   return r;
+   if (va_handle) {
+   r = amdgpu_bo_va_op(bo, 0, size, vmc_addr, 0,
+   AMDGPU_VA_OP_UNMAP);
+   CU_ASSERT_EQUAL(r, 0);
+   if (r)
+   return r;
+
+   r = amdgpu_va_range_free(va_handle);
+   CU_ASSERT_EQUAL(r, 0);
+   if (r)
+   return r;
+   }
  
  	r = amdgpu_bo_free(bo);

CU_ASSERT_EQUAL(r, 0);
diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
index dc2de9b7..7cff4cf7 100644
--- a/tests/amdgpu/bo_tests.c
+++ b/tests/amdgpu/bo_tests.c
@@ -242,6 +242,27 @@ static void amdgpu_memory_alloc(void)
  
  	r = gpu_mem_free(bo, va_handle, bo_mc, 4096);

CU_ASSERT_EQUAL(r, 0);
+
+   /* Test GDS */
+   bo = gpu_mem_alloc(device_handle, 1024, 0,
+   AMDGPU_GEM_DOMAIN_GDS, 0,
+   NULL, NULL);
+   r = gpu_mem_free(bo, NULL, 0, 4096);
+   CU_ASSERT_EQUAL(r, 0);
+
+   /* Test GWS */
+   bo = gpu_mem_alloc(device_handle, 1, 0,
+   AMDGPU_GEM_DOMAIN_GWS, 0,
+   NULL, NULL);
+   r = gpu_mem_free(bo, NULL, 0, 4096);
+   CU_ASSERT_EQUAL(r, 0);
+
+   /* Test OA */
+   bo = gpu_mem_alloc(device_handle, 1, 0,
+   AMDGPU_GEM_DOMAIN_OA, 0,
+   NULL, NULL);
+   r = gpu_mem_free(bo, NULL, 0, 4096);
+   CU_ASSERT_EQUAL(r, 0);
  }
  
  static void amdgpu_mem_fail_alloc(void)


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH libdrm 1/3] amdgpu: remove invalid check in amdgpu_bo_alloc

2018-09-18 Thread Zhang, Jerry(Junwei)

On 09/14/2018 09:09 PM, Christian König wrote:

The heap is checked by the kernel and not libdrm, to make it even worse
it prevented allocating resources other than VRAM and GTT.

Signed-off-by: Christian König 

Reviewed-by: Junwei Zhang 


---
  amdgpu/amdgpu_bo.c | 9 ++---
  1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 6a95929c..34904e38 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -74,19 +74,14 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
amdgpu_bo_handle *buf_handle)
  {
union drm_amdgpu_gem_create args;
-   unsigned heap = alloc_buffer->preferred_heap;
-   int r = 0;
-
-   /* It's an error if the heap is not specified */
-   if (!(heap & (AMDGPU_GEM_DOMAIN_GTT | AMDGPU_GEM_DOMAIN_VRAM)))
-   return -EINVAL;
+   int r;
  
  	memset(, 0, sizeof(args));

args.in.bo_size = alloc_buffer->alloc_size;
args.in.alignment = alloc_buffer->phys_alignment;
  
  	/* Set the placement. */

-   args.in.domains = heap;
+   args.in.domains = alloc_buffer->preferred_heap;
args.in.domain_flags = alloc_buffer->flags;
  
  	/* Allocate the buffer with the preferred heap. */


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH libdrm 2/3] test/amdgpu: add proper error handling

2018-09-18 Thread Zhang, Jerry(Junwei)

On 09/14/2018 09:09 PM, Christian König wrote:

Otherwise the calling function won't notice that something is wrong.

Signed-off-by: Christian König 
---
  tests/amdgpu/amdgpu_test.h | 23 ++-
  1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h
index f2ece3c3..d1e14e23 100644
--- a/tests/amdgpu/amdgpu_test.h
+++ b/tests/amdgpu/amdgpu_test.h
@@ -219,17 +219,31 @@ static inline amdgpu_bo_handle gpu_mem_alloc(
  
  	r = amdgpu_bo_alloc(device_handle, , _handle);

CU_ASSERT_EQUAL(r, 0);
+   if (r)
+   return NULL;
  
  	r = amdgpu_va_range_alloc(device_handle,

  amdgpu_gpu_va_range_general,
  size, alignment, 0, vmc_addr,
  va_handle, 0);
CU_ASSERT_EQUAL(r, 0);
+   if (r)
+   goto error_free_bo;
  
  	r = amdgpu_bo_va_op(buf_handle, 0, size, *vmc_addr, 0, AMDGPU_VA_OP_MAP);

CU_ASSERT_EQUAL(r, 0);


We may also add error check for bo map.

Regards,
Jerry
  
  	return buf_handle;

+
+error_free_va:
+   r = amdgpu_va_range_free(*va_handle);
+   CU_ASSERT_EQUAL(r, 0);
+
+error_free_bo:
+   r = amdgpu_bo_free(buf_handle);
+   CU_ASSERT_EQUAL(r, 0);
+
+   return NULL;
  }
  
  static inline int gpu_mem_free(amdgpu_bo_handle bo,

@@ -239,16 +253,23 @@ static inline int gpu_mem_free(amdgpu_bo_handle bo,
  {
int r;
  
+	if (!bo)

+   return 0;
+
r = amdgpu_bo_va_op(bo, 0, size, vmc_addr, 0, AMDGPU_VA_OP_UNMAP);
CU_ASSERT_EQUAL(r, 0);
+   if (r)
+   return r;
  
  	r = amdgpu_va_range_free(va_handle);

CU_ASSERT_EQUAL(r, 0);
+   if (r)
+   return r;
  
  	r = amdgpu_bo_free(bo);

CU_ASSERT_EQUAL(r, 0);
  
-	return 0;

+   return r;
  }
  
  static inline int


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] list: introduce list_bulk_move_tail helper

2018-09-18 Thread Zhang, Jerry(Junwei)

On 09/17/2018 08:08 PM, Christian König wrote:

Move all entries between @first and including @last before @head.

This is useful for LRU lists where a whole block of entries should be
moved to the end of the list.

Used as a band aid in TTM, but better placed in the common list headers.

Signed-off-by: Christian König 

Reviewed-by: Junwei Zhang 

---
  drivers/gpu/drm/ttm/ttm_bo.c | 25 +
  include/linux/list.h | 23 +++
  2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index b2a33bf1ef10..26b889f86670 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -247,20 +247,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
  }
  EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
  
-static void ttm_list_move_bulk_tail(struct list_head *list,

-   struct list_head *first,
-   struct list_head *last)
-{
-   first->prev->next = last->next;
-   last->next->prev = first->prev;
-
-   list->prev->next = first;
-   first->prev = list->prev;
-
-   last->next = list;
-   list->prev = last;
-}
-
  void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
  {
unsigned i;
@@ -276,8 +262,8 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
  
  		man = >first->bdev->man[TTM_PL_TT];

-   ttm_list_move_bulk_tail(>lru[i], >first->lru,
-   >last->lru);
+   list_bulk_move_tail(>lru[i], >first->lru,
+   >last->lru);
}
  
  	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {

@@ -291,8 +277,8 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
  
  		man = >first->bdev->man[TTM_PL_VRAM];

-   ttm_list_move_bulk_tail(>lru[i], >first->lru,
-   >last->lru);
+   list_bulk_move_tail(>lru[i], >first->lru,
+   >last->lru);
}
  
  	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {

@@ -306,8 +292,7 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move 
*bulk)
reservation_object_assert_held(pos->last->resv);
  
  		lru = >first->bdev->glob->swap_lru[i];

-   ttm_list_move_bulk_tail(lru, >first->swap,
-   >last->swap);
+   list_bulk_move_tail(lru, >first->swap, >last->swap);
}
  }
  EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
diff --git a/include/linux/list.h b/include/linux/list.h
index de04cc5ed536..edb7628e46ed 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -183,6 +183,29 @@ static inline void list_move_tail(struct list_head *list,
list_add_tail(list, head);
  }
  
+/**

+ * list_bulk_move_tail - move a subsection of a list to its tail
+ * @head: the head that will follow our entry
+ * @first: first entry to move
+ * @last: last entry to move, can be the same as first
+ *
+ * Move all entries between @first and including @last before @head.
+ * All three entries must belong to the same linked list.
+ */
+static inline void list_bulk_move_tail(struct list_head *head,
+  struct list_head *first,
+  struct list_head *last)
+{
+   first->prev->next = last->next;
+   last->next->prev = first->prev;
+
+   head->prev->next = first;
+   first->prev = head->prev;
+
+   last->next = head;
+   head->prev = last;
+}
+
  /**
   * list_is_last - tests whether @list is the last entry in list @head
   * @list: the entry to test


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx