Re: [PATCH] drm/i915/guc: Protect against HAS_GUC_* returning true values other than one

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

 



On 04/07/16 15:30, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

At the moment HAS_GUC_UCODE == HAS_GUC == IS_GEN9 ==
(INTEL_INFO(dev)->gen_mask & BIT(8)), which is true but not one. And
module parameters are integers and not booleans so compiler will not
normalize the value for us.

Quick and easy fix for the GuC loading code and the whole area can
be evaluated afterwards.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Reported-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Dave Gordon <david.s.gordon@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_guc_loader.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index d925e2daeb24..72ea5b97e242 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -687,9 +687,9 @@ void intel_guc_init(struct drm_device *dev)

  	/* A negative value means "use platform default" */
  	if (i915.enable_guc_loading < 0)
-		i915.enable_guc_loading = HAS_GUC_UCODE(dev);
+		i915.enable_guc_loading = !!HAS_GUC_UCODE(dev);
  	if (i915.enable_guc_submission < 0)
-		i915.enable_guc_submission = HAS_GUC_SCHED(dev);
+		i915.enable_guc_submission = !!HAS_GUC_SCHED(dev);

  	if (!HAS_GUC_UCODE(dev)) {
  		fw_path = NULL;

Or we could just fix the IS_GENx() macros:

.Dave.

>From 4c82153bd0a520d1d85757ccfc2241776c7634af Mon Sep 17 00:00:00 2001
From: Dave Gordon <david.s.gordon@xxxxxxxxx>
Date: Tue, 5 Jul 2016 12:11:12 +0100
Subject: [PATCH] drm/i915: IS_GENx() must return bool
Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ

Since "ae5702d2 drm/i915: Make IS_GENx macros work on a mask" which
optimised the IS_GENx() macros to perform a simple bitmask operation
rather than an arithmetic comparison, the values of these macros have
been powers-of-2 integers rather than true booleans.

This confuses some code that expects them to be specifically 0 or 1
rather than just 0 or nonzero.

So here we convert all the individual GENx() macros to use a single
underlying common macro, to which we add "!!" to convert the result to
an actual bool. The compiler knows when this actually makes a difference
and doesn't insert any instructions if it only needs a zero/nonzero
test, so this patch increases the binary size by only ~40 bytes total,
for the cases where we actually want the values 0 or 1.

Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f0b1f43..431d862 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2763,14 +2763,15 @@ struct drm_i915_cmd_table {
  * have their own (e.g. HAS_PCH_SPLIT for ILK+ display, IS_foo for particular
  * chips, etc.).
  */
-#define IS_GEN2(dev)	(INTEL_INFO(dev)->gen_mask & BIT(1))
-#define IS_GEN3(dev)	(INTEL_INFO(dev)->gen_mask & BIT(2))
-#define IS_GEN4(dev)	(INTEL_INFO(dev)->gen_mask & BIT(3))
-#define IS_GEN5(dev)	(INTEL_INFO(dev)->gen_mask & BIT(4))
-#define IS_GEN6(dev)	(INTEL_INFO(dev)->gen_mask & BIT(5))
-#define IS_GEN7(dev)	(INTEL_INFO(dev)->gen_mask & BIT(6))
-#define IS_GEN8(dev)	(INTEL_INFO(dev)->gen_mask & BIT(7))
-#define IS_GEN9(dev)	(INTEL_INFO(dev)->gen_mask & BIT(8))
+#define	_IS_GEN(x, dev)	(!!(INTEL_INFO(dev)->gen_mask & BIT((x)-1)))
+#define IS_GEN2(dev)	_IS_GEN(2, dev)
+#define IS_GEN3(dev)	_IS_GEN(3, dev)
+#define IS_GEN4(dev)	_IS_GEN(4, dev)
+#define IS_GEN5(dev)	_IS_GEN(5, dev)
+#define IS_GEN6(dev)	_IS_GEN(6, dev)
+#define IS_GEN7(dev)	_IS_GEN(7, dev)
+#define IS_GEN8(dev)	_IS_GEN(8, dev)
+#define IS_GEN9(dev)	_IS_GEN(9, dev)
 
 #define ENGINE_MASK(id)	BIT(id)
 #define RENDER_RING	ENGINE_MASK(RCS)
-- 
1.9.1

_______________________________________________
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