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.
