Re: [Intel-gfx] [RFC 2/7] drm/i915/guc: Update GuC ADS size for error capture lists

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

Michal, wrt this one:

+/************ FIXME: Populate tables for other devices in subsequent patch ************/
> > > +
> > > +static struct __guc_mmio_reg_descr_group *
> > > +guc_capture_get_device_reglist(struct drm_i915_private *dev_priv)
> > 
> > in new code we are using "i915" instead of "dev_priv" and since this
> > function has "guc" prefix it shall rather take "guc" as param:
> > 
> > guc_capture_get_device_reglist(struct intel_guc *guc)
> > {
> > 	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> > 	...
> > 

if its a static function that is only used internally, does the rule still apply?
I thought rules on primary handle inputs are for cross-i915-component interfaces that should start with an "intel_" in
front no? I will fix the others, but will keep static internal only functions the way they are (to avoid unnecessary de-
refencing).


...alan



On Wed, 2021-11-24 at 09:35 -0800, Alan Previn Teres Alexis wrote:
> Thanks Michal for the thorough review of the code (and the other patches). I will fix them all.
> 
> On the register-to-string helper function,
> i'll have to think it through because i do want to keep future development
> maintenance work when adding new registers simple (in the sense that
> adding a single line into the table will be all thats needed).
> 
> Unless you are suggesting keeping a global i915-wide list somewhere?
> which might be a bit of an overhead when searching through an offset list
> to find the mmio being requested for string return - unless i keep a sorted tree
> initialized with registers ordered by address, but would not work well for
> different registers that share addresses on diff gen's).
> 
> 
> ...alan
> 
> 
> On Tue, 2021-11-23 at 22:46 +0100, Michal Wajdeczko wrote:
> > Hi,
> > 
> > just few random nits below
> > 
> > -Michal
> > 
> > 
> > On 23.11.2021 00:03, Alan Previn wrote:
> > > Update GuC ADS size allocation to include space for
> > > the lists of error state capture register descriptors.
> > > 
> > > Also, populate the lists of registers we want GuC to report back to
> > > Host on engine reset events. This list should include global,
> > > engine-class and engine-instance registers for every engine-class
> > > type on the current hardware.
> > > 
> > > NOTE: Start with a fake table of register lists to layout the
> > > framework before adding real registers in subsequent patch.
> > > 
> > > Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/Makefile                 |   1 +
> > >  drivers/gpu/drm/i915/gt/uc/intel_guc.c        |  10 +-
> > >  drivers/gpu/drm/i915/gt/uc/intel_guc.h        |   5 +
> > >  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c    | 176 ++++++++++++-
> > >  .../gpu/drm/i915/gt/uc/intel_guc_capture.c    | 232 ++++++++++++++++++
> > >  .../gpu/drm/i915/gt/uc/intel_guc_capture.h    |  47 ++++
> > >  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h   |  19 +-
> > >  7 files changed, 476 insertions(+), 14 deletions(-)
> > >  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> > >  create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h
> > >