[Intel-gfx] [PATCH 2/2] drm/i915/guc: Use a single pass to calculate regset

[Lucas De Marchi]

On Tue, Feb 08, 2022 at 05:15:02PM -0800, Daniele Ceraolo Spurio wrote:
>
>
>On 2/7/2022 11:01 PM, Lucas De Marchi wrote:
>>The ADS initialitazion was using 2 passes to calculate the regset sent
>>to GuC to initialize each engine: the first pass to just have the final
>>object size and the second to set each register in place in the final
>>gem object.
>>
>>However in order to maintain an ordered set of registers to pass to guc,
>>each register needs to be added and moved in the final array. The second
>>phase may actually happen in IO memory rather than system memory and
>>accessing IO memory by simply dereferencing the pointer doesn't work on
>>all architectures. Other places of the ADS initializaition were
>>converted to use the iosys_map API, but here there may be a lot more
>>accesses to IO memory. So, instead of following that same approach,
>>convert the regset initialization to calculate the final array in 1
>>pass and in the second pass that array is just copied to its final
>>location, updating the pointers for each engine written to the ADS blob.
>>
>>One important thing is that struct temp_regset now have
>>different semantics: `registers` continues to track the registers of a
>>single engine, however the other fields are updated together, according
>>to the newly added `storage`, which tracks the memory allocated for
>>all the registers. So rename some of these fields and add a
>>__mmio_reg_add(): this function (possibly) allocates memory and operates
>>on the storage pointer while guc_mmio_reg_add() continues to manage the
>>registers pointer.
>>
>>On a Tiger Lake system using enable_guc=3, the following log message is
>>now seen:
>>
>>	[  187.334310] i915 0000:00:02.0: [drm:intel_guc_ads_create [i915]] Used 4 KB for temporary ADS regset
>>
>>This change has also been tested on an ARM64 host with DG2 and other
>>discrete graphics cards.
>>
>>v2 (Daniele):
>>   - Fix leaking tempset on error path
>>   - Add comments on struct temp_regset to document the meaning of each
>>     field
>>
>>Cc: Matt Roper <matthew.d.roper at intel.com>
>>Cc: John Harrison <John.C.Harrison at Intel.com>
>>Cc: Matthew Brost <matthew.brost at intel.com>
>>Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>
>Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>

Thanks for the review. Applied to drm-intel-gt-next.

Lucas De Marchi