* [PATCH 2/3] drm/i915: add constants to size fence arrays and fields
2011-10-09 19:52 [PATCH 1/3] drm/i915: Ivybridge still has fences! Daniel Vetter
@ 2011-10-09 19:52 ` Daniel Vetter
2011-10-09 19:54 ` Daniel Vetter
2011-10-09 19:52 ` [PATCH 3/3] drm/i915: enable support for 32 fences on ivybridge Daniel Vetter
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2011-10-09 19:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
In preparation of to support 32 fences on Ivybdrigde.
---
drivers/gpu/drm/i915/i915_drv.h | 15 ++++++++-------
drivers/gpu/drm/i915/i915_gem.c | 4 ++--
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34f37d1..0068f56 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -126,6 +126,9 @@ struct drm_i915_master_private {
struct _drm_i915_sarea *sarea_priv;
};
#define I915_FENCE_REG_NONE -1
+#define I915_MAX_NUM_FENCES 16
+/* 16 fences + sign bit for FENCE_REG_NONE */
+#define I915_MAX_NUM_FENCE_BITS 5
struct drm_i915_fence_reg {
struct list_head lru_list;
@@ -169,7 +172,7 @@ struct drm_i915_error_state {
u32 instdone1;
u32 seqno;
u64 bbaddr;
- u64 fence[16];
+ u64 fence[I915_MAX_NUM_FENCES];
struct timeval time;
struct drm_i915_error_object {
int page_count;
@@ -183,7 +186,7 @@ struct drm_i915_error_state {
u32 gtt_offset;
u32 read_domains;
u32 write_domain;
- s32 fence_reg:5;
+ s32 fence_reg:I915_MAX_NUM_FENCE_BITS;
s32 pinned:2;
u32 tiling:2;
u32 dirty:1;
@@ -377,7 +380,7 @@ typedef struct drm_i915_private {
struct notifier_block lid_notifier;
int crt_ddc_pin;
- struct drm_i915_fence_reg fence_regs[16]; /* assume 965 */
+ struct drm_i915_fence_reg fence_regs[I915_MAX_NUM_FENCES]; /* assume 965 */
int fence_reg_start; /* 4 if userland hasn't ioctl'd us yet */
int num_fence_regs; /* 8 on pre-965, 16 otherwise */
@@ -508,7 +511,7 @@ typedef struct drm_i915_private {
u8 saveAR[21];
u8 saveDACMASK;
u8 saveCR[37];
- uint64_t saveFENCE[16];
+ uint64_t saveFENCE[I915_MAX_NUM_FENCES];
u32 saveCURACNTR;
u32 saveCURAPOS;
u32 saveCURABASE;
@@ -780,10 +783,8 @@ struct drm_i915_gem_object {
* Fence register bits (if any) for this object. Will be set
* as needed when mapped into the GTT.
* Protected by dev->struct_mutex.
- *
- * Size: 4 bits for 16 fences + sign (for FENCE_REG_NONE)
*/
- signed int fence_reg:5;
+ signed int fence_reg:I915_MAX_NUM_FENCE_BITS;
/**
* Advice: are the backing pages purgeable?
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5f0f46e..30774e9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1743,7 +1743,7 @@ static void i915_gem_reset_fences(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
int i;
- for (i = 0; i < 16; i++) {
+ for (i = 0; i < dev_priv->num_fence_regs; i++) {
struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i];
struct drm_i915_gem_object *obj = reg->obj;
@@ -3905,7 +3905,7 @@ i915_gem_load(struct drm_device *dev)
INIT_LIST_HEAD(&dev_priv->mm.gtt_list);
for (i = 0; i < I915_NUM_RINGS; i++)
init_ring_lists(&dev_priv->ring[i]);
- for (i = 0; i < 16; i++)
+ for (i = 0; i < I915_MAX_NUM_FENCES; i++)
INIT_LIST_HEAD(&dev_priv->fence_regs[i].lru_list);
INIT_DELAYED_WORK(&dev_priv->mm.retire_work,
i915_gem_retire_work_handler);
--
1.7.6.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/3] drm/i915: enable support for 32 fences on ivybridge
2011-10-09 19:52 [PATCH 1/3] drm/i915: Ivybridge still has fences! Daniel Vetter
2011-10-09 19:52 ` [PATCH 2/3] drm/i915: add constants to size fence arrays and fields Daniel Vetter
@ 2011-10-09 19:52 ` Daniel Vetter
2011-10-10 17:58 ` [PATCH 1/3] drm/i915: Ivybridge still has fences! Jesse Barnes
2011-10-23 11:23 ` Chris Wilson
3 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2011-10-09 19:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/i915_gem.c | 4 +++-
drivers/gpu/drm/i915/i915_irq.c | 2 +-
drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0068f56..be40782 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -126,9 +126,9 @@ struct drm_i915_master_private {
struct _drm_i915_sarea *sarea_priv;
};
#define I915_FENCE_REG_NONE -1
-#define I915_MAX_NUM_FENCES 16
+#define I915_MAX_NUM_FENCES 32
/* 16 fences + sign bit for FENCE_REG_NONE */
-#define I915_MAX_NUM_FENCE_BITS 5
+#define I915_MAX_NUM_FENCE_BITS 6
struct drm_i915_fence_reg {
struct list_head lru_list;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 30774e9..93f0848 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3927,7 +3927,9 @@ i915_gem_load(struct drm_device *dev)
if (!drm_core_check_feature(dev, DRIVER_MODESET))
dev_priv->fence_reg_start = 3;
- if (INTEL_INFO(dev)->gen >= 4 || IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))
+ if (INTEL_INFO(dev)->gen >= 7)
+ dev_priv->num_fence_regs = 32;
+ else if (INTEL_INFO(dev)->gen >= 4 || IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))
dev_priv->num_fence_regs = 16;
else
dev_priv->num_fence_regs = 8;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 128ada2..d9b52e8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -824,7 +824,7 @@ static void i915_gem_record_fences(struct drm_device *dev,
switch (INTEL_INFO(dev)->gen) {
case 7:
case 6:
- for (i = 0; i < 16; i++)
+ for (i = 0; i < dev_priv->num_fence_regs; i++)
error->fence[i] = I915_READ64(FENCE_REG_SANDYBRIDGE_0 + (i * 8));
break;
case 5:
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 7886e4f..0db9630 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -372,7 +372,7 @@ static void i915_save_modeset_reg(struct drm_device *dev)
switch (INTEL_INFO(dev)->gen) {
case 7:
case 6:
- for (i = 0; i < 16; i++)
+ for (i = 0; i < dev_priv->num_fence_regs; i++)
dev_priv->saveFENCE[i] = I915_READ64(FENCE_REG_SANDYBRIDGE_0 + (i * 8));
break;
case 5:
@@ -407,7 +407,7 @@ static void i915_restore_modeset_reg(struct drm_device *dev)
switch (INTEL_INFO(dev)->gen) {
case 7:
case 6:
- for (i = 0; i < 16; i++)
+ for (i = 0; i < dev_priv->num_fence_regs; i++)
I915_WRITE64(FENCE_REG_SANDYBRIDGE_0 + (i * 8), dev_priv->saveFENCE[i]);
break;
case 5:
--
1.7.6.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-09 19:52 [PATCH 1/3] drm/i915: Ivybridge still has fences! Daniel Vetter
2011-10-09 19:52 ` [PATCH 2/3] drm/i915: add constants to size fence arrays and fields Daniel Vetter
2011-10-09 19:52 ` [PATCH 3/3] drm/i915: enable support for 32 fences on ivybridge Daniel Vetter
@ 2011-10-10 17:58 ` Jesse Barnes
2011-10-23 10:12 ` Daniel Vetter
2011-10-23 11:23 ` Chris Wilson
3 siblings, 1 reply; 13+ messages in thread
From: Jesse Barnes @ 2011-10-10 17:58 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, stable
[-- Attachment #1.1: Type: text/plain, Size: 330 bytes --]
On Sun, 9 Oct 2011 21:52:01 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> So don't forget to restore them on resume and dump them into
> the error state.
This should probably just be >= 6 instead; I don't think we're getting
rid of fences anytime soon.
--
Jesse Barnes, Intel Open Source Technology Center
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-10 17:58 ` [PATCH 1/3] drm/i915: Ivybridge still has fences! Jesse Barnes
@ 2011-10-23 10:12 ` Daniel Vetter
2011-10-23 10:18 ` Jesse Barnes
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2011-10-23 10:12 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Daniel Vetter, intel-gfx, stable
On Mon, Oct 10, 2011 at 10:58:22AM -0700, Jesse Barnes wrote:
> On Sun, 9 Oct 2011 21:52:01 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > So don't forget to restore them on resume and dump them into
> > the error state.
>
> This should probably just be >= 6 instead; I don't think we're getting
> rid of fences anytime soon.
As discussed on irc >= 6 is a bit hard to do in a switch statement ;-) Do
you want me to resend the patches using the gcc ranged switch extension
suggested by Adam Jackson (i.e. 6..UINT_MAX)? Or can you slap your r-b on
them as-is?
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-23 10:12 ` Daniel Vetter
@ 2011-10-23 10:18 ` Jesse Barnes
0 siblings, 0 replies; 13+ messages in thread
From: Jesse Barnes @ 2011-10-23 10:18 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, intel-gfx, stable
On Sun, 23 Oct 2011 12:12:00 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Oct 10, 2011 at 10:58:22AM -0700, Jesse Barnes wrote:
> > On Sun, 9 Oct 2011 21:52:01 +0200
> > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > > So don't forget to restore them on resume and dump them into
> > > the error state.
> >
> > This should probably just be >= 6 instead; I don't think we're
> > getting rid of fences anytime soon.
>
> As discussed on irc >= 6 is a bit hard to do in a switch
> statement ;-) Do you want me to resend the patches using the gcc
> ranged switch extension suggested by Adam Jackson (i.e. 6..UINT_MAX)?
> Or can you slap your r-b on them as-is?
I was suggesting that you convert it to an if ladder with the >= 6 at
the top. That should be a separate patch though.
Jesse
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-09 19:52 [PATCH 1/3] drm/i915: Ivybridge still has fences! Daniel Vetter
` (2 preceding siblings ...)
2011-10-10 17:58 ` [PATCH 1/3] drm/i915: Ivybridge still has fences! Jesse Barnes
@ 2011-10-23 11:23 ` Chris Wilson
2011-10-23 12:09 ` Daniel Vetter
2011-10-23 20:45 ` Kenneth Graunke
3 siblings, 2 replies; 13+ messages in thread
From: Chris Wilson @ 2011-10-23 11:23 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter, stable
Regardless of the outcome of Jesse's request for an if-ladder, the
substance of the patches look sound.
However, I remain unconvinced that there are 32 fence registers on IVB.
Daniel's evidence is based upon the size of the register map (and not
on the BSPEC explicitly stating a change to 32 ;-), but most tellingly
the bitfields for fence-number in other registers have not been updated -
so we can only safely allocated the first 16 anyway...
(For instance, FBC_CTL).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-23 11:23 ` Chris Wilson
@ 2011-10-23 12:09 ` Daniel Vetter
2011-10-23 12:35 ` Chris Wilson
2011-10-23 20:45 ` Kenneth Graunke
1 sibling, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2011-10-23 12:09 UTC (permalink / raw)
To: Chris Wilson, Keith Packard; +Cc: Daniel Vetter, intel-gfx, stable
On Sun, Oct 23, 2011 at 12:23:14PM +0100, Chris Wilson wrote:
> Regardless of the outcome of Jesse's request for an if-ladder, the
> substance of the patches look sound.
>
> However, I remain unconvinced that there are 32 fence registers on IVB.
> Daniel's evidence is based upon the size of the register map (and not
> on the BSPEC explicitly stating a change to 32 ;-), but most tellingly
> the bitfields for fence-number in other registers have not been updated -
> so we can only safely allocated the first 16 anyway...
> (For instance, FBC_CTL).
Ok, I've rechecked bspec. The FBC_CTL fence number is indeed only 4 bits
wide, but on snb+ is must be written as 0. The cpu fence stuff for fbc
moved to DPFC_CONTROL_SA, which has room enough for 5 bits. Unfortunately
bspec is silent on whether that has actually grown from 4 bits for ivb. On
a future hw iteration I'm not really allowed to talk about it is all
correctly in place (i.e. bspec definitions for all 32 fence regs plus the
5 bit wide fence number in DPFC_CTL_SA).
So I think I'll drop this patch till things clear up.
Keith, can take a look at patches 1-2 and consider merging them for 3.2?
Yours, Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-23 12:09 ` Daniel Vetter
@ 2011-10-23 12:35 ` Chris Wilson
2011-11-03 16:23 ` Keith Packard
0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2011-10-23 12:35 UTC (permalink / raw)
To: Daniel Vetter, Keith Packard; +Cc: Daniel Vetter, intel-gfx, stable
On Sun, 23 Oct 2011 14:09:07 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> Keith, can take a look at patches 1-2 and consider merging them for 3.2?
Those two are
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-23 12:35 ` Chris Wilson
@ 2011-11-03 16:23 ` Keith Packard
0 siblings, 0 replies; 13+ messages in thread
From: Keith Packard @ 2011-11-03 16:23 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter; +Cc: Daniel Vetter, intel-gfx, stable
[-- Attachment #1.1: Type: text/plain, Size: 373 bytes --]
On Sun, 23 Oct 2011 13:35:45 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Sun, 23 Oct 2011 14:09:07 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> > Keith, can take a look at patches 1-2 and consider merging them for 3.2?
>
> Those two are
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
And merged to -next.
--
keith.packard@intel.com
[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-23 11:23 ` Chris Wilson
2011-10-23 12:09 ` Daniel Vetter
@ 2011-10-23 20:45 ` Kenneth Graunke
2011-10-23 22:16 ` Daniel Vetter
1 sibling, 1 reply; 13+ messages in thread
From: Kenneth Graunke @ 2011-10-23 20:45 UTC (permalink / raw)
To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx
On 10/23/2011 04:23 AM, Chris Wilson wrote:
> Regardless of the outcome of Jesse's request for an if-ladder, the
> substance of the patches look sound.
>
> However, I remain unconvinced that there are 32 fence registers on IVB.
> Daniel's evidence is based upon the size of the register map (and not
> on the BSPEC explicitly stating a change to 32 ;-), but most tellingly
> the bitfields for fence-number in other registers have not been updated -
> so we can only safely allocated the first 16 anyway...
> (For instance, FBC_CTL).
> -Chris
It sure looks like it has 32 fence registers: BSpec vol1g GT Interface
Register [DevIVB] / GT Interface Register DevIVB / System Agent Config
Space lists FENCE0 through FENCE31. The simulator seems to indicate
this as well.
You're right that FBC_CTL still only has 4 bits for selecting a fence,
but notably, in the latest (WIP) version of the BSpec, it says "This
field must be programmed to 0000b." I'm not sure how it's supposed to
work now, but likely something has changed.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] drm/i915: Ivybridge still has fences!
2011-10-23 20:45 ` Kenneth Graunke
@ 2011-10-23 22:16 ` Daniel Vetter
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2011-10-23 22:16 UTC (permalink / raw)
To: Kenneth Graunke; +Cc: Daniel Vetter, intel-gfx
On Sun, Oct 23, 2011 at 01:45:21PM -0700, Kenneth Graunke wrote:
> On 10/23/2011 04:23 AM, Chris Wilson wrote:
> > Regardless of the outcome of Jesse's request for an if-ladder, the
> > substance of the patches look sound.
> >
> > However, I remain unconvinced that there are 32 fence registers on IVB.
> > Daniel's evidence is based upon the size of the register map (and not
> > on the BSPEC explicitly stating a change to 32 ;-), but most tellingly
> > the bitfields for fence-number in other registers have not been updated -
> > so we can only safely allocated the first 16 anyway...
> > (For instance, FBC_CTL).
> > -Chris
>
> It sure looks like it has 32 fence registers: BSpec vol1g GT Interface
> Register [DevIVB] / GT Interface Register DevIVB / System Agent Config
> Space lists FENCE0 through FENCE31. The simulator seems to indicate
> this as well.
>
> You're right that FBC_CTL still only has 4 bits for selecting a fence,
> but notably, in the latest (WIP) version of the BSpec, it says "This
> field must be programmed to 0000b." I'm not sure how it's supposed to
> work now, but likely something has changed.
See my other mail. The cpu fence is specified in DPCF_CONTROL_SA, which
has again 4 bits on snb, but enough free room for 5 bits. Now on some
future hw I'm not allowed to talk about, these 5 bits exist (see
MPGFXTRK_CR_DPFC_CONTROL_SA_0_2_0_GTTMMADR in bspec), but unfortunately
not on ivb. So if you could hunt around in the sim code and check whether
that is already implemented on ivb, that'd be awesome.
Thanks, Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 13+ messages in thread