Is MI_FLUSH_ENABLE bit 12?

Is MI_FLUSH_ENABLE bit 12?

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 432 bytes --]


Just reading through vol1c.4 of the bspec  this evening and found something odd.

Bit 11 of MI_MODE is "Invalidate UHPTR enable".
Bit 12 of MI_MODE is "MI_FLUSH Enable"

And, yet, in i915_reg.h:

#define MI_MODE		0x0209c
# define VS_TIMER_DISPATCH				(1 << 6)
# define MI_FLUSH_ENABLE				(1 << 11)

Are we off-by-one on MI_FLUSH_ENABLE? Seems like this would cause
serious problems...

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

Re: Is MI_FLUSH_ENABLE bit 12?

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 714 bytes --]

On Mon, 28 Nov 2011 18:48:04 -0800, "Keith Packard" <keithp@keithp.com> wrote:
Non-text part: multipart/mixed
Non-text part: multipart/signed
> 
> Just reading through vol1c.4 of the bspec  this evening and found something odd.
> 
> Bit 11 of MI_MODE is "Invalidate UHPTR enable".
> Bit 12 of MI_MODE is "MI_FLUSH Enable"
> 
> And, yet, in i915_reg.h:
> 
> #define MI_MODE		0x0209c
> # define VS_TIMER_DISPATCH				(1 << 6)
> # define MI_FLUSH_ENABLE				(1 << 11)
> 
> Are we off-by-one on MI_FLUSH_ENABLE? Seems like this would cause
> serious problems...

I think we are.  On the other hand, based on actual behavior plus
reading of simulator, I believe that the bit does nothing, regardless.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 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

Re: Is MI_FLUSH_ENABLE bit 12?

[Ben Widawsky]

[-- Attachment #1.1: Type: text/plain, Size: 1040 bytes --]

On Tue, Nov 29, 2011 at 04:47:57PM -0800, Eric Anholt wrote:
> On Mon, 28 Nov 2011 18:48:04 -0800, "Keith Packard" <keithp@keithp.com> wrote:
> Non-text part: multipart/mixed
> Non-text part: multipart/signed
> > 
> > Just reading through vol1c.4 of the bspec  this evening and found something odd.
> > 
> > Bit 11 of MI_MODE is "Invalidate UHPTR enable".
> > Bit 12 of MI_MODE is "MI_FLUSH Enable"
> > 
> > And, yet, in i915_reg.h:
> > 
> > #define MI_MODE		0x0209c
> > # define VS_TIMER_DISPATCH				(1 << 6)
> > # define MI_FLUSH_ENABLE				(1 << 11)
> > 
> > Are we off-by-one on MI_FLUSH_ENABLE? Seems like this would cause
> > serious problems...
> 
> I think we are.  On the other hand, based on actual behavior plus
> reading of simulator, I believe that the bit does nothing, regardless.

I do not think so. We've (Chris, I, and perhaps Jesse?) been through
this excercise at least twice before, and both times resulted in hangs
when we switched to bit 12 on Ironlake, not sure about other platforms.

Ben

[-- Attachment #1.2: Type: application/pgp-signature, Size: 490 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

Re: Is MI_FLUSH_ENABLE bit 12?

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 430 bytes --]

On Wed, 30 Nov 2011 03:42:00 +0000, Ben Widawsky <ben@bwidawsk.net> wrote:

> I do not think so. We've (Chris, I, and perhaps Jesse?) been through
> this excercise at least twice before, and both times resulted in hangs
> when we switched to bit 12 on Ironlake, not sure about other
> platforms.

Then, perhaps, a comment in the code would let other people learn from
your mis-adventures?

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

Re: Is MI_FLUSH_ENABLE bit 12?

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1297 bytes --]

