Re: [PATCH v2 5/7] drm/i915/guc: Combine enable_guc_loading|submission modparams

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 01 Dec 2017 13:36:06 +0100, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:

Quoting Michal Wajdeczko (2017-12-01 10:33:15)
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 load and verify the HuC.

Lets combine above module parameters into one "enable_guc"
modparam. New supported bit values are:

 0=disable GuC (no GuC submission, no HuC)
 1=enable GuC submission
 2=enable HuC load

Special value "-1" can be used to let driver decide what
option should be enabled for given platform based on
hardware/firmware availability or preference.

Explicit enabling any of the GuC features makes GuC load
a required step, fallback to non-GuC mode will not be
supported.

v2: Don't use -EIO

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@xxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_drv.h    |   5 +-
 drivers/gpu/drm/i915/i915_params.c |  11 ++--
 drivers/gpu/drm/i915/i915_params.h |   3 +-
drivers/gpu/drm/i915/intel_uc.c | 100 +++++++++++++++++++++----------------
 4 files changed, 65 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2c386c7..9162a60 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3238,8 +3238,9 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define HAS_HUC_UCODE(dev_priv)        (HAS_GUC(dev_priv))

 /* Having a GuC is not the same as using a GuC */
-#define USES_GUC(dev_priv) (i915_modparams.enable_guc_loading) -#define USES_GUC_SUBMISSION(dev_priv) (i915_modparams.enable_guc_submission) +#define USES_GUC(dev_priv) (!!(i915_modparams.enable_guc > 0)) +#define USES_GUC_SUBMISSION(dev_priv) (!!(i915_modparams.enable_guc & 1)) +#define USES_HUC(dev_priv) (!!(i915_modparams.enable_guc & 2))

* channelling Joonas

Please use a magic name for each bit and so

#define USE_GUC_SUBMISSION_BIT 0

I was considering that, but then I decided to follow existing code
(see USES_PPGTT* and enable_ppgtt where we use plain numbers only)

But if we want to start using magic values, then these values should
be defined in i915_params.h and rather in this way:

#define ENABLE_GUC_SUBMISSION   BIT(0)
#define ENABLE_GUC_HUC_LOAD     BIT(1)
        ^^^^^^^^^^
        this part matches modparam name

#define USES_GUC_SUBMISSION (!!(i915_modparams.enable_guc & BIT(USE_GUC_SUBMISSION_BIT)))

Gah, that's so ugly.

or

static inline bool intel_use_guc_submission(void)

"intel_" ? maybe correct prefix should be "i915_" ?

{
	return i915_modparams.enable_guc & BIT(USE_GUC_SUBMISSION_BIT);
}

I assume that above will still be wrapped inside macro

#define USES_GUC_SUBMISSION(dev_priv) intel_uses_guc_submission()


which at least stops being so shouty.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux