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

[Sagar Arun Kamble]

On 1/31/2018 4:07 PM, Michał Winiarski wrote:
> 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).
True. Setup spread over different stages of load made it little 
complicated to change and your change will help.
> 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.
Yes. Agree on this approach. Thanks.
> I'm working on a series that does all of the above.
>
> -Michał
>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Michal Winiarski <michal.winiarski at intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc.c     |  2 +-
>>   drivers/gpu/drm/i915/intel_guc_log.c | 97 ++++++++++++++++++++++++++----------
>>   drivers/gpu/drm/i915/intel_guc_log.h | 16 +++++-
>>   drivers/gpu/drm/i915/intel_uc.c      | 14 ++----
>>   4 files changed, 90 insertions(+), 39 deletions(-)

-- 
Thanks,
Sagar