[Intel-gfx] [PATCH v9 6/8] drm/i915/guc : Introducing enable_guc module parameter
Sujaritha
sujaritha.sundaresan at intel.com
Wed Nov 22 21:54:25 UTC 2017
On 11/12/2017 08:52 AM, Michal Wajdeczko wrote:
> On Sat, 11 Nov 2017 01:06:36 +0100, Sujaritha Sundaresan
> <sujaritha.sundaresan at intel.com> wrote:
>
>> Replacing enable_guc_submission with enable_guc modparam.
>> In effect enable_guc is replacing enable_guc_loading and
>> enable_guc_submission.
>
> Maybe it will be better if we replace guc_loading and guc_submission
> modparams with single guc_enable in one patch, as now you are touching
> almost the same places again...
>
I will merge this with removing enable_guc_loading :)
>>
>> Suggested by : Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Jani Nikula <jani.nikula at linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Oscar Mateo <oscar.mateo at intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 2 +-
>> drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
>> drivers/gpu/drm/i915/i915_irq.c | 2 +-
>> drivers/gpu/drm/i915/i915_params.c | 7 ++++---
>> drivers/gpu/drm/i915/i915_params.h | 2 +-
>> drivers/gpu/drm/i915/intel_guc.c | 2 +-
>> drivers/gpu/drm/i915/intel_uc.c | 35
>> +++++++++++++++++----------------
>> 7 files changed, 27 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 798fa8a..ad73cee 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3225,7 +3225,7 @@ static inline unsigned int
>> i915_sg_segment_size(void)
>> #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)
>> + (HAS_GUC(dev_priv) && i915_modparams.enable_guc)
>> #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 6a819c0..43210df 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -409,7 +409,7 @@ struct i915_gem_context *
>> i915_gem_context_set_closed(ctx); /* not user accessible */
>> i915_gem_context_clear_bannable(ctx);
>> i915_gem_context_set_force_single_submission(ctx);
>> - if (!i915_modparams.enable_guc_submission)
>> + if (!i915_modparams.enable_guc)
>> ctx->ring_size = 512 * PAGE_SIZE; /* Max ring buffer size */
>> GEM_BUG_ON(i915_gem_context_is_kernel(ctx));
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c
>> b/drivers/gpu/drm/i915/i915_irq.c
>> index a414bca..693b345 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1400,7 +1400,7 @@ static void snb_gt_irq_handler(struct
>> drm_i915_private *dev_priv,
>> if (iir & (GT_RENDER_USER_INTERRUPT << test_shift)) {
>> notify_ring(engine);
>> - tasklet |= i915_modparams.enable_guc_submission;
>> + tasklet |= i915_modparams.enable_guc;
>> }
>> if (tasklet)
>> diff --git a/drivers/gpu/drm/i915/i915_params.c
>> b/drivers/gpu/drm/i915/i915_params.c
>> index 1c25f45..51cf6bd 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -162,9 +162,10 @@ 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_submission, int, 0400,
>> - "Enable GuC submission "
>> - "(-1=auto, 0=never [default], 1=if available, 2=required)");
>> +i915_param_named_unsafe(enable_guc, int, 0400,
>> + "Enable GuC submission and loading "
>> + "(-1=auto [default], 0=No GuC or HuC, 1=Load & use GuC, HuC on
>> the side"
>> + " 2=Load GuC only for HuC)");
>
> What about direct description:
>
> "Enable GuC"
> -1=auto [default]
> 0=disable GuC loading
> 1=enable GuC submission
> 2=enable HuC
> 3=enable GuC submission and HuC
>
>> i915_param_named(guc_log_level, int, 0400,
>> "GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
>> diff --git a/drivers/gpu/drm/i915/i915_params.h
>> b/drivers/gpu/drm/i915/i915_params.h
>> index 9e1e231..7bf4dce 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -44,7 +44,7 @@
>> param(int, disable_power_well, -1) \
>> param(int, enable_ips, 1) \
>> param(int, invert_brightness, 0) \
>> - param(int, enable_guc_submission, 0) \
>> + param(int, enable_guc, -1) \
>> param(int, guc_log_level, -1) \
>> param(char *, guc_firmware_path, NULL) \
>> param(char *, huc_firmware_path, NULL) \
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index 823d0c2..629ef5d 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -128,7 +128,7 @@ void intel_guc_init_params(struct intel_guc *guc)
>> }
>> /* If GuC submission is enabled, set up additional parameters
>> here */
>> - if (i915_modparams.enable_guc_submission) {
>> + if (i915_modparams.enable_guc) {
>> u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
>> u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
>> u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 320165a..980de7a 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -51,33 +51,34 @@ void intel_uc_sanitize_options(struct
>> drm_i915_private *dev_priv)
>> {
>> /* Verify Hardware version */
>> if (!HAS_GUC(dev_priv)) {
>> - if (i915_modparams.enable_guc_submission > 0)
>> + if (i915_modparams.enable_guc > 0)
>> - DRM_INFO("Ignoring option %s - no hardware",
>> "enable_guc_submission");
>> + DRM_INFO("Ignoring option %s - no hardware", "enable_guc");
>> - i915_modparams.enable_guc_submission = 0;
>> + i915_modparams.enable_guc = 0;
>> return;
>> }
>> /* Verify Firmware version */
>> if (!HAS_HUC_UCODE(dev_priv)) {
>> - if (i915_modparams.enable_guc_submission > 0) {
>> + if (i915_modparams.enable_guc > 0) {
>> - DRM_INFO("Ignoring option %s - no firmware",
>> "enable_guc_submission");
>> + DRM_INFO("Ignoring option %s - no firmware", "enable_guc");
>> - i915_modparams.enable_guc_submission = 0;
>> + i915_modparams.enable_guc = 0;
>> return;
>> }
>>
>> - if (i915_modparams.enable_guc_submission < 0) {
>> - i915_modparams.enable_guc_submission = 0;
>> - return;
>> + if (i915_modparams.enable_guc < 0) {
>> + i915_modparams.enable_guc = 0;
>> + return;
>> }
>> }
>> +
>> /*
>> * A negative value means "use platform default" (enabled if we
>> have
>> * survived to get here)
>> */
>> - if (i915_modparams.enable_guc_submission == 1)
>> - i915_modparams.enable_guc_submission = HAS_GUC(dev_priv);
>> + if (i915_modparams.enable_guc < 0)
>> + i915_modparams.enable_guc = 1;
>> }
>> void intel_uc_init_early(struct drm_i915_private *dev_priv)
>> @@ -169,7 +170,7 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> /* We need to notify the guc whenever we change the GGTT */
>> i915_ggtt_enable_guc(dev_priv);
>> - if (i915_modparams.enable_guc_submission) {
>> + if (i915_modparams.enable_guc) {
>> /*
>> * This is stuff we need to have available at fw load time
>> * if we are planning to enable submission later
>> @@ -219,7 +220,7 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> goto err_log_capture;
>> intel_huc_auth(&dev_priv->huc);
>> - if (i915_modparams.enable_guc_submission) {
>> + if (i915_modparams.enable_guc) {
>> if (i915_modparams.guc_log_level >= 0)
>> gen9_enable_guc_interrupts(dev_priv);
>> @@ -229,7 +230,7 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> }
>> dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version
>> %u.%u])\n",
>> - i915_modparams.enable_guc_submission ? "submission enabled" :
>> + i915_modparams.enable_guc ? "submission enabled" :
>> "loaded",
>> guc->fw.path,
>> guc->fw.major_ver_found, guc->fw.minor_ver_found);
>> @@ -251,15 +252,15 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> err_log_capture:
>> guc_capture_load_err_log(guc);
>> err_submission:
>> - if (i915_modparams.enable_guc_submission)
>> + if (i915_modparams.enable_guc)
>> intel_guc_submission_fini(guc);
>> err_guc:
>> i915_ggtt_disable_guc(dev_priv);
>> - if (i915_modparams.enable_guc_submission > 1) {
>> + if (i915_modparams.enable_guc > 1) {
>> DRM_ERROR("GuC init failed. Firmware loading disabled.\n");
>> ret = -EIO;
>> - } else if (i915_modparams.enable_guc_submission == 1) {
>> + } else if (i915_modparams.enable_guc == 1) {
>> DRM_NOTE("Falling back from GuC submission to execlist
>> mode\n");
>> ret = 0;
>> } else {
>> @@ -276,12 +277,12 @@ void intel_uc_fini_hw(struct drm_i915_private
>> *dev_priv)
>> if (!NEEDS_GUC_FW(dev_priv))
>> return;
>> - if (i915_modparams.enable_guc_submission)
>> + if (i915_modparams.enable_guc)
>> intel_guc_submission_disable(&dev_priv->guc);
>> guc_disable_communication(&dev_priv->guc);
>> - if (i915_modparams.enable_guc_submission) {
>> + if (i915_modparams.enable_guc) {
>> gen9_disable_guc_interrupts(dev_priv);
>> intel_guc_submission_fini(&dev_priv->guc);
>> }
Thanks for the review.
Regards,
Sujaritha
More information about the Intel-gfx
mailing list