[Intel-gfx] [PATCH v7 3/6] drm/i915/guc: Move the pin bias value from GuC to GGTT

[Michal Wajdeczko]

On Fri, 27 Jul 2018 14:18:57 +0200, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> Quoting Jakub Bartmiński (2018-07-27 13:08:53)
>> Removing the pin bias from GuC allows us to not check for GuC every time
>> we pin a context, which fixes the assertion error on unresolved GuC
>> platform default in mock contexts selftest.
>>
>> It also seems that we were using uninitialized WOPCM variables when
>> setting the GuC pin bias. The pin bias has to be set after the WOPCM,
>> but before the call to i915_gem_contexts_init where the first contexts
>> are pinned.
>>
>> v2:
>> This also makes it so that there's no need to set GuC variables from
>> within the WOPCM init function or to move the WOPCM init, while keeping
>> the correct initialization order. Also for mock tests the pin bias is
>> left at 0 and we make sure that the pin bias with GuC will not be
>> smaller than without GuC.
>>
>> v3:
>> Avoid unused i915 in intel_guc_ggtt_offset if debug is disabled.
>>
>> v4:
>> Squash with WOPCM init reordering.
>> Moved the i915_ggtt_pin_bias helper to this patch, and made some
>> functions use it instead of directly dereferencing i915->ggtt.
>>
>> v5:
>> Since we now don't use wopcm.guc.base for the pin bias there's no need  
>> to
>> validate it. It also has already been verified in WOPCM init.
>>
>> v6:
>> Moved and renamed the function which now returns the wopcm.guc.size to
>> intel_guc.c:intel_guc_wopcm_region_size to avoid any possible confusion
>> with the pin_bias in ggtt, which should be used for pinning.
>> Deleted the now unnecessarily introduced includes from previous  
>> versions.
>> Dropped naming changes from dev_priv to i915 for better patch  
>> readability.
>>
>> v7:
>> Moved the same function again to intel_wopcm.c and renamed it to
>> intel_wopcm_reserved_gtt_size to better represent its use from the
>> gem_init_ggtt perspective.
>> Changed some comments to make more sense in the context they're in.
>>
>> Fixes: f7dc0157e4b5 ("drm/i915/uc: Fetch GuC/HuC firmwares from guc/huc  
>> specific init")
>> Testcase: igt/drv_selftest/mock_contexts #GuC
>> Signed-off-by: Jakub Bartmiński <jakub.bartminski at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
>
> Michal may prefer to call it intel_guc_reserved_gtt_size (or something
> along those lines) in which case just resend this patch with the change
> (in reply to this patch).

u32 intel_guc_reserved_gtt_size(struct intel_guc *guc) would be fine.

> However, we do have intel_wopcm_init et al
> which I think weakens the argument that this belongs only to the guc.

But WOPCM itself does not care about GGTT. Only GuC needs to tweak its
GGTT mapping according to size of the GUC_WOPCM region, and only GuC
knowns other restrictions that have to applied (see GUC_GGTT_TOP)

Michal