public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Vijay Purushothaman <vijay.a.purushothaman@intel.com>
Cc: Intel Graphics <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Fix correct FIFO size for Baytrail
Date: Fri, 7 Feb 2014 17:58:16 +0200	[thread overview]
Message-ID: <20140207155816.GY3891@intel.com> (raw)
In-Reply-To: <1391785992-28063-1-git-send-email-vijay.a.purushothaman@intel.com>

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 <vijay.a.purushothaman@intel.com>
> ---
>  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älä <ville.syrjala@linux.intel.com>

Not that we actually use the value anywhere at the moment.

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.

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...

-- 
Ville Syrjälä
Intel OTC

  reply	other threads:[~2014-02-07 15:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-07 15:13 [PATCH] drm/i915: Fix correct FIFO size for Baytrail Vijay Purushothaman
2014-02-07 15:58 ` Ville Syrjälä [this message]
2014-02-07 16:26   ` Vijay Purushothaman
2014-02-07 19:38     ` Ville Syrjälä
2014-02-07 18:58   ` Daniel Vetter
2014-02-12 17:24     ` Imre Deak
2014-02-12 17:27       ` Imre Deak
2014-02-12 17:53         ` Daniel Vetter
2014-02-13 15:46         ` Purushothaman, Vijay A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140207155816.GY3891@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=vijay.a.purushothaman@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox