On Thu, Jul 21, 2016 at 01:00:47PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Static table wastes space for invalid combinations and
> engines which are not supported by Gen6 (legacy semaphores).
>
> Replace it with a function devised by Dave Gordon.
>
> I have verified that it generates the same mappings between
> mbox selectors and signalling registers.
So just how big was that table? How big are the functions replacing it?
> v2: Add a comment describing what gen6_sem_f does.
> v3: This time with git add.
I like having the table a lot... Even if we don't find the function
convincing we should add that comment.
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Dave Gordon <david.s.gordon@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 7 +--
> drivers/gpu/drm/i915/intel_engine_cs.c | 93 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_ringbuffer.c | 40 +-------------
> drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++
> 4 files changed, 102 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9397ddec26b9..c2fe718582c8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1604,9 +1604,10 @@ enum skl_disp_power_wells {
> #define RING_HEAD(base) _MMIO((base)+0x34)
> #define RING_START(base) _MMIO((base)+0x38)
> #define RING_CTL(base) _MMIO((base)+0x3c)
> -#define RING_SYNC_0(base) _MMIO((base)+0x40)
> -#define RING_SYNC_1(base) _MMIO((base)+0x44)
> -#define RING_SYNC_2(base) _MMIO((base)+0x48)
> +#define RING_SYNC(base, n) _MMIO((base) + 0x40 + (n) * 4)
> +#define RING_SYNC_0(base) RING_SYNC(base, 0)
> +#define RING_SYNC_1(base) RING_SYNC(base, 1)
> +#define RING_SYNC_2(base) RING_SYNC(base, 2)
> #define GEN6_RVSYNC (RING_SYNC_0(RENDER_RING_BASE))
> #define GEN6_RBSYNC (RING_SYNC_1(RENDER_RING_BASE))
> #define GEN6_RVESYNC (RING_SYNC_2(RENDER_RING_BASE))
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index f4a35ec78481..19455b20b322 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -209,3 +209,96 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
>
> return i915_cmd_parser_init_ring(engine);
> }
> +
> +#define I915_NUM_GEN6_SEMAPHORE_ENGINES (4)
> +
> +/*
> + * For Gen6 semaphores where the driver issues MI_SEMAPHORE_MBOX commands
> + * with register selects so that a specific engine can wake up another engine
> + * waiting on a matching register, the matrix of required register selects
> + * looks like this:
> + *
> + * | RCS | VCS | BCS | VECS
> + * -----+---------------------------+---------------------------+---------------------------+---------------------------
> + * RCS | MI_SEMAPHORE_SYNC_INVALID | MI_SEMAPHORE_SYNC_VR | MI_SEMAPHORE_SYNC_BR | MI_SEMAPHORE_SYNC_VER
> + * VCS | MI_SEMAPHORE_SYNC_RV | MI_SEMAPHORE_SYNC_INVALID | MI_SEMAPHORE_SYNC_BV | MI_SEMAPHORE_SYNC_VEV
> + * BCS | MI_SEMAPHORE_SYNC_RB | MI_SEMAPHORE_SYNC_VB | MI_SEMAPHORE_SYNC_INVALID | MI_SEMAPHORE_SYNC_VEB
> + * VECS | MI_SEMAPHORE_SYNC_RVE | MI_SEMAPHORE_SYNC_VVE | MI_SEMAPHORE_SYNC_BVE | MI_SEMAPHORE_SYNC_INVALID
> + *
> + * This distilled to integers looks like this:
> + *
> + * | 0 | 1 | 2 | 3
> + * --+-----+-----+-----+-----
> + * 0 | -1 | 0 | 2 | 1
> + * 1 | 2 | -1 | 0 | 1
> + * 2 | 0 | 2 | -1 | 1
> + * 3 | 2 | 1 | 0 | -1
> + *
> + * In the opposite direction, the same table showing register addresses is:
> + *
> + * | RCS | VCS | BCS | VECS
> + * -----+--------------+--------------+--------------+--------------
> + * RCS | GEN6_NOSYNC | GEN6_RVSYNC | GEN6_RBSYNC | GEN6_RVESYNC
> + * VCS | GEN6_VRSYNC | GEN6_NOSYNC | GEN6_VBSYNC | GEN6_VVESYNC
> + * BCS | GEN6_VRSYNC | GEN6_BVSYNC | GEN6_NOSYNC | GEN6_BVESYNC
> + * VECS | GEN6_VERSYNC | GEN6_VEVSYNC | GEN6_VEBSYNC | GEN6_NOSYNC
> + *
> + * Again this distilled to integers looks like this:
> + *
> + * | 0 | 1 | 2 | 3
> + * --+-----+-----+-----+-----
> + * 0 | -1 | 0 | 1 | 2
> + * 1 | 1 | -1 | 0 | 2
> + * 2 | 0 | 1 | -1 | 2
> + * 3 | 1 | 2 | 0 | -1
> + *
> + * The function gen6_sem_f expresses the above table. We also notice that the
> + * difference between the first and second tabe is only a transpose of ones to
> + * twos and twos to ones.
> + */
> +
> +static int gen6_sem_f(unsigned int x, unsigned int y)
gen6_sema_select
gen6_semaphore_flag
> +{
> + if (x == y)
> + return -1;
> +
> + x = intel_engines[x].guc_id;
> + y = intel_engines[y].guc_id;
hw_id.
> +
> + if (x >= I915_NUM_GEN6_SEMAPHORE_ENGINES ||
> + y >= I915_NUM_GEN6_SEMAPHORE_ENGINES)
> + return -1;
> +
/*
* X
* | 0 | 1 | 2 | 3
* --+-----+-----+-----+-----
* 0 | | 0 | 1 | 2
* Y 1 | 1 | | 0 | 2
* 2 | 0 | 1 | | 2
* 3 | 1 | 2 | 0 |
*/
> + x -= x >= y;
> + if (y == 1)
> + x = 3 - x;
> + x += y & 1;
> + return x % 3;
> +}
> +
> +u32 gen6_wait_mbox(enum intel_engine_id x, enum intel_engine_id y)
static...
> +{
> + int r;
> +
> + r = gen6_sem_f(x, y);
> + if (r < 0)
> + return MI_SEMAPHORE_SYNC_INVALID;
> +
> + if (r == 1)
> + r = 2;
> + else if (r == 2)
> + r = 1;
> +
> + return r << 16;
/* Convert semaphore sync field to its wait flag */
switch (gen6_sem_f(x, y)) {
case 0: return 0;
case 1: return 2 << 16;
case 2: return 1 << 16;
default: eturn MI_SEMAPHORE_SYNC_INVALID;
}
> +}
> +
> +i915_reg_t gen6_signal_reg(enum intel_engine_id x, enum intel_engine_id y)
static...
> +{
> + int r;
> +
> + r = gen6_sem_f(x, y);
> + if (r < 0)
> + return GEN6_NOSYNC;
> +
> + return RING_SYNC(intel_engines[y].mmio_base, r);
> +}
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
