On Fri, Oct 14, 2016 at 11:53:44PM +0530, akash.goel@xxxxxxxxx wrote:
> From: Akash Goel <akash.goel@xxxxxxxxx>
>
> Driver accesses the ringbuffer pages, via GMADR BAR, if the pages are
> pinned in mappable aperture portion of GGTT and for ringbuffer pages
> allocated from Stolen memory, access can only be done through GMADR BAR.
> In case of GuC based submission, updates done in ringbuffer via GMADR
> may not get commited to memory by the time the Command streamer starts
> reading them, resulting in fetching of stale data.
Please leave a blank line between paragraphs, or try to not leave so
much whitespace at the end of a sentence.
> For Host based submission, such problem is not there as the write to Ring
> Tail or ELSP register happens from the Host side prior to submission.
> Access to any GFX register from CPU side goes to GTTMMADR BAR and Hw already
> enforces the ordering between outstanding GMADR writes & new GTTMADR access.
> MMIO writes from GuC side do not go to GTTMMADR BAR as GuC communication to
> registers within GT is contained within GT, so ordering is not enforced
> resulting in a race, which can manifest in form of a hang.
> To ensure the flush of in flight GMADR writes, a POSTING READ is done to
> GuC register prior to doorbell ring.
> There is already a similar WA in i915_gem_object_flush_gtt_write_domain(),
> which takes care of GMADR writes from User space to GEM buffers, but not the
> ringbuffer writes from KMD.
> This WA is needed on all recent HW.
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index a1f76c8..43c8a72 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -601,6 +601,7 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
> */
> static void i915_guc_submit(struct drm_i915_gem_request *rq)
> {
> + struct drm_i915_private *dev_priv = rq->i915;
> unsigned int engine_id = rq->engine->id;
> struct intel_guc *guc = &rq->i915->guc;
> struct i915_guc_client *client = guc->execbuf_client;
> @@ -608,6 +609,11 @@ static void i915_guc_submit(struct drm_i915_gem_request *rq)
>
> spin_lock(&client->wq_lock);
> guc_wq_item_append(client, rq);
> +
> + /* WA to flush out the pending GMADR writes to ring buffer. */
> + if (i915_vma_is_map_and_fenceable(rq->ring->vma))
> + POSTING_READ(GUC_STATUS);
Did you test POSTING_READ_FW() ?
Otherwise it makes an unfortunate amount of sense, and I feel justified
in what I had to do in flush_gtt_write_domwin! :)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
