From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Siluvery, Arun" Subject: Re: [PATCH] drm/i915: Add means to apply WA conditionally Date: Thu, 23 Oct 2014 17:07:53 +0100 Message-ID: <544927D9.2070403@linux.intel.com> References: <1414078170-29469-1-git-send-email-arun.siluvery@linux.intel.com> <20141023155125.GR26941@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 768EE6E48D for ; Thu, 23 Oct 2014 09:09:47 -0700 (PDT) In-Reply-To: <20141023155125.GR26941@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 23/10/2014 16:51, Daniel Vetter wrote: > On Thu, Oct 23, 2014 at 04:29:30PM +0100, Arun Siluvery wrote: >> We would want to apply some of the workarounds based on a condition to a >> particular platform or Gen but we may not know all possible controlling >> parameters in advance hence allow to define open conditions; a WA makes >> it to the list only if the condition is true. >> >> With the appropriate conditions we can combine all of the workarounds >> and apply them from a single place irrespective of platform instead of >> having them in separate functions. >> >> For: VIZ-4090 >> Signed-off-by: Arun Siluvery > > Imo we should just pull the condition out into proper control flow. Hiding > it like that in the macro doesn't seem to buy us anything at all, but > obfuscates the code. No we are not hiding the condition, I thought it would be easier to read it this way, e.g., WA_SET_BIT_MASKED_IF(IS_BDW_GT3(dev), WA_REG, WA_MASK); do you prefer adding if(cond) to each WA? regards Arun > -Daniel > >> --- >> drivers/gpu/drm/i915/intel_ringbuffer.c | 35 +++++++++++++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c >> index 497b836..0525a5d 100644 >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c >> @@ -736,6 +736,41 @@ static int wa_add(struct drm_i915_private *dev_priv, >> >> #define WA_WRITE(addr, val) WA_REG(addr, val, 0xffffffff) >> >> +#define WA_SET_BIT_MASKED_IF(cond, addr, mask) \ >> + do { \ >> + if (cond) { \ >> + WA_SET_BIT_MASKED(addr, mask); \ >> + } \ >> + } while(0) >> + >> +#define WA_CLR_BIT_MASKED_IF(cond, addr, mask) \ >> + do { \ >> + if (cond) { \ >> + WA_CLR_BIT_MASKED(addr, mask); \ >> + } \ >> + } while(0) >> + >> +#define WA_SET_BIT_IF(cond, addr, mask) \ >> + do { \ >> + if (cond) { \ >> + WA_SET_BIT(addr, mask); \ >> + } \ >> + } while(0) >> + >> +#define WA_CLR_BIT_IF(cond, addr, mask) \ >> + do { \ >> + if (cond) { \ >> + WA_CLR_BIT(addr, mask); \ >> + } \ >> + } while(0) >> + >> +#define WA_WRITE_IF(cond, addr, val) \ >> + do { \ >> + if (cond) { \ >> + WA_WRITE(addr, val); \ >> + } \ >> + } while(0) >> + >> static int bdw_init_workarounds(struct intel_engine_cs *ring) >> { >> struct drm_device *dev = ring->dev; >> -- >> 2.1.2 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >