On Fri, Jan 08, 2016 at 02:26:40PM +0200, Joonas Lahtinen wrote:
> Hi,
>
> Comments below.
>
> On ma, 2015-11-23 at 11:38 +0000, [email protected] wrote:
> > From: Dave Gordon <[email protected]>
> >
> > Added various definitions that will be useful for the scheduler in
> > general and
> > pre-emptive context switching in particular.
> >
> > Change-Id: Ica805b94160426def51f5d520f5ce51c60864a98
> > For: VIZ-1587
> > Signed-off-by: Dave Gordon <[email protected]>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 30
> > +++++++++++++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 32
> > +++++++++++++++++++++++++++++---
> > 2 files changed, 58 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index db72f98..b986050 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -259,6 +259,10 @@
> > #define MI_GLOBAL_GTT (1<<22)
> >
> > #define MI_NOOP MI_INSTR(0, 0)
> > +#define MI_NOOP_WRITE_ID (1<<22)
> > +#define MI_NOOP_ID_MASK ((1<<22) - 1)
> > +#define MI_NOOP_MID(id) ((id) & MI_NOOP_ID_MASK)
>
> I'd stay consistent with the style of other _MASK/_SHIFT pairs
> elsewhere in this header. I find this rather unintuitive compared to
> 0xff notation especially if it would be with nonzero shift. MI_NOOP_MID
> is extra, too.
>
> Most of these new definitions are not used anywhere in the series?
> Partially due to that I would prefer these to be introduced in smaller
> patches prior to first use, at least to an extent that one patch per
> series, with only stuff being used. Would make reviewing much easier to
> know they have been actually tested too.
>
> Daniel, how do you want to see the register updates? As much added at once as
> possible, or only needed?
I'm fine with a separate patch to just update them all. We do that
regularly for feature work.
-Daniel
>
> > +#define MI_NOOP_WITH_ID(id) MI_INSTR(0,
> > MI_NOOP_WRITE_ID|MI_NOOP_MID(id))
> > #define MI_USER_INTERRUPT MI_INSTR(0x02, 0)
> > #define MI_WAIT_FOR_EVENT MI_INSTR(0x03, 0)
> > #define MI_WAIT_FOR_OVERLAY_FLIP (1<<16)
> > @@ -276,6 +280,7 @@
> > #define MI_ARB_ON_OFF MI_INSTR(0x08, 0)
> > #define MI_ARB_ENABLE (1<<0)
> > #define MI_ARB_DISABLE (0<<0)
> > +#define MI_ARB_CHECK MI_INSTR(0x05, 0)
> > #define MI_BATCH_BUFFER_END MI_INSTR(0x0a, 0)
> > #define MI_SUSPEND_FLUSH MI_INSTR(0x0b, 0)
> > #define MI_SUSPEND_FLUSH_EN (1<<0)
> > @@ -325,6 +330,8 @@
> > #define MI_SEMAPHORE_SYNC_INVALID (3<<16)
> > #define MI_SEMAPHORE_SYNC_MASK (3<<16)
> > #define MI_SET_CONTEXT MI_INSTR(0x18, 0)
> > +#define MI_CONTEXT_ADDR_MASK ((~0)<<12)
> > +#define MI_SET_CONTEXT_FLAG_MASK ((1<<12)-1)
> > #define MI_MM_SPACE_GTT (1<<8)
> > #define MI_MM_SPACE_PHYSICAL (0<<8)
> > #define MI_SAVE_EXT_STATE_EN (1<<3)
> > @@ -344,6 +351,10 @@
> > #define MI_USE_GGTT (1 << 22) /* g4x+ */
> > #define MI_STORE_DWORD_INDEX MI_INSTR(0x21, 1)
> > #define MI_STORE_DWORD_INDEX_SHIFT 2
> > +#define MI_STORE_REG_MEM MI_INSTR(0x24, 1)
> > +#define MI_STORE_REG_MEM_GTT (1 << 22)
> > +#define MI_STORE_REG_MEM_PREDICATE (1 << 21)
> > +
> > /* Official intel docs are somewhat sloppy concerning
> > MI_LOAD_REGISTER_IMM:
> > * - Always issue a MI_NOOP _before_ the MI_LOAD_REGISTER_IMM -
> > otherwise hw
> > * simply ignores the register load under certain conditions.
> > @@ -358,7 +369,10 @@
> > #define MI_FLUSH_DW MI_INSTR(0x26, 1) /* for GEN6 */
> > #define MI_FLUSH_DW_STORE_INDEX (1<<21)
> > #define MI_INVALIDATE_TLB (1<<18)
> > +#define MI_FLUSH_DW_OP_NONE (0<<14)
> > #define MI_FLUSH_DW_OP_STOREDW (1<<14)
> > +#define MI_FLUSH_DW_OP_RSVD (2<<14)
> > +#define MI_FLUSH_DW_OP_STAMP (3<<14)
> > #define MI_FLUSH_DW_OP_MASK (3<<14)
> > #define MI_FLUSH_DW_NOTIFY (1<<8)
> > #define MI_INVALIDATE_BSD (1<<7)
> > @@ -1533,6 +1547,19 @@ enum skl_disp_power_wells {
> >
> > #define HSW_GTT_CACHE_EN 0x4024
> > #define GTT_CACHE_EN_ALL 0xF0007FFF
> > +
> > +/*
> > + * Premption-related registers
> > + */
> > +#define RING_UHPTR(base) ((base)+0x134)
> > +#define UHPTR_GFX_ADDR_ALIGN (0x7)
> > +#define UHPTR_VALID (0x1)
> > +#define RING_PREEMPT_ADDR 0x0214c
> > +#define PREEMPT_BATCH_LEVEL_MASK (0x3)
> > +#define BB_PREEMPT_ADDR 0x02148
> > +#define SBB_PREEMPT_ADDR 0x0213c
> > +#define RS_PREEMPT_STATUS 0x0215c
> > +
> > #define GEN7_WR_WATERMARK 0x4028
> > #define GEN7_GFX_PRIO_CTRL 0x402C
> > #define ARB_MODE 0x4030
> > @@ -6736,7 +6763,8 @@ enum skl_disp_power_wells {
> > #define VLV_SPAREG2H 0xA194
> >
> > #define GTFIFODBG 0x120000
> > -#define GT_FIFO_SBDROPERR (1<<6)
> > +#define GT_FIFO_CPU_ERROR_MASK 0xf
> > +#define GT_FIFO_SDDROPERR (1<<6)
> > #define GT_FIFO_BLOBDROPERR (1<<5)
> > #define GT_FIFO_SB_READ_ABORTERR (1<<4)
> > #define GT_FIFO_DROPERR (1<<3)
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index 1987abd..48f60cc 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -49,6 +49,12 @@ struct intel_hw_status_page {
> > #define I915_READ_MODE(ring) I915_READ(RING_MI_MODE((ring)
> > ->mmio_base))
> > #define I915_WRITE_MODE(ring, val) I915_WRITE(RING_MI_MODE((ring)
> > ->mmio_base), val)
> >
> > +#define I915_READ_UHPTR(ring) \
> > + I915_READ(RING_UHPTR((ring)->mmio_base))
> > +#define I915_WRITE_UHPTR(ring, val) \
> > + I915_WRITE(RING_UHPTR((ring)->mmio_base), val)
> > +#define I915_READ_NOPID(ring) I915_READ(RING_NOPID((ring)
> > ->mmio_base))
> > +
> > /* seqno size is actually only a uint32, but since we plan to use
> > MI_FLUSH_DW to
> > * do the writes, and that must have qw aligned offsets, simply
> > pretend it's 8b.
> > */
> > @@ -426,10 +432,30 @@ intel_write_status_page(struct intel_engine_cs
> > *ring,
> > * 0x20-0x2f: Reserved (Gen6+)
> > *
> > * The area from dword 0x30 to 0x3ff is available for driver usage.
> > + *
> > + * Note: in general the allocation of these indices is arbitrary, as
> > long
> > + * as they are all unique. But a few of them are used with
> > instructions that
> > + * have specific alignment requirements, those particular indices
> > must be
> > + * chosen carefully to meet those requirements. The list below shows
> > the
> > + * currently-known alignment requirements:
> > + *
> > + * I915_GEM_SCRATCH_INDEX must be EVEN
> > + */
> > +
> > +/*
> > + * Tracking; these are updated by the GPU at the beginning and/or
> > end of every
> > + * batch. One pair for regular buffers, the other for preemptive
> > ones.
> > */
> > -#define I915_GEM_HWS_INDEX 0x30
> > -#define I915_GEM_HWS_SCRATCH_INDEX 0x40
> > -#define I915_GEM_HWS_SCRATCH_ADDR (I915_GEM_HWS_SCRATCH_INDEX <<
> > MI_STORE_DWORD_INDEX_SHIFT)
> > +#define I915_BATCH_DONE_SEQNO 0x30 /* Completed
> > batch seqno */
> > +#define I915_BATCH_ACTIVE_SEQNO 0x31 /* In progress
> > batch seqno */
> > +#define I915_PREEMPTIVE_DONE_SEQNO 0x32 /* Completed
> > preemptive batch */
> > +#define I915_PREEMPTIVE_ACTIVE_SEQNO 0x33 /* In progress
> > preemptive batch */
> > +
> > +#define I915_GEM_HWS_INDEX I915_BATCH_DONE_SEQNO
> > /* alias */
> > +#define I915_GEM_ACTIVE_SEQNO_INDEX I915_BATCH_ACTIVE_SEQNO
> > /* alias */
> > +
> > +#define I915_GEM_HWS_SCRATCH_INDEX 0x38 /* QWord */
> > +#define I915_GEM_HWS_SCRATCH_ADDR (I915_GEM_HWS_SCRATCH_INDEX
> > << MI_STORE_DWORD_INDEX_SHIFT)
> >
> > struct intel_ringbuffer *
> > intel_engine_create_ringbuffer(struct intel_engine_cs *engine, int
> > size);
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
