Re: [Intel-gfx] [PATCH v2 09/12] drm/i915/uncore: Add GSI offset to uncore
On Tue, Sep 06, 2022 at 04:14:21PM +0530, Iddamsetty, Aravind wrote:
>
>
> On 03-09-2022 05:02, Matt Roper wrote:
> > GT non-engine registers (referred to as "GSI" registers by the spec)
> > have the same relative offsets on standalone media as they do on the
> > primary GT, just with an additional "GSI offset" added to their MMIO
> > address. If we store this GSI offset in the standalone media's
> > intel_uncore structure, it can be automatically applied to all GSI reg
> > reads/writes that happen on that GT, allowing us to re-use our existing
> > GT code with minimal changes.
> >
> > Forcewake and shadowed register tables for the media GT (which will be
> > added in a future patch) are listed as final addresses that already
> > include the GSI offset, so we also need to add the GSI offset before
> > doing lookups of registers in one of those tables.
> >
> > Cc: Daniele Ceraolo Spurio
> > Signed-off-by: Matt Roper
> > ---
> > drivers/gpu/drm/i915/gt/intel_gt.c | 17 ++---
> > drivers/gpu/drm/i915/intel_device_info.h | 4 +++-
> > drivers/gpu/drm/i915/intel_uncore.c | 10 --
> > drivers/gpu/drm/i915/intel_uncore.h | 22 --
> > 4 files changed, 45 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> > b/drivers/gpu/drm/i915/gt/intel_gt.c
> > index fbb5e32979a4..a6ed11b933eb 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> > @@ -776,10 +776,20 @@ void intel_gt_driver_late_release_all(struct
> > drm_i915_private *i915)
> > }
> > }
> >
> > -static int intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
> > +/*
> > + * Note: the gsi_offset parameter here isn't used, but we want to keep the
> > + * function signature equivalent to gtdef->setup() so that it can be
> > plugged
> > + * in when we enabled remote tiles in the future.
> > + */
> > +static int intel_gt_tile_setup(struct intel_gt *gt,
> > + phys_addr_t phys_addr,
> > + u32 gsi_offset)
> > {
> > int ret;
> >
> > + /* GSI offset is only applicable for media GTs */
> > + drm_WARN_ON(>i915->drm, gsi_offset);
> > +
> > if (!gt_is_root(gt)) {
> > struct intel_uncore *uncore;
> >
> > @@ -832,7 +842,7 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> > gt->info.engine_mask = RUNTIME_INFO(i915)->platform_engine_mask;
> >
> > drm_dbg(>drm, "Setting up %s\n", gt->name);
> > - ret = intel_gt_tile_setup(gt, phys_addr);
> > + ret = intel_gt_tile_setup(gt, phys_addr, 0);
> > if (ret)
> > return ret;
> >
> > @@ -865,7 +875,8 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> > goto err;
> > }
> >
> > - ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base);
> > + ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base,
> > + gtdef->gsi_offset);
> > if (ret)
> > goto err;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> > b/drivers/gpu/drm/i915/intel_device_info.h
> > index b408ce384cd7..85e0ef0e91b1 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -254,8 +254,10 @@ struct intel_gt_definition {
> > enum intel_gt_type type;
> > char *name;
> > int (*setup)(struct intel_gt *gt,
> > -phys_addr_t phys_addr);
> > +phys_addr_t phys_addr,
> > +u32 gsi_offset);
> > u32 mapping_base;
> > + u32 gsi_offset;
> > intel_engine_mask_t engine_mask;
> > };
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> > b/drivers/gpu/drm/i915/intel_uncore.c
> > index 33bdcbc77ab2..ecb02421502d 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -927,6 +927,9 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
> > {
> > const struct intel_forcewake_range *entry;
> >
> > + if (IS_GSI_REG(offset))
> > + offset += uncore->gsi_offset;
> > +
> > entry = BSEARCH(offset,
> > uncore->fw_domains_table,
> > uncore->fw_domains_table_entries,
> > @@ -1142,6 +1145,9 @@ static bool is_shadowed(struct intel_uncore *uncore,
> > u32 offset)
> > if (drm_WARN_ON(>i915->drm, !uncore->shadowed_reg_table))
> > return false;
> >
> > + if (IS_GSI_REG(offset))
> > + offset += uncore->gsi_offset;
> > +
> > return BSEARCH(offset,
> >uncore->shadowed_reg_table,
> >uncore->shadowed_reg_table_entries,
> > @@ -1994,8 +2000,8 @@ static int __fw_domain_init(struct intel_uncore
> > *uncore,
> >
> > d->uncore = uncore;
> > d->wake_count = 0;
> > - d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set);
> > - d->reg_ack = uncore->regs +
Re: [Intel-gfx] [PATCH v2 09/12] drm/i915/uncore: Add GSI offset to uncore
On 03-09-2022 05:02, Matt Roper wrote:
> GT non-engine registers (referred to as "GSI" registers by the spec)
> have the same relative offsets on standalone media as they do on the
> primary GT, just with an additional "GSI offset" added to their MMIO
> address. If we store this GSI offset in the standalone media's
> intel_uncore structure, it can be automatically applied to all GSI reg
> reads/writes that happen on that GT, allowing us to re-use our existing
> GT code with minimal changes.
>
> Forcewake and shadowed register tables for the media GT (which will be
> added in a future patch) are listed as final addresses that already
> include the GSI offset, so we also need to add the GSI offset before
> doing lookups of registers in one of those tables.
>
> Cc: Daniele Ceraolo Spurio
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 17 ++---
> drivers/gpu/drm/i915/intel_device_info.h | 4 +++-
> drivers/gpu/drm/i915/intel_uncore.c | 10 --
> drivers/gpu/drm/i915/intel_uncore.h | 22 --
> 4 files changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
> b/drivers/gpu/drm/i915/gt/intel_gt.c
> index fbb5e32979a4..a6ed11b933eb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -776,10 +776,20 @@ void intel_gt_driver_late_release_all(struct
> drm_i915_private *i915)
> }
> }
>
> -static int intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
> +/*
> + * Note: the gsi_offset parameter here isn't used, but we want to keep the
> + * function signature equivalent to gtdef->setup() so that it can be plugged
> + * in when we enabled remote tiles in the future.
> + */
> +static int intel_gt_tile_setup(struct intel_gt *gt,
> +phys_addr_t phys_addr,
> +u32 gsi_offset)
> {
> int ret;
>
> + /* GSI offset is only applicable for media GTs */
> + drm_WARN_ON(>i915->drm, gsi_offset);
> +
> if (!gt_is_root(gt)) {
> struct intel_uncore *uncore;
>
> @@ -832,7 +842,7 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> gt->info.engine_mask = RUNTIME_INFO(i915)->platform_engine_mask;
>
> drm_dbg(>drm, "Setting up %s\n", gt->name);
> - ret = intel_gt_tile_setup(gt, phys_addr);
> + ret = intel_gt_tile_setup(gt, phys_addr, 0);
> if (ret)
> return ret;
>
> @@ -865,7 +875,8 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> goto err;
> }
>
> - ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base);
> + ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base,
> +gtdef->gsi_offset);
> if (ret)
> goto err;
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h
> b/drivers/gpu/drm/i915/intel_device_info.h
> index b408ce384cd7..85e0ef0e91b1 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -254,8 +254,10 @@ struct intel_gt_definition {
> enum intel_gt_type type;
> char *name;
> int (*setup)(struct intel_gt *gt,
> - phys_addr_t phys_addr);
> + phys_addr_t phys_addr,
> + u32 gsi_offset);
> u32 mapping_base;
> + u32 gsi_offset;
> intel_engine_mask_t engine_mask;
> };
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 33bdcbc77ab2..ecb02421502d 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -927,6 +927,9 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
> {
> const struct intel_forcewake_range *entry;
>
> + if (IS_GSI_REG(offset))
> + offset += uncore->gsi_offset;
> +
> entry = BSEARCH(offset,
> uncore->fw_domains_table,
> uncore->fw_domains_table_entries,
> @@ -1142,6 +1145,9 @@ static bool is_shadowed(struct intel_uncore *uncore,
> u32 offset)
> if (drm_WARN_ON(>i915->drm, !uncore->shadowed_reg_table))
> return false;
>
> + if (IS_GSI_REG(offset))
> + offset += uncore->gsi_offset;
> +
> return BSEARCH(offset,
> uncore->shadowed_reg_table,
> uncore->shadowed_reg_table_entries,
> @@ -1994,8 +2000,8 @@ static int __fw_domain_init(struct intel_uncore *uncore,
>
> d->uncore = uncore;
> d->wake_count = 0;
> - d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set);
> - d->reg_ack = uncore->regs + i915_mmio_reg_offset(reg_ack);
> + d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set) +
> uncore->gsi_offset;
> + d->reg_ack = uncore->regs + i915_mmio_reg_offset(reg_ack) +
> uncore->gsi_offset;
>
> d->id =
[Intel-gfx] [PATCH v2 09/12] drm/i915/uncore: Add GSI offset to uncore
GT non-engine registers (referred to as "GSI" registers by the spec)
have the same relative offsets on standalone media as they do on the
primary GT, just with an additional "GSI offset" added to their MMIO
address. If we store this GSI offset in the standalone media's
intel_uncore structure, it can be automatically applied to all GSI reg
reads/writes that happen on that GT, allowing us to re-use our existing
GT code with minimal changes.
Forcewake and shadowed register tables for the media GT (which will be
added in a future patch) are listed as final addresses that already
include the GSI offset, so we also need to add the GSI offset before
doing lookups of registers in one of those tables.
Cc: Daniele Ceraolo Spurio
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/gt/intel_gt.c | 17 ++---
drivers/gpu/drm/i915/intel_device_info.h | 4 +++-
drivers/gpu/drm/i915/intel_uncore.c | 10 --
drivers/gpu/drm/i915/intel_uncore.h | 22 --
4 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c
b/drivers/gpu/drm/i915/gt/intel_gt.c
index fbb5e32979a4..a6ed11b933eb 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -776,10 +776,20 @@ void intel_gt_driver_late_release_all(struct
drm_i915_private *i915)
}
}
-static int intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
+/*
+ * Note: the gsi_offset parameter here isn't used, but we want to keep the
+ * function signature equivalent to gtdef->setup() so that it can be plugged
+ * in when we enabled remote tiles in the future.
+ */
+static int intel_gt_tile_setup(struct intel_gt *gt,
+ phys_addr_t phys_addr,
+ u32 gsi_offset)
{
int ret;
+ /* GSI offset is only applicable for media GTs */
+ drm_WARN_ON(>i915->drm, gsi_offset);
+
if (!gt_is_root(gt)) {
struct intel_uncore *uncore;
@@ -832,7 +842,7 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
gt->info.engine_mask = RUNTIME_INFO(i915)->platform_engine_mask;
drm_dbg(>drm, "Setting up %s\n", gt->name);
- ret = intel_gt_tile_setup(gt, phys_addr);
+ ret = intel_gt_tile_setup(gt, phys_addr, 0);
if (ret)
return ret;
@@ -865,7 +875,8 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
goto err;
}
- ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base);
+ ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base,
+ gtdef->gsi_offset);
if (ret)
goto err;
diff --git a/drivers/gpu/drm/i915/intel_device_info.h
b/drivers/gpu/drm/i915/intel_device_info.h
index b408ce384cd7..85e0ef0e91b1 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -254,8 +254,10 @@ struct intel_gt_definition {
enum intel_gt_type type;
char *name;
int (*setup)(struct intel_gt *gt,
-phys_addr_t phys_addr);
+phys_addr_t phys_addr,
+u32 gsi_offset);
u32 mapping_base;
+ u32 gsi_offset;
intel_engine_mask_t engine_mask;
};
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 33bdcbc77ab2..ecb02421502d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -927,6 +927,9 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
{
const struct intel_forcewake_range *entry;
+ if (IS_GSI_REG(offset))
+ offset += uncore->gsi_offset;
+
entry = BSEARCH(offset,
uncore->fw_domains_table,
uncore->fw_domains_table_entries,
@@ -1142,6 +1145,9 @@ static bool is_shadowed(struct intel_uncore *uncore, u32
offset)
if (drm_WARN_ON(>i915->drm, !uncore->shadowed_reg_table))
return false;
+ if (IS_GSI_REG(offset))
+ offset += uncore->gsi_offset;
+
return BSEARCH(offset,
uncore->shadowed_reg_table,
uncore->shadowed_reg_table_entries,
@@ -1994,8 +2000,8 @@ static int __fw_domain_init(struct intel_uncore *uncore,
d->uncore = uncore;
d->wake_count = 0;
- d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set);
- d->reg_ack = uncore->regs + i915_mmio_reg_offset(reg_ack);
+ d->reg_set = uncore->regs + i915_mmio_reg_offset(reg_set) +
uncore->gsi_offset;
+ d->reg_ack = uncore->regs + i915_mmio_reg_offset(reg_ack) +
uncore->gsi_offset;
d->id = domain_id;
diff --git a/drivers/gpu/drm/i915/intel_uncore.h
b/drivers/gpu/drm/i915/intel_uncore.h
index 4acb78a03233..7f1d7903a8f3 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++
