Re: [Intel-gfx] [PATCH v7 01/13] drm/i915/guc: Update GuC ADS size for error capture lists

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

Thanks for reviewing Matt.. On one specific open - have replied below. 
Will fix the others.

...alan

On 3/9/2022 9:30 PM, Matthew Brost wrote:
> On Sat, Feb 26, 2022 at 01:55:29AM -0800, Alan Previn wrote:
> +static int
>> +guc_capture_prep_lists(struct intel_guc *guc)
>>   {
>> +	struct intel_gt *gt = guc_to_gt(guc);
>> +	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
>> +	u32 ggtt, capture_offset, null_ggtt, total_size = 0;
>> +	struct guc_gt_system_info local_info;
>> +	struct iosys_map info_map;
>> +	u32 null_header[2]={0};
>> +	struct file *file;
>> +	size_t size = 0;
>>   	int i, j;
>> -	u32 addr_ggtt, offset;
>>   
>> -	offset = guc_ads_capture_offset(guc);
>> -	addr_ggtt = intel_guc_ggtt_offset(guc, guc->ads_vma) + offset;
>> +	if (!iosys_map_is_null(&guc->ads_map)) {
>> +		capture_offset = guc_ads_capture_offset(guc);
>> +		ggtt = intel_guc_ggtt_offset(guc, guc->ads_vma) + capture_offset;
> This should just be capture_offset, right? ads_map is CPU mapped
> address that has nothing do with the GGTT address, it is just a pointer
> to the base of the ADS structure that can be accessed through the
> iosys_map* macros.

Good catch Matt...

note that "ggtt" is being used two ways in this function: [1] to memcpy 
content (register lists) and [2] to write a GGTT offset into members of 
ADS structure (the ptr to #1).

For the usage of [1] such as above, you are right, we shouldnt be 
including the vma's offset offsetting from ads_map. But for [2] we need 
both ads-offset + capture-offset.

for clarity, i can change the variable  name from "ggtt" to "ads_offset" 
and then reuse "capture_ggtt" with the rolling lists' ggtt offset.