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

[Chris Wilson <chris@chris-wilson.co.uk>]

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@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Chris Wilson <chris@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). However, we do have intel_wopcm_init et al
which I think weakens the argument that this belongs only to the guc.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx