| Message ID | 1448016961-25331-6-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 20, 2015 at 10:56:00AM +0000, Chris Wilson wrote: > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_cmd_parser.c | 51 ++++++++-------------------------- > drivers/gpu/drm/i915/i915_drv.h | 4 ++- > 2 files changed, 14 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c > index cfd07bfe6e75..ea9df2bb87de 100644 > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > @@ -113,7 +113,7 @@ > > /* Command Mask Fixed Len Action > ---------------------------------------------------------- */ > -static const struct drm_i915_cmd_descriptor common_cmds[] = { > +static struct drm_i915_cmd_descriptor common_cmds[] = { I'm a little sad to see the const gone. All this gets moved out of rodata. > CMD( MI_NOOP, SMI, F, 1, S ), > CMD( MI_USER_INTERRUPT, SMI, F, 1, R ), > CMD( MI_WAIT_FOR_EVENT, SMI, F, 1, M ), > @@ -146,7 +146,7 @@ static const struct drm_i915_cmd_descriptor common_cmds[] = { > CMD( MI_BATCH_BUFFER_START, SMI, !F, 0xFF, S ), > }; > > -static const struct drm_i915_cmd_descriptor render_cmds[] = { > +static struct drm_i915_cmd_descriptor render_cmds[] = { > CMD( MI_FLUSH, SMI, F, 1, S ), > CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), > CMD( MI_PREDICATE, SMI, F, 1, S ), > @@ -207,7 +207,7 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = { > }}, ), > }; > > -static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = { > +static struct drm_i915_cmd_descriptor hsw_render_cmds[] = { > CMD( MI_SET_PREDICATE, SMI, F, 1, S ), > CMD( MI_RS_CONTROL, SMI, F, 1, S ), > CMD( MI_URB_ATOMIC_ALLOC, SMI, F, 1, S ), > @@ -229,7 +229,7 @@ static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = { > CMD( GFX_OP_3DSTATE_BINDING_TABLE_EDIT_PS, S3D, !F, 0x1FF, S ), > }; > > -static const struct drm_i915_cmd_descriptor video_cmds[] = { > +static struct drm_i915_cmd_descriptor video_cmds[] = { > CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), > CMD( MI_SET_APPID, SMI, F, 1, S ), > CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B, > @@ -273,7 +273,7 @@ static const struct drm_i915_cmd_descriptor video_cmds[] = { > CMD( MFX_WAIT, SMFX, F, 1, S ), > }; > > -static const struct drm_i915_cmd_descriptor vecs_cmds[] = { > +static struct drm_i915_cmd_descriptor vecs_cmds[] = { > CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), > CMD( MI_SET_APPID, SMI, F, 1, S ), > CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B, > @@ -311,7 +311,7 @@ static const struct drm_i915_cmd_descriptor vecs_cmds[] = { > }}, ), > }; > > -static const struct drm_i915_cmd_descriptor blt_cmds[] = { > +static struct drm_i915_cmd_descriptor blt_cmds[] = { > CMD( MI_DISPLAY_FLIP, SMI, !F, 0xFF, R ), > CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3FF, B, > .bits = {{ > @@ -344,7 +344,7 @@ static const struct drm_i915_cmd_descriptor blt_cmds[] = { > CMD( SRC_COPY_BLT, S2D, !F, 0x3F, S ), > }; > > -static const struct drm_i915_cmd_descriptor hsw_blt_cmds[] = { > +static struct drm_i915_cmd_descriptor hsw_blt_cmds[] = { > CMD( MI_LOAD_SCAN_LINES_INCL, SMI, !F, 0x3F, M ), > CMD( MI_LOAD_SCAN_LINES_EXCL, SMI, !F, 0x3F, R ), > }; > @@ -618,11 +618,6 @@ static bool validate_regs_sorted(struct intel_engine_cs *ring) > ring->master_reg_count); > } > > -struct cmd_node { > - const struct drm_i915_cmd_descriptor *desc; > - struct hlist_node node; > -}; > - > /* > * Different command ranges have different numbers of bits for the opcode. For > * example, MI commands use bits 31:23 while 3D commands use bits 31:16. The > @@ -651,16 +646,8 @@ static int init_hash_table(struct intel_engine_cs *ring, > const struct drm_i915_cmd_table *table = &cmd_tables[i]; > > for (j = 0; j < table->count; j++) { > - const struct drm_i915_cmd_descriptor *desc = > - &table->table[j]; > - struct cmd_node *desc_node = > - kmalloc(sizeof(*desc_node), GFP_KERNEL); > - > - if (!desc_node) > - return -ENOMEM; init_hash_table() can no longer fail -> void? > - > - desc_node->desc = desc; > - hash_add(ring->cmd_hash, &desc_node->node, > + struct drm_i915_cmd_descriptor *desc = &table->table[j]; > + hash_add(ring->cmd_hash, &desc->node[ring->id], > desc->cmd.value & CMD_HASH_MASK); > } > } > @@ -668,18 +655,6 @@ static int init_hash_table(struct intel_engine_cs *ring, > return 0; > } > > -static void fini_hash_table(struct intel_engine_cs *ring) > -{ > - struct hlist_node *tmp; > - struct cmd_node *desc_node; > - int i; > - > - hash_for_each_safe(ring->cmd_hash, i, tmp, desc_node, node) { > - hash_del(&desc_node->node); > - kfree(desc_node); > - } > -} > - > /** > * i915_cmd_parser_init_ring() - set cmd parser related fields for a ringbuffer > * @ring: the ringbuffer to initialize > @@ -770,7 +745,6 @@ int i915_cmd_parser_init_ring(struct intel_engine_cs *ring) > ret = init_hash_table(ring, cmd_tables, cmd_table_count); > if (ret) { > DRM_ERROR("CMD: cmd_parser_init failed!\n"); > - fini_hash_table(ring); > return ret; > } > > @@ -790,8 +764,6 @@ void i915_cmd_parser_fini_ring(struct intel_engine_cs *ring) > { > if (!ring->needs_cmd_parser) > return; > - > - fini_hash_table(ring); > } i915_cmd_parser_fini_ring() is a nop now. Kill it? > > /* > @@ -801,11 +773,10 @@ static const struct drm_i915_cmd_descriptor* > find_cmd_in_table(struct intel_engine_cs *ring, > u32 cmd_header) > { > - struct cmd_node *desc_node; > + const struct drm_i915_cmd_descriptor *desc; > > - hash_for_each_possible(ring->cmd_hash, desc_node, node, > + hash_for_each_possible(ring->cmd_hash, desc, node[ring->id], > cmd_header & CMD_HASH_MASK) { > - const struct drm_i915_cmd_descriptor *desc = desc_node->desc; > if (((cmd_header ^ desc->cmd.value) & desc->cmd.mask) == 0) > return desc; At least we still return this as const, so the caller can't accidentally clobber it, even if we lose the rodata protection. Apart from the int vs. void thing and the nop i915_cmd_parser_fini_ring() the patch looks fine to me. > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 28d5bfceae3b..5960f76f1438 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2396,6 +2396,8 @@ struct drm_i915_cmd_descriptor { > u32 condition_offset; > u32 condition_mask; > } bits[MAX_CMD_DESC_BITMASKS]; > + > + struct hlist_node node[I915_NUM_RINGS]; > }; > > /* > @@ -2405,7 +2407,7 @@ struct drm_i915_cmd_descriptor { > * descriptors, which must be sorted with command opcodes in ascending order. > */ > struct drm_i915_cmd_table { > - const struct drm_i915_cmd_descriptor *table; > + struct drm_i915_cmd_descriptor *table; > int count; > }; > > -- > 2.6.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Nov 20, 2015 at 05:27:43PM +0200, Ville Syrjälä wrote: > On Fri, Nov 20, 2015 at 10:56:00AM +0000, Chris Wilson wrote: > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_cmd_parser.c | 51 ++++++++-------------------------- > > drivers/gpu/drm/i915/i915_drv.h | 4 ++- > > 2 files changed, 14 insertions(+), 41 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c > > index cfd07bfe6e75..ea9df2bb87de 100644 > > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > > @@ -113,7 +113,7 @@ > > > > /* Command Mask Fixed Len Action > > ---------------------------------------------------------- */ > > -static const struct drm_i915_cmd_descriptor common_cmds[] = { > > +static struct drm_i915_cmd_descriptor common_cmds[] = { > > I'm a little sad to see the const gone. All this gets moved out of > rodata. I'm open to having this refused. Pretty much both of 4 & 5 can be ignored after reducing the number of hash collisions in 6. Though not having to duplicate the memory for the hash tables is nice. -Chris
On Fri, Nov 20, 2015 at 03:34:22PM +0000, Chris Wilson wrote: > On Fri, Nov 20, 2015 at 05:27:43PM +0200, Ville Syrjälä wrote: > > On Fri, Nov 20, 2015 at 10:56:00AM +0000, Chris Wilson wrote: > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > --- > > > drivers/gpu/drm/i915/i915_cmd_parser.c | 51 ++++++++-------------------------- > > > drivers/gpu/drm/i915/i915_drv.h | 4 ++- > > > 2 files changed, 14 insertions(+), 41 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c > > > index cfd07bfe6e75..ea9df2bb87de 100644 > > > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > > > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > > > @@ -113,7 +113,7 @@ > > > > > > /* Command Mask Fixed Len Action > > > ---------------------------------------------------------- */ > > > -static const struct drm_i915_cmd_descriptor common_cmds[] = { > > > +static struct drm_i915_cmd_descriptor common_cmds[] = { > > > > I'm a little sad to see the const gone. All this gets moved out of > > rodata. > > I'm open to having this refused. Pretty much both of 4 & 5 can be > ignored after reducing the number of hash collisions in 6. Though not > having to duplicate the memory for the hash tables is nice. I don't have a strong opinion either way. On one hand rodata is nice, on the other hand simplicity from avoiding the kmalloc()s is nice.
On Fri, 20 Nov 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Nov 20, 2015 at 03:34:22PM +0000, Chris Wilson wrote: >> On Fri, Nov 20, 2015 at 05:27:43PM +0200, Ville Syrjälä wrote: >> > On Fri, Nov 20, 2015 at 10:56:00AM +0000, Chris Wilson wrote: >> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > > --- >> > > drivers/gpu/drm/i915/i915_cmd_parser.c | 51 ++++++++-------------------------- >> > > drivers/gpu/drm/i915/i915_drv.h | 4 ++- >> > > 2 files changed, 14 insertions(+), 41 deletions(-) >> > > >> > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c >> > > index cfd07bfe6e75..ea9df2bb87de 100644 >> > > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c >> > > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c >> > > @@ -113,7 +113,7 @@ >> > > >> > > /* Command Mask Fixed Len Action >> > > ---------------------------------------------------------- */ >> > > -static const struct drm_i915_cmd_descriptor common_cmds[] = { >> > > +static struct drm_i915_cmd_descriptor common_cmds[] = { >> > >> > I'm a little sad to see the const gone. All this gets moved out of >> > rodata. >> >> I'm open to having this refused. Pretty much both of 4 & 5 can be >> ignored after reducing the number of hash collisions in 6. Though not >> having to duplicate the memory for the hash tables is nice. > > I don't have a strong opinion either way. On one hand rodata is nice, > on the other hand simplicity from avoiding the kmalloc()s is nice. One could allocate (by kmalloc or statically) an array of struct cmd_node instead of each of them individually, however IIUC that was not the main optimization here but rather the reduction of the pointer chasing. BR, Jani.
On Fri, Nov 20, 2015 at 05:27:43PM +0200, Ville Syrjälä wrote: > On Fri, Nov 20, 2015 at 10:56:00AM +0000, Chris Wilson wrote: > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_cmd_parser.c | 51 ++++++++-------------------------- > > drivers/gpu/drm/i915/i915_drv.h | 4 ++- > > 2 files changed, 14 insertions(+), 41 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c > > index cfd07bfe6e75..ea9df2bb87de 100644 > > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > > @@ -113,7 +113,7 @@ > > > > /* Command Mask Fixed Len Action > > ---------------------------------------------------------- */ > > -static const struct drm_i915_cmd_descriptor common_cmds[] = { > > +static struct drm_i915_cmd_descriptor common_cmds[] = { > > I'm a little sad to see the const gone. All this gets moved out of > rodata. > > > CMD( MI_NOOP, SMI, F, 1, S ), > > CMD( MI_USER_INTERRUPT, SMI, F, 1, R ), > > CMD( MI_WAIT_FOR_EVENT, SMI, F, 1, M ), > > @@ -146,7 +146,7 @@ static const struct drm_i915_cmd_descriptor common_cmds[] = { > > CMD( MI_BATCH_BUFFER_START, SMI, !F, 0xFF, S ), > > }; > > > > -static const struct drm_i915_cmd_descriptor render_cmds[] = { > > +static struct drm_i915_cmd_descriptor render_cmds[] = { > > CMD( MI_FLUSH, SMI, F, 1, S ), > > CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), > > CMD( MI_PREDICATE, SMI, F, 1, S ), > > @@ -207,7 +207,7 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = { > > }}, ), > > }; > > > > -static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = { > > +static struct drm_i915_cmd_descriptor hsw_render_cmds[] = { > > CMD( MI_SET_PREDICATE, SMI, F, 1, S ), > > CMD( MI_RS_CONTROL, SMI, F, 1, S ), > > CMD( MI_URB_ATOMIC_ALLOC, SMI, F, 1, S ), > > @@ -229,7 +229,7 @@ static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = { > > CMD( GFX_OP_3DSTATE_BINDING_TABLE_EDIT_PS, S3D, !F, 0x1FF, S ), > > }; > > > > -static const struct drm_i915_cmd_descriptor video_cmds[] = { > > +static struct drm_i915_cmd_descriptor video_cmds[] = { > > CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), > > CMD( MI_SET_APPID, SMI, F, 1, S ), > > CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B, > > @@ -273,7 +273,7 @@ static const struct drm_i915_cmd_descriptor video_cmds[] = { > > CMD( MFX_WAIT, SMFX, F, 1, S ), > > }; > > > > -static const struct drm_i915_cmd_descriptor vecs_cmds[] = { > > +static struct drm_i915_cmd_descriptor vecs_cmds[] = { > > CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), > > CMD( MI_SET_APPID, SMI, F, 1, S ), > > CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B, > > @@ -311,7 +311,7 @@ static const struct drm_i915_cmd_descriptor vecs_cmds[] = { > > }}, ), > > }; > > > > -static const struct drm_i915_cmd_descriptor blt_cmds[] = { > > +static struct drm_i915_cmd_descriptor blt_cmds[] = { > > CMD( MI_DISPLAY_FLIP, SMI, !F, 0xFF, R ), > > CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3FF, B, > > .bits = {{ > > @@ -344,7 +344,7 @@ static const struct drm_i915_cmd_descriptor blt_cmds[] = { > > CMD( SRC_COPY_BLT, S2D, !F, 0x3F, S ), > > }; > > > > -static const struct drm_i915_cmd_descriptor hsw_blt_cmds[] = { > > +static struct drm_i915_cmd_descriptor hsw_blt_cmds[] = { > > CMD( MI_LOAD_SCAN_LINES_INCL, SMI, !F, 0x3F, M ), > > CMD( MI_LOAD_SCAN_LINES_EXCL, SMI, !F, 0x3F, R ), > > }; > > @@ -618,11 +618,6 @@ static bool validate_regs_sorted(struct intel_engine_cs *ring) > > ring->master_reg_count); > > } > > > > -struct cmd_node { > > - const struct drm_i915_cmd_descriptor *desc; > > - struct hlist_node node; > > -}; > > - > > /* > > * Different command ranges have different numbers of bits for the opcode. For > > * example, MI commands use bits 31:23 while 3D commands use bits 31:16. The > > @@ -651,16 +646,8 @@ static int init_hash_table(struct intel_engine_cs *ring, > > const struct drm_i915_cmd_table *table = &cmd_tables[i]; > > > > for (j = 0; j < table->count; j++) { > > - const struct drm_i915_cmd_descriptor *desc = > > - &table->table[j]; > > - struct cmd_node *desc_node = > > - kmalloc(sizeof(*desc_node), GFP_KERNEL); > > - > > - if (!desc_node) > > - return -ENOMEM; > > init_hash_table() can no longer fail -> void? > > > - > > - desc_node->desc = desc; > > - hash_add(ring->cmd_hash, &desc_node->node, > > + struct drm_i915_cmd_descriptor *desc = &table->table[j]; > > + hash_add(ring->cmd_hash, &desc->node[ring->id], > > desc->cmd.value & CMD_HASH_MASK); > > } > > } > > @@ -668,18 +655,6 @@ static int init_hash_table(struct intel_engine_cs *ring, > > return 0; > > } > > > > -static void fini_hash_table(struct intel_engine_cs *ring) > > -{ > > - struct hlist_node *tmp; > > - struct cmd_node *desc_node; > > - int i; > > - > > - hash_for_each_safe(ring->cmd_hash, i, tmp, desc_node, node) { > > - hash_del(&desc_node->node); > > - kfree(desc_node); > > - } > > -} > > - > > /** > > * i915_cmd_parser_init_ring() - set cmd parser related fields for a ringbuffer > > * @ring: the ringbuffer to initialize > > @@ -770,7 +745,6 @@ int i915_cmd_parser_init_ring(struct intel_engine_cs *ring) > > ret = init_hash_table(ring, cmd_tables, cmd_table_count); > > if (ret) { > > DRM_ERROR("CMD: cmd_parser_init failed!\n"); > > - fini_hash_table(ring); > > return ret; > > } > > > > @@ -790,8 +764,6 @@ void i915_cmd_parser_fini_ring(struct intel_engine_cs *ring) > > { > > if (!ring->needs_cmd_parser) > > return; > > - > > - fini_hash_table(ring); > > } > > i915_cmd_parser_fini_ring() is a nop now. Kill it? > > > > > /* > > @@ -801,11 +773,10 @@ static const struct drm_i915_cmd_descriptor* > > find_cmd_in_table(struct intel_engine_cs *ring, > > u32 cmd_header) > > { > > - struct cmd_node *desc_node; > > + const struct drm_i915_cmd_descriptor *desc; > > > > - hash_for_each_possible(ring->cmd_hash, desc_node, node, > > + hash_for_each_possible(ring->cmd_hash, desc, node[ring->id], > > cmd_header & CMD_HASH_MASK) { > > - const struct drm_i915_cmd_descriptor *desc = desc_node->desc; > > if (((cmd_header ^ desc->cmd.value) & desc->cmd.mask) == 0) > > return desc; > > At least we still return this as const, so the caller can't accidentally > clobber it, even if we lose the rodata protection. > > Apart from the int vs. void thing and the nop > i915_cmd_parser_fini_ring() the patch looks fine to me. This too can get Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> since the now unnecessary error handling doesn't actually do anything. Feel free to keep the r-b even if you decide to throw out the error handling parts. > > > } > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 28d5bfceae3b..5960f76f1438 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2396,6 +2396,8 @@ struct drm_i915_cmd_descriptor { > > u32 condition_offset; > > u32 condition_mask; > > } bits[MAX_CMD_DESC_BITMASKS]; > > + > > + struct hlist_node node[I915_NUM_RINGS]; > > }; > > > > /* > > @@ -2405,7 +2407,7 @@ struct drm_i915_cmd_descriptor { > > * descriptors, which must be sorted with command opcodes in ascending order. > > */ > > struct drm_i915_cmd_table { > > - const struct drm_i915_cmd_descriptor *table; > > + struct drm_i915_cmd_descriptor *table; > > int count; > > }; > > > > -- > > 2.6.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index cfd07bfe6e75..ea9df2bb87de 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -113,7 +113,7 @@ /* Command Mask Fixed Len Action ---------------------------------------------------------- */ -static const struct drm_i915_cmd_descriptor common_cmds[] = { +static struct drm_i915_cmd_descriptor common_cmds[] = { CMD( MI_NOOP, SMI, F, 1, S ), CMD( MI_USER_INTERRUPT, SMI, F, 1, R ), CMD( MI_WAIT_FOR_EVENT, SMI, F, 1, M ), @@ -146,7 +146,7 @@ static const struct drm_i915_cmd_descriptor common_cmds[] = { CMD( MI_BATCH_BUFFER_START, SMI, !F, 0xFF, S ), }; -static const struct drm_i915_cmd_descriptor render_cmds[] = { +static struct drm_i915_cmd_descriptor render_cmds[] = { CMD( MI_FLUSH, SMI, F, 1, S ), CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), CMD( MI_PREDICATE, SMI, F, 1, S ), @@ -207,7 +207,7 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = { }}, ), }; -static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = { +static struct drm_i915_cmd_descriptor hsw_render_cmds[] = { CMD( MI_SET_PREDICATE, SMI, F, 1, S ), CMD( MI_RS_CONTROL, SMI, F, 1, S ), CMD( MI_URB_ATOMIC_ALLOC, SMI, F, 1, S ), @@ -229,7 +229,7 @@ static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = { CMD( GFX_OP_3DSTATE_BINDING_TABLE_EDIT_PS, S3D, !F, 0x1FF, S ), }; -static const struct drm_i915_cmd_descriptor video_cmds[] = { +static struct drm_i915_cmd_descriptor video_cmds[] = { CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), CMD( MI_SET_APPID, SMI, F, 1, S ), CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B, @@ -273,7 +273,7 @@ static const struct drm_i915_cmd_descriptor video_cmds[] = { CMD( MFX_WAIT, SMFX, F, 1, S ), }; -static const struct drm_i915_cmd_descriptor vecs_cmds[] = { +static struct drm_i915_cmd_descriptor vecs_cmds[] = { CMD( MI_ARB_ON_OFF, SMI, F, 1, R ), CMD( MI_SET_APPID, SMI, F, 1, S ), CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B, @@ -311,7 +311,7 @@ static const struct drm_i915_cmd_descriptor vecs_cmds[] = { }}, ), }; -static const struct drm_i915_cmd_descriptor blt_cmds[] = { +static struct drm_i915_cmd_descriptor blt_cmds[] = { CMD( MI_DISPLAY_FLIP, SMI, !F, 0xFF, R ), CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3FF, B, .bits = {{ @@ -344,7 +344,7 @@ static const struct drm_i915_cmd_descriptor blt_cmds[] = { CMD( SRC_COPY_BLT, S2D, !F, 0x3F, S ), }; -static const struct drm_i915_cmd_descriptor hsw_blt_cmds[] = { +static struct drm_i915_cmd_descriptor hsw_blt_cmds[] = { CMD( MI_LOAD_SCAN_LINES_INCL, SMI, !F, 0x3F, M ), CMD( MI_LOAD_SCAN_LINES_EXCL, SMI, !F, 0x3F, R ), }; @@ -618,11 +618,6 @@ static bool validate_regs_sorted(struct intel_engine_cs *ring) ring->master_reg_count); } -struct cmd_node { - const struct drm_i915_cmd_descriptor *desc; - struct hlist_node node; -}; - /* * Different command ranges have different numbers of bits for the opcode. For * example, MI commands use bits 31:23 while 3D commands use bits 31:16. The @@ -651,16 +646,8 @@ static int init_hash_table(struct intel_engine_cs *ring, const struct drm_i915_cmd_table *table = &cmd_tables[i]; for (j = 0; j < table->count; j++) { - const struct drm_i915_cmd_descriptor *desc = - &table->table[j]; - struct cmd_node *desc_node = - kmalloc(sizeof(*desc_node), GFP_KERNEL); - - if (!desc_node) - return -ENOMEM; - - desc_node->desc = desc; - hash_add(ring->cmd_hash, &desc_node->node, + struct drm_i915_cmd_descriptor *desc = &table->table[j]; + hash_add(ring->cmd_hash, &desc->node[ring->id], desc->cmd.value & CMD_HASH_MASK); } } @@ -668,18 +655,6 @@ static int init_hash_table(struct intel_engine_cs *ring, return 0; } -static void fini_hash_table(struct intel_engine_cs *ring) -{ - struct hlist_node *tmp; - struct cmd_node *desc_node; - int i; - - hash_for_each_safe(ring->cmd_hash, i, tmp, desc_node, node) { - hash_del(&desc_node->node); - kfree(desc_node); - } -} - /** * i915_cmd_parser_init_ring() - set cmd parser related fields for a ringbuffer * @ring: the ringbuffer to initialize @@ -770,7 +745,6 @@ int i915_cmd_parser_init_ring(struct intel_engine_cs *ring) ret = init_hash_table(ring, cmd_tables, cmd_table_count); if (ret) { DRM_ERROR("CMD: cmd_parser_init failed!\n"); - fini_hash_table(ring); return ret; } @@ -790,8 +764,6 @@ void i915_cmd_parser_fini_ring(struct intel_engine_cs *ring) { if (!ring->needs_cmd_parser) return; - - fini_hash_table(ring); } /* @@ -801,11 +773,10 @@ static const struct drm_i915_cmd_descriptor* find_cmd_in_table(struct intel_engine_cs *ring, u32 cmd_header) { - struct cmd_node *desc_node; + const struct drm_i915_cmd_descriptor *desc; - hash_for_each_possible(ring->cmd_hash, desc_node, node, + hash_for_each_possible(ring->cmd_hash, desc, node[ring->id], cmd_header & CMD_HASH_MASK) { - const struct drm_i915_cmd_descriptor *desc = desc_node->desc; if (((cmd_header ^ desc->cmd.value) & desc->cmd.mask) == 0) return desc; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 28d5bfceae3b..5960f76f1438 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2396,6 +2396,8 @@ struct drm_i915_cmd_descriptor { u32 condition_offset; u32 condition_mask; } bits[MAX_CMD_DESC_BITMASKS]; + + struct hlist_node node[I915_NUM_RINGS]; }; /* @@ -2405,7 +2407,7 @@ struct drm_i915_cmd_descriptor { * descriptors, which must be sorted with command opcodes in ascending order. */ struct drm_i915_cmd_table { - const struct drm_i915_cmd_descriptor *table; + struct drm_i915_cmd_descriptor *table; int count; };
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_cmd_parser.c | 51 ++++++++-------------------------- drivers/gpu/drm/i915/i915_drv.h | 4 ++- 2 files changed, 14 insertions(+), 41 deletions(-)