[Intel-gfx] [PATCH v5 06/10] drm/i915/guc: Update GuC's log-buffer-state access for error capture.

[Teres Alexis, Alan Previn]

Matt, just a final confirmation on below

> > > > > 
> > > > > On Fri, 2022-02-04 at 10:19 -0800, Matthew Brost wrote:
> > > > > > On Wed, Jan 26, 2022 at 02:48:18AM -0800, Alan Previn wrote:
> > > > > > > 
> > If the object lives at the GuC level, operate on it at the GuC level.
> > 
> > e.g.
> > intel_guc_log_init_early calls mutex init on guc->log_state - that is
> > wrong and breaks the layering. intel_guc_log_init_early should only
> > operate on guc_log or below objects, not above it.
> > 
> > The key here is consisteny, if the GuC level owns the object it should
> > be initialized there + passed into the lower levels if possible. The
> > lower levels should avoid reaching back to GuC level for objects
> > whenever possible.
> > 
> > You could have 2 intel_guc_log_stats objects below the guc_log object
> > and 1 intel_guc_log_stats object for capture at the GuC level. That's
> > likely the right approach here.
> 
> Thanks Matt - I'm in agreement... I was concerned about too much of
> change - but you're right, I should be focusing on the design consistency.
> Above sounds like the correct design (these stats and locks should belong
> to their sole user).
> 
> ...alan
> 

So this means:
1. guc[upper] allocates the shared-logging-buffer
   - but would ask the lower level components for the sizes before
     buffering-up or capping-down to match interface spec.
2. guc-log and guc-error-capture requests guc for a vmap at their init.
3. guc-log and guc-error-capture owns independent log-stats and
   (and separate locks if needed).
4. when lower level components are done, they relinquish access to
   their region by requesting guc[upper] to unmap and free

A super clean separation like above could mean ripping apart enums
and other #defines to split them across guc_log and guc_error_capture
headers (such as region sizes).

I believe that separation complicates the understanding of the fw interface
for logging as we break that picture into independant files / components.
For now I want to keep guc[upper] aware of the individual sub-region
allocation requirements (no ripping apart of enums but moving them around)
but only keep the requesting of vmap and independant log-region-stats
within the lower level?

Are you okay with this?

side note: error-capture no longer need locks after the recent G2H triggered
linked-list extraction redesign.