Re: [PATCH] drm/i915/dg2: wait for HuC load completion before running selftests

[John Harrison <john.c.harrison@intel.com>]

On 4/10/2024 13:15, Daniele Ceraolo Spurio wrote:
> On DG2, submissions to VCS engines tied to a gem context are blocked
> until the HuC is loaded. Since some selftests do use a gem context,
> wait for the HuC load to complete before running the tests to avoid
> contamination.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10564
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>

> ---
>   .../gpu/drm/i915/selftests/i915_selftest.c    | 36 ++++++++++++++++---
>   1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> index ee79e0809a6d..fee76c1d2f45 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -154,6 +154,30 @@ __wait_gsc_proxy_completed(struct drm_i915_private *i915)
>   		pr_warn(DRIVER_NAME "Timed out waiting for gsc_proxy_completion!\n");
>   }
>   
> +static void
> +__wait_gsc_huc_load_completed(struct drm_i915_private *i915)
> +{
> +	/* this only applies to DG2, so we only care about GT0 */
> +	struct intel_huc *huc = &to_gt(i915)->uc.huc;
> +	bool need_to_wait = (IS_ENABLED(CONFIG_INTEL_MEI_PXP) &&
> +			     intel_huc_wait_required(huc));
> +	/*
> +	 * The GSC and PXP mei bringup depends on the kernel boot ordering, so
> +	 * to account for the worst case scenario the HuC code waits for up to
> +	 * 10s for the GSC driver to load and then another 5s for the PXP
> +	 * component to bind before giving up, even though those steps normally
> +	 * complete in less than a second from the i915 load. We match that
> +	 * timeout here, but we expect to bail early due to the fence being
> +	 * signalled even in a failure case, as it is extremely unlikely that
> +	 * both components will use their full timeout.
> +	 */
> +	unsigned long timeout_ms = 15000;
> +
> +	if (need_to_wait &&
> +	    wait_for(i915_sw_fence_done(&huc->delayed_load.fence), timeout_ms))
> +		pr_warn(DRIVER_NAME "Timed out waiting for huc load via GSC!\n");
> +}
> +
>   static int __run_selftests(const char *name,
>   			   struct selftest *st,
>   			   unsigned int count,
> @@ -228,14 +252,16 @@ int i915_mock_selftests(void)
>   
>   int i915_live_selftests(struct pci_dev *pdev)
>   {
> +	struct drm_i915_private *i915 = pdev_to_i915(pdev);
>   	int err;
>   
>   	if (!i915_selftest.live)
>   		return 0;
>   
> -	__wait_gsc_proxy_completed(pdev_to_i915(pdev));
> +	__wait_gsc_proxy_completed(i915);
> +	__wait_gsc_huc_load_completed(i915);
>   
> -	err = run_selftests(live, pdev_to_i915(pdev));
> +	err = run_selftests(live, i915);
>   	if (err) {
>   		i915_selftest.live = err;
>   		return err;
> @@ -251,14 +277,16 @@ int i915_live_selftests(struct pci_dev *pdev)
>   
>   int i915_perf_selftests(struct pci_dev *pdev)
>   {
> +	struct drm_i915_private *i915 = pdev_to_i915(pdev);
>   	int err;
>   
>   	if (!i915_selftest.perf)
>   		return 0;
>   
> -	__wait_gsc_proxy_completed(pdev_to_i915(pdev));
> +	__wait_gsc_proxy_completed(i915);
> +	__wait_gsc_huc_load_completed(i915);
>   
> -	err = run_selftests(perf, pdev_to_i915(pdev));
> +	err = run_selftests(perf, i915);
>   	if (err) {
>   		i915_selftest.perf = err;
>   		return err;