On Wed, 30 Nov 2011 03:42:00 +0000, Ben Widawsky <ben@bwidawsk.net> wrote:
Non-text part: multipart/signed
> On Tue, Nov 29, 2011 at 04:47:57PM -0800, Eric Anholt wrote:
> > On Mon, 28 Nov 2011 18:48:04 -0800, "Keith Packard" <keithp@keithp.com> wrote:
> > Non-text part: multipart/mixed
> > Non-text part: multipart/signed
> > > 
> > > Just reading through vol1c.4 of the bspec  this evening and found something odd.
> > > 
> > > Bit 11 of MI_MODE is "Invalidate UHPTR enable".
> > > Bit 12 of MI_MODE is "MI_FLUSH Enable"
> > > 
> > > And, yet, in i915_reg.h:
> > > 
> > > #define MI_MODE		0x0209c
> > > # define VS_TIMER_DISPATCH				(1 << 6)
> > > # define MI_FLUSH_ENABLE				(1 << 11)
> > > 
> > > Are we off-by-one on MI_FLUSH_ENABLE? Seems like this would cause
> > > serious problems...
> > 
> > I think we are.  On the other hand, based on actual behavior plus
> > reading of simulator, I believe that the bit does nothing, regardless.
> 
> I do not think so. We've (Chris, I, and perhaps Jesse?) been through
> this excercise at least twice before, and both times resulted in hangs
> when we switched to bit 12 on Ironlake, not sure about other platforms.

There is no MI_FLUSH enable bit on Ironlake in my copy of the docs.  Bit
12 is MBZ on that hardware.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 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

Re: Is MI_FLUSH_ENABLE bit 12?

[Jesse Barnes]

[-- Attachment #1.1: Type: text/plain, Size: 1674 bytes --]

On Wed, 30 Nov 2011 13:54:47 -0800
Eric Anholt <eric@anholt.net> wrote:

> On Wed, 30 Nov 2011 03:42:00 +0000, Ben Widawsky <ben@bwidawsk.net> wrote:
> Non-text part: multipart/signed
> > On Tue, Nov 29, 2011 at 04:47:57PM -0800, Eric Anholt wrote:
> > > On Mon, 28 Nov 2011 18:48:04 -0800, "Keith Packard" <keithp@keithp.com> wrote:
> > > Non-text part: multipart/mixed
> > > Non-text part: multipart/signed
> > > > 
> > > > Just reading through vol1c.4 of the bspec  this evening and found something odd.
> > > > 
> > > > Bit 11 of MI_MODE is "Invalidate UHPTR enable".
> > > > Bit 12 of MI_MODE is "MI_FLUSH Enable"
> > > > 
> > > > And, yet, in i915_reg.h:
> > > > 
> > > > #define MI_MODE		0x0209c
> > > > # define VS_TIMER_DISPATCH				(1 << 6)
> > > > # define MI_FLUSH_ENABLE				(1 << 11)
> > > > 
> > > > Are we off-by-one on MI_FLUSH_ENABLE? Seems like this would cause
> > > > serious problems...
> > > 
> > > I think we are.  On the other hand, based on actual behavior plus
> > > reading of simulator, I believe that the bit does nothing, regardless.
> > 
> > I do not think so. We've (Chris, I, and perhaps Jesse?) been through
> > this excercise at least twice before, and both times resulted in hangs
> > when we switched to bit 12 on Ironlake, not sure about other platforms.
> 
> There is no MI_FLUSH enable bit on Ironlake in my copy of the docs.  Bit
> 12 is MBZ on that hardware.

It's one of those "been there forever" bits.  A comment wouldn't hurt,
but we should also put in a doc change request (in fact I think I did
that but it went into a black hole).

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

Re: Is MI_FLUSH_ENABLE bit 12?

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 2390 bytes --]

On Mon, 5 Dec 2011 14:57:09 -0800, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Wed, 30 Nov 2011 13:54:47 -0800
> Eric Anholt <eric@anholt.net> wrote:
> 
> > On Wed, 30 Nov 2011 03:42:00 +0000, Ben Widawsky <ben@bwidawsk.net> wrote:
> > Non-text part: multipart/signed
> > > On Tue, Nov 29, 2011 at 04:47:57PM -0800, Eric Anholt wrote:
> > > > On Mon, 28 Nov 2011 18:48:04 -0800, "Keith Packard" <keithp@keithp.com> wrote:
> > > > Non-text part: multipart/mixed
> > > > Non-text part: multipart/signed
> > > > > 
> > > > > Just reading through vol1c.4 of the bspec  this evening and found something odd.
> > > > > 
> > > > > Bit 11 of MI_MODE is "Invalidate UHPTR enable".
> > > > > Bit 12 of MI_MODE is "MI_FLUSH Enable"
> > > > > 
> > > > > And, yet, in i915_reg.h:
> > > > > 
> > > > > #define MI_MODE		0x0209c
> > > > > # define VS_TIMER_DISPATCH				(1 << 6)
> > > > > # define MI_FLUSH_ENABLE				(1 << 11)
> > > > > 
> > > > > Are we off-by-one on MI_FLUSH_ENABLE? Seems like this would cause
> > > > > serious problems...
> > > > 
> > > > I think we are.  On the other hand, based on actual behavior plus
> > > > reading of simulator, I believe that the bit does nothing, regardless.
> > > 
> > > I do not think so. We've (Chris, I, and perhaps Jesse?) been through
> > > this excercise at least twice before, and both times resulted in hangs
> > > when we switched to bit 12 on Ironlake, not sure about other platforms.
> > 
> > There is no MI_FLUSH enable bit on Ironlake in my copy of the docs.  Bit
> > 12 is MBZ on that hardware.
> 
> It's one of those "been there forever" bits.  A comment wouldn't hurt,
> but we should also put in a doc change request (in fact I think I did
> that but it went into a black hole).

