Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Since commit fd8526e50902 ("drm/i915/execlists: Trust the CSB") we
> actually broke the force-mmio mode for our execlists implementation. No
> one noticed, so ergo no one is actually using an old vGPU host (where we
> required the older method) and so can simply remove the broken support.
>
> Reported-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Fixes: fd8526e50902 ("drm/i915/execlists: Trust the CSB")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 14 +++++++++++
> drivers/gpu/drm/i915/intel_lrc.c | 32 +++++++------------------
> drivers/gpu/drm/i915/intel_ringbuffer.h | 9 -------
> 3 files changed, 23 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e39016713464..3e5e2efce670 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1384,6 +1384,20 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
> }
> }
>
> + if (HAS_EXECLISTS(dev_priv)) {
> + /*
> + * Older GVT emulation depends upon intercepting CSB mmio,
> + * which we no longer use, preferring to use the HWSP cache
> + * instead.
> + */
> + if (intel_vgpu_active(dev_priv) &&
> + !intel_vgpu_has_hwsp_emulation(dev_priv)) {
> + i915_report_error(dev_priv,
> + "old vGPU host found, support for HWSP emulation required\n");
> + return -ENXIO;
> + }
> + }
> +
> intel_sanitize_options(dev_priv);
>
> i915_perf_init(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0a690c557113..1848ca2bf9ee 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -748,6 +748,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
>
> static void reset_csb_pointers(struct intel_engine_execlists *execlists)
> {
> + const unsigned int reset_value = GEN8_CSB_ENTRIES - 1;
> +
> /*
> * After a reset, the HW starts writing into CSB entry [0]. We
> * therefore have to set our HEAD pointer back one entry so that
> @@ -757,8 +759,8 @@ 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 = reset_value;
> + WRITE_ONCE(*execlists->csb_write, reset_value);
> }
>
> static void nop_submission_tasklet(unsigned long data)
> @@ -2213,12 +2215,6 @@ logical_ring_setup(struct intel_engine_cs *engine)
> logical_ring_default_irqs(engine);
> }
>
> -static bool csb_force_mmio(struct drm_i915_private *i915)
> -{
> - /* Older GVT emulation depends upon intercepting CSB mmio */
> - return intel_vgpu_active(i915) && !intel_vgpu_has_hwsp_emulation(i915);
> -}
> -
> static int logical_ring_init(struct intel_engine_cs *engine)
> {
> struct drm_i915_private *i915 = engine->i915;
> @@ -2250,22 +2246,12 @@ static int logical_ring_init(struct intel_engine_cs *engine)
>
> execlists->csb_read =
> i915->regs +
> i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine));
This can go too?
-Mika
> - if (csb_force_mmio(i915)) {
> - execlists->csb_status = (u32 __force *)
> - (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_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 =
> - &engine->status_page.page_addr[intel_hws_csb_write_index(i915)];
> - execlists->csb_write_reset = GEN8_CSB_ENTRIES - 1;
> - }
> reset_csb_pointers(execlists);
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 970fb5c05c36..2f7d1ce54f1e 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -338,15 +338,6 @@ 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.
> - */
> - u32 csb_write_reset;
> -
> /**
> * @csb_head: context status buffer head
> */
> --
> 2.20.0.rc1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
