Re: [PATCH v2] drm/i915/cmdparser: Remove stray intel_engine_cs *ring

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ma, 2016-07-25 at 10:06 +0100, Chris Wilson wrote:
> When we refer to intel_engine_cs, we want to use engine so as not to
> confuse ourselves about ringbuffers.
> 
> v2: Rename all the functions as well, as well as a few more stray comments.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Link: http://patchwork.freedesktop.org/patch/msgid/1469432687-22756-6-git-send-email-chris@chris-wilson.co.uk
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_cmd_parser.c     | 72 +++++++++++++++---------------
>  drivers/gpu/drm/i915/i915_drv.h            | 23 +++++-----
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++---
>  drivers/gpu/drm/i915/intel_engine_cs.c     |  2 +-
>  drivers/gpu/drm/i915/intel_lrc.c           |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c    |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.h    | 10 ++---
>  7 files changed, 64 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index b0fd6a7..8db144b 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -62,23 +62,23 @@
>   * The parser always rejects such commands.
>   *
>   * The majority of the problematic commands fall in the MI_* range, with only a
> - * few specific commands on each ring (e.g. PIPE_CONTROL and MI_FLUSH_DW).
> + * few specific commands on each engine (e.g. PIPE_CONTROL and MI_FLUSH_DW).
>   *
>   * Implementation:
> - * Each ring maintains tables of commands and registers which the parser uses in
> - * scanning batch buffers submitted to that ring.
> + * Each engine maintains tables of commands and registers which the parser
> + * uses in scanning batch buffers submitted to that engine.
>   *
>   * Since the set of commands that the parser must check for is significantly
>   * smaller than the number of commands supported, the parser tables contain only
>   * those commands required by the parser. This generally works because command
>   * opcode ranges have standard command length encodings. So for commands that
>   * the parser does not need to check, it can easily skip them. This is
> - * implemented via a per-ring length decoding vfunc.
> + * implemented via a per-engine length decoding vfunc.
>   *
>   * Unfortunately, there are a number of commands that do not follow the standard
>   * length encoding for their opcode range, primarily amongst the MI_* commands.
>   * To handle this, the parser provides a way to define explicit "skip" entries
> - * in the per-ring command tables.
> + * in the per-engine command tables.
>   *
>   * Other command table entries map fairly directly to high level categories
>   * mentioned above: rejected, master-only, register whitelist. The parser
> @@ -603,7 +603,7 @@ static u32 gen7_blt_get_cmd_length_mask(u32 cmd_header)
>  	return 0;
>  }
>  
> -static bool validate_cmds_sorted(struct intel_engine_cs *engine,
> +static bool validate_cmds_sorted(const struct intel_engine_cs *engine,

This otherwise unrelated change is a dependency from a signature change
below.

>  				 const struct drm_i915_cmd_table *cmd_tables,
>  				 int cmd_table_count)
>  {
> @@ -624,8 +624,9 @@ static bool validate_cmds_sorted(struct intel_engine_cs *engine,
>  			u32 curr = desc->cmd.value & desc->cmd.mask;
>  
>  			if (curr < previous) {
> -				DRM_ERROR("CMD: table not sorted ring=%d table=%d entry=%d cmd=0x%08X prev=0x%08X\n",
> -					  engine->id, i, j, curr, previous);
> +				DRM_ERROR("CMD: %s [%d] command table not sorted: table=%d entry=%d cmd=0x%08X prev=0x%08X\n",

Cut this line here like;

DRM_ERROR("CMD: %s [%d] command table not sorted: "
	  "table=%d entry=%d cmd=0x%08X prev=0x%08X\n"

> +					  engine->name, engine->id,
> +					  i, j, curr, previous);

Then this cut makes more sense, too.

Apart from that, looks fairly mechanical;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx