Re: [Intel-gfx] [PATCH] Revert "drm/i915: disable set/get_tiling ioctl on gen12+"
On Thu, Nov 21, 2019 at 06:11:01PM +, Chris Wilson wrote:
Restore the DRI2/DRI3 uABI backchannel that was removed by ab016914984e
("drm/i915: disable set/get_tiling ioctl on gen12+") before the ABI
change was agreed upon.
Fixes: ab016914984e ("drm/i915: disable set/get_tiling ioctl on gen12+")
Signed-off-by: Chris Wilson
Cc: Joonas Lahtinen
Acked-by: Vanshidhar Konda
---
drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index 1fa592d82af5..39f3bd5defd6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -317,14 +317,10 @@ int
i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_i915_gem_set_tiling *args = data;
struct drm_i915_gem_object *obj;
int err;
- if (!dev_priv->ggtt.num_fences)
- return -EOPNOTSUPP;
-
obj = i915_gem_object_lookup(file, args->handle);
if (!obj)
return -ENOENT;
@@ -405,9 +401,6 @@ i915_gem_get_tiling_ioctl(struct drm_device *dev, void
*data,
struct drm_i915_gem_object *obj;
int err = -ENOENT;
- if (!dev_priv->ggtt.num_fences)
- return -EOPNOTSUPP;
-
rcu_read_lock();
obj = i915_gem_object_lookup_rcu(file, args->handle);
if (obj) {
--
2.24.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v5 4/4] tests/gem_ctx_shared: Align objects using minimum GTT alignment
On Mon, Nov 04, 2019 at 06:13:30PM +0100, Janusz Krzysztofik wrote: exec-shared-gtt-* subtests use hardcoded values for object size and softpin offset, based on 4kB GTT alignment assumption. That may result in those subtests failing when run on future backing stores with possibly larger minimum page sizes. Replace hardcoded constants with values calculated from minimum GTT alignment of actual backing store the test is running on. v2: Update helper name, use 'minimum GTT alignment' term across the change, adjust variable name. v3: Name the variable 'stride', not 'alignment', it better reflects its purpose (Chris). Signed-off-by: Janusz Krzysztofik Reviewed-by: Chris Wilson --- tests/i915/gem_ctx_shared.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/i915/gem_ctx_shared.c b/tests/i915/gem_ctx_shared.c index f7852482..cb726b4d 100644 --- a/tests/i915/gem_ctx_shared.c +++ b/tests/i915/gem_ctx_shared.c @@ -41,6 +41,7 @@ #include "igt_rand.h" #include "igt_vgem.h" #include "sync_file.h" +#include "i915/gem_gtt_topology.c" #define LO 0 #define HI 1 @@ -195,6 +196,7 @@ static void exec_shared_gtt(int i915, unsigned int ring) uint32_t scratch, *s; uint32_t batch, cs[16]; uint64_t offset; + uint64_t stride; int i; gem_require_ring(i915, ring); @@ -203,7 +205,8 @@ static void exec_shared_gtt(int i915, unsigned int ring) clone = gem_context_clone(i915, 0, I915_CONTEXT_CLONE_VM, 0); /* Find a hole big enough for both objects later */ - scratch = gem_create(i915, 16384); + stride = 2 * gem_gtt_min_alignment(i915); + scratch = gem_create(i915, 2 * stride); gem_write(i915, scratch, 0, , sizeof(bbe)); obj.handle = scratch; gem_execbuf(i915, ); @@ -246,7 +249,7 @@ static void exec_shared_gtt(int i915, unsigned int ring) gem_write(i915, batch, 0, cs, sizeof(cs)); obj.handle = batch; - obj.offset += 8192; /* make sure we don't cause an eviction! */ + obj.offset += stride; /* make sure we don't cause an eviction! */ execbuf.rsvd1 = clone; if (gen > 3 && gen < 6) execbuf.flags |= I915_EXEC_SECURE; -- 2.21.0 Reviewed-by: Vanshidhar Konda ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v5 3/4] tests/gem_exec_reloc: Calculate offsets from minimum GTT alignment
On Mon, Nov 04, 2019 at 06:13:29PM +0100, Janusz Krzysztofik wrote:
The basic-range subtest assumes 4kB alignment while calculating softpin
offsets. On future backends with possibly larger minimum page sizes
the test will fail as a half of calculated offsets to be tested will be
incorrectly aligned.
Replace hardcoded constants corresponding to the assumed 4kB page size
with variables initialized with actual minimum GTT alignment size and
order.
v2: Simplify the code by reversing the size->order conversion,
- drop irrelevant modifications of requested object sizes.
v3: Reword commit message after removal of patch "Don't filter out
addresses on full PPGTT" from the series,
- initialize page size order with an actual minimum returned by a new
helper (inspired by Chris).
v4: Update the helper name, use the term 'minimum GTT alignment' across
the change, adjust variable names,
- refresh the commit message on top of the reintroduced patch that
fixes invalid offsets incorrectly assumed as occupied.
Signed-off-by: Janusz Krzysztofik
Cc: Katarzyna Dec
Cc: Stuart Summers
Reviewed-by: Chris Wilson
---
tests/i915/gem_exec_reloc.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index e46a4df7..a2a04343 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -521,14 +521,16 @@ static void basic_range(int fd, unsigned flags)
uint64_t gtt_size = gem_aperture_size(fd);
const uint32_t bbe = MI_BATCH_BUFFER_END;
igt_spin_t *spin = NULL;
+ int alignment_order = gem_gtt_min_alignment_order(fd);
+ uint64_t alignment = 1ull << alignment_order;
int count, n, err;
igt_require(gem_has_softpin(fd));
- for (count = 12; gtt_size >> (count + 1); count++)
+ for (count = alignment_order; gtt_size >> (count + 1); count++)
;
- count -= 12;
+ count -= alignment_order;
memset(obj, 0, sizeof(obj));
memset(reloc, 0, sizeof(reloc));
@@ -537,7 +539,7 @@ static void basic_range(int fd, unsigned flags)
n = 0;
for (int i = 0; i <= count; i++) {
obj[n].handle = gem_create(fd, 4096);
- obj[n].offset = (1ull << (i + 12)) - 4096;
+ obj[n].offset = (1ull << (i + alignment_order)) - alignment;
obj[n].offset = gen8_canonical_address(obj[n].offset);
obj[n].flags = EXEC_OBJECT_PINNED |
EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
err = gem_gtt_validate_object(fd, [n]);
@@ -558,7 +560,7 @@ static void basic_range(int fd, unsigned flags)
}
for (int i = 1; i < count; i++) {
obj[n].handle = gem_create(fd, 4096);
- obj[n].offset = 1ull << (i + 12);
+ obj[n].offset = 1ull << (i + alignment_order);
obj[n].offset = gen8_canonical_address(obj[n].offset);
obj[n].flags = EXEC_OBJECT_PINNED |
EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
err = gem_gtt_validate_object(fd, [n]);
--
2.21.0
Reviewed-by: Vanshidhar Konda
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v5 2/4] tests/gem_exec_reloc: Don't filter out invalid addresses
On Mon, Nov 04, 2019 at 06:13:28PM +0100, Janusz Krzysztofik wrote:
Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable
addresses for !ppgtt") introduced filtering of addresses possibly
occupied by other users of shared GTT. Unfortunately, that filtering
doesn't distinguish between actually occupied addresses and otherwise
invalid softpin offsets. If incorrect GTT alignment will be assumed
when running on future backends with possibly larger minimum page
sizes, a half of calculated offsets to be tested will be incorrectly
detected as occupied by other users and silently skipped instead of
reported as a problem. That will significantly distort the intended
test pattern.
Filter out failing addresses only if reported as occupied.
v2: Skip unavailable addresses only when not running on full PPGTT.
v3: Replace the not on full PPGTT requirement for skipping with error
code checking.
v4: Silently skip only those offsets which have been explicitly
reported as overlapping with shared GTT reserved space, not simply
all which raise failures other than -EINVAL (Chris),
- as an implementation of moving the probe out of line so it's not
easily confused with the central point of the test (Chris), use
object validation library helper just introduced.
Signed-off-by: Janusz Krzysztofik
Cc: Chris Wilson
---
tests/i915/gem_exec_reloc.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index fdd9661d..e46a4df7 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -23,6 +23,7 @@
#include "igt.h"
#include "igt_dummyload.h"
+#include "i915/gem_gtt_topology.c"
IGT_TEST_DESCRIPTION("Basic sanity check of execbuf-ioctl relocations.");
@@ -520,7 +521,7 @@ static void basic_range(int fd, unsigned flags)
uint64_t gtt_size = gem_aperture_size(fd);
const uint32_t bbe = MI_BATCH_BUFFER_END;
igt_spin_t *spin = NULL;
- int count, n;
+ int count, n, err;
igt_require(gem_has_softpin(fd));
@@ -539,11 +540,12 @@ static void basic_range(int fd, unsigned flags)
obj[n].offset = (1ull << (i + 12)) - 4096;
obj[n].offset = gen8_canonical_address(obj[n].offset);
obj[n].flags = EXEC_OBJECT_PINNED |
EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
- gem_write(fd, obj[n].handle, 0, , sizeof(bbe));
- execbuf.buffers_ptr = to_user_pointer([n]);
- execbuf.buffer_count = 1;
- if (__gem_execbuf(fd, ))
+ err = gem_gtt_validate_object(fd, [n]);
Would it be better to name this function as
gem_gtt_validate_object_offset? From the earlier code is it was easy to
see that the intention of the test was to use gem_execbuf to map the
object to the offset. From the new method name unless I check the code
it's not straight forward that we are just checking the offset.
+ if (err) {
+ /* Iff using a shared GTT, some of it may be reserved */
Nit-pick: If, not Iff
+ igt_assert_eq(err, -ENOSPC);
continue;
+ }
igt_debug("obj[%d] handle=%d, address=%llx\n",
n, obj[n].handle, (long long)obj[n].offset);
@@ -559,11 +561,12 @@ static void basic_range(int fd, unsigned flags)
obj[n].offset = 1ull << (i + 12);
obj[n].offset = gen8_canonical_address(obj[n].offset);
obj[n].flags = EXEC_OBJECT_PINNED |
EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
- gem_write(fd, obj[n].handle, 0, , sizeof(bbe));
- execbuf.buffers_ptr = to_user_pointer([n]);
- execbuf.buffer_count = 1;
- if (__gem_execbuf(fd, ))
+ err = gem_gtt_validate_object(fd, [n]);
+ if (err) {
+ /* Iff using a shared GTT, some of it may be reserved */
Nit-pick: If, not Iff
+ igt_assert_eq(err, -ENOSPC);
continue;
+ }
igt_debug("obj[%d] handle=%d, address=%llx\n",
n, obj[n].handle, (long long)obj[n].offset);
--
Other than the above comments, looks good to me.
Reviewed-by: Vanshidhar Konda
Vanshi
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/selftests: Spin on all engines simultaneously
On Thu, Oct 31, 2019 at 09:23:36PM +, Chris Wilson wrote:
Vanshidhar Konda asked for the simplest test "to verify that the kernel
can submit and hardware can execute batch buffers on all the command
streamers in parallel." We have a number of tests in userspace that
submit load to each engine and verify that it is present, but strictly
we have no selftest to prove that the kernel can _simultaneously_
execute on all known engines. (We have tests to demonstrate that we can
submit to HW in parallel, but we don't insist that they execute in
parallel.)
Suggested-by: Vanshidhar Konda
Signed-off-by: Chris Wilson
Cc: Vanshidhar Konda
Cc: Matthew Auld
---
drivers/gpu/drm/i915/i915_drv.h | 6 ++
drivers/gpu/drm/i915/selftests/i915_request.c | 63 +++
2 files changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a22d969cb352..0c3ab6020bc6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -891,6 +891,10 @@ struct intel_cdclk_state {
u8 voltage_level;
};
+struct i915_selftest_stash {
+ atomic_t counter;
+};
+
struct drm_i915_private {
struct drm_device drm;
@@ -1286,6 +1290,8 @@ struct drm_i915_private {
/* Mutex to protect the above hdcp component related values. */
struct mutex hdcp_comp_mutex;
+ I915_SELFTEST_DECLARE(struct i915_selftest_stash selftest;)
+
/*
* NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
* will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c
b/drivers/gpu/drm/i915/selftests/i915_request.c
index 30ae34f62176..6181b327b4ac 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -32,6 +32,7 @@
#include "i915_random.h"
#include "i915_selftest.h"
#include "igt_live_test.h"
+#include "igt_spinner.h"
#include "lib_sw_fence.h"
#include "mock_drm.h"
@@ -1115,12 +1116,72 @@ static int __live_parallel_engineN(void *arg)
return 0;
}
+static int wait_for_all(struct drm_i915_private *i915)
+{
+ if (atomic_dec_and_test(>selftest.counter)) {
+ wake_up_var(>selftest.counter);
+ return 0;
+ }
+
+ if (wait_var_event_timeout(>selftest.counter,
+ !atomic_read(>selftest.counter),
+ i915_selftest.timeout_jiffies))
+ return 0;
+
+ return -ETIME;
+}
+
+static int __live_parallel_spin(void *arg)
+{
+ struct intel_engine_cs *engine = arg;
+ struct igt_spinner spin;
+ struct i915_request *rq;
+ int err = 0;
+
+ /*
+* Create a spinner running for eternity on each engine. If a second
+* spinner is incorrectly placed on the same engine, it will not be
+* able to start in time.
+*/
+
+ if (igt_spinner_init(, engine->gt))
+ return -ENOMEM;
+
+ rq = igt_spinner_create_request(,
+ engine->kernel_context,
+ MI_NOOP); /* no preemption */
+ if (IS_ERR(rq)) {
+ err = PTR_ERR(rq);
+ goto out_spin;
+ }
+
+ i915_request_get(rq);
+ i915_request_add(rq);
+ if (igt_wait_for_spinner(, rq)) {
+ /* Occupy this engine for the whole test */
+ err = wait_for_all(engine->i915);
+ } else {
+ pr_err("Failed to start spinner on %s\n", engine->name);
+ err = -EINVAL;
+ }
+ igt_spinner_end();
+
+ if (err == 0 && i915_request_wait(rq, 0, HZ / 5) < 0)
+ err = -EIO;
+ i915_request_put(rq);
+
+out_spin:
+ igt_spinner_fini();
+ return err;
+}
+
static int live_parallel_engines(void *arg)
{
struct drm_i915_private *i915 = arg;
static int (* const func[])(void *arg) = {
__live_parallel_engine1,
__live_parallel_engineN,
+ __live_parallel_spin,
NULL,
};
const unsigned int nengines = num_uabi_engines(i915);
@@ -1146,6 +1207,8 @@ static int live_parallel_engines(void *arg)
if (err)
break;
+ atomic_set(>selftest.counter, nengines);
+
idx = 0;
for_each_uabi_engine(engine, i915) {
tsk[idx] = kthread_run(*fn, engine,
--
2.24.0.rc2
Reviewed-by: Vanshidhar Konda
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH i-g-t v4 4/4] tests/gem_ctx_shared: Align objects using minimum GTT alignment
On Thu, Oct 31, 2019 at 04:28:57PM +0100, Janusz Krzysztofik wrote: exec-shared-gtt-* subtests use hardcoded values for object size and softpin offset, based on 4kB GTT alignment assumption. That may result in those subtests failing when run on future backing stores with possibly larger minimum page sizes. Replace hardcoded constants with values calculated from minimum GTT alignment of actual backing store the test is running on. v2: Update helper name, use 'minimum GTT alignment' term across the change, adjust variable name. Signed-off-by: Janusz Krzysztofik Cc: Chris Wilson --- tests/i915/gem_ctx_shared.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/i915/gem_ctx_shared.c b/tests/i915/gem_ctx_shared.c index 6d8cbcce..1e9c7f78 100644 --- a/tests/i915/gem_ctx_shared.c +++ b/tests/i915/gem_ctx_shared.c @@ -195,6 +195,7 @@ static void exec_shared_gtt(int i915, unsigned int ring) uint32_t scratch, *s; uint32_t batch, cs[16]; uint64_t offset; + uint64_t alignment; int i; gem_require_ring(i915, ring); @@ -203,7 +204,8 @@ static void exec_shared_gtt(int i915, unsigned int ring) clone = gem_context_clone(i915, 0, I915_CONTEXT_CLONE_VM, 0); /* Find a hole big enough for both objects later */ - scratch = gem_create(i915, 16384); + alignment = 2 * gem_gtt_min_alignment(i915); + scratch = gem_create(i915, 2 * alignment); gem_write(i915, scratch, 0, , sizeof(bbe)); obj.handle = scratch; gem_execbuf(i915, ); @@ -246,7 +248,7 @@ static void exec_shared_gtt(int i915, unsigned int ring) gem_write(i915, batch, 0, cs, sizeof(cs)); obj.handle = batch; - obj.offset += 8192; /* make sure we don't cause an eviction! */ + obj.offset += alignment; /* make sure we don't cause an eviction! */ execbuf.rsvd1 = clone; if (gen > 3 && gen < 6) execbuf.flags |= I915_EXEC_SECURE; -- 2.21.0 Looks good to me. Vanshi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH i-g-t v4 1/4] tests/gem_exec_reloc: Don't filter out invalid addresses
May be this patch can just be merged with the other patch in this series
that changes gem_exec_reloc.
Vanshi
On Thu, Oct 31, 2019 at 04:28:54PM +0100, Janusz Krzysztofik wrote:
Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable
addresses for !ppgtt") introduced filtering of addresses possibly
occupied by other users of shared GTT. Unfortunately, that filtering
doesn't distinguish between actually occupied addresses and otherwise
invalid softpin offsets. As soon as incorrect GTT alignment is assumed
when running on future backends with possibly larger minimum page
sizes, a half of calculated offsets to be tested will be incorrectly
detected as occupied by other users and silently skipped instead of
reported as a problem. That will significantly distort the intended
test pattern.
Filter out failing addresses only if not reported as invalid.
v2: Skip unavailable addresses only when not running on full PPGTT.
v3: Replace the not on full PPGTT requirement for skipping with error
code checking.
Signed-off-by: Janusz Krzysztofik
Cc: Chris Wilson
---
tests/i915/gem_exec_reloc.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index 5f59fe99..423fed8b 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -520,7 +520,7 @@ static void basic_range(int fd, unsigned flags)
uint64_t gtt_size = gem_aperture_size(fd);
const uint32_t bbe = MI_BATCH_BUFFER_END;
igt_spin_t *spin = NULL;
- int count, n;
+ int count, n, err;
igt_require(gem_has_softpin(fd));
@@ -542,8 +542,11 @@ static void basic_range(int fd, unsigned flags)
gem_write(fd, obj[n].handle, 0, , sizeof(bbe));
execbuf.buffers_ptr = to_user_pointer([n]);
execbuf.buffer_count = 1;
- if (__gem_execbuf(fd, ))
+ err = __gem_execbuf(fd, );
+ if (err) {
+ igt_assert(err != -EINVAL);
continue;
+ }
igt_debug("obj[%d] handle=%d, address=%llx\n",
n, obj[n].handle, (long long)obj[n].offset);
@@ -562,8 +565,11 @@ static void basic_range(int fd, unsigned flags)
gem_write(fd, obj[n].handle, 0, , sizeof(bbe));
execbuf.buffers_ptr = to_user_pointer([n]);
execbuf.buffer_count = 1;
- if (__gem_execbuf(fd, ))
+ err = __gem_execbuf(fd, );
+ if (err) {
+ igt_assert(err != -EINVAL);
continue;
+ }
igt_debug("obj[%d] handle=%d, address=%llx\n",
n, obj[n].handle, (long long)obj[n].offset);
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH i-g-t v4 2/4] lib: Add minimum GTT alignment helper
On Thu, Oct 31, 2019 at 04:28:55PM +0100, Janusz Krzysztofik wrote:
Some tests assume 4kB offset alignment while using softpin. That
assumption may be wrong on future GEM backends with possibly larger
minimum page sizes. As a result, those tests may either fail on
softpin at offsets which are incorrectly aligned, may silently skip
such incorrectly aligned addresses assuming them occupied by other
users if incorrect detection method is used, or may always succeed
when examining invalid use patterns.
Provide a helper function that detects minimum GTT alignment. Tests
may use it to calculate softpin offsets valid for actually used backing
store.
v2: Rename the helper, use 'minimum GTT alignment' term across the
change (Chris),
- use error numbers to distinguish between invalid offsets and
addresses occupied by other users, then
- simplify the code (Chris).
Signed-off-by: Janusz Krzysztofik
Cc: Chris Wilson
Cc: Daniele Ceraolo Spurio
Cc: Stuart Summers
---
lib/ioctl_wrappers.c | 46
lib/ioctl_wrappers.h | 2 ++
2 files changed, 48 insertions(+)
diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 628f8b83..f0ef8b2e 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -54,6 +54,7 @@
#include "intel_io.h"
#include "igt_debugfs.h"
#include "igt_sysfs.h"
+#include "igt_x86.h"
#include "config.h"
#ifdef HAVE_VALGRIND
@@ -1158,6 +1159,51 @@ bool gem_has_softpin(int fd)
return has_softpin;
}
+/**
+ * gem_gtt_min_alignment_order:
+ * @fd: open i915 drm file descriptor
+ *
+ * This function detects the minimum possible alignment of a soft-pinned gem
+ * object allocated from a default backing store. It is useful for calculating
+ * correctly aligned softpin offsets.
+ * Since size order to size conversion (size = 1 << order) is less trivial
+ * than the opposite, the function returns the alignment order as more handy.
+ *
+ * Returns:
+ * Size order of the minimum GTT alignment
+ */
+int gem_gtt_min_alignment_order(int fd)
+{
+ struct drm_i915_gem_exec_object2 obj;
+ struct drm_i915_gem_execbuffer2 eb;
+ const uint32_t bbe = MI_BATCH_BUFFER_END;
+ int order;
+
+ /* no softpin => 4kB page size */
+ if (!gem_has_softpin(fd))
+ return 12;
+
+ memset(, 0, sizeof(obj));
+ memset(, 0, sizeof(eb));
+
+ obj.handle = gem_create(fd, 4096);
+ obj.flags = EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+ eb.buffers_ptr = to_user_pointer();
+ eb.buffer_count = 1;
+ gem_write(fd, obj.handle, 0, , sizeof(bbe));
I think it will be safer to create a new context to execute this
execbuffer. For a new context the address space should be empty reducing
the chance that there is another object mapped by the caller of the
helper function at the address we start testing.
Otherwise it looks good to me.
Vanshi
+
+ for (order = 12; order < 64; order++) {
+ obj.offset = 1ull << order;
+ if (__gem_execbuf(fd, ) != -EINVAL)
+ break;
+ }
+ igt_assert(obj.offset < gem_aperture_size(fd));
+
+ gem_close(fd, obj.handle);
+ igt_debug("minimum GTT alignment is %#llx\n", (long long)obj.offset);
+ return order;
+}
+
/**
* gem_has_exec_fence:
* @fd: open i915 drm file descriptor
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 03211c97..c8d57a7c 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -138,6 +138,8 @@ uint64_t gem_aperture_size(int fd);
uint64_t gem_global_aperture_size(int fd);
uint64_t gem_mappable_aperture_size(void);
bool gem_has_softpin(int fd);
+int gem_gtt_min_alignment_order(int fd);
+#define gem_gtt_min_alignment(fd) (1ull << gem_gtt_min_alignment_order(fd))
bool gem_has_exec_fence(int fd);
/* check functions which auto-skip tests by calling igt_skip() */
--
2.21.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v3] tests/gem_exec_reloc: Don't filter out invalid addresses
On Thu, Oct 31, 2019 at 08:40:58AM +0100, Janusz Krzysztofik wrote:
On Wednesday, October 30, 2019 10:19:43 PM CET Vanshidhar Konda wrote:
On Wed, Oct 30, 2019 at 06:15:35PM +0100, Janusz Krzysztofik wrote:
>Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable
>addresses for !ppgtt") introduced filtering of addresses possibly
>occupied by other users of shared GTT. Unfortunately, that filtering
>doesn't distinguish actually occupied addresses from otherwise invalid
>softpin offsets. For example, on a future hardware backing store with
>a page size larger than 4 kB incorrect object alignment is assumed and
>the test results are distorted as it happily skips over incorrectly
>aligned objects instead of reporting the problem.
>
>Filter out failing addresses only if not reported as invalid.
>
>Signed-off-by: Janusz Krzysztofik
>Cc: Chris Wilson
>---
> tests/i915/gem_exec_reloc.c | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
>index fdd9661d..1d0c791e 100644
>--- a/tests/i915/gem_exec_reloc.c
>+++ b/tests/i915/gem_exec_reloc.c
>@@ -520,7 +520,7 @@ static void basic_range(int fd, unsigned flags)
>uint64_t gtt_size = gem_aperture_size(fd);
>const uint32_t bbe = MI_BATCH_BUFFER_END;
>igt_spin_t *spin = NULL;
>- int count, n;
>+ int count, n, err;
>
>igt_require(gem_has_softpin(fd));
>
>@@ -542,8 +542,11 @@ static void basic_range(int fd, unsigned flags)
>gem_write(fd, obj[n].handle, 0, , sizeof(bbe));
>execbuf.buffers_ptr = to_user_pointer([n]);
>execbuf.buffer_count = 1;
>- if (__gem_execbuf(fd, ))
>+ err = __gem_execbuf(fd, );
>+ if (err) {
>+ igt_assert(err != -EINVAL);
>continue;
The addresses to which the object is being pinned is generated as part
of the test. The code is just assuming that the address needs to be 4K
aligned instead of figuring out what the alignment requirement for the
device is.
Shouldn't the test be updated to generate virtual addresses per the
alignment requirements of the test device instead of just assuming 4K
increments are good?
You're perfectly right, and that's what I've been trying to achieve in my
series https://lists.freedesktop.org/archives/igt-dev/2019-October/017081.html.
Thanks for providing the context for the changes.
As suggested by Chris
(https://lists.freedesktop.org/archives/igt-dev/2019-October/016936.html),
I've been trying to add a library function that detects the alignment
requirement of a device. We haven't agreed yet on necessity of my approach to
distinguish failures caused by incorrect offset alignment from those which are
simply coming from addresses being occupied by other users, and how to do
For a context that has just been created, I would assume that the PPGTTs would
be
empty as no object should be mapped by default in it. With this
assumption, is it safe to assume that unaligned addresses would be the
only ones that return an EINVAL error?
I'll take a look at the latest version of the patch series for this.
Thanks,
Vanshi
that distinction. In my current approach, I'm retrying at different offsets
to conclude possible failure reasons, but maybe error codes can be used for
that.
Back to this patch, skipping over invalid offsets, calculated from incorrectly
assumed or detected alignment requirements still seems wrong to me, anyway.
That's the reason for this patch.
Thanks,
Janusz
Vanshi
>+ }
>
>igt_debug("obj[%d] handle=%d, address=%llx\n",
> n, obj[n].handle, (long long)obj[n].offset);
>@@ -562,8 +565,11 @@ static void basic_range(int fd, unsigned flags)
>gem_write(fd, obj[n].handle, 0, , sizeof(bbe));
>execbuf.buffers_ptr = to_user_pointer([n]);
>execbuf.buffer_count = 1;
>- if (__gem_execbuf(fd, ))
>+ err = __gem_execbuf(fd, );
>+ if (err) {
>+ igt_assert(err != -EINVAL);
>continue;
>+ }
>
>igt_debug("obj[%d] handle=%d, address=%llx\n",
> n, obj[n].handle, (long long)obj[n].offset);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v3] tests/gem_exec_reloc: Don't filter out invalid addresses
On Wed, Oct 30, 2019 at 06:15:35PM +0100, Janusz Krzysztofik wrote:
Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable
addresses for !ppgtt") introduced filtering of addresses possibly
occupied by other users of shared GTT. Unfortunately, that filtering
doesn't distinguish actually occupied addresses from otherwise invalid
softpin offsets. For example, on a future hardware backing store with
a page size larger than 4 kB incorrect object alignment is assumed and
the test results are distorted as it happily skips over incorrectly
aligned objects instead of reporting the problem.
Filter out failing addresses only if not reported as invalid.
Signed-off-by: Janusz Krzysztofik
Cc: Chris Wilson
---
tests/i915/gem_exec_reloc.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
index fdd9661d..1d0c791e 100644
--- a/tests/i915/gem_exec_reloc.c
+++ b/tests/i915/gem_exec_reloc.c
@@ -520,7 +520,7 @@ static void basic_range(int fd, unsigned flags)
uint64_t gtt_size = gem_aperture_size(fd);
const uint32_t bbe = MI_BATCH_BUFFER_END;
igt_spin_t *spin = NULL;
- int count, n;
+ int count, n, err;
igt_require(gem_has_softpin(fd));
@@ -542,8 +542,11 @@ static void basic_range(int fd, unsigned flags)
gem_write(fd, obj[n].handle, 0, , sizeof(bbe));
execbuf.buffers_ptr = to_user_pointer([n]);
execbuf.buffer_count = 1;
- if (__gem_execbuf(fd, ))
+ err = __gem_execbuf(fd, );
+ if (err) {
+ igt_assert(err != -EINVAL);
continue;
The addresses to which the object is being pinned is generated as part
of the test. The code is just assuming that the address needs to be 4K
aligned instead of figuring out what the alignment requirement for the
device is.
Shouldn't the test be updated to generate virtual addresses per the
alignment requirements of the test device instead of just assuming 4K
increments are good?
Vanshi
+ }
igt_debug("obj[%d] handle=%d, address=%llx\n",
n, obj[n].handle, (long long)obj[n].offset);
@@ -562,8 +565,11 @@ static void basic_range(int fd, unsigned flags)
gem_write(fd, obj[n].handle, 0, , sizeof(bbe));
execbuf.buffers_ptr = to_user_pointer([n]);
execbuf.buffer_count = 1;
- if (__gem_execbuf(fd, ))
+ err = __gem_execbuf(fd, );
+ if (err) {
+ igt_assert(err != -EINVAL);
continue;
+ }
igt_debug("obj[%d] handle=%d, address=%llx\n",
n, obj[n].handle, (long long)obj[n].offset);
--
2.21.0
___
igt-dev mailing list
igt-...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Configurable GT idle frequency
On Tue, Apr 16, 2019 at 08:30:22AM -0700, Bob Paauwe wrote:
On Mon, 15 Apr 2019 17:33:30 -0700
Vanshidhar Konda wrote:
On Mon, Apr 15, 2019 at 04:05:26PM -0700, Bob Paauwe wrote:
>There are real-time use cases where having deterministic CPU processes
>can be more important than GPU power/performance. Parking the GPU at a
>specific freqency by setting idle, min and max prohibits the normal
>dynamic GPU frequency switching which can introduce significant PCI-E
>latency. This adds the ability to configure the GPU "idle" frequecy
>using the same method that already exists for minimum and maximum
>frequencies.
>
>In addition, parking the idle frequency may reduce spool up latencies
>on GPU workloads.
>
>Signed-off-by: Bob Paauwe
>---
> drivers/gpu/drm/i915/i915_sysfs.c | 60 +++
> 1 file changed, 60 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
>index 41313005af42..62612c23d514 100644
>--- a/drivers/gpu/drm/i915/i915_sysfs.c
>+++ b/drivers/gpu/drm/i915/i915_sysfs.c
>@@ -454,11 +454,69 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>return ret ?: count;
> }
>
>+static ssize_t gt_idle_freq_mhz_show(struct device *kdev, struct
device_attribute *attr, char *buf)
>+{
>+ struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>+
>+ return snprintf(buf, PAGE_SIZE, "%d\n",
>+ intel_gpu_freq(dev_priv,
>+ dev_priv->gt_pm.rps.idle_freq));
>+}
>+
>+static ssize_t gt_idle_freq_mhz_store(struct device *kdev,
>+ struct device_attribute *attr,
>+ const char *buf, size_t count)
>+{
>+ struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>+ struct intel_rps *rps = _priv->gt_pm.rps;
>+ intel_wakeref_t wakeref;
>+ u32 val;
val can probably just be u8. max_freq, min_freq, etc. are only u8 in
struct intel_rps *rps.
Using u32 is consistent with all the other _store functions in the file
and changing it would also mean changing the kstrtou32 call below. I'd
rather this function stay consistent with the min/max/boost frequency
functions. Changing to u8 would be a separate change and should be
applied to all the similar functions.
Thanks for pointing that out.
I recently joined the team, so not sure if you would like someone else
to review as well.
Reviewed-by: Vanshidhar Konda
>+ ssize_t ret;
>+
>+ ret = kstrtou32(buf, 0, );
>+ if (ret)
>+ return ret;
>+
>+ wakeref = intel_runtime_pm_get(dev_priv);
>+
>+ mutex_lock(_priv->pcu_lock);
>+
>+ val = intel_freq_opcode(dev_priv, val);
>+
>+ if (val < rps->min_freq ||
>+ val > rps->max_freq ||
>+ val > rps->max_freq_softlimit) {
>+ mutex_unlock(_priv->pcu_lock);
>+ intel_runtime_pm_put(dev_priv, wakeref);
>+ return -EINVAL;
>+ }
>+
>+ rps->idle_freq = val;
>+
>+ val = clamp_t(int, rps->cur_freq,
>+ rps->idle_freq,
>+ rps->max_freq_softlimit);
This should probably be clamped to u8 instead of int.
Similar to above, this is consistent with the other similar functions.
Vanshi
>+
>+ /*
>+* If the current freq is at the old idle freq we should
>+* ajust it to the new idle. Calling *_set_rps will also
>+* update the interrupt limits and PMINTRMSK if ncessary.
>+*/
>+ ret = intel_set_rps(dev_priv, val);
>+
>+ mutex_unlock(_priv->pcu_lock);
>+
>+ intel_runtime_pm_put(dev_priv, wakeref);
>+
>+ return ret ?: count;
>+}
>+
> static DEVICE_ATTR_RO(gt_act_freq_mhz);
> static DEVICE_ATTR_RO(gt_cur_freq_mhz);
> static DEVICE_ATTR_RW(gt_boost_freq_mhz);
> static DEVICE_ATTR_RW(gt_max_freq_mhz);
> static DEVICE_ATTR_RW(gt_min_freq_mhz);
>+static DEVICE_ATTR_RW(gt_idle_freq_mhz);
>
> static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
>
>@@ -492,6 +550,7 @@ static const struct attribute * const gen6_attrs[] = {
>_attr_gt_boost_freq_mhz.attr,
>_attr_gt_max_freq_mhz.attr,
>_attr_gt_min_freq_mhz.attr,
>+ _attr_gt_idle_freq_mhz.attr,
>_attr_gt_RP0_freq_mhz.attr,
>_attr_gt_RP1_freq_mhz.attr,
>_attr_gt_RPn_freq_mhz.attr,
>@@ -504,6 +563,7 @@ static const struct attribute * const vlv_attrs[] = {
>_attr_gt_boost_freq_mhz.attr,
>_attr_gt_max_freq_mhz.attr,
>_attr_gt_min_freq_mhz.attr,
>+ _attr_gt_idle_freq_mhz.attr,
>_attr_gt_RP0_freq_mhz.attr,
>_attr_gt_RP1_freq_mhz.attr,
>_attr_gt_RPn_freq_mhz.attr,
>--
>2.19.2
>
>___
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
--
Bob Paauwe
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp. Folsom, CA
(916) 356-6193
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Configurable GT idle frequency
On Mon, Apr 15, 2019 at 04:05:26PM -0700, Bob Paauwe wrote:
There are real-time use cases where having deterministic CPU processes
can be more important than GPU power/performance. Parking the GPU at a
specific freqency by setting idle, min and max prohibits the normal
dynamic GPU frequency switching which can introduce significant PCI-E
latency. This adds the ability to configure the GPU "idle" frequecy
using the same method that already exists for minimum and maximum
frequencies.
In addition, parking the idle frequency may reduce spool up latencies
on GPU workloads.
Signed-off-by: Bob Paauwe
---
drivers/gpu/drm/i915/i915_sysfs.c | 60 +++
1 file changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index 41313005af42..62612c23d514 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -454,11 +454,69 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
return ret ?: count;
}
+static ssize_t gt_idle_freq_mhz_show(struct device *kdev, struct
device_attribute *attr, char *buf)
+{
+ struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
+
+ return snprintf(buf, PAGE_SIZE, "%d\n",
+ intel_gpu_freq(dev_priv,
+ dev_priv->gt_pm.rps.idle_freq));
+}
+
+static ssize_t gt_idle_freq_mhz_store(struct device *kdev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
+ struct intel_rps *rps = _priv->gt_pm.rps;
+ intel_wakeref_t wakeref;
+ u32 val;
val can probably just be u8. max_freq, min_freq, etc. are only u8 in
struct intel_rps *rps.
+ ssize_t ret;
+
+ ret = kstrtou32(buf, 0, );
+ if (ret)
+ return ret;
+
+ wakeref = intel_runtime_pm_get(dev_priv);
+
+ mutex_lock(_priv->pcu_lock);
+
+ val = intel_freq_opcode(dev_priv, val);
+
+ if (val < rps->min_freq ||
+ val > rps->max_freq ||
+ val > rps->max_freq_softlimit) {
+ mutex_unlock(_priv->pcu_lock);
+ intel_runtime_pm_put(dev_priv, wakeref);
+ return -EINVAL;
+ }
+
+ rps->idle_freq = val;
+
+ val = clamp_t(int, rps->cur_freq,
+ rps->idle_freq,
+ rps->max_freq_softlimit);
This should probably be clamped to u8 instead of int.
Vanshi
+
+ /*
+* If the current freq is at the old idle freq we should
+* ajust it to the new idle. Calling *_set_rps will also
+* update the interrupt limits and PMINTRMSK if ncessary.
+*/
+ ret = intel_set_rps(dev_priv, val);
+
+ mutex_unlock(_priv->pcu_lock);
+
+ intel_runtime_pm_put(dev_priv, wakeref);
+
+ return ret ?: count;
+}
+
static DEVICE_ATTR_RO(gt_act_freq_mhz);
static DEVICE_ATTR_RO(gt_cur_freq_mhz);
static DEVICE_ATTR_RW(gt_boost_freq_mhz);
static DEVICE_ATTR_RW(gt_max_freq_mhz);
static DEVICE_ATTR_RW(gt_min_freq_mhz);
+static DEVICE_ATTR_RW(gt_idle_freq_mhz);
static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
@@ -492,6 +550,7 @@ static const struct attribute * const gen6_attrs[] = {
_attr_gt_boost_freq_mhz.attr,
_attr_gt_max_freq_mhz.attr,
_attr_gt_min_freq_mhz.attr,
+ _attr_gt_idle_freq_mhz.attr,
_attr_gt_RP0_freq_mhz.attr,
_attr_gt_RP1_freq_mhz.attr,
_attr_gt_RPn_freq_mhz.attr,
@@ -504,6 +563,7 @@ static const struct attribute * const vlv_attrs[] = {
_attr_gt_boost_freq_mhz.attr,
_attr_gt_max_freq_mhz.attr,
_attr_gt_min_freq_mhz.attr,
+ _attr_gt_idle_freq_mhz.attr,
_attr_gt_RP0_freq_mhz.attr,
_attr_gt_RP1_freq_mhz.attr,
_attr_gt_RPn_freq_mhz.attr,
--
2.19.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Update size upon return from GEM_CREATE
On Tue, Mar 26, 2019 at 06:02:18PM +0100, Michał Winiarski wrote:
Since GEM_CREATE is trying to outsmart the user by rounding up unaligned
objects, we used to update the size returned to userspace.
This update seems to have been lost throughout the history.
v2: Use round_up(), reorder locals (Chris)
References: ff72145badb8 ("drm: dumb scanout create/mmap for intel/radeon (v3)")
Signed-off-by: Michał Winiarski
Cc: Chris Wilson
Cc: Janusz Krzysztofik
Cc: Joonas Lahtinen
Reviewed-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f6cdd5fb9deb..e506e43cfade 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -622,14 +622,15 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
static int
i915_gem_create(struct drm_file *file,
struct drm_i915_private *dev_priv,
- u64 size,
+ u64 *size_p,
u32 *handle_p)
{
struct drm_i915_gem_object *obj;
- int ret;
u32 handle;
+ u64 size;
+ int ret;
- size = roundup(size, PAGE_SIZE);
+ size = round_up(*size_p, PAGE_SIZE);
if (size == 0)
return -EINVAL;
Does it make more sense to check the size prior to doing a roundup?
@@ -645,6 +646,7 @@ i915_gem_create(struct drm_file *file,
return ret;
*handle_p = handle;
+ *size_p = obj->base.size;
return 0;
}
@@ -657,7 +659,7 @@ i915_gem_dumb_create(struct drm_file *file,
args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64);
args->size = args->pitch * args->height;
return i915_gem_create(file, to_i915(dev),
- args->size, >handle);
+ >size, >handle);
}
static bool gpu_write_needs_clflush(struct drm_i915_gem_object *obj)
@@ -682,7 +684,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
i915_gem_flush_free_objects(dev_priv);
return i915_gem_create(file, dev_priv,
- args->size, >handle);
+ >size, >handle);
}
static inline enum fb_op_origin
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal
On Fri, Mar 15, 2019 at 02:43:46PM -0700, Rodrigo Vivi wrote:
On Fri, Mar 15, 2019 at 02:39:25PM -0700, Rodrigo Vivi wrote:
On Fri, Mar 15, 2019 at 02:31:40PM -0700, Rodrigo Vivi wrote:
> On Fri, Mar 15, 2019 at 01:27:22PM -0700, Vanshidhar Konda wrote:
> > On Fri, Mar 15, 2019 at 12:38:41PM -0700, Rodrigo Vivi wrote:
> > > On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
> > > > Extend the timeout for the hardware to signal SEND_BUSY on the DP
> > > > Aux Channel Controller register.
> > > >
> > > > This is needed to address FDO #109982
> > > > https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > >
> > > instead of mentioning like this, please use:
> > > Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
> > >
> > > Also instead of the "needed to address" it would be better
> > > to add some reasoning explaining that
> > > "empirically we got some bugs workarounded by increasing the timeout
> > > from 10ms to 15ms although spec was only requiring 4ms"
> > > or something like that...
> > >
> > Thanks! I'll keep these in mind for next patches.
> > > >
> > > > Cc: Ville Syrjälä
> > > > Cc: Imre Deak
> > > > Signed-off-by: Vanshidhar Konda
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 47857f96c3b1..fd6de33c5664 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
> > > > }
> > > > }
> > > >
> > > > +#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
> > >
> > > This define is spurious since it's in use in a single place.
> > >
> > > Also, giving the timeout a name, like this, makes it appear it came from
> > > the spec. Well, if it came from Spec it should be defined in the proper .h
> > > files.
> > >
> > > Since I don't believe this came from spec I believe we can just remove it
> > > and go for the timeout directly on the function below.
> > >
> > I'll make this change in the next patch.
> > > > +
> > > > static u32
> > > > intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > > {
> > > > @@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> > > >
> > > > #define C (((status = I915_READ_NOTRACE(ch_ctl)) &
DP_AUX_CH_CTL_SEND_BUSY) == 0)
> > > > done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
> > > > - msecs_to_jiffies_timeout(10));
> > > > +
msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
> > >
> > > Is this just a guess that you are trying to check?
> > > I'm asking because I didn't see any indication that the increase
> > > really fixed the issue.
> > >
> > > So, if you are trying to just validate your approach maybe the try-bot
> > > could be used?
> > >
> > I also would have preferred to use the trybot. The error in Bugzilla 109982
is only reproduced on only shard-iclb2 machine once
> > in approximately 2 days. So the trybot is not of much help for this.
> >
> > Is there another way of testing experimental patches for bugs that are
> > not reproduced locally or on other machines? Or, if the issue is only
> > happening on one machine, should it be lowered in priority/whitelisted
> > on the specific CI machine?
>
> For now I believe the best alternative is to --subject-prefix="[CI]"
>
> But also you need to be sure that the fail rate don't fool us...
hmm... this testcase seems to have a passrate of 77% of the runs.
So probably using the CI infrastructure here isn't the best alternative
to validate this approach in general.
ops... I forgot to paste the link from these data:
https://intel-gfx-ci.01.org/tree/drm-tip/shard-iclb.html
(30 runs out of 45 passing)
Sorry, I don't think I understood what you are saying here. The CI/IGT
test that is reported in the bug itself doesn't have an issue. It pases
successfully.
The issue reported by Martin is that there is an error printed out to dmesg.
<3> [1128.098368] [drm:intel_dp_aux_xfer [i915]] *ERROR* dp aux hw did
not signal ti
Re: [Intel-gfx] [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal
On Fri, Mar 15, 2019 at 12:38:41PM -0700, Rodrigo Vivi wrote:
On Fri, Mar 15, 2019 at 11:39:54AM -0700, Vanshidhar Konda wrote:
Extend the timeout for the hardware to signal SEND_BUSY on the DP
Aux Channel Controller register.
This is needed to address FDO #109982
https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
instead of mentioning like this, please use:
Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
Also instead of the "needed to address" it would be better
to add some reasoning explaining that
"empirically we got some bugs workarounded by increasing the timeout
from 10ms to 15ms although spec was only requiring 4ms"
or something like that...
Thanks! I'll keep these in mind for next patches.
Cc: Ville Syrjälä
Cc: Imre Deak
Signed-off-by: Vanshidhar Konda
---
drivers/gpu/drm/i915/intel_dp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 47857f96c3b1..fd6de33c5664 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
}
}
+#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
This define is spurious since it's in use in a single place.
Also, giving the timeout a name, like this, makes it appear it came from
the spec. Well, if it came from Spec it should be defined in the proper .h
files.
Since I don't believe this came from spec I believe we can just remove it
and go for the timeout directly on the function below.
I'll make this change in the next patch.
+
static u32
intel_dp_aux_wait_done(struct intel_dp *intel_dp)
{
@@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) ==
0)
done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
- msecs_to_jiffies_timeout(10));
+
msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
Is this just a guess that you are trying to check?
I'm asking because I didn't see any indication that the increase
really fixed the issue.
So, if you are trying to just validate your approach maybe the try-bot
could be used?
I also would have preferred to use the trybot. The error in Bugzilla 109982 is
only reproduced on only shard-iclb2 machine once
in approximately 2 days. So the trybot is not of much help for this.
Is there another way of testing experimental patches for bugs that are
not reproduced locally or on other machines? Or, if the issue is only
happening on one machine, should it be lowered in priority/whitelisted
on the specific CI machine?
Thanks,
Rodrigo.
/* just trace the final value */
trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/display: Increase timeout for DP Aux channel ctl signal
Extend the timeout for the hardware to signal SEND_BUSY on the DP
Aux Channel Controller register.
This is needed to address FDO #109982
https://bugzilla.freedesktop.org/show_bug.cgi?id=109982
Cc: Ville Syrjälä
Cc: Imre Deak
Signed-off-by: Vanshidhar Konda
---
drivers/gpu/drm/i915/intel_dp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 47857f96c3b1..fd6de33c5664 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1053,6 +1053,8 @@ intel_dp_check_edp(struct intel_dp *intel_dp)
}
}
+#define DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS 15
+
static u32
intel_dp_aux_wait_done(struct intel_dp *intel_dp)
{
@@ -1063,7 +1065,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) ==
0)
done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
- msecs_to_jiffies_timeout(10));
+
msecs_to_jiffies_timeout(DP_AUX_CH_CTL_SIGNAL_TIMEOUT_MS));
/* just trace the final value */
trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Reduce log level for DP command signal timeout
On Fri, Mar 15, 2019 at 03:35:04PM +0200, Ville Syrjälä wrote:
On Thu, Mar 14, 2019 at 06:37:22PM -0700, Vanshidhar Konda wrote:
On Thu, Mar 14, 2019 at 11:09:38PM +0200, Ville Syrjälä wrote:
>On Thu, Mar 14, 2019 at 02:00:29PM -0700, Vanshidhar Konda wrote:
>> On Thu, Mar 14, 2019 at 10:47:56PM +0200, Ville Syrjälä wrote:
>> >On Thu, Mar 14, 2019 at 01:26:00PM -0700, Vanshidhar Konda wrote:
>> >> On Thu, Mar 14, 2019 at 08:39:11PM +0200, Ville Syrjälä wrote:
>> >> >On Thu, Mar 14, 2019 at 10:58:49AM -0700, Vanshidhar Konda wrote:
>> >> >> The log level for timeout after waiting for a signal on the DP aux
>> >> >> channel control register is set to DRM_ERROR. But this is timeout not a
>> >> >> fatal error as the driver is expected to retry the command. Failure
>> >> >> after all retries is already captured as an error. Hence, reducing the
>> >> >> log for a timeout to warning instead of error.
>> >> >> ---
>> >> >> drivers/gpu/drm/i915/intel_dp.c | 2 +-
>> >> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >>
>> >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
>> >> >> index 47857f96c3b1..f51e8b2ccb17 100644
>> >> >> --- a/drivers/gpu/drm/i915/intel_dp.c
>> >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> >> >> @@ -1069,7 +1069,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>> >> >> trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
>> >> >>
>> >> >> if (!done)
>> >> >> -DRM_ERROR("dp aux hw did not signal timeout!\n");
>> >> >> +DRM_WARN("dp aux hw did not signal timeout!\n");
>> >> >
>> >> >IIRC this indicates the hw is broken.
>> >> >
>> >> Does this indicate an issue with Intel GFX/display device, or the
>> >> display/monitor connected to the DP port?
>> >>
>> >> FYI, this is for FDO #109982
>> >> (https://bugzilla.freedesktop.org/show_bug.cgi?id=109982).
>> >
>> >There's nothing connected I believe. But in either case I believe
>>
>> >From the two logs for the issue, e-DP1 is the only display connected to
>> the test machine when it generated this error.
>>
>> >the aux hw should terminate with a proper timeout. I'm tempted to
>>
>> If we think that there should be no timeout, would it make more sense to
>> return an error to the caller and having the caller handle the error?
>
>How would it handle the error?
>
>>
>> >blame the typec/tbt stuff here too.
Could it be possible that the addition of typec/tbt to ICL has added
additional latency to the DP register being signaled? Would it make
sense to increase the 10 ms timeout to something larger?
Imre just told me the hw timeout was increased to 4ms. So 10ms should
still be sufficient I guess. But it wouldn't hurt to at least test
longer timeouts a bit to see if the hw ever gets around to signalling
the timeout.
Ok. Let me send a patch for that then. I'll cc you and Imre on that
patch. Thank you!
>> >
>> >>
>> >> >From the logs, it seems like this timeout only occurs once. The next try
>> >> succeeds without issues.
>> >>
>> >> >> #undef C
>> >> >>
>> >> >> return status;
>> >> >> --
>> >> >> 2.20.1
>> >> >>
>> >> >> ___
>> >> >> Intel-gfx mailing list
>> >> >> Intel-gfx@lists.freedesktop.org
>> >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> >
>> >> >--
>> >> >Ville Syrjälä
>> >> >Intel
>> >
>> >--
>> >Ville Syrjälä
>> >Intel
>
>--
>Ville Syrjälä
>Intel
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Reduce log level for DP command signal timeout
On Thu, Mar 14, 2019 at 11:09:38PM +0200, Ville Syrjälä wrote:
On Thu, Mar 14, 2019 at 02:00:29PM -0700, Vanshidhar Konda wrote:
On Thu, Mar 14, 2019 at 10:47:56PM +0200, Ville Syrjälä wrote:
>On Thu, Mar 14, 2019 at 01:26:00PM -0700, Vanshidhar Konda wrote:
>> On Thu, Mar 14, 2019 at 08:39:11PM +0200, Ville Syrjälä wrote:
>> >On Thu, Mar 14, 2019 at 10:58:49AM -0700, Vanshidhar Konda wrote:
>> >> The log level for timeout after waiting for a signal on the DP aux
>> >> channel control register is set to DRM_ERROR. But this is timeout not a
>> >> fatal error as the driver is expected to retry the command. Failure
>> >> after all retries is already captured as an error. Hence, reducing the
>> >> log for a timeout to warning instead of error.
>> >> ---
>> >> drivers/gpu/drm/i915/intel_dp.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
>> >> index 47857f96c3b1..f51e8b2ccb17 100644
>> >> --- a/drivers/gpu/drm/i915/intel_dp.c
>> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> >> @@ -1069,7 +1069,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>> >> trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
>> >>
>> >> if (!done)
>> >> - DRM_ERROR("dp aux hw did not signal timeout!\n");
>> >> + DRM_WARN("dp aux hw did not signal timeout!\n");
>> >
>> >IIRC this indicates the hw is broken.
>> >
>> Does this indicate an issue with Intel GFX/display device, or the
>> display/monitor connected to the DP port?
>>
>> FYI, this is for FDO #109982
>> (https://bugzilla.freedesktop.org/show_bug.cgi?id=109982).
>
>There's nothing connected I believe. But in either case I believe
>From the two logs for the issue, e-DP1 is the only display connected to
the test machine when it generated this error.
>the aux hw should terminate with a proper timeout. I'm tempted to
If we think that there should be no timeout, would it make more sense to
return an error to the caller and having the caller handle the error?
How would it handle the error?
>blame the typec/tbt stuff here too.
Could it be possible that the addition of typec/tbt to ICL has added
additional latency to the DP register being signaled? Would it make
sense to increase the 10 ms timeout to something larger?
>
>>
>> >From the logs, it seems like this timeout only occurs once. The next try
>> succeeds without issues.
>>
>> >> #undef C
>> >>
>> >> return status;
>> >> --
>> >> 2.20.1
>> >>
>> >> ___
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> >--
>> >Ville Syrjälä
>> >Intel
>
>--
>Ville Syrjälä
>Intel
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Reduce log level for DP command signal timeout
On Thu, Mar 14, 2019 at 10:47:56PM +0200, Ville Syrjälä wrote:
On Thu, Mar 14, 2019 at 01:26:00PM -0700, Vanshidhar Konda wrote:
On Thu, Mar 14, 2019 at 08:39:11PM +0200, Ville Syrjälä wrote:
>On Thu, Mar 14, 2019 at 10:58:49AM -0700, Vanshidhar Konda wrote:
>> The log level for timeout after waiting for a signal on the DP aux
>> channel control register is set to DRM_ERROR. But this is timeout not a
>> fatal error as the driver is expected to retry the command. Failure
>> after all retries is already captured as an error. Hence, reducing the
>> log for a timeout to warning instead of error.
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
>> index 47857f96c3b1..f51e8b2ccb17 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1069,7 +1069,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
>>trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
>>
>>if (!done)
>> - DRM_ERROR("dp aux hw did not signal timeout!\n");
>> + DRM_WARN("dp aux hw did not signal timeout!\n");
>
>IIRC this indicates the hw is broken.
>
Does this indicate an issue with Intel GFX/display device, or the
display/monitor connected to the DP port?
FYI, this is for FDO #109982
(https://bugzilla.freedesktop.org/show_bug.cgi?id=109982).
There's nothing connected I believe. But in either case I believe
From the two logs for the issue, e-DP1 is the only display connected to
the test machine when it generated this error.
the aux hw should terminate with a proper timeout. I'm tempted to
If we think that there should be no timeout, would it make more sense to
return an error to the caller and having the caller handle the error?
blame the typec/tbt stuff here too.
>From the logs, it seems like this timeout only occurs once. The next try
succeeds without issues.
>> #undef C
>>
>>return status;
>> --
>> 2.20.1
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Reduce log level for DP command signal timeout
On Thu, Mar 14, 2019 at 08:39:11PM +0200, Ville Syrjälä wrote:
On Thu, Mar 14, 2019 at 10:58:49AM -0700, Vanshidhar Konda wrote:
The log level for timeout after waiting for a signal on the DP aux
channel control register is set to DRM_ERROR. But this is timeout not a
fatal error as the driver is expected to retry the command. Failure
after all retries is already captured as an error. Hence, reducing the
log for a timeout to warning instead of error.
---
drivers/gpu/drm/i915/intel_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 47857f96c3b1..f51e8b2ccb17 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1069,7 +1069,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
if (!done)
- DRM_ERROR("dp aux hw did not signal timeout!\n");
+ DRM_WARN("dp aux hw did not signal timeout!\n");
IIRC this indicates the hw is broken.
Does this indicate an issue with Intel GFX/display device, or the
display/monitor connected to the DP port?
FYI, this is for FDO #109982
(https://bugzilla.freedesktop.org/show_bug.cgi?id=109982).
From the logs, it seems like this timeout only occurs once. The next try
succeeds without issues.
#undef C
return status;
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/display: Reduce log level for DP command signal timeout
I'll add the Signed Off by line when I re-submit the patch after a
review.
Thanks,
Vanshi
On Thu, Mar 14, 2019 at 10:58:49AM -0700, Vanshidhar Konda wrote:
The log level for timeout after waiting for a signal on the DP aux
channel control register is set to DRM_ERROR. But this is timeout not a
fatal error as the driver is expected to retry the command. Failure
after all retries is already captured as an error. Hence, reducing the
log for a timeout to warning instead of error.
---
drivers/gpu/drm/i915/intel_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 47857f96c3b1..f51e8b2ccb17 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1069,7 +1069,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
if (!done)
- DRM_ERROR("dp aux hw did not signal timeout!\n");
+ DRM_WARN("dp aux hw did not signal timeout!\n");
#undef C
return status;
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/display: Reduce log level for DP command signal timeout
The log level for timeout after waiting for a signal on the DP aux
channel control register is set to DRM_ERROR. But this is timeout not a
fatal error as the driver is expected to retry the command. Failure
after all retries is already captured as an error. Hence, reducing the
log for a timeout to warning instead of error.
---
drivers/gpu/drm/i915/intel_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 47857f96c3b1..f51e8b2ccb17 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1069,7 +1069,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
if (!done)
- DRM_ERROR("dp aux hw did not signal timeout!\n");
+ DRM_WARN("dp aux hw did not signal timeout!\n");
#undef C
return status;
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
