From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
To: Mika Kuoppala <mika.kuoppala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/i915: Prepare for larger CSB status FIFO size
Date: Tue, 27 Nov 2018 11:00:41 -0800 [thread overview]
Message-ID: <6c7785c3-c00b-2f78-054a-6411f263ca00@intel.com> (raw)
In-Reply-To: <20181127173845.17403-1-mika.kuoppala@linux.intel.com>
On 27/11/2018 09:38, Mika Kuoppala wrote:
> Make csb entry count variable in preparation for larger
> CSB status FIFO size found on gen11+ hardware.
>
Note that not all registers in the 12-deep CSB fifo are in a contiguous
range, the new ones (6-11) start at mmio_base + 0x3C0 (Bspec: 11724). If
we read contiguously we'll access the registers that are in between the
2 CSB ranges (e.g. RING_CONTEXT_STATUS_PTR). Not a real issue for us
since we always read from HWSP where the CSBs are contiguous, but we
need to at least update intel_engine_print_registers(). Also not sure if
GVT still needs the MMIO mode, if not we could maybe just rip it out
instead of adapting it.
Daniele
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_engine_cs.c | 16 +++++++++-------
> drivers/gpu/drm/i915/intel_lrc.c | 24 +++++++++++++++++-------
> drivers/gpu/drm/i915/intel_lrc.h | 9 +++++----
> drivers/gpu/drm/i915/intel_ringbuffer.h | 15 +++++++++------
> 4 files changed, 40 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 759c0fd58f8c..218ef817da1d 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1347,27 +1347,29 @@ static void intel_engine_print_registers(const struct intel_engine_cs *engine,
> unsigned int idx;
> u8 read, write;
>
> - drm_printf(m, "\tExeclist status: 0x%08x %08x\n",
> + drm_printf(m, "\tExeclist status: 0x%08x %08x, entries %u\n",
> I915_READ(RING_EXECLIST_STATUS_LO(engine)),
> - I915_READ(RING_EXECLIST_STATUS_HI(engine)));
> + I915_READ(RING_EXECLIST_STATUS_HI(engine)),
> + execlists->csb_entries);
>
> read = execlists->csb_head;
> write = READ_ONCE(*execlists->csb_write);
>
> drm_printf(m, "\tExeclist CSB read %d, write %d [mmio:%d], tasklet queued? %s (%s)\n",
> read, write,
> - GEN8_CSB_WRITE_PTR(I915_READ(RING_CONTEXT_STATUS_PTR(engine))),
> + I915_READ(RING_CONTEXT_STATUS_PTR(engine)) &
> + GEN11_CSB_WRITE_PTR_MASK,
> yesno(test_bit(TASKLET_STATE_SCHED,
> &engine->execlists.tasklet.state)),
> enableddisabled(!atomic_read(&engine->execlists.tasklet.count)));
> - if (read >= GEN8_CSB_ENTRIES)
> + if (read >= execlists->csb_entries)
> read = 0;
> - if (write >= GEN8_CSB_ENTRIES)
> + if (write >= execlists->csb_entries)
> write = 0;
> if (read > write)
> - write += GEN8_CSB_ENTRIES;
> + write += execlists->csb_entries;
> while (read < write) {
> - idx = ++read % GEN8_CSB_ENTRIES;
> + idx = ++read % execlists->csb_entries;
> drm_printf(m, "\tExeclist CSB[%d]: 0x%08x [mmio:0x%08x], context: %d [mmio:%d]\n",
> idx,
> hws[idx * 2],
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 08fd9b12e4d7..5487fe496bb6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -767,6 +767,7 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
>
> static void reset_csb_pointers(struct intel_engine_execlists *execlists)
> {
> + u32 reset_val;
> /*
> * After a reset, the HW starts writing into CSB entry [0]. We
> * therefore have to set our HEAD pointer back one entry so that
> @@ -776,8 +777,19 @@ static void reset_csb_pointers(struct intel_engine_execlists *execlists)
> * inline comparison of our cached head position against the last HW
> * write works even before the first interrupt.
> */
> - execlists->csb_head = execlists->csb_write_reset;
> - WRITE_ONCE(*execlists->csb_write, execlists->csb_write_reset);
> + execlists->csb_head = execlists->csb_entries - 1;
> +
> + if (execlists_mmio_mode(execlists)) {
> + const u32 mask = execlists->csb_entries == GEN8_CSB_ENTRIES ?
> + GEN8_CSB_WRITE_PTR_MASK :
> + GEN11_CSB_WRITE_PTR_MASK;
> +
> + reset_val = _MASKED_FIELD(mask, execlists->csb_head);
> + } else {
> + reset_val = execlists->csb_head;
> + }
> +
> + WRITE_ONCE(*execlists->csb_write, reset_val);
> }
>
> static void nop_submission_tasklet(unsigned long data)
> @@ -895,7 +907,7 @@ static void process_csb(struct intel_engine_cs *engine)
> unsigned int status;
> unsigned int count;
>
> - if (++head == GEN8_CSB_ENTRIES)
> + if (++head == execlists->csb_entries)
> head = 0;
>
> /*
> @@ -2264,6 +2276,8 @@ static int logical_ring_init(struct intel_engine_cs *engine)
> upper_32_bits(ce->lrc_desc);
> }
>
> + execlists->csb_entries = GEN8_CSB_ENTRIES;
> +
> execlists->csb_read =
> i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
> if (csb_force_mmio(i915)) {
> @@ -2271,16 +2285,12 @@ static int logical_ring_init(struct intel_engine_cs *engine)
> (i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0)));
>
> execlists->csb_write = (u32 __force *)execlists->csb_read;
> - execlists->csb_write_reset =
> - _MASKED_FIELD(GEN8_CSB_WRITE_PTR_MASK,
> - GEN8_CSB_ENTRIES - 1);
> } else {
> execlists->csb_status =
> &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
>
> execlists->csb_write =
> &engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
> - execlists->csb_write_reset = GEN8_CSB_ENTRIES - 1;
> }
> reset_csb_pointers(execlists);
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index f5a5502ecf70..e865cba43a26 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -55,10 +55,11 @@
> #define GEN8_CSB_PTR_MASK 0x7
> #define GEN8_CSB_READ_PTR_MASK (GEN8_CSB_PTR_MASK << 8)
> #define GEN8_CSB_WRITE_PTR_MASK (GEN8_CSB_PTR_MASK << 0)
> -#define GEN8_CSB_WRITE_PTR(csb_status) \
> - (((csb_status) & GEN8_CSB_WRITE_PTR_MASK) >> 0)
> -#define GEN8_CSB_READ_PTR(csb_status) \
> - (((csb_status) & GEN8_CSB_READ_PTR_MASK) >> 8)
> +
> +#define GEN11_CSB_ENTRIES 12
> +#define GEN11_CSB_PTR_MASK 0xf
> +#define GEN11_CSB_READ_PTR_MASK (GEN11_CSB_PTR_MASK << 8)
> +#define GEN11_CSB_WRITE_PTR_MASK (GEN11_CSB_PTR_MASK << 0)
>
> enum {
> INTEL_CONTEXT_SCHEDULE_IN = 0,
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 8a2270b209b0..d9520a52fe29 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -339,13 +339,9 @@ struct intel_engine_execlists {
> u32 preempt_complete_status;
>
> /**
> - * @csb_write_reset: reset value for CSB write pointer
> - *
> - * As the CSB write pointer maybe either in HWSP or as a field
> - * inside an mmio register, we want to reprogram it slightly
> - * differently to avoid later confusion.
> + * @csb_entries: number of CSB entries
> */
> - u32 csb_write_reset;
> + u8 csb_entries;
>
> /**
> * @csb_head: context status buffer head
> @@ -714,6 +710,13 @@ execlists_is_active(const struct intel_engine_execlists *execlists,
> return test_bit(bit, (unsigned long *)&execlists->active);
> }
>
> +static inline bool
> +execlists_mmio_mode(const struct intel_engine_execlists * const execlists)
> +{
> + return (unsigned long)execlists->csb_read ==
> + (unsigned long)execlists->csb_write;
> +}
> +
> void execlists_user_begin(struct intel_engine_execlists *execlists,
> const struct execlist_port *port);
> void execlists_user_end(struct intel_engine_execlists *execlists);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-11-27 19:00 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-27 17:38 [PATCH 1/3] drm/i915: Prepare for larger CSB status FIFO size Mika Kuoppala
2018-11-27 17:38 ` [PATCH 2/3] drm/i915/icl: Switch to using 12 deep CSB status FIFO Mika Kuoppala
2018-11-27 17:38 ` [PATCH 3/3] drm/i915: Assert the csb tail is within bounds Mika Kuoppala
2018-11-27 17:48 ` Chris Wilson
2018-11-27 17:46 ` [PATCH 1/3] drm/i915: Prepare for larger CSB status FIFO size Chris Wilson
2018-11-27 18:43 ` ✓ Fi.CI.BAT: success for series starting with [1/3] " Patchwork
2018-11-27 19:00 ` Daniele Ceraolo Spurio [this message]
2018-11-28 1:16 ` ✓ Fi.CI.IGT: " Patchwork
2018-12-18 13:24 [PATCH 1/3] " Mika Kuoppala
2018-12-18 13:47 ` Chris Wilson
2018-12-19 12:27 ` Mika Kuoppala
2018-12-19 12:40 ` Chris Wilson
2018-12-19 13:17 ` Mika Kuoppala
2018-12-19 13:22 ` Mika Kuoppala
2018-12-19 13:26 ` Mika Kuoppala
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6c7785c3-c00b-2f78-054a-6411f263ca00@intel.com \
--to=daniele.ceraolospurio@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=mika.kuoppala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.