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@xxxxxxxxx>
> > > ---
> > > 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
> > >
