Re: [PATCH 2/5] drm/i915: Use an array of register tables in command parser

Francisco Jerez
2016-03-16T23:34:16Z

[-- Attachment #1.1.1: Type: text/plain, Size: 7157 bytes --]

Jordan Justen <jordan.l.justen@intel.com> writes:

> For Haswell, we will want another table of registers while retaining
> the large common table of whitelisted registers shared by all gen7
> devices.
>
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>

Reviewed-by: Francisco Jerez <currojerez@riseup.net>

> ---
> drivers/gpu/drm/i915/i915_cmd_parser.c | 101 +++++++++++++++++++++++---------
> drivers/gpu/drm/i915/intel_ringbuffer.h | 13 +---
> 2 files changed, 75 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 86d7cda..46ea40b 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -501,6 +501,32 @@ static const struct drm_i915_reg_descriptor hsw_master_regs[] = {
> #undef REG64
> #undef REG32
>
> +struct drm_i915_reg_table {
> + const struct drm_i915_reg_descriptor *regs;
> + int num_regs;
> + bool master;
> +};
> +
> +static const struct drm_i915_reg_table ivb_render_reg_tables[] = {
> + { gen7_render_regs, ARRAY_SIZE(gen7_render_regs), false },
> + { ivb_master_regs, ARRAY_SIZE(ivb_master_regs), true },
> +};
> +
> +static const struct drm_i915_reg_table ivb_blt_reg_tables[] = {
> + { gen7_blt_regs, ARRAY_SIZE(gen7_blt_regs), false },
> + { ivb_master_regs, ARRAY_SIZE(ivb_master_regs), true },
> +};
> +
> +static const struct drm_i915_reg_table hsw_render_reg_tables[] = {
> + { gen7_render_regs, ARRAY_SIZE(gen7_render_regs), false },
> + { hsw_master_regs, ARRAY_SIZE(hsw_master_regs), true },
> +};
> +
> +static const struct drm_i915_reg_table hsw_blt_reg_tables[] = {
> + { gen7_blt_regs, ARRAY_SIZE(gen7_blt_regs), false },
> + { hsw_master_regs, ARRAY_SIZE(hsw_master_regs), true },
> +};
> +
> static u32 gen7_render_get_cmd_length_mask(u32 cmd_header)
> {
> u32 client = (cmd_header & INSTR_CLIENT_MASK) >> INSTR_CLIENT_SHIFT;
> @@ -614,9 +640,16 @@ static bool check_sorted(int ring_id,
>
> static bool validate_regs_sorted(struct intel_engine_cs *ring)
> {
> - return check_sorted(ring->id, ring->reg_table, ring->reg_count) &&
> - check_sorted(ring->id, ring->master_reg_table,
> - ring->master_reg_count);
> + int i;
> + const struct drm_i915_reg_table *table;
> +
> + for (i = 0; i < ring->reg_table_count; i++) {
> + table = &ring->reg_tables[i];
> + if (!check_sorted(ring->id, table->regs, table->num_regs))
> + return false;
> + }
> +
> + return true;
> }
>
> struct cmd_node {
> @@ -711,15 +744,12 @@ int i915_cmd_parser_init_ring(struct intel_engine_cs *ring)
> cmd_table_count = ARRAY_SIZE(gen7_render_cmds);
> }
>
> - ring->reg_table = gen7_render_regs;
> - ring->reg_count = ARRAY_SIZE(gen7_render_regs);
> -
> if (IS_HASWELL(ring->dev)) {
> - ring->master_reg_table = hsw_master_regs;
> - ring->master_reg_count = ARRAY_SIZE(hsw_master_regs);
> + ring->reg_tables = hsw_render_reg_tables;
> + ring->reg_table_count = ARRAY_SIZE(hsw_render_reg_tables);
> } else {
> - ring->master_reg_table = ivb_master_regs;
> - ring->master_reg_count = ARRAY_SIZE(ivb_master_regs);
> + ring->reg_tables = ivb_render_reg_tables;
> + ring->reg_table_count = ARRAY_SIZE(ivb_render_reg_tables);
> }
>
> ring->get_cmd_length_mask = gen7_render_get_cmd_length_mask;
> @@ -738,15 +768,12 @@ int i915_cmd_parser_init_ring(struct intel_engine_cs *ring)
> cmd_table_count = ARRAY_SIZE(gen7_blt_cmds);
> }
>
> - ring->reg_table = gen7_blt_regs;
> - ring->reg_count = ARRAY_SIZE(gen7_blt_regs);
> -
> if (IS_HASWELL(ring->dev)) {
> - ring->master_reg_table = hsw_master_regs;
> - ring->master_reg_count = ARRAY_SIZE(hsw_master_regs);
> + ring->reg_tables = hsw_blt_reg_tables;
> + ring->reg_table_count = ARRAY_SIZE(hsw_blt_reg_tables);
> } else {
> - ring->master_reg_table = ivb_master_regs;
> - ring->master_reg_count = ARRAY_SIZE(ivb_master_regs);
> + ring->reg_tables = ivb_blt_reg_tables;
> + ring->reg_table_count = ARRAY_SIZE(ivb_blt_reg_tables);
> }
>
> ring->get_cmd_length_mask = gen7_blt_get_cmd_length_mask;
> @@ -849,12 +876,31 @@ static const struct drm_i915_reg_descriptor *
> find_reg(const struct drm_i915_reg_descriptor *table,
> int count, u32 addr)
> {
> - if (table) {
> - int i;
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + if (i915_mmio_reg_offset(table[i].addr) == addr)
> + return &table[i];
> + }
>
> - for (i = 0; i < count; i++) {
> - if (i915_mmio_reg_offset(table[i].addr) == addr)
> - return &table[i];
> + return NULL;
> +}
> +
> +static const struct drm_i915_reg_descriptor *
> +find_reg_in_tables(const struct drm_i915_reg_table *tables,
> + int count, bool is_master, u32 addr)
> +{
> + int i;
> + const struct drm_i915_reg_table *table;
> + const struct drm_i915_reg_descriptor *reg;
> +
> + for (i = 0; i < count; i++) {
> + table = &tables[i];
> + if (!table->master || is_master) {
> + reg = find_reg(table->regs, table->num_regs,
> + addr);
> + if (reg != NULL)
> + return reg;
> }
> }
>
> @@ -1005,13 +1051,10 @@ static bool check_cmd(const struct intel_engine_cs *ring,
> offset += step) {
> const u32 reg_addr = cmd[offset] & desc->reg.mask;
> const struct drm_i915_reg_descriptor *reg =
> - find_reg(ring->reg_table, ring->reg_count,
> - reg_addr);
> -
> - if (!reg && is_master)
> - reg = find_reg(ring->master_reg_table,
> - ring->master_reg_count,
> - reg_addr);
> + find_reg_in_tables(ring->reg_tables,
> + ring->reg_table_count,
> + is_master,
> + reg_addr);
>
> if (!reg) {
> DRM_DEBUG_DRIVER("CMD: Rejected register 0x%08X in command: 0x%08X (ring=%d)\n",
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 566b0ae..5f89261 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -125,7 +125,7 @@ struct intel_ringbuffer {
> };
>
> struct intel_context;
> -struct drm_i915_reg_descriptor;
> +struct drm_i915_reg_table;
>
> /*
> * we use a single page to load ctx workarounds so all of these
> @@ -332,15 +332,8 @@ struct intel_engine_cs {
> /*
> * Table of registers allowed in commands that read/write registers.
> */
> - const struct drm_i915_reg_descriptor *reg_table;
> - int reg_count;
> -
> - /*
> - * Table of registers allowed in commands that read/write registers, but
> - * only from the DRM master.
> - */
> - const struct drm_i915_reg_descriptor *master_reg_table;
> - int master_reg_count;
> + const struct drm_i915_reg_table *reg_tables;
> + int reg_table_count;
>
> /*
> * Returns the bitmask for the length field of the specified command.
> --
> 2.7.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]