From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vijay Purushothaman Subject: Re: [PATCH] drm/i915: Fix correct FIFO size for Baytrail Date: Fri, 07 Feb 2014 21:56:29 +0530 Message-ID: <52F50935.9060604@intel.com> References: <1391785992-28063-1-git-send-email-vijay.a.purushothaman@intel.com> <20140207155816.GY3891@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 0C0CBFBBF8 for ; Fri, 7 Feb 2014 08:26:43 -0800 (PST) In-Reply-To: <20140207155816.GY3891@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: =?ISO-8859-1?Q?Ville_Syrj=E4l=E4?= Cc: Intel Graphics List-Id: intel-gfx@lists.freedesktop.org On 2/7/2014 9:28 PM, Ville Syrj=E4l=E4 wrote: > On Fri, Feb 07, 2014 at 08:43:12PM +0530, Vijay Purushothaman wrote: >> B-spec says the FIFO total size is 512. So fix this to 512. >> >> Signed-off-by: Vijay Purushothaman >> --- >> drivers/gpu/drm/i915/i915_reg.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915= _reg.h >> index cc3ea04..fb73031 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -3395,7 +3395,7 @@ >> #define I915_FIFO_LINE_SIZE 64 >> #define I830_FIFO_LINE_SIZE 32 >> >> -#define VALLEYVIEW_FIFO_SIZE 255 >> +#define VALLEYVIEW_FIFO_SIZE 511 >> #define G4X_FIFO_SIZE 127 >> #define I965_FIFO_SIZE 512 >> #define I945_FIFO_SIZE 127 > > Reviewed-by: Ville Syrj=E4l=E4 Thanks for the review. > > Not that we actually use the value anywhere at the moment. This value is used when the display controller is configured in Max FIFO = mode. This is working fine in the android tree. At this moment i am = rewriting some logic related to this Max FIFO, memory arbiter credits = and drain latency handling for the sprite planes. I should have the = patches ready over the week end, will test it on monday once i get to = office. > > As a side note the FIFO sizing for gmch platforms seems to be a place > where the documentation is rather poor. It kind of looks like there > are off by one errors in the text, and yet when I was playing around > with this stuff on gen2/gen4 machines it kind of looks like the > hardware has the same off by one issues too. IIRC my conlusion was > that the last cacheline in the FIFO can't actually be used. So > specifying 511 matches with my conclusion. I agree.. I was thrown off by this oddity as well and it took some time = for me to understand this completely. The display block in Baytrail is a = mix and match of features from gen4 & gen5 (Cantiga, Crestline and = Ironlake). No wonder this chip has the same h/w issues.. > > One other thing I did notice now that I look at our g4x/vlv watermark > code. We seem to assume the watermarks for g4x/vlv work the same way > as pch platforms. Ie. you specify the minimum level of data left in the > FIFO before it needs to start fetching more. But the documentation > suggests that it's the other way around, where you specify the max > amount of free space allowed in the FIFO before more data needs to be > fetched. We use the latter logic for gen2-gen4. I wonder if the spec > is wrong, or if your code is wrong. I guess I just need to verify it > on real hardware at some point... > I think this is implemented correctly in the android kernel.. On a high = level the split is something like 32 KB FIFO per pipe - 16 KB for = primary plane and 8 KB for each sprite. When we are in single display = mode we can configure the entire 64KB FIFO for the same pipe. There is = another trick to enable trickle feed.. With all these tricks i am seeing = good memory self refresh numbers - almost on par with the theoretical = target. I should be able to post the patch series on monday once i do = some sanity testing.. Thanks, Vijay