Well, by "been there forever", you mean "introduced in:

commit a69ffdbfcba8eabf2ca9d384b578e6f28b339c61
Author: Zhenyu Wang <zhenyuw@linux.intel.com>
Date:   Mon Aug 30 16:12:42 2010 +0800

    drm/i915: Enable MI_FLUSH on Sandybridge
    
    MI_FLUSH is being deprecated, but still available on Sandybridge.
    Make sure it's enabled as userspace still uses MI_FLUSH.
    
    Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
    Cc: stable@kernel.org
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>"

It should be removed on Ironlake, and fixed to be 12 on SNB or just
removed.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 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

Re: Is MI_FLUSH_ENABLE bit 12?

[Jesse Barnes]

[-- Attachment #1.1: Type: text/plain, Size: 1225 bytes --]

On Tue, 06 Dec 2011 11:56:07 -0800
Eric Anholt <eric@anholt.net> wrote:
> > It's one of those "been there forever" bits.  A comment wouldn't hurt,
> > but we should also put in a doc change request (in fact I think I did
> > that but it went into a black hole).
> 
> Well, by "been there forever", you mean "introduced in:
> 
> commit a69ffdbfcba8eabf2ca9d384b578e6f28b339c61
> Author: Zhenyu Wang <zhenyuw@linux.intel.com>
> Date:   Mon Aug 30 16:12:42 2010 +0800
> 
>     drm/i915: Enable MI_FLUSH on Sandybridge
>     
>     MI_FLUSH is being deprecated, but still available on Sandybridge.
>     Make sure it's enabled as userspace still uses MI_FLUSH.
>     
>     Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
>     Cc: stable@kernel.org
>     Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>"
> 
> It should be removed on Ironlake, and fixed to be 12 on SNB or just
> removed.

No, "fixed to be 12" would just encourage people to still try it and
break their machines.  We can safely remove it now that we don't use it
anywhere.  And we can fix the docs so that if people do want to mess
with it they won't try bit 12.

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

Re: Is MI_FLUSH_ENABLE bit 12?

[Daniel Vetter]

On Tue, Dec 06, 2011 at 12:21:02PM -0800, Jesse Barnes wrote:
> On Tue, 06 Dec 2011 11:56:07 -0800
> Eric Anholt <eric@anholt.net> wrote:
> > > It's one of those "been there forever" bits.  A comment wouldn't hurt,
> > > but we should also put in a doc change request (in fact I think I did
> > > that but it went into a black hole).
> >
> > Well, by "been there forever", you mean "introduced in:
> >
> > commit a69ffdbfcba8eabf2ca9d384b578e6f28b339c61
> > Author: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Date:   Mon Aug 30 16:12:42 2010 +0800
> >
> >     drm/i915: Enable MI_FLUSH on Sandybridge
> >
> >     MI_FLUSH is being deprecated, but still available on Sandybridge.
> >     Make sure it's enabled as userspace still uses MI_FLUSH.
> >
> >     Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> >     Cc: stable@kernel.org
> >     Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>"
> >
> > It should be removed on Ironlake, and fixed to be 12 on SNB or just
> > removed.
>
> No, "fixed to be 12" would just encourage people to still try it and
> break their machines.  We can safely remove it now that we don't use it
> anywhere.  And we can fix the docs so that if people do want to mess
> with it they won't try bit 12.

If userspace ever relied on this being enabled we cannot just disable it
now - it would break the kernel<->userspace abi on snb. You have to wait
for 5-10 years, eternity or until the last userspace using this
disappeared, whichever comes first ;-)

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48