Re: [PATCH v15 1/5] drm/xe/guc: Prepare GuC register list and update ADS size for error capture

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

On Tue, 2024-08-06 at 07:57 -0700, Zhanjun Dong wrote:
> Add referenced registers defines and list of registers.
> Update GuC ADS size allocation to include space for
> the lists of error state capture register descriptors.
> 
> Then, populate GuC ADS with 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.
> 
> Ensure we allocate a persistent store for the register lists
> that are populated into ADS so that we don't need to allocate
> memory during GT resets when GuC is reloaded and ADS population
> happens again.
> 
> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com> #rev11

alan: took a look at this patch again since some changes come in after rev11
all LGTM but i believe u missed one comment syling gap that Michal requested in rev12.
see below - i'll give you a conditional RB once u fix that:

Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>

alan:snip


> diff --git a/drivers/gpu/drm/xe/xe_guc_capture_types.h b/drivers/gpu/drm/xe/xe_guc_capture_types.h
> new file mode 100644
> index 000000000000..839148c9fa0f
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_guc_capture_types.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2021-2024 Intel Corporation
> + */
> +
> +#ifndef _XE_GUC_CAPTURE_TYPES_H
> +#define _XE_GUC_CAPTURE_TYPES_H
> +
> +#include <linux/types.h>
> +#include "regs/xe_reg_defs.h"
> +
> +struct xe_guc;
> +
> +/**
> + * struct __guc_mmio_reg_descr - GuC mmio register descriptor
> + *
> + * xe_guc_capture module uses these structures to define a register
> + * (offsets, names, flags,...) that are used at the ADS regisration
> + * time as well as during runtime processing and reporting of error-
> + * capture states generated by GuC just prior to engine reset events.
> + */
> +struct __guc_mmio_reg_descr {
> +       /** @reg: the register */
> +       struct xe_reg reg;
> +       /** @flags: Flags for the register */
> +       u32 flags;
> +       /** @mask: The mask to apply */
> +       u32 mask;
> +       /** @regname: Name of the register */
> +       const char *regname;
> +};
> +
> +/**
> + * struct __guc_mmio_reg_descr_group - The group of register descriptor
> + *
> + * xe_guc_capture module uses these structures to maintain static
> + * tables (per unique platform) that consists of lists of registers
> + * (offsets, names, flags,...) that are used at the ADS regisration
> + * time as well as during runtime processing and reporting of error-
> + * capture states generated by GuC just prior to engine reset events.
> + */
> +struct __guc_mmio_reg_descr_group {
> +       /** @list: The register list */
> +       const struct __guc_mmio_reg_descr *list;
> +       /** @num_regs: Count of registers in the list */
> +       u32 num_regs;
> +       /** @owner: PF/VF owner */
> +       u32 owner; /* see enum guc_capture_list_index_type */
> +       /** @type: GuC capture register type */
> +       u32 type; /* see enum guc_state_capture_type */
> +       /** @engine: The engine class */
> +       u32 engine; /* see enum guc_capture_list_class_type */
alan: conditonal rb: re-quote from Michal in rev12: ix comments (keep in one block)
(the last 3 above, owner, type and engine).

alan:snip