[Intel-gfx] [RFC 5/7] drm/i915/guc: Update GuC's log-buffer-state access for error capture.

[Teres Alexis, Alan Previn]

Thank you for the detailed review Matt. Responses and follow up questions on some of them below (wanna
make sure i dont misunderstand).

Will fix all the rest - glad we dont have any design problems .. so far :)

...alan

On Tue, 2021-12-07 at 14:31 -0800, Matthew Brost wrote:
> On Mon, Nov 22, 2021 at 03:04:00PM -0800, Alan Previn wrote:
> > -static bool guc_check_log_buf_overflow(struct intel_guc_log *log,
> > -				       enum guc_log_buffer_type type,
> > -				       unsigned int full_cnt)
> > +bool guc_check_log_buf_overflow(struct intel_guc *guc,
> > +				struct intel_guc_log_stats *log_state,
> > +				unsigned int full_cnt)
> 
> I don't think you meant to drop the 'static' here.
actually i do need to call it from guc_capture - but that was on the next patch.
my action would be to move this change to the next patch and i guess change the name to "intel_guc..."?
(im assuming we dont wanna duplicate this).

> 
> > +
> > +	guc_log_size = PAGE_SIZE + DEBUG_BUFFER_SIZE + CRASH_BUFFER_SIZE + CAPTURE_BUFFER_SIZE;
> 
> I'd personally keep the original formatting here.
> 
Question: - You are refering to just that last line of the "guc_log_size = .." above right?

> >   struct intel_guc_log {
> >  	u32 level;
> >  	struct i915_vma *vma;
> > +	void *buf_addr;
> 
> I don't think you need both 'buf_addr' and 'relay.buf_addr' as they are
> the same value, right?
> 
Matt
> 
Clarification: In the baseline code, i believe we use the "relay.buf_addr" like an enable flag
way to verify that the guc relay debugfs was invoked by user space and is currently active.
(which can be disabled. That said I can definitely remove that relay.buf_addr by introducing
a more descriptive flag such as "relay.active". Assume this is ok right?