From patchwork Sat Nov 11 00:06:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Sundaresan, Sujaritha" X-Patchwork-Id: 10053963 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DF9AC6032D for ; Sat, 11 Nov 2017 00:10:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D28152B514 for ; Sat, 11 Nov 2017 00:10:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C7A1D2B523; Sat, 11 Nov 2017 00:10:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 32ADD2B514 for ; Sat, 11 Nov 2017 00:10:51 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7F50C6EBC9; Sat, 11 Nov 2017 00:10:49 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4D27F6EBB6 for ; Sat, 11 Nov 2017 00:10:45 +0000 (UTC) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Nov 2017 16:10:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,377,1505804400"; d="scan'208";a="1057185" Received: from sujaritha-z170x-ud5.fm.intel.com ([10.1.27.118]) by orsmga003.jf.intel.com with ESMTP; 10 Nov 2017 16:10:44 -0800 From: Sujaritha Sundaresan To: intel-gfx@lists.freedesktop.org Date: Fri, 10 Nov 2017 16:06:32 -0800 Message-Id: <1510358798-21566-3-git-send-email-sujaritha.sundaresan@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1510358798-21566-1-git-send-email-sujaritha.sundaresan@intel.com> References: <1510358798-21566-1-git-send-email-sujaritha.sundaresan@intel.com> Cc: Sujaritha Sundaresan Subject: [Intel-gfx] [PATCH v9 2/8] drm/i915/guc : Removing i915_modparams.enable_guc_loading module parameter X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP We currently have two module parameters that control GuC: "enable_guc_loading" and "enable_guc_submission". Whenever we need submission=1, we also need loading=1.We also need loading=1 when we want to want to verify the HuC, which is every time we have a HuC (but all platforms with HuC have a GuC and viceversa). Also if we have HuC have firmware to be loaded, we need to have GuC to actually load it. So if the user wants to avoid the GuC from getting loaded, they must not have a HuC firmware to be loaded, in addition to not using submission. v2: Clarifying the commit message (Anusha) v3: Unify seq_puts messages, Re-factoring code as per review (Michal) v4: Rebase v5: Separating message unification into a separate patch v6: Re-factoring code (Sagar, Michal) Rebase v7: Applying review comments (Sagar) Rebase v8: Change to NEEDS_GUC_FW (Chris) Applying review comments (Michal) Clarifying commit message (Joonas) v9: Applying review comments (Michal) Suggested by; Oscar Mateo Signed-off-by: Sujaritha Sundaresan Cc: Chris Wilson Cc: Daniele Ceraolo Spurio Cc: Joonas Lahtinen Cc: Michal Wajdeczko Cc: Oscar Mateo Cc: Sagar Arun Kamble --- drivers/gpu/drm/i915/i915_drv.h | 9 +++-- drivers/gpu/drm/i915/i915_gem_context.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 4 --- drivers/gpu/drm/i915/i915_params.h | 1 - drivers/gpu/drm/i915/intel_uc.c | 59 ++++++++++++++++------------------ 7 files changed, 35 insertions(+), 44 deletions(-) struct intel_guc *guc = &dev_priv->guc; int ret, attempts; - if (!i915_modparams.enable_guc_loading) + if (!NEEDS_GUC_FW(dev_priv)) return 0; guc_disable_communication(guc); @@ -250,22 +249,16 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) err_guc: i915_ggtt_disable_guc(dev_priv); - if (i915_modparams.enable_guc_loading > 1 || - i915_modparams.enable_guc_submission > 1) { + if (i915_modparams.enable_guc_submission > 1) { DRM_ERROR("GuC init failed. Firmware loading disabled.\n"); ret = -EIO; + } else if (i915_modparams.enable_guc_submission == 1) { + DRM_NOTE("Falling back from GuC submission to execlist mode\n"); + ret = 0; } else { - DRM_NOTE("GuC init failed. Firmware loading disabled.\n"); ret = 0; } - if (i915_modparams.enable_guc_submission) { - i915_modparams.enable_guc_submission = 0; - DRM_NOTE("Falling back from GuC submission to execlist mode\n"); - } - - i915_modparams.enable_guc_loading = 0; - return ret; } @@ -273,7 +266,7 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv) { guc_free_load_err_log(&dev_priv->guc); - if (!i915_modparams.enable_guc_loading) + if (!NEEDS_GUC_FW(dev_priv)) return; if (i915_modparams.enable_guc_submission) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c94f34f..798fa8a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3219,10 +3219,13 @@ static inline unsigned int i915_sg_segment_size(void) * properties, so we have separate macros to test them. */ #define HAS_GUC(dev_priv) ((dev_priv)->info.has_guc) +#define HAS_HUC(dev_priv) (HAS_GUC(dev_priv)) #define HAS_GUC_CT(dev_priv) ((dev_priv)->info.has_guc_ct) -#define HAS_GUC_UCODE(dev_priv) (HAS_GUC(dev_priv)) -#define HAS_GUC_SCHED(dev_priv) (HAS_GUC(dev_priv)) -#define HAS_HUC_UCODE(dev_priv) (HAS_GUC(dev_priv)) +#define HAS_GUC_UCODE(dev_priv) ((dev_priv)->guc.fw.path != NULL) +#define HAS_HUC_UCODE(dev_priv) ((dev_priv)->huc.fw.path != NULL) + +#define NEEDS_GUC_FW(dev_priv) \ + (HAS_GUC(dev_priv) && i915_modparams.enable_guc_submission) #define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index c05c3d7..6a819c0 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -316,7 +316,7 @@ static u32 default_desc_template(const struct drm_i915_private *i915, * present or not in use we still need a small bias as ring wraparound * at offset 0 sometimes hangs. No idea why. */ - if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading) + if (NEEDS_GUC_FW(dev_priv)) ctx->ggtt_offset_bias = GUC_WOPCM_TOP; else ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 1e40eeb..b634edf 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3476,7 +3476,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv) * currently don't have any bits spare to pass in this upper * restriction! */ - if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading) { + if (NEEDS_GUC_FW(dev_priv)) { ggtt->base.total = min_t(u64, ggtt->base.total, GUC_GGTT_TOP); ggtt->mappable_end = min(ggtt->mappable_end, ggtt->base.total); } diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ff00e46..a414bca 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4032,7 +4032,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv) for (i = 0; i < MAX_L3_SLICES; ++i) dev_priv->l3_parity.remap_info[i] = NULL; - if (HAS_GUC_SCHED(dev_priv)) + if (HAS_GUC(dev_priv)) dev_priv->pm_guc_events = GEN9_GUC_TO_HOST_INT_EVENT; /* Let's track the enabled rps events */ diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index b4faeb6..1c25f45 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -162,10 +162,6 @@ struct i915_params i915_modparams __read_mostly = { "(0=use value from vbt [default], 1=low power swing(200mV)," "2=default swing(400mV))"); -i915_param_named_unsafe(enable_guc_loading, int, 0400, - "Enable GuC firmware loading " - "(-1=auto, 0=never [default], 1=if available, 2=required)"); - i915_param_named_unsafe(enable_guc_submission, int, 0400, "Enable GuC submission " "(-1=auto, 0=never [default], 1=if available, 2=required)"); diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index c729226..9e1e231 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -44,7 +44,6 @@ param(int, disable_power_well, -1) \ param(int, enable_ips, 1) \ param(int, invert_brightness, 0) \ - param(int, enable_guc_loading, 0) \ param(int, enable_guc_submission, 0) \ param(int, guc_log_level, -1) \ param(char *, guc_firmware_path, NULL) \ diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index e85b268..648e59c 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -49,36 +49,35 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv) void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) { + /* Verify Hardware version */ if (!HAS_GUC(dev_priv)) { - if (i915_modparams.enable_guc_loading > 0 || - i915_modparams.enable_guc_submission > 0) + if (i915_modparams.enable_guc_submission > 0) + DRM_INFO("Ignoring option %s - no hardware", "enable_guc_submission"); - - i915_modparams.enable_guc_loading = 0; i915_modparams.enable_guc_submission = 0; return; } - /* A negative value means "use platform default" */ - if (i915_modparams.enable_guc_loading < 0) - i915_modparams.enable_guc_loading = HAS_GUC_UCODE(dev_priv); - - /* Verify firmware version */ - if (i915_modparams.enable_guc_loading) { - if (HAS_HUC_UCODE(dev_priv)) - intel_huc_select_fw(&dev_priv->huc); - - if (intel_guc_fw_select(&dev_priv->guc)) - i915_modparams.enable_guc_loading = 0; + /* Verify Firmware version */ + if (!HAS_HUC_UCODE(dev_priv)) { + if (i915_modparams.enable_guc_submission > 0) { + DRM_INFO("Ignoring option %s - no firmware", "enable_guc_submission"); + i915_modparams.enable_guc_submission = 0; + return; + } + + if (i915_modparams.enable_guc_submission < 0) { + i915_modparams.enable_guc_submission = 0; + return; + } } - /* Can't enable guc submission without guc loaded */ - if (!i915_modparams.enable_guc_loading) - i915_modparams.enable_guc_submission = 0; + /* + * A negative value means "use platform default" (enabled if we have + * survived to get here) + */ - /* A negative value means "use platform default" */ - if (i915_modparams.enable_guc_submission < 0) - i915_modparams.enable_guc_submission = HAS_GUC_SCHED(dev_priv); + if (i915_modparams.enable_guc_submission == 1) + i915_modparams.enable_guc_submission = HAS_GUC(dev_priv); } void intel_uc_init_early(struct drm_i915_private *dev_priv) @@ -154,7 +153,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)