On 3/9/2022 9:30 PM, Matthew Brost wrote:
On Sat, Feb 26, 2022 at 01:55:29AM -0800, Alan Previn wrote:
+intel_guc_capture_getlist(struct intel_guc *guc, u32 owner, u32 type,
u32 classid,
+ struct file **fileoutptr)
+{
+ struct __guc_state_capture_priv *gc = guc->capture.priv;
+ struct __guc_capture_ads_cache *cache = &gc->ads_cache[owner][type][classid];
+ struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
+ struct guc_debug_capture_list *listnode;
+ struct file *file;
+ u8 *caplist, *tmp;
+ size_t size = 0;
+ int ret, num_regs;
+
+ if (!gc->reglists)
+ return -ENODEV;
+
+ if (cache->is_valid) {
+ *fileoutptr = cache->file;
+ return cache->status;
+ }
+
+ ret = intel_guc_capture_getlistsize(guc, owner, type, classid, &size);
+ if (ret) {
+ cache->is_valid = true;
+ cache->file = NULL;
+ cache->size = 0;
+ cache->status = ret;
+ return ret;
+ }
+
+ caplist = kzalloc(size, GFP_KERNEL);
+ if (!caplist)
+ return -ENOMEM;
+
+ /* populate capture list header */
+ tmp = caplist;
+ num_regs = guc_cap_list_num_regs(guc->capture.priv, owner, type, classid);
+ listnode = (struct guc_debug_capture_list *)tmp;
+ listnode->header.info = FIELD_PREP(GUC_CAPTURELISTHDR_NUMDESCR, (u32)num_regs);
+
+ /* populate list of register descriptor */
+ tmp += sizeof(struct guc_debug_capture_list);
+ guc_capture_list_init(guc, owner, type, classid, (struct guc_mmio_reg *)tmp, num_regs);
+
+ /* ADS only takes file handles, so we keep that as our cache */
+ file = shmem_create_from_data("guc-err-cap", caplist, size);
I don't think you need a file here, why can't this just be a peice of
kalloc'd memory that is populated and copied via IOSYS map calls?
Agreed on above.
The memory will need to be persistent (allocated at driver load, free'd
during unload) as can't allocate memory in the reset path tho.
Of course (as per prior rev comments)
Beyond that, you are parsing a static table to populate memory. That
almost certainly means you could just have static memory which we
directly copy into the ADS.
On this, we dont actually copy the static tables from the top of this
src file as-is directly into ADS because the format of the static tables
is different from what ADS expects - i need both formats since the
latter is lean (just register info) but the former also contains the
name of registers (strings) to be used at printout time.
Matt
