From patchwork Tue Jul 5 11:50:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 9214139 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 646FF60752 for ; Tue, 5 Jul 2016 11:50:10 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4A80628A04 for ; Tue, 5 Jul 2016 11:50:10 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3B10728A22; Tue, 5 Jul 2016 11:50:10 +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]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C0AA528A04 for ; Tue, 5 Jul 2016 11:50:09 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1EB346E0AF; Tue, 5 Jul 2016 11:50:09 +0000 (UTC) X-Original-To: Intel-gfx@lists.freedesktop.org Delivered-To: Intel-gfx@lists.freedesktop.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id E1E2E6E0AF for ; Tue, 5 Jul 2016 11:50:07 +0000 (UTC) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 05 Jul 2016 04:50:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,579,1459839600"; d="scan'208,223";a="1001201443" Received: from dsgordon-linux2.isw.intel.com (HELO [10.102.226.88]) ([10.102.226.88]) by fmsmga001.fm.intel.com with ESMTP; 05 Jul 2016 04:50:06 -0700 To: Tvrtko Ursulin , Intel-gfx@lists.freedesktop.org References: <1467642633-35255-1-git-send-email-tvrtko.ursulin@linux.intel.com> From: Dave Gordon Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ Message-ID: <577B9EEE.8060103@intel.com> Date: Tue, 5 Jul 2016 12:50:06 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <1467642633-35255-1-git-send-email-tvrtko.ursulin@linux.intel.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915/guc: Protect against HAS_GUC_* returning true values other than one 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP On 04/07/16 15:30, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > 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 > Reported-by: Chris Wilson > Cc: Dave Gordon > --- > 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 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 --- 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