Re: [Intel-gfx] [PATCH 1/2] drm/i915: Prepare for multiple GTs
Hi Andi,
On 2022-01-11 14:15:51, Andi Shyti wrote:
>
> From: Tvrtko Ursulin
>
> On a multi-tile platform, each tile has its own registers + GGTT
> space, and BAR 0 is extended to cover all of them.
>
> Up to four gts are supported in i915->gt[], with slot zero
> shadowing the existing i915->gt0 to enable source compatibility
> with legacy driver paths. A for_each_gt macro is added to iterate
> over the GTs and will be used by upcoming patches that convert
> various parts of the driver to be multi-gt aware.
>
> Only the primary/root tile is initialized for now; the other
> tiles will be detected and plugged in by future patches once the
> necessary infrastructure is in place to handle them.
>
> Signed-off-by: Abdiel Janulgue
> Signed-off-by: Daniele Ceraolo Spurio
> Signed-off-by: Tvrtko Ursulin
> Signed-off-by: Matt Roper
> Signed-off-by: Andi Shyti
> Cc: Daniele Ceraolo Spurio
> Cc: Joonas Lahtinen
> Cc: Matthew Auld
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c| 139 --
> drivers/gpu/drm/i915/gt/intel_gt.h| 14 +-
> drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 +-
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 7 +
> drivers/gpu/drm/i915/i915_driver.c| 29 ++--
> drivers/gpu/drm/i915/i915_drv.h | 6 +
> drivers/gpu/drm/i915/intel_memory_region.h| 3 +
> drivers/gpu/drm/i915/intel_uncore.c | 12 +-
> drivers/gpu/drm/i915/intel_uncore.h | 3 +-
> .../gpu/drm/i915/selftests/mock_gem_device.c | 5 +-
> 10 files changed, 185 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 298ff32c8d0c..5e062c9525f8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -26,7 +26,8 @@
> #include "shmem_utils.h"
> #include "pxp/intel_pxp.h"
>
> -void __intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private
> *i915)
> +static void
> +__intel_gt_init_early(struct intel_gt *gt)
> {
> spin_lock_init(>irq_lock);
>
> @@ -46,19 +47,27 @@ void __intel_gt_init_early(struct intel_gt *gt, struct
> drm_i915_private *i915)
> intel_rps_init_early(>rps);
> }
>
> +/* Preliminary initialization of Tile 0 */
> void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
> {
> gt->i915 = i915;
> gt->uncore = >uncore;
> +
> + __intel_gt_init_early(gt);
> }
>
> -int intel_gt_probe_lmem(struct intel_gt *gt)
> +static int intel_gt_probe_lmem(struct intel_gt *gt)
> {
> struct drm_i915_private *i915 = gt->i915;
> + unsigned int instance = gt->info.id;
> struct intel_memory_region *mem;
> int id;
> int err;
>
> + id = INTEL_REGION_LMEM + instance;
> + if (drm_WARN_ON(>drm, id >= INTEL_REGION_STOLEN_SMEM))
> + return -ENODEV;
> +
> mem = intel_gt_setup_lmem(gt);
> if (mem == ERR_PTR(-ENODEV))
> mem = intel_gt_setup_fake_lmem(gt);
> @@ -73,9 +82,8 @@ int intel_gt_probe_lmem(struct intel_gt *gt)
> return err;
> }
>
> - id = INTEL_REGION_LMEM;
> -
> mem->id = id;
> + mem->instance = instance;
>
> intel_memory_region_set_name(mem, "local%u", mem->instance);
>
> @@ -790,16 +798,21 @@ void intel_gt_driver_release(struct intel_gt *gt)
> intel_gt_fini_buffer_pool(gt);
> }
>
> -void intel_gt_driver_late_release(struct intel_gt *gt)
> +void intel_gt_driver_late_release(struct drm_i915_private *i915)
> {
> + struct intel_gt *gt;
> + unsigned int id;
> +
> /* We need to wait for inflight RCU frees to release their grip */
> rcu_barrier();
>
> - intel_uc_driver_late_release(>uc);
> - intel_gt_fini_requests(gt);
> - intel_gt_fini_reset(gt);
> - intel_gt_fini_timelines(gt);
> - intel_engines_free(gt);
> + for_each_gt(gt, i915, id) {
> + intel_uc_driver_late_release(>uc);
> + intel_gt_fini_requests(gt);
> + intel_gt_fini_reset(gt);
> + intel_gt_fini_timelines(gt);
> + intel_engines_free(gt);
> + }
> }
>
> /**
> @@ -908,6 +921,112 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt,
> i915_reg_t reg)
> return intel_uncore_read_fw(gt->uncore, reg);
> }
>
> +static int
> +intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
> +{
> + struct drm_i915_private *i915 = gt->i915;
> + unsigned int id = gt->info.id;
> + int ret;
> +
> + if (id) {
> + struct intel_uncore_mmio_debug *mmio_debug;
> + struct intel_uncore *uncore;
> +
> + /* For multi-tile platforms BAR0 must have at least 16MB per
> tile */
> + if (GEM_WARN_ON(pci_resource_len(to_pci_dev(i915->drm.dev), 0) <
> + (id + 1) * SZ_16M))
> + return -EINVAL;
> +
> + uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);
> +
Re: [Intel-gfx] [PATCH v3 05/10] drm/i915: Prepare for multiple gts
[Redundant sending of this email due to some mail issues]
On 2021-10-28 20:28:12, Matt Roper wrote:
> From: Tvrtko Ursulin
>
> Add some basic plumbing to support more than one dynamically allocated
> struct intel_gt. Up to four gts are supported in i915->gts[], with slot
> zero shadowing the existing i915->gt to enable source compatibility with
> legacy driver paths. A for_each_gt macro is added to iterate over the
> GTs and will be used by upcoming patches that convert various parts of
> the driver to be multi-gt aware.
>
> v2:
> - Rename init function to i915_init_tile_memory() and move it to
>i915_drv.c. (Lucas)
> - Squash in patch from Sandeep to release the per-gt resources during
>driver teardown.
>
> Cc: Lucas De Marchi
> Signed-off-by: Tvrtko Ursulin
> Signed-off-by: Venkata Sandeep Dhanalakota
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 57 +++---
> drivers/gpu/drm/i915/gt/intel_gt.h | 6 +++
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 2 +
> drivers/gpu/drm/i915/i915_drv.c| 30 +++-
> drivers/gpu/drm/i915/i915_drv.h| 6 +++
> drivers/gpu/drm/i915/intel_memory_region.h | 3 ++
> 6 files changed, 96 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 098cd8843c38..d02a09653033 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -23,10 +23,13 @@
> #include "shmem_utils.h"
> #include "pxp/intel_pxp.h"
>
> -void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
> +static void
> +__intel_gt_init_early(struct intel_gt *gt,
> + struct intel_uncore *uncore,
> + struct drm_i915_private *i915)
> {
> gt->i915 = i915;
> - gt->uncore = >uncore;
> + gt->uncore = uncore;
>
> spin_lock_init(>irq_lock);
>
> @@ -49,10 +52,15 @@ void intel_gt_init_early(struct intel_gt *gt, struct
> drm_i915_private *i915)
> int intel_gt_probe_lmem(struct intel_gt *gt)
> {
> struct drm_i915_private *i915 = gt->i915;
> + unsigned int instance = gt->info.id;
> struct intel_memory_region *mem;
> int id;
> int err;
>
> + id = INTEL_REGION_LMEM + instance;
> + if (drm_WARN_ON(>drm, id >= INTEL_REGION_STOLEN_SMEM))
> + return -ENODEV;
> +
> mem = intel_gt_setup_lmem(gt);
> if (mem == ERR_PTR(-ENODEV))
> mem = intel_gt_setup_fake_lmem(gt);
> @@ -67,9 +75,8 @@ int intel_gt_probe_lmem(struct intel_gt *gt)
> return err;
> }
>
> - id = INTEL_REGION_LMEM;
> -
> mem->id = id;
> + mem->instance = instance;
>
> intel_memory_region_set_name(mem, "local%u", mem->instance);
>
> @@ -80,6 +87,11 @@ int intel_gt_probe_lmem(struct intel_gt *gt)
> return 0;
> }
>
> +void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
> +{
> + __intel_gt_init_early(gt, >uncore, i915);
> +}
> +
> void intel_gt_init_hw_early(struct intel_gt *gt, struct i915_ggtt *ggtt)
> {
> gt->ggtt = ggtt;
> @@ -903,9 +915,29 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt,
> i915_reg_t reg)
> static int
> intel_gt_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t
> phys_addr)
> {
> + struct drm_i915_private *i915 = gt->i915;
> + struct intel_uncore *uncore;
> + struct intel_uncore_mmio_debug *mmio_debug;
> int ret;
>
> - intel_uncore_init_early(gt->uncore, gt);
> + if (id) {
> + uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);
> + if (!uncore)
> + return -ENOMEM;
> +
> + mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL);
> + if (!mmio_debug) {
> + kfree(uncore);
> + return -ENOMEM;
> + }
> +
> + __intel_gt_init_early(gt, uncore, i915);
> + } else {
> + uncore = >uncore;
> + mmio_debug = >mmio_debug;
> + }
> +
> + intel_uncore_init_early(uncore, gt);
>
> ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
> if (ret)
> @@ -920,6 +952,11 @@ static void
> intel_gt_tile_cleanup(struct intel_gt *gt)
> {
> intel_uncore_cleanup_mmio(gt->uncore);
> +
> + if (gt->info.id) {
> + kfree(gt->uncore);
> + kfree(gt);
> + }
> }
>
> int intel_gt_probe_all(struct drm_i915_private *i915)
> @@ -937,13 +974,21 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> if (ret)
> return ret;
>
> + i915->gts[0] = >gt;
> +
> /* TODO: add more tiles */
> return 0;
> }
>
> void intel_gt_release_all(struct drm_i915_private *i915)
> {
> - intel_gt_tile_cleanup(>gt);
> + struct intel_gt *gt;
> + unsigned int id;
> +
> + for_each_gt(i915, id, gt) {
> + intel_gt_tile_cleanup(gt);
Re: [Intel-gfx] [PATCH i-g-t 2/5] i915/gem_ctx_isolation: Check engine relative registers
On 2020-02-28 10:43:37, Chris Wilson wrote:
> Some of the non-privileged registers are at the same offset on each
> engine. We can improve our coverage for unknown HW layout by using the
> reported engine->mmio_base for relative offsets.
>
> Signed-off-by: Chris Wilson
Reviewed-by: Dale B Stimson
> ---
> tests/i915/gem_ctx_isolation.c | 164 -
> 1 file changed, 100 insertions(+), 64 deletions(-)
>
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 1b66fec11..eff4b1df2 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -70,6 +70,7 @@ static const struct named_register {
> uint32_t ignore_bits;
> uint32_t write_mask; /* some registers bits do not exist */
> bool masked;
> + bool relative;
> } nonpriv_registers[] = {
> { "NOPID", NOCTX, RCS0, 0x2094 },
> { "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
> @@ -109,7 +110,6 @@ static const struct named_register {
> { "PS_DEPTH_COUNT_1", GEN8, RCS0, 0x22f8, 2 },
> { "BB_OFFSET", GEN8, RCS0, 0x2158, .ignore_bits = 0x7 },
> { "MI_PREDICATE_RESULT_1", GEN8, RCS0, 0x241c },
> - { "CS_GPR", GEN8, RCS0, 0x2600, 32 },
> { "OA_CTX_CONTROL", GEN8, RCS0, 0x2360 },
> { "OACTXID", GEN8, RCS0, 0x2364 },
> { "PS_INVOCATION_COUNT_2", GEN8, RCS0, 0x2448, 2, .write_mask = ~0x3 },
> @@ -138,79 +138,56 @@ static const struct named_register {
>
> { "CTX_PREEMPT", NOCTX /* GEN10 */, RCS0, 0x2248 },
> { "CS_CHICKEN1", GEN11, RCS0, 0x2580, .masked = true },
> - { "HDC_CHICKEN1", GEN_RANGE(10, 10), RCS0, 0x7304, .masked = true },
>
> /* Privileged (enabled by w/a + FORCE_TO_NONPRIV) */
> { "CTX_PREEMPT", NOCTX /* GEN9 */, RCS0, 0x2248 },
> { "CS_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x2580, .masked = true },
> { "COMMON_SLICE_CHICKEN2", GEN_RANGE(9, 9), RCS0, 0x7014, .masked =
> true },
> - { "HDC_CHICKEN1", GEN_RANGE(9, 9), RCS0, 0x7304, .masked = true },
> + { "HDC_CHICKEN1", GEN_RANGE(9, 10), RCS0, 0x7304, .masked = true },
> { "SLICE_COMMON_ECO_CHICKEN1", GEN_RANGE(11, 11) /* + glk */, RCS0,
> 0x731c, .masked = true },
> { "L3SQREG4", NOCTX /* GEN9:skl,kbl */, RCS0, 0xb118, .write_mask =
> ~0x10 },
> { "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
> true },
> { "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
>
> - { "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
> { "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
> },
>
> { "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
> { "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
>
> - { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
> - { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
> - { "VECS_GPR", GEN_RANGE(9, 10), VECS0, 0x1a600, 32 },
> -
> - { "VCS0_GPR", GEN11, VCS0, 0x1c0600, 32 },
> - { "VCS1_GPR", GEN11, VCS1, 0x1c4600, 32 },
> - { "VCS2_GPR", GEN11, VCS2, 0x1d0600, 32 },
> - { "VCS3_GPR", GEN11, VCS3, 0x1d4600, 32 },
> - { "VECS_GPR", GEN11, VECS0, 0x1c8600, 32 },
> + { "xCS_GPR", GEN9, ALL, 0x600, 32, .relative = true },
>
> {}
> }, ignore_registers[] = {
> { "RCS timestamp", GEN6, ~0u, 0x2358 },
> { "BCS timestamp", GEN7, ~0u, 0x22358 },
>
> - { "VCS0 timestamp", GEN_RANGE(7, 10), ~0u, 0x12358 },
> - { "VCS1 timestamp", GEN_RANGE(7, 10), ~0u, 0x1c358 },
> - { "VECS timestamp", GEN_RANGE(8, 10), ~0u, 0x1a358 },
> -
> - { "VCS0 timestamp", GEN11, ~0u, 0x1c0358 },
> - { "VCS1 timestamp", GEN11, ~0u, 0x1c4358 },
> - { "VCS2 timestamp", GEN11, ~0u, 0x1d0358 },
> - { "VCS3 timestamp", GEN11, ~0u, 0x1d4358 },
> - { "VECS timestamp", GEN11, ~0u, 0x1c8358 },
> + { "xCS timestamp", GEN8, ALL, 0x358, .relative = true },
>
> /* huc read only */
> - { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2000 },
> - { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2014 },
> - { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x23b0 },
> -
> - { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2000 },
> - { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2014 },
> - { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x23b0 },
> -
> - { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2000 },
> - { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2014 },
> - { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x23b0 },
> -
> - { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2000 },
> - { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2014 },
> - { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x23b0 },
> + { "BSD 0x2000", GEN11, ALL, 0x2000, .relative = true },
> + { "BSD 0x2014", GEN11, ALL, 0x2014, .relative = true },
> + { "BSD 0x23b0", GEN11, ALL, 0x23b0, .relative = true },
>
> {}
> };
>
> -static const char *register_name(uint32_t offset, char *buf, size_t len)
> +static const char *
> +register_name(uint32_t offset, uint32_t
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/5] i915: Start putting the mmio_base to wider use
On 2020-02-28 10:43:36, Chris Wilson wrote:
> Several tests depend upon the implicit engine->mmio_base but have no
> means of determining the physical layout. Since the kernel has started
> providing this information, start putting it to use.
>
> Signed-off-by: Chris Wilson
Reviewed-by: Dale B Stimson
> ---
> lib/i915/gem_engine_topology.c | 84 ++
> lib/i915/gem_engine_topology.h | 5 ++
> tests/i915/gem_ctx_shared.c| 38 +--
> tests/i915/gem_exec_latency.c | 17 ---
> 4 files changed, 111 insertions(+), 33 deletions(-)
>
> diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
> index 58dff3208..640777ae4 100644
> --- a/lib/i915/gem_engine_topology.c
> +++ b/lib/i915/gem_engine_topology.c
> @@ -21,7 +21,12 @@
> * IN THE SOFTWARE.
> */
>
> +#include
> +#include
> +
> #include "drmtest.h"
> +#include "igt_sysfs.h"
> +#include "intel_chipset.h"
> #include "ioctl_wrappers.h"
>
> #include "i915/gem_engine_topology.h"
> @@ -327,3 +332,82 @@ bool gem_engine_is_equal(const struct
> intel_execution_engine2 *e1,
> {
> return e1->class == e2->class && e1->instance == e2->instance;
> }
> +
> +static int descend(int dir, const char *path)
> +{
> + int fd;
> +
> + fd = openat(dir, path, O_RDONLY);
> + close(dir);
> +
> + return fd;
> +}
> +
> +int gem_engine_property_scanf(int i915, const char *engine, const char *attr,
> + const char *fmt, ...)
> +{
> + FILE *file;
> + va_list ap;
> + int ret;
> + int fd;
> +
> + fd = igt_sysfs_open(i915);
> + if (fd < 0)
> + return fd;
> +
> + fd = descend(fd, "engine");
> + if (fd < 0)
> + return fd;
> +
> + fd = descend(fd, engine);
> + if (fd < 0)
> + return fd;
> +
> + fd = descend(fd, attr);
> + if (fd < 0)
> + return fd;
> +
> + file = fdopen(fd, "r");
> + if (!file) {
> + close(fd);
> + return -1;
> + }
> +
> + va_start(ap, fmt);
> + ret = vfscanf(file, fmt, ap);
> + va_end(ap);
> +
> + fclose(file);
> + return ret;
> +}
> +
> +uint32_t gem_engine_mmio_base(int i915, const char *engine)
> +{
> + unsigned int mmio = 0;
> +
> + if (gem_engine_property_scanf(i915, engine, "mmio_base",
> + "%x", ) < 0) {
> + int gen = intel_gen(intel_get_drm_devid(i915));
> +
> + /* The layout of xcs1+ is unreliable -- hence the property! */
> + if (!strcmp(engine, "rcs0")) {
> + mmio = 0x2000;
> + } else if (!strcmp(engine, "bcs0")) {
> + mmio = 0x22000;
> + } else if (!strcmp(engine, "vcs0")) {
> + if (gen < 6)
> + mmio = 0x4000;
> + else if (gen < 11)
> + mmio = 0x12000;
> + else
> + mmio = 0x1c;
> + } else if (!strcmp(engine, "vecs0")) {
> + if (gen < 11)
> + mmio = 0x1a000;
> + else
> + mmio = 0x1c8000;
> + }
> + }
> +
> + return mmio;
> +}
> diff --git a/lib/i915/gem_engine_topology.h b/lib/i915/gem_engine_topology.h
> index 027d86be2..219c84b72 100644
> --- a/lib/i915/gem_engine_topology.h
> +++ b/lib/i915/gem_engine_topology.h
> @@ -75,4 +75,9 @@ struct intel_execution_engine2
> gem_eb_flags_to_engine(unsigned int flags);
> #define __for_each_physical_engine(fd__, e__) \
> for_each_physical_engine(fd__, 0, e__)
>
> +__attribute__((format(scanf, 4, 5)))
> +int gem_engine_property_scanf(int i915, const char *engine, const char *attr,
> + const char *fmt, ...);
> +uint32_t gem_engine_mmio_base(int i915, const char *engine);
> +
> #endif /* GEM_ENGINE_TOPOLOGY_H */
> diff --git a/tests/i915/gem_ctx_shared.c b/tests/i915/gem_ctx_shared.c
> index 820b96c1a..9ea020d7b 100644
> --- a/tests/i915/gem_ctx_shared.c
> +++ b/tests/i915/gem_ctx_shared.c
> @@ -38,6 +38,7 @@
>
> #include
>
> +#include "i915/gem_engine_topology.h"
> #include "igt_rand.h"
> #include "igt_vgem.h"
> #include "sync_file.h"
> @@ -548,6 +549,14 @@ static uint32_t store_timestamp(int i915,
> return obj.handle;
> }
>
> +static uint32_t ring_base(int i915, unsigned ring)
> +{
> + if (ring == I915_EXEC_DEFAULT)
> + ring = I915_EXEC_RENDER; /* XXX */
> +
> + return gem_engine_mmio_base(i915, gem_eb_flags_to_engine(ring).name);
> +}
> +
> static void independent(int i915, unsigned ring, unsigned flags)
> {
> const int TIMESTAMP = 1023;
> @@ -555,33 +564,8 @@ static void independent(int i915, unsigned ring,
> unsigned flags)
> igt_spin_t *spin[MAX_ELSP_QLEN];
> unsigned int mmio_base;
>
> - /* XXX i915_query()! */
> -
Re: [Intel-gfx] [PATCH] drm/i915: stop assigning drm->dev_private pointer
On 2020-02-24 13:33:12, Jani Nikula wrote: > We no longer need or use it as we subclass struct drm_device in our > struct drm_i915_private, and can always use to_i915() to get at > i915. Stop assigning the pointer to catch anyone trying to add new users > for ->dev_private. > > Cc: Chris Wilson > Signed-off-by: Jani Nikula Reviewed-by: Dale B Stimson > --- > drivers/gpu/drm/i915/i915_drv.c | 2 -- > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 1 - > 2 files changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 4034e431cc4c..d5aed3b7d7e3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1372,8 +1372,6 @@ i915_driver_create(struct pci_dev *pdev, const struct > pci_device_id *ent) > return ERR_PTR(err); > } > > - i915->drm.dev_private = i915; > - > i915->drm.pdev = pdev; > pci_set_drvdata(pdev, i915); > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > index 3b8986983afc..754d0eb6beaa 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > @@ -144,7 +144,6 @@ struct drm_i915_private *mock_gem_device(void) > goto put_device; > } > i915->drm.pdev = pdev; > - i915->drm.dev_private = i915; > > intel_runtime_pm_init_early(>runtime_pm); > > -- > 2.20.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/4] lib/i915: Don't confuse param.size
On 2020-02-14 19:40:15, Chris Wilson wrote:
> If the context has no engines, it has no engines -- do not override the
> user's setup.
>
> Signed-off-by: Chris Wilson
Reviewed-by: Dale B Stimson
> ---
> lib/i915/gem_engine_topology.c | 19 +++
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/lib/i915/gem_engine_topology.c b/lib/i915/gem_engine_topology.c
> index 9daa03df4..ca1c1fdb9 100644
> --- a/lib/i915/gem_engine_topology.c
> +++ b/lib/i915/gem_engine_topology.c
> @@ -195,17 +195,6 @@ static int gem_topology_get_param(int fd,
> if (__gem_context_get_param(fd, p))
> return -1; /* using default engine map */
>
> - if (!p->size)
> - return 0;
> -
> - /* size will store the engine count */
> - p->size = (p->size - sizeof(struct i915_context_param_engines)) /
> - (offsetof(struct i915_context_param_engines,
> - engines[1]) -
> - sizeof(struct i915_context_param_engines));
> -
> - igt_assert_f(p->size <= GEM_MAX_ENGINES, "unsupported engine count\n");
> -
> return 0;
> }
>
> @@ -242,7 +231,13 @@ struct intel_engine_data intel_init_engine_list(int fd,
> uint32_t ctx_id)
> query_engine_list(fd, _data);
> ctx_map_engines(fd, _data, );
> } else {
> - /* param.size contains the engine count */
> + /* engine count can be inferred from size */
> + param.size -= sizeof(struct i915_context_param_engines);
> + param.size /= sizeof(struct i915_engine_class_instance);
> +
> + igt_assert_f(param.size <= GEM_MAX_ENGINES,
> + "unsupported engine count\n");
> +
> for (i = 0; i < param.size; i++)
> init_engine(_data.engines[i],
> engines.engines[i].engine_class,
> --
> 2.25.0
>
> ___
> igt-dev mailing list
> igt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/2] i915/gem_ctx_isolation: Check engine relative registers
The functionality provided by this patch is something we would like to have.
What are the prospects for having it merged soon?
-Dale
On 2019-10-21 12:01:38, Chris Wilson wrote:
> Some of the non-privileged registers are at the same offset on each
> engine. We can improve our coverage for unknown HW layout by using the
> reported engine->mmio_base for relative offsets.
>
> Signed-off-by: Chris Wilson
> ---
> tests/i915/gem_ctx_isolation.c | 160 -
> 1 file changed, 99 insertions(+), 61 deletions(-)
>
> diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> index 6aa27133c..2ed71dd34 100644
> --- a/tests/i915/gem_ctx_isolation.c
> +++ b/tests/i915/gem_ctx_isolation.c
> @@ -70,6 +70,7 @@ static const struct named_register {
> uint32_t ignore_bits;
> uint32_t write_mask; /* some registers bits do not exist */
> bool masked;
> + bool relative;
> } nonpriv_registers[] = {
> { "NOPID", NOCTX, RCS0, 0x2094 },
> { "MI_PREDICATE_RESULT_2", NOCTX, RCS0, 0x23bc },
> @@ -150,67 +151,45 @@ static const struct named_register {
> { "HALF_SLICE_CHICKEN7", GEN_RANGE(11, 11), RCS0, 0xe194, .masked =
> true },
> { "SAMPLER_MODE", GEN_RANGE(11, 11), RCS0, 0xe18c, .masked = true },
>
> - { "BCS_GPR", GEN9, BCS0, 0x22600, 32 },
> { "BCS_SWCTRL", GEN8, BCS0, 0x22200, .write_mask = 0x3, .masked = true
> },
>
> { "MFC_VDBOX1", NOCTX, VCS0, 0x12800, 64 },
> { "MFC_VDBOX2", NOCTX, VCS1, 0x1c800, 64 },
>
> - { "VCS0_GPR", GEN_RANGE(9, 10), VCS0, 0x12600, 32 },
> - { "VCS1_GPR", GEN_RANGE(9, 10), VCS1, 0x1c600, 32 },
> - { "VECS_GPR", GEN_RANGE(9, 10), VECS0, 0x1a600, 32 },
> -
> - { "VCS0_GPR", GEN11, VCS0, 0x1c0600, 32 },
> - { "VCS1_GPR", GEN11, VCS1, 0x1c4600, 32 },
> - { "VCS2_GPR", GEN11, VCS2, 0x1d0600, 32 },
> - { "VCS3_GPR", GEN11, VCS3, 0x1d4600, 32 },
> - { "VECS_GPR", GEN11, VECS0, 0x1c8600, 32 },
> + { "xCS_GPR", GEN9, ALL, 0x600, 32, .relative = true },
>
> {}
> }, ignore_registers[] = {
> { "RCS timestamp", GEN6, ~0u, 0x2358 },
> { "BCS timestamp", GEN7, ~0u, 0x22358 },
>
> - { "VCS0 timestamp", GEN_RANGE(7, 10), ~0u, 0x12358 },
> - { "VCS1 timestamp", GEN_RANGE(7, 10), ~0u, 0x1c358 },
> - { "VECS timestamp", GEN_RANGE(8, 10), ~0u, 0x1a358 },
> -
> - { "VCS0 timestamp", GEN11, ~0u, 0x1c0358 },
> - { "VCS1 timestamp", GEN11, ~0u, 0x1c4358 },
> - { "VCS2 timestamp", GEN11, ~0u, 0x1d0358 },
> - { "VCS3 timestamp", GEN11, ~0u, 0x1d4358 },
> - { "VECS timestamp", GEN11, ~0u, 0x1c8358 },
> + { "xCS timestamp", GEN8, ALL, 0x358, .relative = true },
>
> /* huc read only */
> - { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2000 },
> - { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x2014 },
> - { "BSD0 0x2000", GEN11, ~0u, 0x1c + 0x23b0 },
> -
> - { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2000 },
> - { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x2014 },
> - { "BSD1 0x2000", GEN11, ~0u, 0x1c4000 + 0x23b0 },
> -
> - { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2000 },
> - { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x2014 },
> - { "BSD2 0x2000", GEN11, ~0u, 0x1d + 0x23b0 },
> -
> - { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2000 },
> - { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x2014 },
> - { "BSD3 0x2000", GEN11, ~0u, 0x1d4000 + 0x23b0 },
> + { "BSD 0x2000", GEN11, ALL, 0x2000, .relative = true },
> + { "BSD 0x2014", GEN11, ALL, 0x2014, .relative = true },
> + { "BSD 0x23b0", GEN11, ALL, 0x23b0, .relative = true},
>
> {}
> };
>
> -static const char *register_name(uint32_t offset, char *buf, size_t len)
> +static const char *
> +register_name(uint32_t offset, uint32_t mmio_base, char *buf, size_t len)
> {
> for (const struct named_register *r = nonpriv_registers; r->name; r++) {
> unsigned int width = r->count ? 4*r->count : 4;
> - if (offset >= r->offset && offset < r->offset + width) {
> + uint32_t base;
> +
> + base = r->offset;
> + if (r->relative)
> + base += mmio_base;
> +
> + if (offset >= base && offset < base + width) {
> if (r->count <= 1)
> return r->name;
>
> snprintf(buf, len, "%s[%d]",
> - r->name, (offset - r->offset)/4);
> + r->name, (offset - base) / 4);
> return buf;
> }
> }
> @@ -218,22 +197,35 @@ static const char *register_name(uint32_t offset, char
> *buf, size_t len)
> return "unknown";
> }
>
> -static const struct named_register *lookup_register(uint32_t offset)
> +static const struct named_register *
> +lookup_register(uint32_t offset, uint32_t mmio_base)
> {
> for (const struct named_register *r = nonpriv_registers; r->name;
Re: [Intel-gfx] [PATCH i-g-t] i915/gem_ctx_isolation: Bump support for Tigerlake
> On Wed, Oct 02, 2019 at 12:26:48PM +0100, Chris Wilson wrote:
> > There's very little variation in non-privileged registers for Tigerlake,
> > so we can mostly inherit the set from gen11. There is no whitelist at
> > present, so we do not need to add any special registers.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111599
> > Signed-off-by: Chris Wilson
> > ---
> > tests/i915/gem_ctx_isolation.c | 11 ++-
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/i915/gem_ctx_isolation.c b/tests/i915/gem_ctx_isolation.c
> > index df1d655ae..819daafc3 100644
> > --- a/tests/i915/gem_ctx_isolation.c
> > +++ b/tests/i915/gem_ctx_isolation.c
> > @@ -55,10 +55,11 @@ enum {
> > #define GEN9 (ALL << 9)
> > #define GEN10 (ALL << 10)
> > #define GEN11 (ALL << 11)
> > +#define GEN12 (ALL << 12)
> >
> > #define NOCTX 0
> >
> > -#define LAST_KNOWN_GEN 11
> > +#define LAST_KNOWN_GEN 12
> >
> > static const struct named_register {
> > const char *name;
> > @@ -116,9 +117,9 @@ static const struct named_register {
> > { "Cache_Mode_0", GEN7, RCS0, 0x7000, .masked = true },
> > { "Cache_Mode_1", GEN7, RCS0, 0x7004, .masked = true },
> > { "GT_MODE", GEN8, RCS0, 0x7008, .masked = true },
> > - { "L3_Config", GEN8, RCS0, 0x7034 },
> > - { "TD_CTL", GEN8, RCS0, 0xe400, .write_mask = 0x },
> > - { "TD_CTL2", GEN8, RCS0, 0xe404 },
> > + { "L3_Config", GEN_RANGE(8, 11), RCS0, 0x7034 },
> > + { "TD_CTL", GEN_RANGE(8, 11), RCS0, 0xe400, .write_mask = 0x },
> > + { "TD_CTL2", GEN_RANGE(8, 11), RCS0, 0xe404 },
> > { "SO_NUM_PRIMS_WRITTEN0", GEN6, RCS0, 0x5200, 2 },
> > { "SO_NUM_PRIMS_WRITTEN1", GEN6, RCS0, 0x5208, 2 },
> > { "SO_NUM_PRIMS_WRITTEN2", GEN6, RCS0, 0x5210, 2 },
> > @@ -852,7 +853,7 @@ igt_main
> > gen = intel_gen(intel_get_drm_devid(fd));
> >
> > igt_warn_on_f(gen > LAST_KNOWN_GEN,
> > - "GEN not recognized! Test needs to be
> > updated to run.");
> > + "GEN not recognized! Test needs to be updated to
> > run.");
> > igt_skip_on(gen > LAST_KNOWN_GEN);
On 2019-10-02 14:38:31, Petri Latvala wrote:
> Thanks to this editorial change, we're able to see that this string is
> missing a newline character.
Your patch looks good (as does Petri's comment).
I had identified the same registers as in the patch, but had one additional
register. Should it be included?
+ { "COMMON_SLICE_CHICKEN2", GEN_RANGE(12, 12), RCS0, 0x7014, .masked =
true },
I did some testing on a TGL with your patch. There are two pre-existing
issues, both of which I have encountered before. These are that the S3/S4 test
never wakes up, and errors reported by nonpriv for vcs'2 registers. See below.
Because of the S3/S4 issues, running gem_ctx_isolation for Gen12 will require
subsequent reboot. Should gem_ctx_isolation temporarily disable the S3/S4
tests for Gen12 until this problem is resolved?
I have been doing some work to address the vcs issue, which I will send
to the mailing list soon. The vcs issue is due to confusion between the
physical engine really being vcs'2, and the kernel presenting the engine to
usermode as vcs1. The test refers to the vcs'2 registers via the mmio_base
expected for vcs1 and therefore fails. Planned solution: "MMIO Remapping"
for ICL and later.
Results for gem_ctx_isolation
Never wakes from rcs0-S3 or rcs0-S4. (Probably also true for *-S3 and *-s4).
Starting subtest: rcs0-S3
[cmd] rtcwake: assuming RTC uses UTC ...
rtcwake: wakeup from "mem" using /dev/rtc0 at Thu Oct 3 17:45:41 2019
The following diagnostic is due to confusion between the physical engine
really being vcs'2, and the kernel presenting the engine to usermode as vcs1.
The test refers to the registers via the mmio_base expected for vcs1 and
therefore fails.
Starting subtest: vcs1-nonpriv
(gem_ctx_isolation:2152) WARNING: Register 0x1c4600 (VCS1_GPR[0]):
A= B=
(gem_ctx_isolation:2152) WARNING: Register 0x1c4604 (VCS1_GPR[1]):
A= B=
...and so on
Tested with:
-
Local kernel branch:
Based on git://anongit.freedesktop.org/drm-tip
Branch drm-tip
fd44976bff7a drm-tip: 2019y-10m-03d-15h-13m-54s UTC integration manifest
-
Local igt branch:
Based on https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
branch master:
74f55119 (public/master) i915/gem_eio: Relax timeout for forced resets
Plus your patch:
i915/gem_ctx_isolation: Bump support for Tigerlake
Plus a test patch to bypass S3 and S4 tests so other results could be seen:
tests/i915/gem_ctx_isolation.c - Suppress suspend/resume tests
-Dale
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
