[Intel-gfx] [PATCH v2 5/6] drm/i915: Reduce pointer indirection during cmd parser lookup
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Nov 20 07:27:43 PST 2015
On Fri, Nov 20, 2015 at 10:56:00AM +0000, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list