Re: [PATCH 2/3] drm/i915/guc: Handle interrupt based logging with lack of SSE4.1, relay and other setup

From: Chris Wilson <chris@chris-wilson.co.uk>
To: "Michał Winiarski" <michal.winiarski@intel.com>,
	"Sagar Arun Kamble" <sagar.a.kamble@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/i915/guc: Handle interrupt based logging with lack of SSE4.1, relay and other setup
Date: Wed, 31 Jan 2018 10:45:44 +0000	[thread overview]
Message-ID: <151739554419.10214.18219263381508194374@mail.alporthouse.com> (raw)
In-Reply-To: <20180131103720.4f4xf42yhqkn2o7w@mwiniars-main.ger.corp.intel.com>

Quoting Michał Winiarski (2018-01-31 10:37:20)
> On Wed, Jan 31, 2018 at 11:44:38AM +0530, Sagar Arun Kamble wrote:
> > On some systems like skl-gvtdvm, SSE4.1 movntdqa might not be available.
> > movntdqa is needed for efficient capture of the logs from uncached GuC
> > log buffer. GuC init was tied with this support and other setup needed
> > for interrupt based GuC log capture like relay channel/file support and
> > uncached mapping support. With this patch, GuC init is now unblocked from
> > lack of this support.
> > SSE and relay support init/fini is now being done by new functions
> > intel_guc_log_init|fini_runtime() which makes relay functions static.
> > We have introduced two states "supported" and "enabled". Supported is set
> > when we have SSE4.1 support and have relay, GuC log, WC mapping available.
> > Enabled is set when support is present and user has requested logging
> > through i915_modparams.guc_log_level.
> > While at this change, fixed unwind order in intel_uc_fini_misc.
> 
> Or, we could rework GuC log a bit.
> We could change the meaning of guc_log_level - it would only affect the
> "internal" GuC state (verbosity - in other words, the content of buffer used by
> GuC). This allows userspace to inspect the content of GuC buffer
> (i915_guc_log_dump), but i915 is not copying the data to the relay (we're
> ignoring the interrupts recieved from GuC).
> 
> This means that we need to introduce alternative way of controling the relay,
> let's say another file called "i915_guc_log_relay". Opening this file causes the
> relay to be created, we can throw an error if we don't have SSE4.1 there.
> (well - the whole runtime is created actually, things are quite badly named
> today, runtime means mapping of buffer shared with GuC, relay is the buffer
> "shared" with userspace, functions operating on "runtime" should probably setup
> both of those rather than just the mapping).
> 
> We're also solving the problem of overflows if GuC is loaded with
> "guc_log_level > 0" while nobody is consuming the data from the relay.
> And we don't really need to handle the "runtime" at module load time at all,
> which simplifies things.
> 
> I'm working on a series that does all of the above.

I'll push the 1st patch (the -EEXISTS fix) and we can come back for the
rest.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx