Re: [Intel-gfx] [PATCH 11/19] drm/i915: Update the helper to set correct mapping
On 14/04/2021 17:20, Matthew Auld wrote:
On Wed, 14 Apr 2021 at 16:22, Tvrtko Ursulin
wrote:
On 12/04/2021 10:05, Matthew Auld wrote:
From: Venkata Sandeep Dhanalakota
Determine the possible coherent map type based on object location,
and if target has llc or if user requires an always coherent
mapping.
Cc: Matthew Auld
Cc: CQ Tang
Suggested-by: Michal Wajdeczko
Signed-off-by: Venkata Sandeep Dhanalakota
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c| 3 ++-
drivers/gpu/drm/i915/gt/intel_engine_pm.c| 2 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
drivers/gpu/drm/i915/gt/intel_ring.c | 9 ++---
drivers/gpu/drm/i915/gt/selftest_context.c | 3 ++-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++--
drivers/gpu/drm/i915/gt/selftest_lrc.c | 4 +++-
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 4 +++-
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 4 +++-
drivers/gpu/drm/i915/i915_drv.h | 11 +--
drivers/gpu/drm/i915/selftests/igt_spinner.c | 4 ++--
11 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index efe935f80c1a..b79568d370f5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -664,7 +664,8 @@ static int init_status_page(struct intel_engine_cs *engine)
if (ret)
goto err;
- vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+ vaddr = i915_gem_object_pin_map(obj,
+ i915_coherent_map_type(engine->i915, obj,
true));
if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr);
goto err_unpin;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 7c9af86fdb1e..47f4397095e5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -23,7 +23,7 @@ static void dbg_poison_ce(struct intel_context *ce)
if (ce->state) {
struct drm_i915_gem_object *obj = ce->state->obj;
- int type = i915_coherent_map_type(ce->engine->i915);
+ int type = i915_coherent_map_type(ce->engine->i915, obj, true);
void *map;
if (!i915_gem_object_trylock(obj))
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index e86897cde984..aafe2a4df496 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -903,7 +903,9 @@ lrc_pre_pin(struct intel_context *ce,
GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
*vaddr = i915_gem_object_pin_map(ce->state->obj,
- i915_coherent_map_type(ce->engine->i915)
|
+ i915_coherent_map_type(ce->engine->i915,
+ ce->state->obj,
+ false) |
I915_MAP_OVERRIDE);
return PTR_ERR_OR_ZERO(*vaddr);
diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c
b/drivers/gpu/drm/i915/gt/intel_ring.c
index aee0a77c77e0..3cf6c7e68108 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -53,9 +53,12 @@ int intel_ring_pin(struct intel_ring *ring, struct
i915_gem_ww_ctx *ww)
if (i915_vma_is_map_and_fenceable(vma))
addr = (void __force *)i915_vma_pin_iomap(vma);
- else
- addr = i915_gem_object_pin_map(vma->obj,
-
i915_coherent_map_type(vma->vm->i915));
+ else {
+ int type = i915_coherent_map_type(vma->vm->i915, vma->obj, false);
+
+ addr = i915_gem_object_pin_map(vma->obj, type);
+ }
+
if (IS_ERR(addr)) {
ret = PTR_ERR(addr);
goto err_ring;
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c
b/drivers/gpu/drm/i915/gt/selftest_context.c
index b9bdd1d23243..26685b927169 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -88,7 +88,8 @@ static int __live_context_size(struct intel_engine_cs *engine)
goto err;
vaddr = i915_gem_object_pin_map_unlocked(ce->state->obj,
-
i915_coherent_map_type(engine->i915));
+
i915_coherent_map_type(engine->i915,
+
ce->state->obj, false));
if (IS_ERR(vaddr)) {
err = PTR_ERR(vaddr);
intel_context_unpin(ce);
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 746985971c3a..5b63d4df8c93 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b
[Intel-gfx] [PATCH][V2] drm/i915/gt: Fix a lockdep warning on RT kernel
Don`t simple disable all the HD-irq, should race the region in the
intel_breadcrumbs_disarm_irq() only.
BUG: sleeping function called from invalid context at
kernel/locking/rtmutex.c:969
#0: 89c4c00ca970 ((wq_completion)events){+.+.}-{0:0}, at:
process_one_work+0x1cf/0x6d0
#1: a433c1f53e60 ((work_completion)(&engine->retire_work)){+.+.}-{0:0},
at: process_one_work+0x1cf 0x6d
#2: 89c4ccb0a0a8 (kernel_context){+.+.}-{0:0}, at:
engine_retire+0x62/0x110 [i915]
#3: 89c4cf682300 (wakeref.mutex#3){+.+.}-{0:0}, at:
__intel_wakeref_put_last+0x20/0x60 [i915]
#4: 89c4ccb08398 (&b->irq_lock){+.+.}-{0:0}, at:
intel_breadcrumbs_disarm_irq+0x20/0xd0 [i915]
irq event stamp: 2126
hardirqs last enabled at (2125): []
cancel_delayed_work+0xa9/0xc0
hardirqs last disabled at (2126): []
__intel_breadcrumbs_park+0x76/0x80 [i915]
softirqs last enabled at (0): [] copy_process+0x63e/0x1630
softirqs last disabled at (0): [<>] 0x0
CPU: 3 PID: 281 Comm: kworker/3:3 Not tainted 5.10.27-rt34-yocto-preempt-rt #1
Hardware name: Intel(R) Client Systems NUC7i5DNKE/NUC7i5DNB, BIOS
DNKBLi5v.86A.0064.2019.0523.1933 05/23 2019
Workqueue: events engine_retire [i915]
Call Trace:
show_stack+0x52/0x58
dump_stack+0x7d/0x9f
___might_sleep.cold+0xe3/0xf4
rt_spin_lock+0x3f/0xc0
? intel_breadcrumbs_disarm_irq+0x20/0xd0 [i915]
intel_breadcrumbs_disarm_irq+0x20/0xd0 [i915]
signal_irq_work+0x241/0x660 [i915]
? __this_cpu_preempt_check+0x13/0x20
? lockdep_hardirqs_off+0x106/0x120
__intel_breadcrumbs_park+0x3f/0x80 [i915]
__engine_park+0xbd/0xe0 [i915]
intel_wakeref_put_last+0x22/0x60 [i915]
__intel_wakeref_put_last+0x50/0x60 [i915]
intel_context_exit_engine+0x5f/0x70 [i915]
i915_request_retire+0x139/0x2d0 [i915]
engine_retire+0xb0/0x110 [i915]
process_one_work+0x26d/0x6d0
worker_thread+0x53/0x330
kthread+0x1b0/0x1d0
? process_one_work+0x6d0/0x6d0
? __kthread_parkme+0xc0/0xc0
ret_from_fork+0x22/0x30
Fixes: 9d5612ca165a ("drm/i915/gt: Defer enabling the breadcrumb interrupt to
after submission")
Signed-off-by: Jun Miao
---
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index 34a645d..0589b1a 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -103,10 +103,12 @@ static void __intel_breadcrumbs_disarm_irq(struct
intel_breadcrumbs *b)
static void intel_breadcrumbs_disarm_irq(struct intel_breadcrumbs *b)
{
- spin_lock(&b->irq_lock);
+ unsigned long flags;
+
+ spin_lock_irqsave(&b->irq_lock, flags);
if (b->irq_armed)
__intel_breadcrumbs_disarm_irq(b);
- spin_unlock(&b->irq_lock);
+ spin_unlock_irqrestore(&b->irq_lock, flags);
}
static void add_signaling_context(struct intel_breadcrumbs *b,
@@ -337,9 +339,7 @@ void __intel_breadcrumbs_park(struct intel_breadcrumbs *b)
/* Kick the work once more to drain the signalers, and disarm the irq */
irq_work_sync(&b->irq_work);
while (READ_ONCE(b->irq_armed) && !atomic_read(&b->active)) {
- local_irq_disable();
signal_irq_work(&b->irq_work);
- local_irq_enable();
cond_resched();
}
}
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/19] drm/i915: Update the helper to set correct mapping
On Thu, 15 Apr 2021 at 09:21, Tvrtko Ursulin
wrote:
>
>
> On 14/04/2021 17:20, Matthew Auld wrote:
> > On Wed, 14 Apr 2021 at 16:22, Tvrtko Ursulin
> > wrote:
> >>
> >>
> >> On 12/04/2021 10:05, Matthew Auld wrote:
> >>> From: Venkata Sandeep Dhanalakota
> >>>
> >>> Determine the possible coherent map type based on object location,
> >>> and if target has llc or if user requires an always coherent
> >>> mapping.
> >>>
> >>> Cc: Matthew Auld
> >>> Cc: CQ Tang
> >>> Suggested-by: Michal Wajdeczko
> >>> Signed-off-by: Venkata Sandeep Dhanalakota
> >>>
> >>> ---
> >>>drivers/gpu/drm/i915/gt/intel_engine_cs.c| 3 ++-
> >>>drivers/gpu/drm/i915/gt/intel_engine_pm.c| 2 +-
> >>>drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
> >>>drivers/gpu/drm/i915/gt/intel_ring.c | 9 ++---
> >>>drivers/gpu/drm/i915/gt/selftest_context.c | 3 ++-
> >>>drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++--
> >>>drivers/gpu/drm/i915/gt/selftest_lrc.c | 4 +++-
> >>>drivers/gpu/drm/i915/gt/uc/intel_guc.c | 4 +++-
> >>>drivers/gpu/drm/i915/gt/uc/intel_huc.c | 4 +++-
> >>>drivers/gpu/drm/i915/i915_drv.h | 11 +--
> >>>drivers/gpu/drm/i915/selftests/igt_spinner.c | 4 ++--
> >>>11 files changed, 36 insertions(+), 16 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >>> index efe935f80c1a..b79568d370f5 100644
> >>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >>> @@ -664,7 +664,8 @@ static int init_status_page(struct intel_engine_cs
> >>> *engine)
> >>>if (ret)
> >>>goto err;
> >>>
> >>> - vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
> >>> + vaddr = i915_gem_object_pin_map(obj,
> >>> +
> >>> i915_coherent_map_type(engine->i915, obj, true));
> >>>if (IS_ERR(vaddr)) {
> >>>ret = PTR_ERR(vaddr);
> >>>goto err_unpin;
> >>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> >>> b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> >>> index 7c9af86fdb1e..47f4397095e5 100644
> >>> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> >>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> >>> @@ -23,7 +23,7 @@ static void dbg_poison_ce(struct intel_context *ce)
> >>>
> >>>if (ce->state) {
> >>>struct drm_i915_gem_object *obj = ce->state->obj;
> >>> - int type = i915_coherent_map_type(ce->engine->i915);
> >>> + int type = i915_coherent_map_type(ce->engine->i915, obj,
> >>> true);
> >>>void *map;
> >>>
> >>>if (!i915_gem_object_trylock(obj))
> >>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>> index e86897cde984..aafe2a4df496 100644
> >>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >>> @@ -903,7 +903,9 @@ lrc_pre_pin(struct intel_context *ce,
> >>>GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
> >>>
> >>>*vaddr = i915_gem_object_pin_map(ce->state->obj,
> >>> -
> >>> i915_coherent_map_type(ce->engine->i915) |
> >>> +
> >>> i915_coherent_map_type(ce->engine->i915,
> >>> +
> >>> ce->state->obj,
> >>> + false) |
> >>> I915_MAP_OVERRIDE);
> >>>
> >>>return PTR_ERR_OR_ZERO(*vaddr);
> >>> diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c
> >>> b/drivers/gpu/drm/i915/gt/intel_ring.c
> >>> index aee0a77c77e0..3cf6c7e68108 100644
> >>> --- a/drivers/gpu/drm/i915/gt/intel_ring.c
> >>> +++ b/drivers/gpu/drm/i915/gt/intel_ring.c
> >>> @@ -53,9 +53,12 @@ int intel_ring_pin(struct intel_ring *ring, struct
> >>> i915_gem_ww_ctx *ww)
> >>>
> >>>if (i915_vma_is_map_and_fenceable(vma))
> >>>addr = (void __force *)i915_vma_pin_iomap(vma);
> >>> - else
> >>> - addr = i915_gem_object_pin_map(vma->obj,
> >>> -
> >>> i915_coherent_map_type(vma->vm->i915));
> >>> + else {
> >>> + int type = i915_coherent_map_type(vma->vm->i915, vma->obj,
> >>> false);
> >>> +
> >>> + addr = i915_gem_object_pin_map(vma->obj, type);
> >>> + }
> >>> +
> >>>if (IS_ERR(addr)) {
> >>>ret = PTR_ERR(addr);
> >>>goto err_ring;
> >>> diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c
> >>> b/drivers/gpu/drm/i915/gt/selftest_context.c
> >>> index b9bdd1d23243..26685b927169 100644
> >>> --- a/drivers/gpu/drm/i915/gt/selftest_context.c
> >>> +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> >>> @@ -88,7 +88,8 @@ static int __live_con
Re: [Intel-gfx] [RFC PATCH] tests/gem_userptr_blits: Check for banned mmap-offset
On Fri, 2021-04-09 at 10:57 +0200, Janusz Krzysztofik wrote:
> Support for mmap-offset to userptr has been obsoleted, then related
> lockdep splat reported issues are not going to be resolved other than
> still banning mmap-offset to userptr attempts.
>
> Replace "mmap-offset-invalidate-*" and "readonly-mmap-unsync"
> subtests
> which now skip with a negative "mmap-offset-banned" that fails if a
> mmap-offset attempt to a userptr object doesn't return ENODEV. Also,
> remove mmap-offset to userptr dependent processing paths from other
> subtest bodies and drop obsolete subtest variants.
>
> Signed-off-by: Janusz Krzysztofik >
> ---
> tests/i915/gem_userptr_blits.c | 324 +++--
>
> 1 file changed, 30 insertions(+), 294 deletions(-)
>
> diff --git a/tests/i915/gem_userptr_blits.c
> b/tests/i915/gem_userptr_blits.c
> index 7a80c0161..aad5f141b 100644
> --- a/tests/i915/gem_userptr_blits.c
> +++ b/tests/i915/gem_userptr_blits.c
> @@ -70,52 +70,12 @@
> #endif
>
> static uint32_t userptr_flags;
> -static bool *can_mmap;
>
> #define WIDTH 512
> #define HEIGHT 512
>
> static uint32_t linear[WIDTH*HEIGHT];
>
> -static bool has_mmap(int i915, const struct mmap_offset *t)
> -{
> - void *ptr, *map;
> - uint32_t handle;
> -
> - handle = gem_create(i915, PAGE_SIZE);
> - map = __gem_mmap_offset(i915, handle, 0, PAGE_SIZE, PROT_WRITE,
> - t->type);
> - gem_close(i915, handle);
> - if (map) {
> - munmap(map, PAGE_SIZE);
> - } else {
> - igt_debug("no HW / kernel support for mmap-
> offset(%s)\n",
> - t->name);
> - return false;
> - }
> - map = NULL;
> -
> - igt_assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE) == 0);
> -
> - if (__gem_userptr(i915, ptr, 4096, 0,
> - I915_USERPTR_UNSYNCHRONIZED, &handle))
> - goto out_ptr;
> - igt_assert(handle != 0);
> -
> - map = __gem_mmap_offset(i915, handle, 0, 4096, PROT_WRITE, t-
> >type);
> - if (map)
> - munmap(map, 4096);
> - else
> - igt_debug("mmap-offset(%s) banned, lockdep loop
> prevention\n",
> - t->name);
> -
> - gem_close(i915, handle);
> -out_ptr:
> - free(ptr);
> -
> - return map != NULL;
> -}
> -
> static void gem_userptr_test_unsynchronized(void)
> {
> userptr_flags = I915_USERPTR_UNSYNCHRONIZED;
> @@ -914,28 +874,13 @@ static int test_invalid_mapping(int fd, const
> struct mmap_offset *t)
> }
>
> #define PE_BUSY 0x1
> -static void test_process_exit(int fd, const struct mmap_offset *mmo,
> int flags)
> +static void test_process_exit(int fd, int flags)
> {
> - if (mmo)
> - igt_require_f(can_mmap[mmo->type],
> - "HW & kernel support for LLC and mmap-
> offset(%s) over userptr\n",
> - mmo->name);
> -
> igt_fork(child, 1) {
> uint32_t handle;
>
> handle = create_userptr_bo(fd, sizeof(linear));
>
> - if (mmo) {
> - uint32_t *ptr;
> -
> - ptr = __gem_mmap_offset(fd, handle, 0,
> sizeof(linear),
> - PROT_READ | PROT_WRITE,
> - mmo->type);
> - if (ptr)
> - *ptr = 0;
> - }
> -
> if (flags & PE_BUSY)
> igt_assert_eq(copy(fd, handle, handle), 0);
> }
> @@ -1064,53 +1009,30 @@ static int test_map_fixed_invalidate(int fd,
> uint32_t flags,
> return 0;
> }
>
> -static void test_mmap_offset_invalidate(int fd,
> - const struct mmap_offset *t,
> - unsigned int flags)
> -#define MMOI_ACTIVE (1u << 0)
> +static void test_mmap_offset_banned(int fd, const struct mmap_offset
> *t)
> {
> - igt_spin_t *spin = NULL;
> - uint32_t handle;
> - uint32_t *map;
> + struct drm_i915_gem_mmap_offset arg;
> void *ptr;
>
> /* check if mmap_offset type is supported by hardware, skip if
> not */
> - handle = gem_create(fd, PAGE_SIZE);
> - map = __gem_mmap_offset(fd, handle, 0, PAGE_SIZE,
> - PROT_READ | PROT_WRITE, t->type);
> - igt_require_f(map,
> - "HW & kernel support for mmap_offset(%s)\n", t-
> >name);
> - munmap(map, PAGE_SIZE);
> - gem_close(fd, handle);
> + memset(&arg, 0, sizeof(arg));
> + arg.flags = t->type;
> + arg.handle = gem_create(fd, PAGE_SIZE);
> + igt_skip_on_f(igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_OFFSET,
> &arg),
> + "HW & kernel support for
> mmap_offset(%s)\n", t->name);
> + gem_close(fd, arg.handle);
>
> /* create userptr object */
> + memset(&arg, 0, sizeof(arg));
> + arg.flags = t->type;
> igt_assert_eq
[Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/gt: Fix a lockdep warning on RT kernel
== Series Details == Series: drm/i915/gt: Fix a lockdep warning on RT kernel URL : https://patchwork.freedesktop.org/series/89108/ State : warning == Summary == $ make htmldocs 2>&1 > /dev/null | grep i915 ./drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:102: warning: Function parameter or member 'ww' not described in 'i915_gem_shrink' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'jump_whitelist' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'shadow_map' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'batch_map' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Fix a lockdep warning on RT kernel
== Series Details == Series: drm/i915/gt: Fix a lockdep warning on RT kernel URL : https://patchwork.freedesktop.org/series/89108/ State : failure == Summary == CI Bug Log - changes from CI_DRM_9973 -> Patchwork_19941 Summary --- **FAILURE** Serious unknown changes coming with Patchwork_19941 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_19941, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/index.html Possible new issues --- Here are the unknown changes that may have been introduced in Patchwork_19941: ### IGT changes ### Possible regressions * igt@gem_exec_create@basic: - fi-ivb-3770:[PASS][1] -> [DMESG-WARN][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-ivb-3770/igt@gem_exec_cre...@basic.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-ivb-3770/igt@gem_exec_cre...@basic.html * igt@gem_exec_fence@basic-busy@rcs0: - fi-kbl-x1275: [PASS][3] -> [DMESG-WARN][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-kbl-x1275/igt@gem_exec_fence@basic-b...@rcs0.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-kbl-x1275/igt@gem_exec_fence@basic-b...@rcs0.html * igt@gem_exec_fence@basic-busy@vecs0: - fi-cfl-8700k: [PASS][5] -> [DMESG-WARN][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-cfl-8700k/igt@gem_exec_fence@basic-b...@vecs0.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-cfl-8700k/igt@gem_exec_fence@basic-b...@vecs0.html - fi-cml-s: [PASS][7] -> [DMESG-WARN][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-cml-s/igt@gem_exec_fence@basic-b...@vecs0.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-cml-s/igt@gem_exec_fence@basic-b...@vecs0.html - fi-cfl-guc: [PASS][9] -> [DMESG-WARN][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-cfl-guc/igt@gem_exec_fence@basic-b...@vecs0.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-cfl-guc/igt@gem_exec_fence@basic-b...@vecs0.html * igt@gem_exec_fence@basic-wait@rcs0: - fi-bsw-n3050: [PASS][11] -> [DMESG-WARN][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-bsw-n3050/igt@gem_exec_fence@basic-w...@rcs0.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-bsw-n3050/igt@gem_exec_fence@basic-w...@rcs0.html * igt@gem_exec_fence@basic-wait@vcs0: - fi-skl-6700k2: [PASS][13] -> [DMESG-WARN][14] [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-skl-6700k2/igt@gem_exec_fence@basic-w...@vcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-skl-6700k2/igt@gem_exec_fence@basic-w...@vcs0.html * igt@gem_exec_gttfill@basic: - fi-skl-guc: [PASS][15] -> [DMESG-WARN][16] [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-skl-guc/igt@gem_exec_gttf...@basic.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-skl-guc/igt@gem_exec_gttf...@basic.html - fi-hsw-4770:[PASS][17] -> [DMESG-WARN][18] [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-hsw-4770/igt@gem_exec_gttf...@basic.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-hsw-4770/igt@gem_exec_gttf...@basic.html - fi-tgl-u2: [PASS][19] -> [DMESG-WARN][20] [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-tgl-u2/igt@gem_exec_gttf...@basic.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-tgl-u2/igt@gem_exec_gttf...@basic.html * igt@gem_exec_parallel@engines@fds: - fi-apl-guc: [PASS][21] -> [DMESG-WARN][22] [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-apl-guc/igt@gem_exec_parallel@engi...@fds.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-apl-guc/igt@gem_exec_parallel@engi...@fds.html * igt@i915_selftest@live@dmabuf: - fi-snb-2520m: [PASS][23] -> [DMESG-FAIL][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-snb-2520m/igt@i915_selftest@l...@dmabuf.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-snb-2520m/igt@i915_selftest@l...@dmabuf.html * igt@i915_selftest@live@execlists: - fi-glk-dsi: [PASS][25] -> [DMESG-FAIL][26] [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9973/fi-glk-dsi/igt@i915_selftest@l...@execlists.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19941/fi-glk-dsi/igt@i915_selftest@l...@execlists.html - fi-skl-6600u: NOTRUN -> [DMESG-FAIL][27] [27]: https://inte
Re: [Intel-gfx] [PATCH v2 08/12] drm/i915: finish removal of gen_mask
On Wed, 14 Apr 2021, Tvrtko Ursulin wrote:
> On 14/04/2021 14:13, Jani Nikula wrote:
>> On Wed, 14 Apr 2021, Tvrtko Ursulin wrote:
>>> On 13/04/2021 06:09, Lucas De Marchi wrote:
Now that it's not used anywhere, remove it from struct
intel_device_info. To allow a period in which code will be converted to
the new macro, keep IS_GEN_RANGE() around, just redefining it to use
the new fields. The size advantage from IS_GEN_RANGE() using a mask is
not that big as it has pretty limited use througout the driver:
textdata bss dec hex filename
2758497 959656496 2860958 2ba79e drivers/gpu/drm/i915/i915.ko.old
2758586 959536496 2861035 2ba7eb drivers/gpu/drm/i915/i915.ko.new
>>>
>>> This delta refers to this patch - I mean this point in the series?
>>> Asking because it may not be 100% representative since some of the
>>> previous patches have already removed some gen mask usages.
>>>
>>> While I am here, I am a bit fond of the mask approach and wonder if
>>> using it for all (gt/media/whatelse) new fields would still make sense.
>>>
>>> Presence of the range check helpers suggests that it might, but I
>>> haven't looked at how prevalent their usage ends up after the series is
>>> done. So just in principle, I don't see why not still go with masks
>>> since that guarantees elegant check at each range check site. It would
>>> be all hidden in the macro implementation so easy.
>>>
>>> Also for historical reference, another reason why I went for masks
>>> everywhere approach is that at some point we had a feature request to
>>> allow compiling out platforms/gens. I *think* that was much easier to do
>>> with masking and in experiments back then I was able for instance to
>>> build just for Gen9+ and drop like 30% of the binary size.
>>>
>>> Oh I found the branch now.. The reason for IS_GEN(p, v) was also in that
>>> series. I don't know if I ever RFC-ed or trybotted it.. google suggests
>>> no and I neither can find it in my mailboxes. I could send out the old
>>> patches for reference? But to be honest I have no idea if this feature
>>> request (targeted driver builds) will ever resurface..
>>
>> I completely agreed with the direction of using the masks way back when,
>> especially with the goal of the conditional/targeted compilation.
>>
>> I think the question now is whether we want to keep maintaining them
>> just for the sake of the masks. Keeping them means having three masks
>> instead of one. And we wouldn't be using most of the benefits with them,
>> we'd mostly just get the downsides.
>>
>> Having the masks per se is not such a big deal, but they're also not
>> such a big deal to add back later on if needed. It's the codebase all
>> over that's the hard part. And arguably it's not getting that much
>> different with the series at hand; the direct uses of INTEL_GEN() and
>> DISPLAY_VER() vastly outnumber IS_GEN(), IS_GEN_RANGE() and
>> IS_DISPLAY_RANGE() which could benefit from the mask.
>>
>> We'd still be retaining the range macros as IS_GRAPHICS_VER(),
>> IS_MEDIA_VER() and IS_DISPLAY_VER(), although more for clarity than for
>> any other reason.
>
> Adding masks later would not a big deal, but another cycle of changing
> "xxx_VER == n" to "IS_xxx_VER(n)" is a churn which could presumably be
> avoided.
Direct xxx_VER <, >, <= and >= already exist all over the place, and
their numbers trump the == cases. Seems confusing to treat ==
differently.
> It is moot yes, but I don't see a clear case for doing the reversal as
> part of this series. With a disclaimer that I only glanced over the
> commit messages today for the first time.
So I wanted to keep using the range check macros for a couple of
reasons. Having (VER >= x && VER <= y) gets long, it needs braces, and
we use a bunch of negation !(VER >= x && VER <= y) vs. VER < x || VER >
y. !IS_GEN_RANGE() has more clarity.
Now, adding IS_GRAPHICS_VER_RANGE() gets long. Dropping the VER for
IS_GRAPHICS_RANGE() gets confusing ("what graphics range?"). Now, if we
use == for specific version check, we can repurpose IS_GRAPHICS_VER() to
do the ranges.
> So I think from me its neither ack or nack, at least since I don't
> understand the attractiveness of using the "ver == n" and numerical
> range check forms everywhere. As said, if we are churning I'd rather go
> the other direction. But that's a soft objection only so feel free to
> proceed.
Thanks, noted. However, unless stronger objections arise, I think we're
going to go with the patches at hand.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/19] drm/i915: Update the helper to set correct mapping
On 15/04/2021 10:23, Matthew Auld wrote:
On Thu, 15 Apr 2021 at 09:21, Tvrtko Ursulin
wrote:
On 14/04/2021 17:20, Matthew Auld wrote:
On Wed, 14 Apr 2021 at 16:22, Tvrtko Ursulin
wrote:
On 12/04/2021 10:05, Matthew Auld wrote:
From: Venkata Sandeep Dhanalakota
Determine the possible coherent map type based on object location,
and if target has llc or if user requires an always coherent
mapping.
Cc: Matthew Auld
Cc: CQ Tang
Suggested-by: Michal Wajdeczko
Signed-off-by: Venkata Sandeep Dhanalakota
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c| 3 ++-
drivers/gpu/drm/i915/gt/intel_engine_pm.c| 2 +-
drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
drivers/gpu/drm/i915/gt/intel_ring.c | 9 ++---
drivers/gpu/drm/i915/gt/selftest_context.c | 3 ++-
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++--
drivers/gpu/drm/i915/gt/selftest_lrc.c | 4 +++-
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 4 +++-
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 4 +++-
drivers/gpu/drm/i915/i915_drv.h | 11 +--
drivers/gpu/drm/i915/selftests/igt_spinner.c | 4 ++--
11 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index efe935f80c1a..b79568d370f5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -664,7 +664,8 @@ static int init_status_page(struct intel_engine_cs *engine)
if (ret)
goto err;
- vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
+ vaddr = i915_gem_object_pin_map(obj,
+ i915_coherent_map_type(engine->i915, obj,
true));
if (IS_ERR(vaddr)) {
ret = PTR_ERR(vaddr);
goto err_unpin;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 7c9af86fdb1e..47f4397095e5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -23,7 +23,7 @@ static void dbg_poison_ce(struct intel_context *ce)
if (ce->state) {
struct drm_i915_gem_object *obj = ce->state->obj;
- int type = i915_coherent_map_type(ce->engine->i915);
+ int type = i915_coherent_map_type(ce->engine->i915, obj, true);
void *map;
if (!i915_gem_object_trylock(obj))
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index e86897cde984..aafe2a4df496 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -903,7 +903,9 @@ lrc_pre_pin(struct intel_context *ce,
GEM_BUG_ON(!i915_vma_is_pinned(ce->state));
*vaddr = i915_gem_object_pin_map(ce->state->obj,
- i915_coherent_map_type(ce->engine->i915)
|
+ i915_coherent_map_type(ce->engine->i915,
+ ce->state->obj,
+ false) |
I915_MAP_OVERRIDE);
return PTR_ERR_OR_ZERO(*vaddr);
diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c
b/drivers/gpu/drm/i915/gt/intel_ring.c
index aee0a77c77e0..3cf6c7e68108 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -53,9 +53,12 @@ int intel_ring_pin(struct intel_ring *ring, struct
i915_gem_ww_ctx *ww)
if (i915_vma_is_map_and_fenceable(vma))
addr = (void __force *)i915_vma_pin_iomap(vma);
- else
- addr = i915_gem_object_pin_map(vma->obj,
-
i915_coherent_map_type(vma->vm->i915));
+ else {
+ int type = i915_coherent_map_type(vma->vm->i915, vma->obj, false);
+
+ addr = i915_gem_object_pin_map(vma->obj, type);
+ }
+
if (IS_ERR(addr)) {
ret = PTR_ERR(addr);
goto err_ring;
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c
b/drivers/gpu/drm/i915/gt/selftest_context.c
index b9bdd1d23243..26685b927169 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -88,7 +88,8 @@ static int __live_context_size(struct intel_engine_cs *engine)
goto err;
vaddr = i915_gem_object_pin_map_unlocked(ce->state->obj,
-
i915_coherent_map_type(engine->i915));
+
i915_coherent_map_type(engine->i915,
+
ce->state->obj, false));
if (IS_ERR(vaddr)) {
err = PTR_ERR(vaddr);
intel_context_unpin(ce);
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
b/drivers/
Re: [Intel-gfx] [PATCH][V2] drm/i915/gt: Fix a lockdep warning on RT kernel
Hi,
On 14/04/2021 15:48, Jun Miao wrote:
Don`t simple disable all the HD-irq, should race the region in the
intel_breadcrumbs_disarm_irq() only.
What is HD-irq, I am, not familiar with that term?
BUG: sleeping function called from invalid context at
kernel/locking/rtmutex.c:969
#0: 89c4c00ca970 ((wq_completion)events){+.+.}-{0:0}, at:
process_one_work+0x1cf/0x6d0
#1: a433c1f53e60 ((work_completion)(&engine->retire_work)){+.+.}-{0:0},
at: process_one_work+0x1cf 0x6d
#2: 89c4ccb0a0a8 (kernel_context){+.+.}-{0:0}, at:
engine_retire+0x62/0x110 [i915]
#3: 89c4cf682300 (wakeref.mutex#3){+.+.}-{0:0}, at:
__intel_wakeref_put_last+0x20/0x60 [i915]
#4: 89c4ccb08398 (&b->irq_lock){+.+.}-{0:0}, at:
intel_breadcrumbs_disarm_irq+0x20/0xd0 [i915]
irq event stamp: 2126
hardirqs last enabled at (2125): []
cancel_delayed_work+0xa9/0xc0
hardirqs last disabled at (2126): []
__intel_breadcrumbs_park+0x76/0x80 [i915]
softirqs last enabled at (0): [] copy_process+0x63e/0x1630
softirqs last disabled at (0): [<>] 0x0
CPU: 3 PID: 281 Comm: kworker/3:3 Not tainted 5.10.27-rt34-yocto-preempt-rt #1
Hardware name: Intel(R) Client Systems NUC7i5DNKE/NUC7i5DNB, BIOS
DNKBLi5v.86A.0064.2019.0523.1933 05/23 2019
Workqueue: events engine_retire [i915]
Call Trace:
show_stack+0x52/0x58
dump_stack+0x7d/0x9f
___might_sleep.cold+0xe3/0xf4
rt_spin_lock+0x3f/0xc0
? intel_breadcrumbs_disarm_irq+0x20/0xd0 [i915]
intel_breadcrumbs_disarm_irq+0x20/0xd0 [i915]
signal_irq_work+0x241/0x660 [i915]
? __this_cpu_preempt_check+0x13/0x20
? lockdep_hardirqs_off+0x106/0x120
__intel_breadcrumbs_park+0x3f/0x80 [i915]
__engine_park+0xbd/0xe0 [i915]
intel_wakeref_put_last+0x22/0x60 [i915]
__intel_wakeref_put_last+0x50/0x60 [i915]
intel_context_exit_engine+0x5f/0x70 [i915]
i915_request_retire+0x139/0x2d0 [i915]
engine_retire+0xb0/0x110 [i915]
process_one_work+0x26d/0x6d0
worker_thread+0x53/0x330
kthread+0x1b0/0x1d0
? process_one_work+0x6d0/0x6d0
? __kthread_parkme+0xc0/0xc0
ret_from_fork+0x22/0x30
Fixes: 9d5612ca165a ("drm/i915/gt: Defer enabling the breadcrumb interrupt to after
submission")
Signed-off-by: Jun Miao
---
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index 34a645d..0589b1a 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -103,10 +103,12 @@ static void __intel_breadcrumbs_disarm_irq(struct
intel_breadcrumbs *b)
static void intel_breadcrumbs_disarm_irq(struct intel_breadcrumbs *b)
{
- spin_lock(&b->irq_lock);
+ unsigned long flags;
+
+ spin_lock_irqsave(&b->irq_lock, flags);
if (b->irq_armed)
__intel_breadcrumbs_disarm_irq(b);
- spin_unlock(&b->irq_lock);
+ spin_unlock_irqrestore(&b->irq_lock, flags);
}
static void add_signaling_context(struct intel_breadcrumbs *b,
@@ -337,9 +339,7 @@ void __intel_breadcrumbs_park(struct intel_breadcrumbs *b)
/* Kick the work once more to drain the signalers, and disarm the irq */
irq_work_sync(&b->irq_work);
while (READ_ONCE(b->irq_armed) && !atomic_read(&b->active)) {
- local_irq_disable();
signal_irq_work(&b->irq_work);
- local_irq_enable();
Unfortunately there is another lock inside signal_irq_work (rq->lock)
which needs to be taken irq safe.
RT patches are in tree or out of the tree these days?
Regards,
Tvrtko
cond_resched();
}
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/11] drm/i915/adl_p: Add support for Display Page Tables
On Wed, 14 Apr 2021, Imre Deak wrote: > Alder Lake-P adds a new Display Page Table hardware structure, mapping > tiled framebuffer pages to the display engine, reducing the address > space required in GGTT for these framebuffers. > > This patchset adds support for this taking a minimum set of dependency > patches from the ADL_P enabling patchset at > https://patchwork.freedesktop.org/series/87897/ Cc: Daniel I guess we'll need a topic branch for the base enabling to merge to both din and dign? I guess it'll need to include the stuff in topic/intel-gen-to-ver too. Shared stuff like this keeps being a problem with the separate dign branch, especially when the only way to sync is to merge both din and dign to drm-next and then backmerge to both. BR, Jani. > > Clinton Taylor (2): > drm/i915/adl_p: Add PCI Devices IDs > drm/i915/adl_p: ADL_P device info enabling > > Imre Deak (4): > drm/i915: Pass intel_framebuffer instad of drm_framebuffer to > intel_fill_fb_info() > drm/i915/adl_p: Disable support for 90/270 FB rotation > drm/i915/adl_p: Require a minimum of 8 tiles stride for DPT FBs > drm/i915/adl_p: Enable remapping to pad DPT FB strides to POT > > José Roberto de Souza (2): > drm/i915/xelpd: Fallback to plane stride limitations when using DPT > drm/i915/adl_p: Add stride restriction when using DPT > > Juha-Pekka Heikkilä (1): > drm/i915/xelpd: Support 128k plane stride > > Matt Roper (1): > drm/i915/xelpd: add XE_LPD display characteristics > > Ville Syrjälä (1): > drm/i915/xelpd: First stab at DPT support > > arch/x86/kernel/early-quirks.c| 1 + > .../gpu/drm/i915/display/intel_atomic_plane.c | 7 +- > drivers/gpu/drm/i915/display/intel_display.c | 381 -- > drivers/gpu/drm/i915/display/intel_display.h | 1 + > .../drm/i915/display/intel_display_types.h| 25 +- > drivers/gpu/drm/i915/display/intel_fb.c | 92 +++-- > drivers/gpu/drm/i915/display/intel_fb.h | 5 +- > drivers/gpu/drm/i915/display/intel_fbc.c | 6 +- > .../drm/i915/display/skl_universal_plane.c| 68 +++- > drivers/gpu/drm/i915/gt/gen8_ppgtt.h | 7 + > drivers/gpu/drm/i915/gt/intel_ggtt.c | 7 +- > drivers/gpu/drm/i915/gt/intel_gtt.h | 5 + > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_pci.c | 22 + > drivers/gpu/drm/i915/i915_reg.h | 2 + > drivers/gpu/drm/i915/intel_device_info.c | 1 + > drivers/gpu/drm/i915/intel_device_info.h | 1 + > include/drm/i915_pciids.h | 21 + > 18 files changed, 567 insertions(+), 86 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm/tegra: Don't set allow_fb_modifiers explicitly
On Tue, Apr 13, 2021 at 01:37:38PM +0200, Thierry Reding wrote:
> On Tue, Apr 13, 2021 at 11:49:01AM +0200, Daniel Vetter wrote:
> > Since
> >
> > commit 890880ddfdbe256083170866e49c87618b706ac7
> > Author: Paul Kocialkowski
> > Date: Fri Jan 4 09:56:10 2019 +0100
> >
> > drm: Auto-set allow_fb_modifiers when given modifiers at plane init
> >
> > this is done automatically as part of plane init, if drivers set the
> > modifier list correctly. Which is the case here.
> >
> > It was slightly inconsistently though, since planes with only linear
> > modifier support haven't listed that explicitly. Fix that, and cc:
> > stable to allow userspace to rely on this. Again don't backport
> > further than where Paul's patch got added.
> >
> > Cc: sta...@vger.kernel.org # v5.1 +
> > Cc: Pekka Paalanen
> > Signed-off-by: Daniel Vetter
> > Cc: Thierry Reding
> > Cc: Jonathan Hunter
> > Cc: linux-te...@vger.kernel.org
> > ---
> > drivers/gpu/drm/tegra/dc.c | 10 --
> > drivers/gpu/drm/tegra/drm.c | 2 --
> > 2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > index c9385cfd0fc1..f9845a50f866 100644
> > --- a/drivers/gpu/drm/tegra/dc.c
> > +++ b/drivers/gpu/drm/tegra/dc.c
> > @@ -959,6 +959,11 @@ static const struct drm_plane_helper_funcs
> > tegra_cursor_plane_helper_funcs = {
> > .atomic_disable = tegra_cursor_atomic_disable,
> > };
> >
> > +static const uint64_t linear_modifiers[] = {
> > + DRM_FORMAT_MOD_LINEAR,
> > + DRM_FORMAT_MOD_INVALID
> > +};
> > +
> > static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device
> > *drm,
> > struct tegra_dc *dc)
> > {
> > @@ -987,7 +992,7 @@ static struct drm_plane
> > *tegra_dc_cursor_plane_create(struct drm_device *drm,
> >
> > err = drm_universal_plane_init(drm, &plane->base, possible_crtcs,
> >&tegra_plane_funcs, formats,
> > - num_formats, NULL,
> > + num_formats, linear_modifiers,
> >DRM_PLANE_TYPE_CURSOR, NULL);
> > if (err < 0) {
> > kfree(plane);
> > @@ -1106,7 +,8 @@ static struct drm_plane
> > *tegra_dc_overlay_plane_create(struct drm_device *drm,
> >
> > err = drm_universal_plane_init(drm, &plane->base, possible_crtcs,
> >&tegra_plane_funcs, formats,
> > - num_formats, NULL, type, NULL);
> > + num_formats, linear_modifiers,
> > + type, NULL);
>
> I think we can do better than linear_modifiers for overlay planes, but
> given that this doesn't change existing behaviour, I'll do that in a
> separate patch.
>
> Acked-by: Thierry Reding
Applied to drm-misc-next, thanks for taking a look.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH] tests/gem_userptr_blits: Check for banned mmap-offset
On czwartek, 15 kwietnia 2021 11:47:29 CEST Marcin Bernatowicz wrote: > On Fri, 2021-04-09 at 10:57 +0200, Janusz Krzysztofik wrote: > > Support for mmap-offset to userptr has been obsoleted, then related > > lockdep splat reported issues are not going to be resolved other than > > still banning mmap-offset to userptr attempts. > > > > Replace "mmap-offset-invalidate-*" and "readonly-mmap-unsync" > > subtests > > which now skip with a negative "mmap-offset-banned" that fails if a > > mmap-offset attempt to a userptr object doesn't return ENODEV. Also, > > remove mmap-offset to userptr dependent processing paths from other > > subtest bodies and drop obsolete subtest variants. > > > > Signed-off-by: Janusz Krzysztofik LGTM, > Reviewed-by: Marcin Bernatowicz Thank you Marcin, pushed. Janusz ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/12] drm/imx: Don't set allow_fb_modifiers explicitly
On Tue, Apr 13, 2021 at 01:47:28PM +0200, Lucas Stach wrote: > Am Dienstag, dem 13.04.2021 um 11:48 +0200 schrieb Daniel Vetter: > > Since > > > > commit 890880ddfdbe256083170866e49c87618b706ac7 > > Author: Paul Kocialkowski > > Date: Fri Jan 4 09:56:10 2019 +0100 > > > > drm: Auto-set allow_fb_modifiers when given modifiers at plane init > > > > this is done automatically as part of plane init, if drivers set the > > modifier list correctly. Which is the case here. > > > > This one actually set it twice on top of what drm_plane_init does, so > > double-redundant! > > That's not true. imx-dcss and imx-drm are two totally separate drivers. > Maybe we should move imx-drm into its own ipuv3 directory one day to > make this more clear. Change is still correct, though. I've fixed the commit message to reflect reality and merged to drm-misc-next. Thanks for taking a look. -Daniel > > Reviewed-by: Lucas Stach > > > Signed-off-by: Daniel Vetter > > Cc: Philipp Zabel > > Cc: Shawn Guo > > Cc: Sascha Hauer > > Cc: Pengutronix Kernel Team > > Cc: Fabio Estevam > > Cc: NXP Linux Team > > Cc: linux-arm-ker...@lists.infradead.org > > --- > > drivers/gpu/drm/imx/dcss/dcss-kms.c | 1 - > > drivers/gpu/drm/imx/imx-drm-core.c | 1 - > > 2 files changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c > > b/drivers/gpu/drm/imx/dcss/dcss-kms.c > > index b549ce5e7607..37ae68a7fba5 100644 > > --- a/drivers/gpu/drm/imx/dcss/dcss-kms.c > > +++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c > > @@ -52,7 +52,6 @@ static void dcss_kms_mode_config_init(struct dcss_kms_dev > > *kms) > > config->min_height = 1; > > config->max_width = 4096; > > config->max_height = 4096; > > - config->allow_fb_modifiers = true; > > config->normalize_zpos = true; > > > > > > > > > > config->funcs = &dcss_drm_mode_config_funcs; > > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c > > b/drivers/gpu/drm/imx/imx-drm-core.c > > index 2ded8e4f32d0..8be4edaec958 100644 > > --- a/drivers/gpu/drm/imx/imx-drm-core.c > > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > > @@ -209,7 +209,6 @@ static int imx_drm_bind(struct device *dev) > > drm->mode_config.max_height = 4096; > > drm->mode_config.funcs = &imx_drm_mode_config_funcs; > > drm->mode_config.helper_private = &imx_drm_mode_config_helpers; > > - drm->mode_config.allow_fb_modifiers = true; > > drm->mode_config.normalize_zpos = true; > > > > > > > > > > ret = drmm_mode_config_init(drm); > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 1/2] tests/core_hotunplug: Add perf health check
On środa, 14 kwietnia 2021 11:50:10 CEST Marcin Bernatowicz wrote: > On Thu, 2021-04-08 at 10:31 +0200, Janusz Krzysztofik wrote: > > Sometimes CI reports skips of perf subtests when run subsequently > > after > > core_hotunplug. That may be an indication of issues with restoring > > device perf features on driver (hot)rebind. > > > > Detect device perf support at test start and check if still available > > after driver rebind. If that fails, a post-subtest device recovery > > step restores the device perf support so no subsequently executed > > tests > > are affected. > > > > Signed-off-by: Janusz Krzysztofik LGTM, > Acked-by: Marcin Bernatowicz Thank you Marcin, pushed. Janusz > > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gvt: Fix error code in intel_gvt_init_device()
On 2021.04.14 09:01:38 +0300, Dan Carpenter wrote:
> The intel_gvt_init_vgpu_type_groups() function is only called from
> intel_gvt_init_device(). If it fails then the intel_gvt_init_device()
> prints the error code and propagates it back again. That's a bug
> because false is zero/success. The fix is to modify it to return zero
> or negative error codes and make everything consistent.
>
> Fixes: c5d71cb31723 ("drm/i915/gvt: Move vGPU type related code into gvt
> file")
> Signed-off-by: Dan Carpenter
> ---
Reviewed-by: Zhenyu Wang
Thanks, Dan! Applied this.
> drivers/gpu/drm/i915/gvt/gvt.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 2ecb8534930b..1deb253ffe80 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -126,7 +126,7 @@ static bool intel_get_gvt_attrs(struct attribute_group
> ***intel_vgpu_type_groups
> return true;
> }
>
> -static bool intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
> +static int intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
> {
> int i, j;
> struct intel_vgpu_type *type;
> @@ -144,7 +144,7 @@ static bool intel_gvt_init_vgpu_type_groups(struct
> intel_gvt *gvt)
> gvt_vgpu_type_groups[i] = group;
> }
>
> - return true;
> + return 0;
>
> unwind:
> for (j = 0; j < i; j++) {
> @@ -152,7 +152,7 @@ static bool intel_gvt_init_vgpu_type_groups(struct
> intel_gvt *gvt)
> kfree(group);
> }
>
> - return false;
> + return -ENOMEM;
> }
>
> static void intel_gvt_cleanup_vgpu_type_groups(struct intel_gvt *gvt)
> @@ -373,7 +373,7 @@ int intel_gvt_init_device(struct drm_i915_private *i915)
> goto out_clean_thread;
>
> ret = intel_gvt_init_vgpu_type_groups(gvt);
> - if (ret == false) {
> + if (ret) {
> gvt_err("failed to init vgpu type groups: %d\n", ret);
> goto out_clean_types;
> }
> --
> 2.30.2
>
> ___
> intel-gvt-dev mailing list
> intel-gvt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave and Daniel, Here goes drm-intel-fixes-2021-04-15: Display panel & power related fixes: - Backlight fix (Lyude) - Display watermark fix (Ville) - VLV panel power fix (Hans) Thanks, Rodrigo. The following changes since commit d434405aaab7d0ebc516b68a8fc4100922d7f5ef: Linux 5.12-rc7 (2021-04-11 15:16:13 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2021-04-15 for you to fetch changes up to aee6f25e9c911323aa89a200e1bb160c1613ed3d: drm/i915/display/vlv_dsi: Do not skip panel_pwr_cycle_delay when disabling the panel (2021-04-12 08:00:33 -0400) Display panel & power related fixes: - Backlight fix (Lyude) - Display watermark fix (Ville) - VLV panel power fix (Hans) Hans de Goede (1): drm/i915/display/vlv_dsi: Do not skip panel_pwr_cycle_delay when disabling the panel Lyude Paul (1): drm/i915/dpcd_bl: Don't try vesa interface unless specified by VBT Ville Syrjälä (1): drm/i915: Don't zero out the Y plane's watermarks drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 1 - drivers/gpu/drm/i915/display/vlv_dsi.c| 4 ++-- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-intel-fixes
On Thu, Apr 15, 2021 at 08:59:11AM -0400, Rodrigo Vivi wrote: > Hi Dave and Daniel, > > Here goes drm-intel-fixes-2021-04-15: > > Display panel & power related fixes: > > - Backlight fix (Lyude) > - Display watermark fix (Ville) > - VLV panel power fix (Hans) > > Thanks, > Rodrigo. > > The following changes since commit d434405aaab7d0ebc516b68a8fc4100922d7f5ef: > > Linux 5.12-rc7 (2021-04-11 15:16:13 -0700) > > are available in the Git repository at: > > git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2021-04-15 > > for you to fetch changes up to aee6f25e9c911323aa89a200e1bb160c1613ed3d: > > drm/i915/display/vlv_dsi: Do not skip panel_pwr_cycle_delay when disabling > the panel (2021-04-12 08:00:33 -0400) Applied to drm-fixes, thanks for the pull. -Daniel > > > Display panel & power related fixes: > > - Backlight fix (Lyude) > - Display watermark fix (Ville) > - VLV panel power fix (Hans) > > > Hans de Goede (1): > drm/i915/display/vlv_dsi: Do not skip panel_pwr_cycle_delay when > disabling the panel > > Lyude Paul (1): > drm/i915/dpcd_bl: Don't try vesa interface unless specified by VBT > > Ville Syrjälä (1): > drm/i915: Don't zero out the Y plane's watermarks > > drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 1 - > drivers/gpu/drm/i915/display/vlv_dsi.c| 4 ++-- > drivers/gpu/drm/i915/intel_pm.c | 4 ++-- > 3 files changed, 4 insertions(+), 5 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Cross-driver ww transaction lock lists
On Tue, Apr 13, 2021 at 09:57:06AM +0200, Christian König wrote:
>
>
> Am 13.04.21 um 09:50 schrieb Thomas Hellström:
> > Hi!
> >
> > During the dma_resv conversion of the i915 driver, we've been using ww
> > transaction lock lists to keep track of ww_mutexes that are locked
> > during the transaction so that they can be batch unlocked at suitable
> > locations. Including also the LMEM/VRAM eviction we've ended up with
> > two static lists per transaction context; one typically unlocked at the
> > end of transaction and one initialized before and unlocked after each
> > buffer object validate. This enables us to do sleeping locking at
> > eviction and keep objects locked on the eviction list until we
> > eventually succeed allocating memory (modulo minor flaws of course).
> >
> > It would be beneficial with the i915 TTM conversion to be able to
> > introduce a similar functionality that would work in ttm but also
> > cross-driver in, for example move_notify. It would also be beneficial
> > to be able to put any dma_resv ww mutex on the lists, and not require
> > it to be embedded in a particular object type.
> >
> > I started scetching on some utilities for this. For TTM, for example,
> > the idea would be to pass a list head for the ww transaction lock list
> > in the ttm_operation_ctx. A function taking a ww_mutex could then
> > either attach a grabbed lock to the list for batch unlocking, or be
> > responsible for unlocking when the lock's scope is exited. It's also
> > possible to create sublists if so desired. I believe the below would be
> > sufficient to cover the i915 functionality.
> >
> > Any comments and suggestions appreciated!
>
> ah yes Daniel and I haven been discussing something like this for years.
>
> I also came up with rough implementation, but we always ran into lifetime
> issues.
>
> In other words the ww_mutexes which are on the list would need to be kept
> alive until unlocked.
>
> Because of this we kind of backed up and said we would need this on the GEM
> level instead of working with dma_resv objects.
Yeah there's a few funny concerns here that make this awkward:
- For simplicity doing these helpers at the gem level should make things a
bit easier, because then we have a standard way to drop the reference.
It does mean that the only thing you can lock like this are gem objects,
but I think that's fine. At least for a first cut.
- This is a bit awkward for vmwgfx, but a) Zack has mentioned he's looking
into adopting gem bo internally to be able to drop a pile of code and
stop making vmwgfx the only special-case we have b) drivers which don't
need this won't need this, so should be fine.
The other awkward thing I guess is that ttm would need to use the
embedded kref from the gem bo, but that should be transparent I think.
- Next up is dma-buf: For i915 we'd like to do the same eviction trick
also through p2p dma-buf callbacks, so that this works the same as
eviction/reservation within a gpu. But for these internal bo you might
not have a dma-buf, so we can't just lift the trick to the dma-buf
level. But I think if we pass e.g. a struct list_head and a callback to
unreference/unlock all the buffers in there to the exporter, plus
similar for the slowpath lock, then that should be doable without
glorious layering inversions between dma-buf and gem.
I think for dma-buf it should even be ok if this requires that we
allocate an entire structure with kmalloc or something, allocating
memory while holding dma_resv is ok.
- Another reason for doing it at the gem level is that the SoC drivers
should probably use dma_resv more, so having some competent cs/eviction
helpers derived from what we have in ttm would be nice I think.
But also I never got anywhere with anything, since like Christian said if
we just link up ww_mutex, or dma_resv, it always dies on some lifetime
handling issues.
-Daniel
>
> Regards,
> Christian.
>
> >
> > 8<--
> >
> > #ifndef _TRANSACTION_LOCKLIST_H_
> > #define _TRANSACTION_LOCKLIST_H_
> >
> > struct trans_lockitem;
> >
> > /**
> > * struct trans_locklist_ops - Ops structure for the ww locklist
> > functionality.
> > *
> > * Typically a const struct trans_locklist_ops is defined for each type
> > that
> > * embeds a struct trans_lockitem, or hav a struct trans_lockitem
> > pointing
> > * at it using the private pointer. It can be a buffer object,
> > reservation
> > * object, a single ww_mutex or even a sublist of trans_lockitems.
> > */
> > struct trans_locklist_ops {
> > /**
> > * slow_lock: Slow lock to relax the transaction. Only used by
> > * a contending lock item.
> > * @item: The struct trans_lockitem to lock
> > * @intr: Whether to lock interruptible
> > *
> > * Return: -ERESTARTSYS: Hit a signal when relaxing,
> > * -EAGAIN, relaxing succesful, but the contending lock
> > * remains unloc
Re: [Intel-gfx] [Linaro-mm-sig] [RFC] Cross-driver ww transaction lock lists
On 4/15/21 3:37 PM, Daniel Vetter wrote: On Tue, Apr 13, 2021 at 09:57:06AM +0200, Christian König wrote: Am 13.04.21 um 09:50 schrieb Thomas Hellström: Hi! During the dma_resv conversion of the i915 driver, we've been using ww transaction lock lists to keep track of ww_mutexes that are locked during the transaction so that they can be batch unlocked at suitable locations. Including also the LMEM/VRAM eviction we've ended up with two static lists per transaction context; one typically unlocked at the end of transaction and one initialized before and unlocked after each buffer object validate. This enables us to do sleeping locking at eviction and keep objects locked on the eviction list until we eventually succeed allocating memory (modulo minor flaws of course). It would be beneficial with the i915 TTM conversion to be able to introduce a similar functionality that would work in ttm but also cross-driver in, for example move_notify. It would also be beneficial to be able to put any dma_resv ww mutex on the lists, and not require it to be embedded in a particular object type. I started scetching on some utilities for this. For TTM, for example, the idea would be to pass a list head for the ww transaction lock list in the ttm_operation_ctx. A function taking a ww_mutex could then either attach a grabbed lock to the list for batch unlocking, or be responsible for unlocking when the lock's scope is exited. It's also possible to create sublists if so desired. I believe the below would be sufficient to cover the i915 functionality. Any comments and suggestions appreciated! ah yes Daniel and I haven been discussing something like this for years. I also came up with rough implementation, but we always ran into lifetime issues. In other words the ww_mutexes which are on the list would need to be kept alive until unlocked. Because of this we kind of backed up and said we would need this on the GEM level instead of working with dma_resv objects. Yeah there's a few funny concerns here that make this awkward: - For simplicity doing these helpers at the gem level should make things a bit easier, because then we have a standard way to drop the reference. It does mean that the only thing you can lock like this are gem objects, but I think that's fine. At least for a first cut. - This is a bit awkward for vmwgfx, but a) Zack has mentioned he's looking into adopting gem bo internally to be able to drop a pile of code and stop making vmwgfx the only special-case we have b) drivers which don't need this won't need this, so should be fine. The other awkward thing I guess is that ttm would need to use the embedded kref from the gem bo, but that should be transparent I think. - Next up is dma-buf: For i915 we'd like to do the same eviction trick also through p2p dma-buf callbacks, so that this works the same as eviction/reservation within a gpu. But for these internal bo you might not have a dma-buf, so we can't just lift the trick to the dma-buf level. But I think if we pass e.g. a struct list_head and a callback to unreference/unlock all the buffers in there to the exporter, plus similar for the slowpath lock, then that should be doable without glorious layering inversions between dma-buf and gem. I think for dma-buf it should even be ok if this requires that we allocate an entire structure with kmalloc or something, allocating memory while holding dma_resv is ok. Yes, the thing here with the suggested helpers is that you would just embed a trans_lockitem struct in the gem object (and defines the gem object ops). Otherwise and for passing to dma-buf this is pretty much exactly what you are suggesting, but the huge benefit of encapsulating the needed members like this is that when we need to change something we change it in just one place. For anything that doesn't have a gem object (dma-buf, vmwgfx or whatever) you have the choice of either allocating a struct trans_lockitem or embed it wherever you prefer. In particular, this is beneficial where you have a single dma-resv class ww-mutex sitting somewhere in the way and you don't want to needlessly have a gem object that embeds it. /Thomas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Linaro-mm-sig] [RFC] Cross-driver ww transaction lock lists
On Thu, Apr 15, 2021 at 4:02 PM Thomas Hellström (Intel) wrote: > > > On 4/15/21 3:37 PM, Daniel Vetter wrote: > > On Tue, Apr 13, 2021 at 09:57:06AM +0200, Christian König wrote: > >> > >> Am 13.04.21 um 09:50 schrieb Thomas Hellström: > >>> Hi! > >>> > >>> During the dma_resv conversion of the i915 driver, we've been using ww > >>> transaction lock lists to keep track of ww_mutexes that are locked > >>> during the transaction so that they can be batch unlocked at suitable > >>> locations. Including also the LMEM/VRAM eviction we've ended up with > >>> two static lists per transaction context; one typically unlocked at the > >>> end of transaction and one initialized before and unlocked after each > >>> buffer object validate. This enables us to do sleeping locking at > >>> eviction and keep objects locked on the eviction list until we > >>> eventually succeed allocating memory (modulo minor flaws of course). > >>> > >>> It would be beneficial with the i915 TTM conversion to be able to > >>> introduce a similar functionality that would work in ttm but also > >>> cross-driver in, for example move_notify. It would also be beneficial > >>> to be able to put any dma_resv ww mutex on the lists, and not require > >>> it to be embedded in a particular object type. > >>> > >>> I started scetching on some utilities for this. For TTM, for example, > >>> the idea would be to pass a list head for the ww transaction lock list > >>> in the ttm_operation_ctx. A function taking a ww_mutex could then > >>> either attach a grabbed lock to the list for batch unlocking, or be > >>> responsible for unlocking when the lock's scope is exited. It's also > >>> possible to create sublists if so desired. I believe the below would be > >>> sufficient to cover the i915 functionality. > >>> > >>> Any comments and suggestions appreciated! > >> ah yes Daniel and I haven been discussing something like this for years. > >> > >> I also came up with rough implementation, but we always ran into lifetime > >> issues. > >> > >> In other words the ww_mutexes which are on the list would need to be kept > >> alive until unlocked. > >> > >> Because of this we kind of backed up and said we would need this on the GEM > >> level instead of working with dma_resv objects. > > Yeah there's a few funny concerns here that make this awkward: > > - For simplicity doing these helpers at the gem level should make things a > >bit easier, because then we have a standard way to drop the reference. > >It does mean that the only thing you can lock like this are gem objects, > >but I think that's fine. At least for a first cut. > > > > - This is a bit awkward for vmwgfx, but a) Zack has mentioned he's looking > >into adopting gem bo internally to be able to drop a pile of code and > >stop making vmwgfx the only special-case we have b) drivers which don't > >need this won't need this, so should be fine. > > > >The other awkward thing I guess is that ttm would need to use the > >embedded kref from the gem bo, but that should be transparent I think. > > > > - Next up is dma-buf: For i915 we'd like to do the same eviction trick > >also through p2p dma-buf callbacks, so that this works the same as > >eviction/reservation within a gpu. But for these internal bo you might > >not have a dma-buf, so we can't just lift the trick to the dma-buf > >level. But I think if we pass e.g. a struct list_head and a callback to > >unreference/unlock all the buffers in there to the exporter, plus > >similar for the slowpath lock, then that should be doable without > >glorious layering inversions between dma-buf and gem. > > > >I think for dma-buf it should even be ok if this requires that we > >allocate an entire structure with kmalloc or something, allocating > >memory while holding dma_resv is ok. > > Yes, the thing here with the suggested helpers is that you would just > embed a trans_lockitem struct in the gem object (and defines the gem > object ops). Otherwise and for passing to dma-buf this is pretty much > exactly what you are suggesting, but the huge benefit of encapsulating > the needed members like this is that when we need to change something we > change it in just one place. > > For anything that doesn't have a gem object (dma-buf, vmwgfx or > whatever) you have the choice of either allocating a struct > trans_lockitem or embed it wherever you prefer. In particular, this is > beneficial where you have a single dma-resv class ww-mutex sitting > somewhere in the way and you don't want to needlessly have a gem object > that embeds it. The thing is, everyone who actually uses dma_resv_lock has a gem_buffer_object underneath. So it feels a bit like flexibility for no real need, and I think it would make it slightly more awkard for gem drivers to neatly integrate into their cs path. The lockitem struct works, but it is a bit cumbersome. Also if we add some wrappers to e.g. add a gem_bo to the ctx, then if
Re: [Intel-gfx] [PATCH 2/8] drm/i915: Rewrite the FBC tiling check a bit
On Wed, Apr 14, 2021 at 06:09:23PM +0300, Jani Nikula wrote:
> On Wed, 14 Apr 2021, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > Write the tiling check in a nicer form.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 04d9c7d22b04..178243a6d3a2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -681,11 +681,9 @@ static bool tiling_is_valid(struct drm_i915_private
> > *dev_priv,
> > {
> > switch (modifier) {
> > case DRM_FORMAT_MOD_LINEAR:
> > - if (DISPLAY_VER(dev_priv) >= 9)
> > - return true;
> > - return false;
> > - case I915_FORMAT_MOD_X_TILED:
> > case I915_FORMAT_MOD_Y_TILED:
> > + return DISPLAY_VER(dev_priv) >= 9;
>
> So this adds the version check on I915_FORMAT_MOD_Y_TILED which didn't
> have it before?
Yeah, but Y tile scanout is gen9+ anyway. Should have pointed
that out in the commit msg I suppose.
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Linaro-mm-sig] [RFC] Cross-driver ww transaction lock lists
On 4/15/21 4:40 PM, Daniel Vetter wrote: On Thu, Apr 15, 2021 at 4:02 PM Thomas Hellström (Intel) wrote: On 4/15/21 3:37 PM, Daniel Vetter wrote: On Tue, Apr 13, 2021 at 09:57:06AM +0200, Christian König wrote: Am 13.04.21 um 09:50 schrieb Thomas Hellström: Hi! During the dma_resv conversion of the i915 driver, we've been using ww transaction lock lists to keep track of ww_mutexes that are locked during the transaction so that they can be batch unlocked at suitable locations. Including also the LMEM/VRAM eviction we've ended up with two static lists per transaction context; one typically unlocked at the end of transaction and one initialized before and unlocked after each buffer object validate. This enables us to do sleeping locking at eviction and keep objects locked on the eviction list until we eventually succeed allocating memory (modulo minor flaws of course). It would be beneficial with the i915 TTM conversion to be able to introduce a similar functionality that would work in ttm but also cross-driver in, for example move_notify. It would also be beneficial to be able to put any dma_resv ww mutex on the lists, and not require it to be embedded in a particular object type. I started scetching on some utilities for this. For TTM, for example, the idea would be to pass a list head for the ww transaction lock list in the ttm_operation_ctx. A function taking a ww_mutex could then either attach a grabbed lock to the list for batch unlocking, or be responsible for unlocking when the lock's scope is exited. It's also possible to create sublists if so desired. I believe the below would be sufficient to cover the i915 functionality. Any comments and suggestions appreciated! ah yes Daniel and I haven been discussing something like this for years. I also came up with rough implementation, but we always ran into lifetime issues. In other words the ww_mutexes which are on the list would need to be kept alive until unlocked. Because of this we kind of backed up and said we would need this on the GEM level instead of working with dma_resv objects. Yeah there's a few funny concerns here that make this awkward: - For simplicity doing these helpers at the gem level should make things a bit easier, because then we have a standard way to drop the reference. It does mean that the only thing you can lock like this are gem objects, but I think that's fine. At least for a first cut. - This is a bit awkward for vmwgfx, but a) Zack has mentioned he's looking into adopting gem bo internally to be able to drop a pile of code and stop making vmwgfx the only special-case we have b) drivers which don't need this won't need this, so should be fine. The other awkward thing I guess is that ttm would need to use the embedded kref from the gem bo, but that should be transparent I think. - Next up is dma-buf: For i915 we'd like to do the same eviction trick also through p2p dma-buf callbacks, so that this works the same as eviction/reservation within a gpu. But for these internal bo you might not have a dma-buf, so we can't just lift the trick to the dma-buf level. But I think if we pass e.g. a struct list_head and a callback to unreference/unlock all the buffers in there to the exporter, plus similar for the slowpath lock, then that should be doable without glorious layering inversions between dma-buf and gem. I think for dma-buf it should even be ok if this requires that we allocate an entire structure with kmalloc or something, allocating memory while holding dma_resv is ok. Yes, the thing here with the suggested helpers is that you would just embed a trans_lockitem struct in the gem object (and defines the gem object ops). Otherwise and for passing to dma-buf this is pretty much exactly what you are suggesting, but the huge benefit of encapsulating the needed members like this is that when we need to change something we change it in just one place. For anything that doesn't have a gem object (dma-buf, vmwgfx or whatever) you have the choice of either allocating a struct trans_lockitem or embed it wherever you prefer. In particular, this is beneficial where you have a single dma-resv class ww-mutex sitting somewhere in the way and you don't want to needlessly have a gem object that embeds it. The thing is, everyone who actually uses dma_resv_lock has a gem_buffer_object underneath. So it feels a bit like flexibility for no real need, and I think it would make it slightly more awkard for gem drivers to neatly integrate into their cs path. The lockitem struct works, but it is a bit cumbersome. Well that's partly because of it's impossible to use a standalone ww_mutex in a locking transaction that can only add gem objects to the list :/. Already in the i915 driver we have and may want to add various places where we have dead gem objects sitting because of this. Also, more importantly, If we pass a list down the dma-buf move_notify(),
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix "mitigations" parsing if i915 is builtin
On Wed, Apr 14, 2021 at 02:06:43PM +0800, Jisheng Zhang wrote:
> I met below error during boot with i915 builtin if pass
> "i915.mitigations=off":
> [0.015589] Booting kernel: `off' invalid for parameter `i915.mitigations'
>
> The reason is slab subsystem isn't ready at that time, so kstrdup()
> returns NULL. Fix this issue by using stack var instead of kstrdup().
>
> Fixes: 984cadea032b ("drm/i915: Allow the sysadmin to override security
> mitigations")
> Signed-off-by: Jisheng Zhang
> ---
> Since v1:
> - Ensure "str" is properly terminated. Thanks Ville for pointing this out.
>
> drivers/gpu/drm/i915/i915_mitigations.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_mitigations.c
> b/drivers/gpu/drm/i915/i915_mitigations.c
> index 84f12598d145..231aad5ff46c 100644
> --- a/drivers/gpu/drm/i915/i915_mitigations.c
> +++ b/drivers/gpu/drm/i915/i915_mitigations.c
> @@ -29,15 +29,14 @@ bool i915_mitigate_clear_residuals(void)
> static int mitigations_set(const char *val, const struct kernel_param *kp)
> {
> unsigned long new = ~0UL;
> - char *str, *sep, *tok;
> + char str[64], *sep, *tok;
> bool first = true;
> int err = 0;
>
> BUILD_BUG_ON(ARRAY_SIZE(names) >= BITS_PER_TYPE(mitigations));
>
> - str = kstrdup(val, GFP_KERNEL);
> - if (!str)
> - return -ENOMEM;
> + strncpy(str, val, sizeof(str) - 1);
> + str[sizeof(str) - 1] = '\0';
Looks correct, however strscpy() seems to be the thing we should
be using these days.
>
> for (sep = str; (tok = strsep(&sep, ","));) {
> bool enable = true;
> @@ -86,7 +85,6 @@ static int mitigations_set(const char *val, const struct
> kernel_param *kp)
> break;
> }
> }
> - kfree(str);
> if (err)
> return err;
>
> --
> 2.31.0
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Linaro-mm-sig] [RFC] Cross-driver ww transaction lock lists
On Thu, Apr 15, 2021 at 05:40:24PM +0200, Thomas Hellström (Intel) wrote: > > On 4/15/21 4:40 PM, Daniel Vetter wrote: > > On Thu, Apr 15, 2021 at 4:02 PM Thomas Hellström (Intel) > > wrote: > > > > > > On 4/15/21 3:37 PM, Daniel Vetter wrote: > > > > On Tue, Apr 13, 2021 at 09:57:06AM +0200, Christian König wrote: > > > > > Am 13.04.21 um 09:50 schrieb Thomas Hellström: > > > > > > Hi! > > > > > > > > > > > > During the dma_resv conversion of the i915 driver, we've been using > > > > > > ww > > > > > > transaction lock lists to keep track of ww_mutexes that are locked > > > > > > during the transaction so that they can be batch unlocked at > > > > > > suitable > > > > > > locations. Including also the LMEM/VRAM eviction we've ended up with > > > > > > two static lists per transaction context; one typically unlocked at > > > > > > the > > > > > > end of transaction and one initialized before and unlocked after > > > > > > each > > > > > > buffer object validate. This enables us to do sleeping locking at > > > > > > eviction and keep objects locked on the eviction list until we > > > > > > eventually succeed allocating memory (modulo minor flaws of course). > > > > > > > > > > > > It would be beneficial with the i915 TTM conversion to be able to > > > > > > introduce a similar functionality that would work in ttm but also > > > > > > cross-driver in, for example move_notify. It would also be > > > > > > beneficial > > > > > > to be able to put any dma_resv ww mutex on the lists, and not > > > > > > require > > > > > > it to be embedded in a particular object type. > > > > > > > > > > > > I started scetching on some utilities for this. For TTM, for > > > > > > example, > > > > > > the idea would be to pass a list head for the ww transaction lock > > > > > > list > > > > > > in the ttm_operation_ctx. A function taking a ww_mutex could then > > > > > > either attach a grabbed lock to the list for batch unlocking, or be > > > > > > responsible for unlocking when the lock's scope is exited. It's also > > > > > > possible to create sublists if so desired. I believe the below > > > > > > would be > > > > > > sufficient to cover the i915 functionality. > > > > > > > > > > > > Any comments and suggestions appreciated! > > > > > ah yes Daniel and I haven been discussing something like this for > > > > > years. > > > > > > > > > > I also came up with rough implementation, but we always ran into > > > > > lifetime > > > > > issues. > > > > > > > > > > In other words the ww_mutexes which are on the list would need to be > > > > > kept > > > > > alive until unlocked. > > > > > > > > > > Because of this we kind of backed up and said we would need this on > > > > > the GEM > > > > > level instead of working with dma_resv objects. > > > > Yeah there's a few funny concerns here that make this awkward: > > > > - For simplicity doing these helpers at the gem level should make > > > > things a > > > > bit easier, because then we have a standard way to drop the > > > > reference. > > > > It does mean that the only thing you can lock like this are gem > > > > objects, > > > > but I think that's fine. At least for a first cut. > > > > > > > > - This is a bit awkward for vmwgfx, but a) Zack has mentioned he's > > > > looking > > > > into adopting gem bo internally to be able to drop a pile of code > > > > and > > > > stop making vmwgfx the only special-case we have b) drivers which > > > > don't > > > > need this won't need this, so should be fine. > > > > > > > > The other awkward thing I guess is that ttm would need to use the > > > > embedded kref from the gem bo, but that should be transparent I > > > > think. > > > > > > > > - Next up is dma-buf: For i915 we'd like to do the same eviction trick > > > > also through p2p dma-buf callbacks, so that this works the same as > > > > eviction/reservation within a gpu. But for these internal bo you > > > > might > > > > not have a dma-buf, so we can't just lift the trick to the dma-buf > > > > level. But I think if we pass e.g. a struct list_head and a > > > > callback to > > > > unreference/unlock all the buffers in there to the exporter, plus > > > > similar for the slowpath lock, then that should be doable without > > > > glorious layering inversions between dma-buf and gem. > > > > > > > > I think for dma-buf it should even be ok if this requires that we > > > > allocate an entire structure with kmalloc or something, allocating > > > > memory while holding dma_resv is ok. > > > Yes, the thing here with the suggested helpers is that you would just > > > embed a trans_lockitem struct in the gem object (and defines the gem > > > object ops). Otherwise and for passing to dma-buf this is pretty much > > > exactly what you are suggesting, but the huge benefit of encapsulating > > > the needed members like this is that when we need to change something we > > > change it in just one
[Intel-gfx] [PATCH v3 1/4] drm/i915/uapi: hide kernel doc warnings
It's not properly formatted kernel doc, just nerf the warnings for now.
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
---
include/uapi/drm/i915_drm.h | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index ddc47bbf48b6..a50257cde9ff 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1054,12 +1054,12 @@ struct drm_i915_gem_exec_fence {
__u32 flags;
};
-/**
+/*
* See drm_i915_gem_execbuffer_ext_timeline_fences.
*/
#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
-/**
+/*
* This structure describes an array of drm_syncobj and associated points for
* timeline variants of drm_syncobj. It is invalid to append this structure to
* the execbuf if I915_EXEC_FENCE_ARRAY is set.
@@ -1700,7 +1700,7 @@ struct drm_i915_gem_context_param {
__u64 value;
};
-/**
+/*
* Context SSEU programming
*
* It may be necessary for either functional or performance reason to configure
@@ -2067,7 +2067,7 @@ struct drm_i915_perf_open_param {
__u64 properties_ptr;
};
-/**
+/*
* Enable data capture for a stream that was either opened in a disabled state
* via I915_PERF_FLAG_DISABLED or was later disabled via
* I915_PERF_IOCTL_DISABLE.
@@ -2081,7 +2081,7 @@ struct drm_i915_perf_open_param {
*/
#define I915_PERF_IOCTL_ENABLE _IO('i', 0x0)
-/**
+/*
* Disable data capture for a stream.
*
* It is an error to try and read a stream that is disabled.
@@ -2090,7 +2090,7 @@ struct drm_i915_perf_open_param {
*/
#define I915_PERF_IOCTL_DISABLE_IO('i', 0x1)
-/**
+/*
* Change metrics_set captured by a stream.
*
* If the stream is bound to a specific context, the configuration change
@@ -2103,7 +2103,7 @@ struct drm_i915_perf_open_param {
*/
#define I915_PERF_IOCTL_CONFIG _IO('i', 0x2)
-/**
+/*
* Common to all i915 perf records
*/
struct drm_i915_perf_record_header {
@@ -2151,7 +2151,7 @@ enum drm_i915_perf_record_type {
DRM_I915_PERF_RECORD_MAX /* non-ABI */
};
-/**
+/*
* Structure to upload perf dynamic configuration into the kernel.
*/
struct drm_i915_perf_oa_config {
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/4] drm/i915/uapi: convert i915_user_extension to kernel doc
Add some example usage for the extension chaining also, which is quite
nifty.
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
---
include/uapi/drm/i915_drm.h | 46 +
1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index a50257cde9ff..d9c954a5a456 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -62,8 +62,8 @@ extern "C" {
#define I915_ERROR_UEVENT "ERROR"
#define I915_RESET_UEVENT "RESET"
-/*
- * i915_user_extension: Base class for defining a chain of extensions
+/**
+ * struct i915_user_extension - Base class for defining a chain of extensions
*
* Many interfaces need to grow over time. In most cases we can simply
* extend the struct and have userspace pass in more data. Another option,
@@ -76,12 +76,50 @@ extern "C" {
* increasing complexity, and for large parts of that interface to be
* entirely optional. The downside is more pointer chasing; chasing across
* the __user boundary with pointers encapsulated inside u64.
+ *
+ * Example chaining:
+ *
+ * .. code-block:: C
+ *
+ * struct i915_user_extension ext3 {
+ * .next_extension = 0, // end
+ * .name = ...,
+ * };
+ * struct i915_user_extension ext2 {
+ * .next_extension = (uintptr_t)&ext3,
+ * .name = ...,
+ * };
+ * struct i915_user_extension ext1 {
+ * .next_extension = (uintptr_t)&ext2,
+ * .name = ...,
+ * };
+ *
+ * Typically the i915_user_extension would be embedded in some uAPI struct, and
+ * in this case we would feed it the head of the chain(i.e ext1), which would
+ * then apply all of the above extensions.
+ *
*/
struct i915_user_extension {
+ /**
+* @next_extension:
+*
+* Pointer to the next i915_user_extension, or zero if the end.
+*/
__u64 next_extension;
+ /** @name: Name of the extension */
__u32 name;
- __u32 flags; /* All undefined bits must be zero. */
- __u32 rsvd[4]; /* Reserved for future use; must be zero. */
+ /**
+* @flags: MBZ
+*
+* All undefined bits must be zero.
+*/
+ __u32 flags;
+ /**
+* @rsvd: MBZ
+*
+* Reserved for future use; must be zero.
+*/
+ __u32 rsvd[4];
};
/*
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 3/4] drm/i915/uapi: convert i915_query and friend to kernel doc
Add a note about the two-step process.
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
---
include/uapi/drm/i915_drm.h | 57 ++---
1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d9c954a5a456..ef36f1a0adde 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
__u64 flex_regs_ptr;
};
+/**
+ * struct drm_i915_query_item - An individual query for the kernel to process.
+ *
+ * The behaviour is determined by the @query_id. Note that exactly what
+ * @data_ptr is also depends on the specific @query_id.
+ */
struct drm_i915_query_item {
+ /** @query_id: The id for this query */
__u64 query_id;
#define DRM_I915_QUERY_TOPOLOGY_INFO1
#define DRM_I915_QUERY_ENGINE_INFO 2
#define DRM_I915_QUERY_PERF_CONFIG 3
/* Must be kept compact -- no holes and well documented */
- /*
+ /**
+* @length:
+*
* When set to zero by userspace, this is filled with the size of the
* data to be written at the data_ptr pointer. The kernel sets this
* value to a negative value to signal an error on a particular query
@@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
*/
__s32 length;
- /*
+ /**
+* @flags:
+*
* When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
*
* When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
-* following :
-* - DRM_I915_QUERY_PERF_CONFIG_LIST
-* - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
-* - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
+* following:
+*
+* - DRM_I915_QUERY_PERF_CONFIG_LIST
+* - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
+* - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
*/
__u32 flags;
#define DRM_I915_QUERY_PERF_CONFIG_LIST 1
#define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
#define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID 3
- /*
+ /**
+* @data_ptr:
+*
* Data will be written at the location pointed by data_ptr when the
* value of length matches the length of the data to be written by the
* kernel.
@@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
__u64 data_ptr;
};
+/**
+ * struct drm_i915_query - Supply an array of drm_i915_query_item for the
kernel
+ * to fill out.
+ *
+ * Note that this is generally a two step process for each drm_i915_query_item
+ * in the array:
+ *
+ * 1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
+ * drm_i915_query_item, with drm_i915_query_item.size set to zero. The
+ * kernel will then fill in the size, in bytes, which tells userspace how
+ * memory it needs to allocate for the blob(say for an array of
+ * properties).
+ *
+ * 2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
+ * drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
+ * that the i915_query_item.size should still be the same as what the
+ * kernel previously set. At this point the kernel can fill in the blob.
+ *
+ */
struct drm_i915_query {
+ /** @num_items: The number of elements in the @items_ptr array */
__u32 num_items;
- /*
-* Unused for now. Must be cleared to zero.
+ /**
+* @flags: Unused for now. Must be cleared to zero.
*/
__u32 flags;
- /*
-* This points to an array of num_items drm_i915_query_item structures.
+ /**
+* @items_ptr: This points to an array of num_items drm_i915_query_item
+* structures.
*/
__u64 items_ptr;
};
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 4/4] drm/doc/rfc: i915 DG1 uAPI
Add an entry for the new uAPI needed for DG1.
v2(Daniel):
- include the overall upstreaming plan
- add a note for mmap, there are differences here for TTM vs i915
- bunch of other suggestions from Daniel
v3:
(Daniel)
- add a note for set/get caching stuff
- add some more docs for existing query and extensions stuff
- add an actual code example for regions query
- bunch of other stuff
(Jason)
- uAPI change(!):
- try a simpler design with the placements extension
- rather than have a generic setparam which can cover multiple
use cases, have each extension be responsible for one thing
only
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
---
Documentation/gpu/rfc/i915_gem_lmem.h | 255
Documentation/gpu/rfc/i915_gem_lmem.rst | 139 +
Documentation/gpu/rfc/index.rst | 4 +
3 files changed, 398 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h
b/Documentation/gpu/rfc/i915_gem_lmem.h
new file mode 100644
index ..2a82a452e9f2
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_gem_lmem.h
@@ -0,0 +1,255 @@
+/*
+ * Note that drm_i915_query_item and drm_i915_query are existing bits of uAPI.
+ * For the regions query we are just adding a new query id, so no actual new
+ * ioctl or anything, but including it here for reference.
+ */
+struct drm_i915_query_item {
+#define DRM_I915_QUERY_MEMORY_REGIONS 0xdeadbeaf
+
+__u64 query_id;
+
+/*
+ * When set to zero by userspace, this is filled with the size of the
+ * data to be written at the data_ptr pointer. The kernel sets this
+ * value to a negative value to signal an error on a particular query
+ * item.
+ */
+__s32 length;
+
+__u32 flags;
+/*
+ * Data will be written at the location pointed by data_ptr when the
+ * value of length matches the length of the data to be written by the
+ * kernel.
+ */
+__u64 data_ptr;
+};
+
+struct drm_i915_query {
+__u32 num_items;
+/*
+ * Unused for now. Must be cleared to zero.
+ */
+__u32 flags;
+/*
+ * This points to an array of num_items drm_i915_query_item structures.
+ */
+__u64 items_ptr;
+};
+
+#define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY,
struct drm_i915_query)
+
+/**
+ * enum drm_i915_gem_memory_class
+ */
+enum drm_i915_gem_memory_class {
+ /** @I915_MEMORY_CLASS_SYSTEM: system memory */
+ I915_MEMORY_CLASS_SYSTEM = 0,
+ /** @I915_MEMORY_CLASS_DEVICE: device local-memory */
+ I915_MEMORY_CLASS_DEVICE,
+};
+
+/**
+ * struct drm_i915_gem_memory_class_instance
+ */
+struct drm_i915_gem_memory_class_instance {
+ /** @memory_class: see enum drm_i915_gem_memory_class */
+ __u16 memory_class;
+
+ /** @memory_instance: which instance */
+ __u16 memory_instance;
+};
+
+/**
+ * struct drm_i915_memory_region_info
+ *
+ * Describes one region as known to the driver.
+ *
+ * Note that we reserve quite a lot of stuff here for potential future work. As
+ * an example we might want expose the capabilities(see caps) for a given
+ * region, which could include things like if the region is CPU
+ * mappable/accessible etc.
+ */
+struct drm_i915_memory_region_info {
+ /** @region: class:instance pair encoding */
+ struct drm_i915_gem_memory_class_instance region;
+
+ /** @rsvd0: MBZ */
+ __u32 rsvd0;
+
+ /** @caps: MBZ */
+ __u64 caps;
+
+ /** @flags: MBZ */
+ __u64 flags;
+
+ /** @probed_size: Memory probed by the driver (-1 = unknown) */
+ __u64 probed_size;
+
+ /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
+ __u64 unallocated_size;
+
+ /** @rsvd1: MBZ */
+ __u64 rsvd1[8];
+};
+
+/**
+ * struct drm_i915_query_memory_regions
+ *
+ * Region info query enumerates all regions known to the driver by filling in
+ * an array of struct drm_i915_memory_region_info structures.
+ *
+ * Example for getting the list of supported regions:
+ *
+ * .. code-block:: C
+ *
+ * struct drm_i915_query_memory_regions *info;
+ * struct drm_i915_query_item item = {
+ * .query_id = DRM_I915_QUERY_MEMORY_REGIONS;
+ * };
+ * struct drm_i915_query query = {
+ * .num_items = 1,
+ * .items_ptr = (uintptr_t)&item,
+ * };
+ * int err, i;
+ *
+ * // First query the size of the blob we need, this needs to be large
+ * // enough to hold our array of regions. The kernel will fill out the
+ * // item.length for us, whic
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings
== Series Details == Series: series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings URL : https://patchwork.freedesktop.org/series/89121/ State : warning == Summary == $ dim checkpatch origin/drm-tip b67e5ba57e8a drm/i915/uapi: hide kernel doc warnings e5bfc110caaa drm/i915/uapi: convert i915_user_extension to kernel doc 02474e90a224 drm/i915/uapi: convert i915_query and friend to kernel doc 3f2501d0f7a0 drm/doc/rfc: i915 DG1 uAPI -:36: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #36: new file mode 100644 -:41: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1 #41: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:1: +/* -:49: ERROR:CODE_INDENT: code indent should use tabs where possible #49: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:9: +__u64 query_id;$ -:49: WARNING:LEADING_SPACE: please, no spaces at the start of a line #49: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:9: +__u64 query_id;$ -:51: ERROR:CODE_INDENT: code indent should use tabs where possible #51: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:11: +/*$ -:52: ERROR:CODE_INDENT: code indent should use tabs where possible #52: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:12: + * When set to zero by userspace, this is filled with the size of the$ -:53: ERROR:CODE_INDENT: code indent should use tabs where possible #53: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:13: + * data to be written at the data_ptr pointer. The kernel sets this$ -:54: ERROR:CODE_INDENT: code indent should use tabs where possible #54: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:14: + * value to a negative value to signal an error on a particular query$ -:55: ERROR:CODE_INDENT: code indent should use tabs where possible #55: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:15: + * item.$ -:56: ERROR:CODE_INDENT: code indent should use tabs where possible #56: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:16: + */$ -:57: ERROR:CODE_INDENT: code indent should use tabs where possible #57: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:17: +__s32 length;$ -:57: WARNING:LEADING_SPACE: please, no spaces at the start of a line #57: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:17: +__s32 length;$ -:59: ERROR:CODE_INDENT: code indent should use tabs where possible #59: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:19: +__u32 flags;$ -:59: WARNING:LEADING_SPACE: please, no spaces at the start of a line #59: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:19: +__u32 flags;$ -:60: ERROR:CODE_INDENT: code indent should use tabs where possible #60: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:20: +/*$ -:61: ERROR:CODE_INDENT: code indent should use tabs where possible #61: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:21: + * Data will be written at the location pointed by data_ptr when the$ -:62: ERROR:CODE_INDENT: code indent should use tabs where possible #62: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:22: + * value of length matches the length of the data to be written by the$ -:63: ERROR:CODE_INDENT: code indent should use tabs where possible #63: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:23: + * kernel.$ -:64: ERROR:CODE_INDENT: code indent should use tabs where possible #64: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:24: + */$ -:65: ERROR:CODE_INDENT: code indent should use tabs where possible #65: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:25: +__u64 data_ptr;$ -:65: WARNING:LEADING_SPACE: please, no spaces at the start of a line #65: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:25: +__u64 data_ptr;$ -:69: ERROR:CODE_INDENT: code indent should use tabs where possible #69: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:29: +__u32 num_items;$ -:69: WARNING:LEADING_SPACE: please, no spaces at the start of a line #69: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:29: +__u32 num_items;$ -:70: ERROR:CODE_INDENT: code indent should use tabs where possible #70: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:30: +/*$ -:71: ERROR:CODE_INDENT: code indent should use tabs where possible #71: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:31: + * Unused for now. Must be cleared to zero.$ -:72: ERROR:CODE_INDENT: code indent should use tabs where possible #72: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:32: + */$ -:73: ERROR:CODE_INDENT: code indent should use tabs where possible #73: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:33: +__u32 flags;$ -:73: WARNING:LEADING_SPACE: please, no spaces at the start of a line #73: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:33: +__u32 flags;$ -:74: ERROR:CODE_INDENT: code indent should use tabs where possible #74: FILE: Documentation/gpu/rfc/i915_gem_lmem.h:34: +/*$ -:75: ERROR:CODE_INDENT: code indent should use tabs where possible #75: FILE: D
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings
== Series Details == Series: series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings URL : https://patchwork.freedesktop.org/series/89121/ State : warning == Summary == $ dim sparse --fast origin/drm-tip Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. - +drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_reset.c:1329:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block +drivers/gpu/drm/i915/gt/intel_ring_submission.c:1203:24: warning: Using plain integer as NULL pointer +drivers/gpu/drm/i915/gvt/mmio.c:295:23: warning: memcpy with byte count of 279040 +drivers/gpu/drm/i915/i915_perf.c:1434:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/i915_perf.c:1488:15: warning: memset with byte count of 16777216 +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block +./include/linux/spinlock.
[Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings
== Series Details == Series: series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings URL : https://patchwork.freedesktop.org/series/89121/ State : warning == Summary == $ make htmldocs 2>&1 > /dev/null | grep i915 ./drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:102: warning: Function parameter or member 'ww' not described in 'i915_gem_shrink' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'jump_whitelist' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'shadow_map' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'batch_map' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings
== Series Details == Series: series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings URL : https://patchwork.freedesktop.org/series/89121/ State : success == Summary == CI Bug Log - changes from CI_DRM_9976 -> Patchwork_19942 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/index.html Known issues Here are the changes found in Patchwork_19942 that come from known issues: ### IGT changes ### Issues hit * igt@i915_selftest@live@hangcheck: - fi-snb-2600:NOTRUN -> [INCOMPLETE][1] ([i915#2782]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/fi-snb-2600/igt@i915_selftest@l...@hangcheck.html Possible fixes * igt@i915_module_load@reload: - fi-tgl-y: [DMESG-WARN][2] ([i915#1982] / [k.org#205379]) -> [PASS][3] [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/fi-tgl-y/igt@i915_module_l...@reload.html [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/fi-tgl-y/igt@i915_module_l...@reload.html [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#2782]: https://gitlab.freedesktop.org/drm/intel/issues/2782 [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379 Participating hosts (45 -> 42) -- Missing(3): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus Build changes - * Linux: CI_DRM_9976 -> Patchwork_19942 CI-20190529: 20190529 CI_DRM_9976: 73739c865de5c4c504adff6d9873ca5dea2dc704 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_6067: 14317b92a672d9a20cd04fc3b0c80e2fb12d51d5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_19942: 3f2501d0f7a0595fd713c55e204ff7820fab6de7 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 3f2501d0f7a0 drm/doc/rfc: i915 DG1 uAPI 02474e90a224 drm/i915/uapi: convert i915_query and friend to kernel doc e5bfc110caaa drm/i915/uapi: convert i915_user_extension to kernel doc b67e5ba57e8a drm/i915/uapi: hide kernel doc warnings == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/index.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 12/16] drm/i915/uapi: introduce drm_i915_gem_create_ext
On Mon, Mar 29, 2021 at 12:58 AM Daniele Ceraolo Spurio
wrote:
>
> From: Bommu Krishnaiah
>
> Same old gem_create but with now with extensions support. This is needed
> to support various upcoming usecases. For now we use the extensions
> mechanism to support PAVP.
>
> Signed-off-by: Bommu Krishnaiah
> Signed-off-by: Matthew Auld
> Cc: Joonas Lahtinen
> Cc: Matthew Auld
> Cc: Telukuntla Sreedhar
> ---
> drivers/gpu/drm/i915/gem/i915_gem_create.c | 41 ++-
gem changes need to be cc'ed to dri-devel. Also adding Jason on this,
since he just reviewed the gem_create_ext rfc from Matt.
-Daniel
> drivers/gpu/drm/i915/i915_drv.c| 2 +-
> include/uapi/drm/i915_drm.h| 47 ++
> 3 files changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c
> b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> index 45d60e3d98e3..3ad3413c459f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> @@ -7,6 +7,7 @@
> #include "gem/i915_gem_region.h"
>
> #include "i915_drv.h"
> +#include "i915_user_extensions.h"
>
> static int
> i915_gem_create(struct drm_file *file,
> @@ -91,6 +92,35 @@ i915_gem_dumb_create(struct drm_file *file,
>&args->size, &args->handle);
> }
>
> +struct create_ext {
> + struct drm_i915_private *i915;
> +};
> +
> +static int __create_setparam(struct drm_i915_gem_object_param *args,
> +struct create_ext *ext_data)
> +{
> + if (!(args->param & I915_OBJECT_PARAM)) {
> + DRM_DEBUG("Missing I915_OBJECT_PARAM namespace\n");
> + return -EINVAL;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int create_setparam(struct i915_user_extension __user *base, void
> *data)
> +{
> + struct drm_i915_gem_create_ext_setparam ext;
> +
> + if (copy_from_user(&ext, base, sizeof(ext)))
> + return -EFAULT;
> +
> + return __create_setparam(&ext.param, data);
> +}
> +
> +static const i915_user_extension_fn create_extensions[] = {
> + [I915_GEM_CREATE_EXT_SETPARAM] = create_setparam,
> +};
> +
> /**
> * Creates a new mm object and returns a handle to it.
> * @dev: drm device pointer
> @@ -102,10 +132,19 @@ i915_gem_create_ioctl(struct drm_device *dev, void
> *data,
> struct drm_file *file)
> {
> struct drm_i915_private *i915 = to_i915(dev);
> - struct drm_i915_gem_create *args = data;
> + struct create_ext ext_data = { .i915 = i915 };
> + struct drm_i915_gem_create_ext *args = data;
> + int ret;
>
> i915_gem_flush_free_objects(i915);
>
> + ret = i915_user_extensions(u64_to_user_ptr(args->extensions),
> + create_extensions,
> + ARRAY_SIZE(create_extensions),
> + &ext_data);
> + if (ret)
> + return ret;
> +
> return i915_gem_create(file,
>intel_memory_region_by_type(i915,
>
> INTEL_MEMORY_SYSTEM),
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 02d5b2b6ee39..f13e1ca2087b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1707,7 +1707,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
> DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl,
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, drm_noop,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, drm_noop,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> - DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl,
> DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_GEM_CREATE_EXT, i915_gem_create_ioctl,
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl,
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl,
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl,
> DRM_RENDER_ALLOW),
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7a2088eccc9f..d5e502269a55 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -392,6 +392,7 @@ typedef struct _drm_i915_sarea {
> #define DRM_IOCTL_I915_GEM_ENTERVT DRM_IO(DRM_COMMAND_BASE +
> DRM_I915_GEM_ENTERVT)
> #define DRM_IOCTL_I915_GEM_LEAVEVT DRM_IO(DRM_COMMAND_BASE +
> DRM_I915_GEM_LEAVEVT)
> #define DRM_IOCTL_I915_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_CREATE, struct drm_i915_gem_create)
> +#define DRM_IOCTL_I915_GEM_CREATE_EXT DRM_IOWR(DRM_COMMAND_BASE +
> DRM_I915_GEM_CREATE, struct drm_i915_gem_create_ext)
> #define DRM_IOCTL_I915_GEM_PREAD DRM_IOW (DRM_COMMAND_BASE +
> DRM_I91
Re: [Intel-gfx] [PATCH v3 11/16] drm/i915/pxp: interface for marking contexts as using protected content
On Mon, Mar 29, 2021 at 12:58 AM Daniele Ceraolo Spurio
wrote:
>
> Extra tracking and checks around protected objects, coming in a follow-up
> patch, will be enabled only for contexts that opt in. Contexts can only be
> marked as using protected content at creation time and they must be both
> bannable and not recoverable.
>
> When a PXP teardown occurs, all gem contexts marked this way that
> have been used at least once will be marked as invalid and all new
> submissions using them will be rejected. All intel contexts within the
> invalidated gem contexts will be marked banned.
> A new flag has been added to the RESET_STATS ioctl to report the
> invalidation to userspace.
>
> v2: split to its own patch and improve doc (Chris), invalidate contexts
> on teardown
>
> v3: improve doc, use -EACCES for execbuf fail (Chris), make protected
> context flag not mandatory in protected object execbuf to avoid
> abuse (Lionel)
>
> Signed-off-by: Daniele Ceraolo Spurio
> Cc: Chris Wilson
> Cc: Lionel Landwerlin
Same here, needs to be cc'ed to dri-devel. Also Jason is working on a
big cleanup of the context creation uapi, so needs coordination. Also
ping maintainers for awareness, not sure yet how to get this merged
with the branches we have (I forgot the maintainer ping on patch 12).
-Daniel
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 59 ++-
> drivers/gpu/drm/i915/gem/i915_gem_context.h | 18 ++
> .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 18 ++
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 48 +++
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 1 +
> drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 3 +
> include/uapi/drm/i915_drm.h | 26
> 8 files changed, 172 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index fd8ee52e17a4..f3fd302682bb 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -76,6 +76,8 @@
> #include "gt/intel_gpu_commands.h"
> #include "gt/intel_ring.h"
>
> +#include "pxp/intel_pxp.h"
> +
> #include "i915_gem_context.h"
> #include "i915_globals.h"
> #include "i915_trace.h"
> @@ -1972,6 +1974,40 @@ static int set_priority(struct i915_gem_context *ctx,
> return 0;
> }
>
> +static int set_protected(struct i915_gem_context *ctx,
> +const struct drm_i915_gem_context_param *args)
> +{
> + int ret = 0;
> +
> + if (!intel_pxp_is_enabled(&ctx->i915->gt.pxp))
> + ret = -ENODEV;
> + else if (ctx->file_priv) /* can't change this after creation! */
> + ret = -EEXIST;
> + else if (args->size)
> + ret = -EINVAL;
> + else if (!args->value)
> + clear_bit(UCONTEXT_PROTECTED, &ctx->user_flags);
> + else if (i915_gem_context_is_recoverable(ctx) ||
> +!i915_gem_context_is_bannable(ctx))
> + ret = -EPERM;
> + else
> + set_bit(UCONTEXT_PROTECTED, &ctx->user_flags);
> +
> + return ret;
> +}
> +
> +static int get_protected(struct i915_gem_context *ctx,
> +struct drm_i915_gem_context_param *args)
> +{
> + if (!intel_pxp_is_enabled(&ctx->i915->gt.pxp))
> + return -ENODEV;
> +
> + args->size = 0;
> + args->value = i915_gem_context_uses_protected_content(ctx);
> +
> + return 0;
> +}
> +
> static int ctx_setparam(struct drm_i915_file_private *fpriv,
> struct i915_gem_context *ctx,
> struct drm_i915_gem_context_param *args)
> @@ -2004,6 +2040,8 @@ static int ctx_setparam(struct drm_i915_file_private
> *fpriv,
> ret = -EPERM;
> else if (args->value)
> i915_gem_context_set_bannable(ctx);
> + else if (i915_gem_context_uses_protected_content(ctx))
> + ret = -EPERM; /* can't clear this for protected
> contexts */
> else
> i915_gem_context_clear_bannable(ctx);
> break;
> @@ -2011,10 +2049,12 @@ static int ctx_setparam(struct drm_i915_file_private
> *fpriv,
> case I915_CONTEXT_PARAM_RECOVERABLE:
> if (args->size)
> ret = -EINVAL;
> - else if (args->value)
> - i915_gem_context_set_recoverable(ctx);
> - else
> + else if (!args->value)
> i915_gem_context_clear_recoverable(ctx);
> + else if (i915_gem_context_uses_protected_content(ctx))
> + ret = -EPERM; /* can't set this for protected
> contexts */
> + else
> + i915_gem_context_set_recoverable(ctx);
> break
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings
== Series Details == Series: series starting with [v3,1/4] drm/i915/uapi: hide kernel doc warnings URL : https://patchwork.freedesktop.org/series/89121/ State : success == Summary == CI Bug Log - changes from CI_DRM_9976_full -> Patchwork_19942_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_19942_full that come from known issues: ### IGT changes ### Issues hit * igt@feature_discovery@display-3x: - shard-tglb: NOTRUN -> [SKIP][1] ([i915#1839]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-tglb2/igt@feature_discov...@display-3x.html * igt@gem_create@create-massive: - shard-skl: NOTRUN -> [DMESG-WARN][2] ([i915#3002]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-skl5/igt@gem_cre...@create-massive.html * igt@gem_ctx_persistence@engines-mixed: - shard-snb: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099]) +3 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-snb6/igt@gem_ctx_persiste...@engines-mixed.html * igt@gem_eio@unwedge-stress: - shard-tglb: [PASS][4] -> [TIMEOUT][5] ([i915#2369] / [i915#3063]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-tglb7/igt@gem_...@unwedge-stress.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-tglb2/igt@gem_...@unwedge-stress.html * igt@gem_exec_fair@basic-flow@rcs0: - shard-tglb: [PASS][6] -> [FAIL][7] ([i915#2842]) +1 similar issue [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-tglb7/igt@gem_exec_fair@basic-f...@rcs0.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-tglb7/igt@gem_exec_fair@basic-f...@rcs0.html * igt@gem_exec_fair@basic-pace-solo@rcs0: - shard-glk: [PASS][8] -> [FAIL][9] ([i915#2842]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-glk7/igt@gem_exec_fair@basic-pace-s...@rcs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-glk5/igt@gem_exec_fair@basic-pace-s...@rcs0.html * igt@gem_exec_fair@basic-pace@rcs0: - shard-kbl: [PASS][10] -> [FAIL][11] ([i915#2842]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-kbl1/igt@gem_exec_fair@basic-p...@rcs0.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-kbl2/igt@gem_exec_fair@basic-p...@rcs0.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-tglb: NOTRUN -> [FAIL][12] ([i915#2842]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-tglb2/igt@gem_exec_fair@basic-throt...@rcs0.html * igt@gem_exec_reloc@basic-wide-active@vcs1: - shard-iclb: NOTRUN -> [FAIL][13] ([i915#2389]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-iclb1/igt@gem_exec_reloc@basic-wide-act...@vcs1.html * igt@gem_mmap_gtt@big-copy: - shard-glk: [PASS][14] -> [FAIL][15] ([i915#307]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-glk7/igt@gem_mmap_...@big-copy.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-glk5/igt@gem_mmap_...@big-copy.html * igt@gem_pread@exhaustion: - shard-snb: NOTRUN -> [WARN][16] ([i915#2658]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-snb6/igt@gem_pr...@exhaustion.html * igt@gem_softpin@evict-snoop: - shard-tglb: NOTRUN -> [SKIP][17] ([fdo#109312]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-tglb2/igt@gem_soft...@evict-snoop.html * igt@gem_userptr_blits@set-cache-level: - shard-snb: NOTRUN -> [FAIL][18] ([i915#3324]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-snb5/igt@gem_userptr_bl...@set-cache-level.html - shard-glk: NOTRUN -> [FAIL][19] ([i915#3324]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-glk5/igt@gem_userptr_bl...@set-cache-level.html * igt@gem_userptr_blits@unsync-unmap-cycles: - shard-kbl: NOTRUN -> [SKIP][20] ([fdo#109271]) +10 similar issues [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-kbl3/igt@gem_userptr_bl...@unsync-unmap-cycles.html * igt@gem_userptr_blits@vma-merge: - shard-skl: NOTRUN -> [FAIL][21] ([i915#3318]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-skl5/igt@gem_userptr_bl...@vma-merge.html * igt@gen9_exec_parse@bb-start-far: - shard-tglb: NOTRUN -> [SKIP][22] ([fdo#112306]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19942/shard-tglb2/igt@gen9_exec_pa...@bb-start-far.html * igt@i915_module_load@reload-with-fault-injection: - shard-snb: [PASS][23] -> [INCOMPLETE][24] ([i915#2880]) [23]: https://intel-gfx-c
Re: [Intel-gfx] [PATCH 10/11] drm/i915/adl_p: Require a minimum of 8 tiles stride for DPT FBs
On Wed, Apr 14, 2021 at 06:52:07PM +0300, Imre Deak wrote:
> The specification only requires DPT FB strides to be POT aligned, but
> there seems to be also a minimum of 8 stride tile requirement. Scanning
> out FBs with < 8 stride tiles will result in pipe faults (even though
> the stride is POT aligned).
This is now confirmed in bspec (index 53393).
> Signed-off-by: Imre Deak
> Acked-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/display/intel_fb.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c
> b/drivers/gpu/drm/i915/display/intel_fb.c
> index bd862f77762a2..2ee10ece27c57 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -609,7 +609,11 @@ plane_view_dst_stride_tiles(const struct
> intel_framebuffer *fb, int color_plane,
> unsigned int pitch_tiles)
> {
> if (intel_fb_needs_pot_stride_remap(fb))
> - return roundup_pow_of_two(pitch_tiles);
> + /*
> + * ADL_P, the only platform needing a POT stride has a minimum
> + * of 8 stride tiles.
> + */
> + return roundup_pow_of_two(max(pitch_tiles, 8u));
> else
> return pitch_tiles;
> }
> --
> 2.27.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] [Mesa-dev] [PATCH v3 3/4] drm/i915/uapi: convert i915_query and friend to kernel doc
On 4/15/21 8:59 AM, Matthew Auld wrote:
> Add a note about the two-step process.
>
> Suggested-by: Daniel Vetter
> Signed-off-by: Matthew Auld
> Cc: Joonas Lahtinen
> Cc: Jordan Justen
> Cc: Daniel Vetter
> Cc: Kenneth Graunke
> Cc: Jason Ekstrand
> Cc: Dave Airlie
> Cc: dri-de...@lists.freedesktop.org
> Cc: mesa-...@lists.freedesktop.org
> ---
> include/uapi/drm/i915_drm.h | 57 ++---
> 1 file changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index d9c954a5a456..ef36f1a0adde 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
> __u64 flex_regs_ptr;
> };
>
> +/**
> + * struct drm_i915_query_item - An individual query for the kernel to
> process.
> + *
> + * The behaviour is determined by the @query_id. Note that exactly what
Since we just had a big discussion about this on mesa-dev w.r.t. Mesa
code and documentation... does the kernel have a policy about which
flavor (pun intended) of English should be used?
> + * @data_ptr is also depends on the specific @query_id.
> + */
> struct drm_i915_query_item {
> + /** @query_id: The id for this query */
> __u64 query_id;
> #define DRM_I915_QUERY_TOPOLOGY_INFO1
> #define DRM_I915_QUERY_ENGINE_INFO 2
> #define DRM_I915_QUERY_PERF_CONFIG 3
> /* Must be kept compact -- no holes and well documented */
>
> - /*
> + /**
> + * @length:
> + *
>* When set to zero by userspace, this is filled with the size of the
>* data to be written at the data_ptr pointer. The kernel sets this
>* value to a negative value to signal an error on a particular query
> @@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
>*/
> __s32 length;
>
> - /*
> + /**
> + * @flags:
> + *
>* When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
>*
>* When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
> - * following :
> - * - DRM_I915_QUERY_PERF_CONFIG_LIST
> - * - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> - * - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> + * following:
> + *
> + * - DRM_I915_QUERY_PERF_CONFIG_LIST
> + * - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> + * - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
>*/
> __u32 flags;
> #define DRM_I915_QUERY_PERF_CONFIG_LIST 1
> #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
> #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID 3
>
> - /*
> + /**
> + * @data_ptr:
> + *
>* Data will be written at the location pointed by data_ptr when the
>* value of length matches the length of the data to be written by the
>* kernel.
> @@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
> __u64 data_ptr;
> };
>
> +/**
> + * struct drm_i915_query - Supply an array of drm_i915_query_item for the
> kernel
> + * to fill out.
> + *
> + * Note that this is generally a two step process for each
> drm_i915_query_item
> + * in the array:
> + *
> + * 1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
> + * drm_i915_query_item, with drm_i915_query_item.size set to zero. The
> + * kernel will then fill in the size, in bytes, which tells userspace how
> + * memory it needs to allocate for the blob(say for an array of
> + * properties).
> + *
> + * 2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
> + * drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
> + * that the i915_query_item.size should still be the same as what the
> + * kernel previously set. At this point the kernel can fill in the blob.
> + *
> + */
> struct drm_i915_query {
> + /** @num_items: The number of elements in the @items_ptr array */
> __u32 num_items;
>
> - /*
> - * Unused for now. Must be cleared to zero.
> + /**
> + * @flags: Unused for now. Must be cleared to zero.
>*/
> __u32 flags;
>
> - /*
> - * This points to an array of num_items drm_i915_query_item structures.
> + /**
> + * @items_ptr: This points to an array of num_items drm_i915_query_item
> + * structures.
>*/
> __u64 items_ptr;
> };
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/modifiers: Enforce consistency between the cap an IN_FORMATS
Reviewed-by: Simon Ser ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
