[PATCH v7] drm/i915/selftest/gsc: Ensure GSC Proxy init completes before selftests
Ceraolo Spurio, Daniele
daniele.ceraolospurio at intel.com
Wed Jul 26 21:50:34 UTC 2023
On 7/20/2023 4:01 PM, Alan Previn wrote:
> On MTL, if the GSC Proxy init flows haven't completed, submissions to the
> GSC engine will fail. Those init flows are dependent on the mei's
> gsc_proxy component that is loaded in parallel with i915 and a
> worker that could potentially start after i915 driver init is done.
>
> That said, all subsytems that access the GSC engine today does check
> for such init flow completion before using the GSC engine. However,
> selftests currently don't wait on anything before starting.
>
> To fix this, add a waiter function at the start of __run_selftests
> that waits for gsc-proxy init flows to complete. Selftests shouldn't
> care if the proxy-init failed as that should be flagged elsewhere.
>
> Difference from prior versions:
> v7: - Change the fw status to INTEL_UC_FIRMWARE_LOAD_FAIL if the
> proxy-init fails so that intel_gsc_uc_fw_proxy_get_status
> catches it. (Daniele)
> v6: - Add a helper that returns something more than a boolean
> so we selftest can stop waiting if proxy-init hadn't
> completed but failed (Daniele).
> v5: - Move the call to __wait_gsc_proxy_completed from common
> __run_selftests dispatcher to the group-level selftest
> function (Trvtko).
> - change the pr_info to pr_warn if we hit the timeout.
> v4: - Remove generalized waiters function table framework (Tvrtko).
> - Remove mention of CI-framework-timeout from comments (Tvrtko).
> v3: - Rebase to latest drm-tip.
> v2: - Based on internal testing, increase the timeout for gsc-proxy
> specific case to 8 seconds.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis at intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
Daniele
> ---
> drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c | 14 +++++++++
> drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h | 1 +
> drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c | 13 +++++++-
> .../gpu/drm/i915/selftests/i915_selftest.c | 31 +++++++++++++++++++
> 4 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> index ab1a456f833d..163021705210 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
> @@ -45,6 +45,20 @@ bool intel_gsc_uc_fw_proxy_init_done(struct intel_gsc_uc *gsc, bool needs_wakere
> HECI1_FWSTS1_PROXY_STATE_NORMAL;
> }
>
> +int intel_gsc_uc_fw_proxy_get_status(struct intel_gsc_uc *gsc)
> +{
> + if (!(IS_ENABLED(CONFIG_INTEL_MEI_GSC_PROXY)))
> + return -ENODEV;
> + if (!intel_uc_fw_is_loadable(&gsc->fw))
> + return -ENODEV;
> + if (__intel_uc_fw_status(&gsc->fw) == INTEL_UC_FIRMWARE_LOAD_FAIL)
> + return -ENOLINK;
> + if (!intel_gsc_uc_fw_proxy_init_done(gsc, true))
> + return -EAGAIN;
> +
> + return 0;
> +}
> +
> bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc)
> {
> return gsc_uc_get_fw_status(gsc_uc_to_gt(gsc)->uncore, false) &
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> index ad2167ce9137..bc9dd0de8aaf 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
> @@ -16,5 +16,6 @@ int intel_gsc_fw_get_binary_info(struct intel_uc_fw *gsc_fw, const void *data, s
> int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc);
> bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc);
> bool intel_gsc_uc_fw_proxy_init_done(struct intel_gsc_uc *gsc, bool needs_wakeref);
> +int intel_gsc_uc_fw_proxy_get_status(struct intel_gsc_uc *gsc);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
> index 034b53a71541..0d3b22a74365 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
> @@ -62,8 +62,18 @@ static void gsc_work(struct work_struct *work)
> }
>
> ret = intel_gsc_proxy_request_handler(gsc);
> - if (ret)
> + if (ret) {
> + if (actions & GSC_ACTION_FW_LOAD) {
> + /*
> + * A proxy failure right after firmware load means the proxy-init
> + * step has failed so mark GSC as not usable after this
> + */
> + drm_err(>->i915->drm,
> + "GSC proxy handler failed to init\n");
> + intel_uc_fw_change_status(&gsc->fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
> + }
> goto out_put;
> + }
>
> /* mark the GSC FW init as done the first time we run this */
> if (actions & GSC_ACTION_FW_LOAD) {
> @@ -78,6 +88,7 @@ static void gsc_work(struct work_struct *work)
> } else {
> drm_err(>->i915->drm,
> "GSC status reports proxy init not complete\n");
> + intel_uc_fw_change_status(&gsc->fw, INTEL_UC_FIRMWARE_LOAD_FAIL);
> }
> }
> }
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> index 39da0fb0d6d2..ee79e0809a6d 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -24,6 +24,8 @@
> #include <linux/random.h>
>
> #include "gt/intel_gt_pm.h"
> +#include "gt/uc/intel_gsc_fw.h"
> +
> #include "i915_driver.h"
> #include "i915_drv.h"
> #include "i915_selftest.h"
> @@ -127,6 +129,31 @@ static void set_default_test_all(struct selftest *st, unsigned int count)
> st[i].enabled = true;
> }
>
> +static bool
> +__gsc_proxy_init_progressing(struct intel_gsc_uc *gsc)
> +{
> + return intel_gsc_uc_fw_proxy_get_status(gsc) == -EAGAIN;
> +}
> +
> +static void
> +__wait_gsc_proxy_completed(struct drm_i915_private *i915)
> +{
> + bool need_to_wait = (IS_ENABLED(CONFIG_INTEL_MEI_GSC_PROXY) &&
> + i915->media_gt &&
> + HAS_ENGINE(i915->media_gt, GSC0) &&
> + intel_uc_fw_is_loadable(&i915->media_gt->uc.gsc.fw));
> + /*
> + * The gsc proxy component depends on the kernel component driver load ordering
> + * and in corner cases (the first time after an IFWI flash), init-completion
> + * firmware flows take longer.
> + */
> + unsigned long timeout_ms = 8000;
> +
> + if (need_to_wait && wait_for(!__gsc_proxy_init_progressing(&i915->media_gt->uc.gsc),
> + timeout_ms))
> + pr_warn(DRIVER_NAME "Timed out waiting for gsc_proxy_completion!\n");
> +}
> +
> static int __run_selftests(const char *name,
> struct selftest *st,
> unsigned int count,
> @@ -206,6 +233,8 @@ int i915_live_selftests(struct pci_dev *pdev)
> if (!i915_selftest.live)
> return 0;
>
> + __wait_gsc_proxy_completed(pdev_to_i915(pdev));
> +
> err = run_selftests(live, pdev_to_i915(pdev));
> if (err) {
> i915_selftest.live = err;
> @@ -227,6 +256,8 @@ int i915_perf_selftests(struct pci_dev *pdev)
> if (!i915_selftest.perf)
> return 0;
>
> + __wait_gsc_proxy_completed(pdev_to_i915(pdev));
> +
> err = run_selftests(perf, pdev_to_i915(pdev));
> if (err) {
> i915_selftest.perf = err;
>
> base-commit: cc69df372f21eb3073c062132ee9eb3649605931
More information about the dri-devel
mailing list