From mboxrd@z Thu Jan 1 00:00:00 1970 From: Damien Lespiau Subject: Re: [PATCH 4/4] drm/i915: Rework workaround data exporting to debugfs Date: Mon, 1 Sep 2014 16:04:37 +0100 Message-ID: <20140901150437.GF1118@strange.amr.corp.intel.com> References: <1409578133-2800-1-git-send-email-arun.siluvery@linux.intel.com> <1409578133-2800-5-git-send-email-arun.siluvery@linux.intel.com> <20140901140657.GD1118@strange.amr.corp.intel.com> <54048689.5090203@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 8E4446E34B for ; Mon, 1 Sep 2014 08:04:39 -0700 (PDT) Content-Disposition: inline In-Reply-To: <54048689.5090203@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Siluvery, Arun" Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 01, 2014 at 03:45:29PM +0100, Siluvery, Arun wrote: > >>+ /* > >>+ * Most of workarounds are masked registers; > >>+ * to set a bit in lower 16-bits we set a mask bit in > >>+ * upper 16-bits so we can take either of them as mask but > >>+ * it doesn't work if the w/a is about clearing a bit so > >>+ * use upper 16-bits to cover both cases. > >>+ */ > >>+ mask = ro_data.init_context[i+1] >> 16; > > > >"Most" doesn't seem good here. Either it's "all" and we're happy, or we > >need a generic way to describe the W/A (masked register or not). value + > >mask is generic enough to code for both cases. > > > It seems some of them could be unmasked registers. > We can use 'mask' itself to determine whether it is a > masked/unmasked register. mask == 0 if it is an unmasked register. I don't think we can use mask == 0 when it's an unmasked register. In this case, we still need to be able to have a mask that tells us which are the interesting bits in the value. If we want something generic that can be applied to masked and unmasked register, we'll something like: struct reg_wa { unsigned int masked_register : 1; u32 addr; u32 mask; u32 value; }; So: 1. masked register case: - masked_register = 1 - addr - mask (selects the lower 16 bits that are coding for the W/A value) - value (it only makes sense to some of the lower 16 bits set here) 2. 'normal' register case - masked_register = 0 - addr - mask - value >>From that generic description you can cover all cases, eg. 1. the write for masked registers becomes: mask << 16 | value 2. the write for normal registers becomes: (read(addr) & ~mask) | value 2. check a W/A has been applied (both normal and masked register): read(addr) & mask == value We seem to have only masked registers atm, so it's fine to handle just that case, but if you envision that we'll need more than masked registers, we need a better W/A definition. -- Damien