Re: [Intel-gfx] [PATCH] drm/i915: Add support for resetting the SO write pointers on gen7.

[Eric Anholt]

On Tue, 03 Jan 2012 19:40:36 -0800, Ben Widawsky  wrote:
> On 12/29/2011 05:52 PM, Eric Anholt wrote:
> > These registers are automatically incremented by the hardware during
> > transform feedback to track where the next streamed vertex output
> > should go.  Unlike the previous generation, which had a packet for
> > setting the corresponding registers to a defined value, gen7 only has
> > MI_LOAD_REGISTER_IMM to do so.  That's a secure packet (since it loads
> > an arbitrary register), so we need to do it from the kernel, and it
> > needs to be settable atomically with the batchbuffer execution so that
> > two clients doing transform feedback don't stomp on each others'
> > state.
> >
> > Instead of building a more complicated interface involcing setting the
> > registers to a specific value, just set them to 0 when asked and
> > userland can tweak its pointers accordingly.
> >
> > Signed-off-by: Eric Anholt

> > +/** Resets the SO write offset registers for transform feedback on gen7. */
> > +#define I915_EXEC_GEN7_SOL_RESET   (1<<8)
> > +
> >   struct drm_i915_gem_pin {
> > /** Handle of the buffer to be pinned. */
> > __u32 handle;
> 
> Is this something we want to carry long term wrt ABI. I really want to 
> get context/ppgtt stuff out the door relatively soon (context should be 
> ready to test really soon actually).
> 
> I'm totally unfamiliar with this register, but is this what you want the 
> interface to be permanently?

Ultimately, we want this ABI plus contexts.  Until then, we can live
with this ABI alone.


pgpilHxVAU9RG.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] intel: Add regression tests for batch decode.

[Daniel Vetter]

On Tue, Jan 03, 2012 at 06:29:35PM -0800, Eric Anholt wrote:
> On Wed, 4 Jan 2012 00:24:28 +0100, Daniel Vetter  wrote:
> > On Tue, Jan 03, 2012 at 03:05:26PM -0800, Eric Anholt wrote:
> > > The .batch was generated using the dump-a-batch branch of
> > > 
> > > git://people.freedesktop.org/~anholt/mesa
> > > 
> > > using glxgears on gen7 hardware, using INTEL_DEVID_OVERRIDE for
> > > non-gen7 (this means that offsets in the buffers for non-gen7 are 0!).
> > > The .ref was generated by:
> > > 
> > > ./test_decode tests/gen7-3d.batch -dump.
> > > 
> > > The .sh exists because you can't supply arguments to tests using the
> > > simple automake tests driver.  Something reasonable could be done
> > > using automake's parallel-tests driver (in fact, a previous version of
> > > the patch did that), but I was concerned that:
> > > 
> > > 1) The parallel-tests driver is documented to be unstable -- they may
> > >change interfaces on us later.
> > > 2) The parallel-tests driver hides the output of tests in .log files
> > >scattered all over the tree, which was ugly and more painful to
> > >work with.
> > > ---
> > >  intel/Makefile.am |   17 +
> > >  intel/tests/gen7-3d.batch |  Bin 0 -> 4504 bytes
> > >  intel/tests/gen7-3d.batch-ref.txt | 1350 
> > > +
> > 
> > I'm missing the *.batch* stuff for gen4-gen6 mentioned in the Makefile.am.
> > From looking throught the code I've also expected the *.batch to be a
> > symlink to test-batch.sh.
> 
> Failure to git add.  Fixed locally.

In that case, for the series:

Reviewed-by: Daniel Vetter 
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel: Update for new i915_drm.h defines.

[Daniel Vetter]

On Tue, Jan 03, 2012 at 07:10:00PM -0800, Eric Anholt wrote:
> ---
> 
> This syncs the code up to the kernel as of the gen7 SOL changes.  It
> would be nice if doing this was just a straight copy of the kernel
> code -- there are two diffs left out.  One is this hunk:
> 
> -#ifdef __KERNEL__
> -/* For use by IPS driver */
> -extern unsigned long i915_read_mch_val(void);
> -extern bool i915_gpu_raise(void);
> -extern bool i915_gpu_lower(void);
> -extern bool i915_gpu_busy(void);
> -extern bool i915_gpu_turbo_disable(void);
> -#endif
> 
> Does anyone see any real reason to be dropping this?
> 
> The other is removing the sparse __user annotations on the structs.
> Could we just patch the kernel to #define it away for userland, so we
> could make updating this file just a matter of "cp"?

$ make headers_install 

in the kernel tree should do all this preprocessing for you and drop a
sanitized header into usr/include/drm/i915_drm.h. Lazy me hasn't checked
whether that would match up perfectly with what's in libdrm though ...
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/1] uxa/glamor: Route some drawing function to glamor.

[Zhigang Gong]

> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Wednesday, January 04, 2012 5:47 AM
> To: zhigang.g...@linux.intel.com
> Cc: intel-gfx@lists.freedesktop.org; zhigang.g...@gmail.com;
> zhigang.g...@linux.intel.com
> Subject: Re: [PATCH 0/1] uxa/glamor: Route some drawing function to
> glamor.
> 
> On Sat, 31 Dec 2011 21:18:14 +0800, zhigang.g...@linux.intel.com
> wrote:
> > From: Zhigang Gong 
> >
> > I agree to pending the last patch which is to create glamor pixmap by
> default.
> > This patch is based on the other 3 patch. After apply this patch, and
> > use the latest glamor.
> 
> I've pushed those 4 patches (all bar to use
> intel_glamor_create_textured_pixmap by default). I've a new segfault for
> you in glamor_core.c::glamor_validate_gc(), line 425 as pixmap_priv is
> NULL, which I only hit just as I thought I had completed testing the code.
> 
> Afaics, the remaining big topics for the ddx are how to enable glx and
> integration between glamor/uxa render paths, right?

Exactly.
That's the hardest part now. There are two DRI loaders, one is AIGLX and the
other is EGL. Both loader have their own glapi.c. Currently, the AIGLX
always 
get loaded before the DDX driver, and thus the glapi in glx will be used. 

One possible solution is as below, please help to review and comment:

let the glamor egl module get loaded prior to the AIGLX And load the proper
libGL.so 
thus the egl will create a complete and correct context. And then latter, we
can call 
_glapi_create_table_from_handle to initialize the glx's dispatch structure. 
We can treat the context created by egl for glamor is the context dedicated
for server
client, and each time when call into glamor's rendering path, force current
to glamor's
context and restore to previous context at returning.

Copy to KRH, I know you are the master of DRI2/AIGLX. Any suggestion here?
Thanks.

> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] The latest status of intel graphics on kernel 3.2

[Sun, Yi]

> -Original Message-
> From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
> Sent: Wednesday, January 04, 2012 6:53 AM
> To: Sun, Yi
> Cc: intel-gfx@lists.freedesktop.org; Yang, Guang A
> Subject: Re: [Intel-gfx] The latest status of intel graphics on kernel 3.2
> 
> On Sat, 31 Dec 2011 08:41:34 +
> "Sun, Yi"  wrote:
> 
> > The platforms we covered are IvyBridge, SandyBridge, IronLake,
> > PineView and GM965 The version of Kernel:
> > (drm-intel-next)64a742fac3a22f57303d8f1b7e347350a1c48254
> >
> > General speaking, the issue is mainly related to 3 pipe and eDP on 
> > IvyBridge.
> >
> > 4 new bugs are filed:
> > Bug 44250 -
> > [IVB]"[drm:intel_dsm_platform_mux_info] *ERROR* MUX INFO call failed"
> > while booting with monitor Bug
> > 44304 - [ivb
> > 3pipe] "*ERROR* failed to set mode on [CRTC:5]" while plug in the 3th
> > monitor Bug 44305
> > - [IVB]The Edp can't work while booting with a monitor Bug
> > 44309 - [IVB eDP]
> > 3 pipe doesn't work with eDP monitor
> 
> > Old bugs which still exists or just need code merge:
> > Bug 42263 - [ILK]
> > Plug in a monitor will make the eDP black screen
> > 41976 [IVB] screen
> > turn to be black while switching between console and x-window with
> > 3-pipe active
> > 41917 [IVB] all
> > three screens are frozen while starting gnome-session with 3-pipe
> > active 42731 [IVB]
> > The whole monitor connected to DP port is black screen while hotplugin
> > VGA
> 
> Just to confirm, these are fixed by
>  [PATCH 1/2] drm/i915: don't disable a PCH DPLL that's in use and  [PATCH 2/2]
> drm/i915: only set the intel_crtc DPMS mode to on if the
>mode set succeeded
> 
> right?  Or are there other patches needed as well?  Can you reply to them
> with your tested-by so Keith can pick them up?
> 

Hi Jesse,

I think I had replied the "[PATCH 2/2] drm/i915: only set the 
intel_crtc DPMS mode  to on if the mode set succeeded". And I'll replay it 
again.

And confirm with you. When I tested about 3-pipe feature, I used your 
branch "ivb-3-pipe", which contain 4 commits in all. And 2 patches for Xserver, 
one is on the upstream and the other is attached. So that is to say, 3 commits 
on branch "ivb-3-pipe" and a xserver patch are needed to put on the upstream. 
Right?

Thanks
Yi

> Thanks,
> --
> Jesse Barnes, Intel Open Source Technology Center


xserver-initial-config-match-refresh.patch
Description: xserver-initial-config-match-refresh.patch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: only set the intel_crtc DPMS mode to on if the mode set succeeded

[Sun, Yi]

Now, 3 bugs are fixed by the patches: 

[patch 1/2] drm/i915: don't disable a PCH DPLL that's   in use 
and 
[patch 2/2] drm/i915: only set the intel_crtc DPMS mode to on if the mode set 
succeeded

http://lists.freedesktop.org/archives/intel-gfx/2011-November/013424.html 
http://lists.freedesktop.org/archives/intel-gfx/2011-November/013425.html


Tested-by: Sun Yi 

Thanks
   --Yi,Sun


> -Original Message-
> From: intel-gfx-bounces+yi.sun=intel@lists.freedesktop.org
> [mailto:intel-gfx-bounces+yi.sun=intel@lists.freedesktop.org] On Behalf
> Of Sun, Yi
> Sent: Friday, December 16, 2011 2:36 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: only set the intel_crtc DPMS
> mode to on if the mode set succeeded
> 
> We have tested the patches on Jesse branch ' ivb-3-pipe '. They can fix bug
> 41917, 41919 and 41976.
> 
> Tested-by Sun Yi 
> 
> Thanks
>--Yi,Sun
> 
> 
> > -Original Message-
> > From: intel-gfx-bounces+yi.sun=intel@lists.freedesktop.org
> > [mailto:intel-gfx-bounces+yi.sun=intel@lists.freedesktop.org] On Behalf
> > Of Jesse Barnes
> > Sent: Wednesday, November 16, 2011 2:29 AM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [Intel-gfx] [PATCH 2/2] drm/i915: only set the intel_crtc DPMS mode
> to
> > on if the mode set succeeded
> >
> > If it failed, leave it in the "off" state.
> >
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |6 --
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index bb8ed3d..dc3d735 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5813,10 +5813,12 @@ static int intel_crtc_mode_set(struct drm_crtc
> > *crtc,
> >
> > ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
> >   x, y, old_fb);
> > -
> > drm_vblank_post_modeset(dev, pipe);
> >
> > -   intel_crtc->dpms_mode = DRM_MODE_DPMS_ON;
> > +   if (ret)
> > +   intel_crtc->dpms_mode = DRM_MODE_DPMS_OFF;
> > +   else
> > +   intel_crtc->dpms_mode = DRM_MODE_DPMS_ON;
> >
> > return ret;
> >  }
> > --
> > 1.7.4.1
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel: Update for new i915_drm.h defines.

[Jesse Barnes]

On Tue,  3 Jan 2012 19:10:00 -0800
Eric Anholt  wrote:

> ---
> 
> This syncs the code up to the kernel as of the gen7 SOL changes.  It
> would be nice if doing this was just a straight copy of the kernel
> code -- there are two diffs left out.  One is this hunk:
> 
> -#ifdef __KERNEL__
> -/* For use by IPS driver */
> -extern unsigned long i915_read_mch_val(void);
> -extern bool i915_gpu_raise(void);
> -extern bool i915_gpu_lower(void);
> -extern bool i915_gpu_busy(void);
> -extern bool i915_gpu_turbo_disable(void);
> -#endif
> 
> Does anyone see any real reason to be dropping this?
> 
> The other is removing the sparse __user annotations on the structs.
> Could we just patch the kernel to #define it away for userland, so we
> could make updating this file just a matter of "cp"?

Having just done this locally, I'm in favor.  The stuff under
__KERNEL__ should be harmless; don't see why we can't just copy it.

For the __user define, we must be missing a compiler.h include
somewhere in the libdrm case.  If you explicitly include it do the
errors go away?

-- 
Jesse Barnes, Intel Open Source Technology Center


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: only set the intel_crtc DPMS mode to on if the mode set succeeded

[Jesse Barnes]

Keith, see below in case you want to update the changelogs.

Jesse

On Wed, 4 Jan 2012 13:36:01 +
"Sun, Yi"  wrote:

> Now, 3 bugs are fixed by the patches: 
> 
> [patch 1/2] drm/i915: don't disable a PCH DPLL that's in use 
> and 
> [patch 2/2] drm/i915: only set the intel_crtc DPMS mode to on if the mode set 
> succeeded
> 
> http://lists.freedesktop.org/archives/intel-gfx/2011-November/013424.html 
> http://lists.freedesktop.org/archives/intel-gfx/2011-November/013425.html
> 
> 
> Tested-by: Sun Yi 
> 
> Thanks
>--Yi,Sun
> 
> 
> > -Original Message-
> > From: intel-gfx-bounces+yi.sun=intel@lists.freedesktop.org
> > [mailto:intel-gfx-bounces+yi.sun=intel@lists.freedesktop.org] On Behalf
> > Of Sun, Yi
> > Sent: Friday, December 16, 2011 2:36 PM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: only set the intel_crtc DPMS
> > mode to on if the mode set succeeded
> > 
> > We have tested the patches on Jesse branch ' ivb-3-pipe '. They can fix bug
> > 41917, 41919 and 41976.
> > 
> > Tested-by Sun Yi 
> > 
> > Thanks
> >--Yi,Sun
> > 
> > 
> > > -Original Message-
> > > From: intel-gfx-bounces+yi.sun=intel@lists.freedesktop.org
> > > [mailto:intel-gfx-bounces+yi.sun=intel@lists.freedesktop.org] On 
> > > Behalf
> > > Of Jesse Barnes
> > > Sent: Wednesday, November 16, 2011 2:29 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Subject: [Intel-gfx] [PATCH 2/2] drm/i915: only set the intel_crtc DPMS 
> > > mode
> > to
> > > on if the mode set succeeded
> > >
> > > If it failed, leave it in the "off" state.
> > >
> > > Signed-off-by: Jesse Barnes 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c |6 --
> > >  1 files changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index bb8ed3d..dc3d735 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5813,10 +5813,12 @@ static int intel_crtc_mode_set(struct drm_crtc
> > > *crtc,
> > >
> > >   ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
> > > x, y, old_fb);
> > > -
> > >   drm_vblank_post_modeset(dev, pipe);
> > >
> > > - intel_crtc->dpms_mode = DRM_MODE_DPMS_ON;
> > > + if (ret)
> > > + intel_crtc->dpms_mode = DRM_MODE_DPMS_OFF;
> > > + else
> > > + intel_crtc->dpms_mode = DRM_MODE_DPMS_ON;
> > >
> > >   return ret;
> > >  }
> > > --
> > > 1.7.4.1
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 



signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: paper over missed irq issues with force wake vodoo

[Daniel Vetter]

Two things seem to do the trick on my ivb machine here:
- prevent the gt from powering down while waiting for seqno
  notification interrupts by grabbing the force_wake in get_irq (and
  dropping it in put_irq again).
- ordering writes from the ring's CS by reading a CS register, ACTHD
  seems to work.

Only the blt&bsd ring on ivb seem to be massively affected by this,
but for paranoia do this dance also on the render ring and on snb
(i.e. all gpus with forcewake).

Tested with Eric's glCopyPixels loop which without this patch scores a
missed irq every few seconds.

This patch needs my forcewake rework to use a spinlock instead of
dev->struct_mutex.

Cc: sta...@kernel.org
Cc: Eric Anholt 
Cc: Kenneth Graunke 
Cc: Eugeni Dodonov 
Signed-Off-by: Daniel Vetter 
---

For easier testing grab my -next branch at

http://cgit.freedesktop.org/~danvet/drm/log/?h=my-next

Cheers, Daniel

 drivers/gpu/drm/i915/intel_ringbuffer.c |   21 +++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cf6e159..dbbc565 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -631,6 +631,15 @@ render_ring_add_request(struct intel_ring_buffer *ring,
 }
 
 static u32
+gen6_ring_get_seqno(struct intel_ring_buffer *ring)
+{
+   /* Workaround for force correct ordering between irq and seqno writes on
+* ivb (and maybe also on snb). */
+   intel_ring_get_active_head(ring);
+   return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
+}
+
+static u32
 ring_get_seqno(struct intel_ring_buffer *ring)
 {
return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
@@ -795,6 +804,11 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 
gflag, u32 rflag)
if (!dev->irq_enabled)
   return false;
 
+   /* It looks like we need to prevent the gt from suspending while waiting
+* for an notifiy irq, otherwise irqs seem to get lost on at least the
+* blt/bsd rings. */
+   gen6_gt_force_wake_get(dev_priv);
+
spin_lock(&ring->irq_lock);
if (ring->irq_refcount++ == 0) {
ring->irq_mask &= ~rflag;
@@ -819,6 +833,8 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 
gflag, u32 rflag)
ironlake_disable_irq(dev_priv, gflag);
}
spin_unlock(&ring->irq_lock);
+
+   gen6_gt_force_wake_put(dev_priv);
 }
 
 static bool
@@ -1329,7 +1345,7 @@ static const struct intel_ring_buffer gen6_bsd_ring = {
.write_tail = gen6_bsd_ring_write_tail,
.flush  = gen6_ring_flush,
.add_request= gen6_add_request,
-   .get_seqno  = ring_get_seqno,
+   .get_seqno  = gen6_ring_get_seqno,
.irq_get= gen6_bsd_ring_get_irq,
.irq_put= gen6_bsd_ring_put_irq,
.dispatch_execbuffer= gen6_ring_dispatch_execbuffer,
@@ -1388,7 +1404,7 @@ static const struct intel_ring_buffer gen6_blt_ring = {
.write_tail = ring_write_tail,
.flush  = blt_ring_flush,
.add_request= gen6_add_request,
-   .get_seqno  = ring_get_seqno,
+   .get_seqno  = gen6_ring_get_seqno,
.irq_get= blt_ring_get_irq,
.irq_put= blt_ring_put_irq,
.dispatch_execbuffer= gen6_ring_dispatch_execbuffer,
@@ -1410,6 +1426,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
ring->flush = gen6_render_ring_flush;
ring->irq_get = gen6_render_ring_get_irq;
ring->irq_put = gen6_render_ring_put_irq;
+   ring->get_seqno = gen6_ring_get_seqno;
} else if (IS_GEN5(dev)) {
ring->add_request = pc_render_add_request;
ring->get_seqno = pc_render_get_seqno;
-- 
1.7.7.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] gen7 missed IRQ workaround series.

[Daniel Vetter]

On Tue, Jan 03, 2012 at 03:38:25PM -0800, Keith Packard wrote:
> On Wed, 4 Jan 2012 00:04:18 +0100, Daniel Vetter  wrote:
> 
> > I kinda prefer Chris' approach of sticking with irqs, but backing it up
> > with a timer in the msec range. Can't find that patch though atm (iirc
> > it's in bugzilla somewhere).
> 
> I think we've resolved that the interrupt arrives, but that it is not
> serialized wrt the memory write. So, what I'd love to see is whether
> waiting for the irq and then busy looping for 'a while' works. This
> would allow for long-running operations to idle the CPU and then hit the
> interrupt and spin until the memory write is recognized.
> 
> Any solution which can leave operations unacknowledged for 'ms'
> timeframes seems like a potential for serious performance
> problems. Eric's simple spinning approach seems fine for the BLT ring
> which we don't use that often; something more complicated and yet not
> entirely timer-based might be more efficient for longer-running operations.

Afaics we only spin shortly on the blt ring if
- semaphores are not enabled and
- mesa is the client.
For all other cases it's pretty easy to come up with examples where we
busy-loop for a rather long time. So I really don't like the busy-looping,
especially now that semaphores are enabled by default for ivb.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] gen7 missed IRQ workaround series.

[Keith Packard]

On Wed, 4 Jan 2012 18:02:21 +0100, Daniel Vetter  wrote:

> So I really don't like the busy-looping, especially now that
> semaphores are enabled by default for ivb.

Alternatives are welcome; we have no other known fix for missing
sequence writes.

-- 
keith.pack...@intel.com


pgpHcsh2BtIKu.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/43] drm/i915: protect force_wake_(get|put) with the gt_lock

[Daniel Vetter]

On Wed, Jan 4, 2012 at 00:33, Keith Packard  wrote:
> On Tue, 3 Jan 2012 22:49:52 +0100, Daniel Vetter  
> wrote:
>
>> Nope, current hangcheck blows up, and we have an i-g-t testcase for it
>> (which the commit msg clearly states). There are also numerous bug
>> reports where a dying gpu results in tons of
>> WARN_ON(!mutex_locked(dev->struct_mutex)) noise in dmesg (which drowns
>> out the gpu hang warning). The locking change fixes this.
>
> Ah, ok, that makes sense. Of course, hangcheck *could* have just taken
> struct_mutex were it run in a suitable context.

Nope, we cannot move the hangcheck into process context by using a
delayed work item and then grabbing struct_mutex. If the gpu is dead,
we usually have a task stuck waiting for it and already holding
struct_mutex. It is *absolutely* imperial that the hangcheck and error
state capture code do not block on anything that the i915 gem code
might hold onto.

>> The patch adds the required locking to i915_reset.
>
> No, the spinlock protects the forcewake_count access and not the actual
> register access, which leaves all kinds of potential for races in
> threads not also holding struct_mutex while accessing registers.

Ah, I think I see you're concern: Between the time we reset the gpu
and the time we fix up the forcewake state somebody might sneak in and
see an inconstency between our tracking and the actual hw state, hence
reading garbage. Correct?

> If you want a spinlock to protect the register access, it must surround
> the whole operation.

Between the time the hangcheck declares the gpu dead and the time we
deem it officially resurrected at the end of i915_reset there's no
issue with returning garbage from register writes - after all, the gpu
just went down.

The only thing we have to take care of is that we don't leave behind
an inconsistent state after i915_reset, which the current locking in
my patch takes care of.

Hence I think that no further protection is required.
-Daniel
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] The latest status of intel graphics on kernel 3.2

[Jesse Barnes]

On Wed, 4 Jan 2012 13:00:40 +
"Sun, Yi"  wrote:

> > -Original Message-
> > From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
> > Sent: Wednesday, January 04, 2012 6:53 AM
> > To: Sun, Yi
> > Cc: intel-gfx@lists.freedesktop.org; Yang, Guang A
> > Subject: Re: [Intel-gfx] The latest status of intel graphics on kernel 3.2
> > 
> > On Sat, 31 Dec 2011 08:41:34 +
> > "Sun, Yi"  wrote:
> > 
> > > The platforms we covered are IvyBridge, SandyBridge, IronLake,
> > > PineView and GM965 The version of Kernel:
> > > (drm-intel-next)64a742fac3a22f57303d8f1b7e347350a1c48254
> > >
> > > General speaking, the issue is mainly related to 3 pipe and eDP on 
> > > IvyBridge.
> > >
> > > 4 new bugs are filed:
> > > Bug 44250 -
> > > [IVB]"[drm:intel_dsm_platform_mux_info] *ERROR* MUX INFO call failed"
> > > while booting with monitor Bug
> > > 44304 - [ivb
> > > 3pipe] "*ERROR* failed to set mode on [CRTC:5]" while plug in the 3th
> > > monitor Bug 44305
> > > - [IVB]The Edp can't work while booting with a monitor Bug
> > > 44309 - [IVB eDP]
> > > 3 pipe doesn't work with eDP monitor
> > 
> > > Old bugs which still exists or just need code merge:
> > > Bug 42263 - [ILK]
> > > Plug in a monitor will make the eDP black screen
> > > 41976 [IVB] screen
> > > turn to be black while switching between console and x-window with
> > > 3-pipe active
> > > 41917 [IVB] all
> > > three screens are frozen while starting gnome-session with 3-pipe
> > > active 42731 [IVB]
> > > The whole monitor connected to DP port is black screen while hotplugin
> > > VGA
> > 
> > Just to confirm, these are fixed by
> >  [PATCH 1/2] drm/i915: don't disable a PCH DPLL that's in use and  [PATCH 
> > 2/2]
> > drm/i915: only set the intel_crtc DPMS mode to on if the
> >mode set succeeded
> > 
> > right?  Or are there other patches needed as well?  Can you reply to them
> > with your tested-by so Keith can pick them up?
> > 
> 
> Hi Jesse,
> 
>   I think I had replied the "[PATCH 2/2] drm/i915: only set the 
> intel_crtc DPMS mode  to on if the mode set succeeded". And I'll replay 
> it again.
> 
>   And confirm with you. When I tested about 3-pipe feature, I used your 
> branch "ivb-3-pipe", which contain 4 commits in all. And 2 patches for 
> Xserver, one is on the upstream and the other is attached. So that is to say, 
> 3 commits on branch "ivb-3-pipe" and a xserver patch are needed to put on the 
> upstream. Right?

Thanks, yeah I saw the reply.  Keith just needs to be poked a lot
sometimes to take bug fixes.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] gen7 missed IRQ workaround series.

[Daniel Vetter]

On Wed, Jan 04, 2012 at 09:05:51AM -0800, Keith Packard wrote:
> On Wed, 4 Jan 2012 18:02:21 +0100, Daniel Vetter  wrote:
> 
> > So I really don't like the busy-looping, especially now that
> > semaphores are enabled by default for ivb.
> 
> Alternatives are welcome; we have no other known fix for missing
> sequence writes.

See my patch, I'm pretty positive that I've successfully papered over it,
at least on my machine here. Also, Chris has posted an alternative using
irqs + a short timer as a backup a while ago. I like that much more.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Fan running with Intel Graphics

[Jesse Barnes]

On Thu, 15 Dec 2011 23:04:21 +0100
Johannes Bauer  wrote:

> Am 13.12.2011 22:46, schrieb Jesse Barnes:
> > On Tue, 13 Dec 2011 22:30:42 +0100
> > Johannes Bauer  wrote:
> >>> then echo that value into the max freq file, e.g.:
> >>>
> >>> $ echo 400 > /sys/kernel/debug/dri/0/i915_max_freq
> >>
> >> This doesn't work for
> >>
> >> joelaptop [/sys/kernel/debug/dri]: uname -a
> >> Linux joelaptop 3.0.0-14-generic #23-Ubuntu SMP Mon Nov 21 20:28:43 UTC
> >> 2011 x86_64 x86_64 x86_64 GNU/Linux
> >>
> >> Since i915_max_freq does not exist -- do I need to switch to a more
> >> recent kernel version?
> > 
> > Yes.  Try running drm-intel-next from Keith's git tree:
> > git://people.freedesktop.org/~keithp/linux
> 
> Hmmm:
> 
> joelaptop [~/intel-graphics]: git pull
> git://people.freedesktop.org/~keithp/linux
> fatal: Couldn't find remote ref HEAD
> 
> Is the server currently down? Can I fetch it somewhere else?

Just do a "git checkout drm-intel-next" at that point to get the dev
branch.

-- 
Jesse Barnes, Intel Open Source Technology Center


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/43] drm/i915: protect force_wake_(get|put) with the gt_lock

[Keith Packard]

On Wed, 4 Jan 2012 18:11:18 +0100, Daniel Vetter  wrote:

> Ah, I think I see you're concern: Between the time we reset the gpu
> and the time we fix up the forcewake state somebody might sneak in and
> see an inconstency between our tracking and the actual hw state, hence
> reading garbage. Correct?

Indeed. Plus, holding the spinlock across the whole operation also means
only taking it once, rather than twice. Spinlocks aren't free.

If we change the locking from struct_mutex to the spinlock, we should
actually make it work, independent of what access we have today.

-- 
keith.pack...@intel.com


pgpGsLvKRDnOn.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/43] drm/i915: protect force_wake_(get|put) with the gt_lock

[Daniel Vetter]

On Wed, Jan 04, 2012 at 09:54:08AM -0800, Keith Packard wrote:
> On Wed, 4 Jan 2012 18:11:18 +0100, Daniel Vetter  
> wrote:
> 
> > Ah, I think I see you're concern: Between the time we reset the gpu
> > and the time we fix up the forcewake state somebody might sneak in and
> > see an inconstency between our tracking and the actual hw state, hence
> > reading garbage. Correct?
> 
> Indeed. Plus, holding the spinlock across the whole operation also means
> only taking it once, rather than twice. Spinlocks aren't free.
> 
> If we change the locking from struct_mutex to the spinlock, we should
> actually make it work, independent of what access we have today.

The "Correct?" was just to check my understanding of your concern, I still
think its invalid. You've cut away the second part of my mail where I
explain why and I don't see you responding to that here. Also
micro-optimizing the gpu reset code sounds a bit strange.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: paper over missed irq issues with force wake vodoo

[Eugeni Dodonov]

On Wed, Jan 4, 2012 at 14:52, Daniel Vetter  wrote:

> Two things seem to do the trick on my ivb machine here:
> - prevent the gt from powering down while waiting for seqno
>  notification interrupts by grabbing the force_wake in get_irq (and
>  dropping it in put_irq again).
> - ordering writes from the ring's CS by reading a CS register, ACTHD
>  seems to work.
>
> Only the blt&bsd ring on ivb seem to be massively affected by this,
> but for paranoia do this dance also on the render ring and on snb
> (i.e. all gpus with forcewake).
>
> Tested with Eric's glCopyPixels loop which without this patch scores a
> missed irq every few seconds.
>
> This patch needs my forcewake rework to use a spinlock instead of
> dev->struct_mutex.
>
> Cc: sta...@kernel.org
> Cc: Eric Anholt 
> Cc: Kenneth Graunke 
> Cc: Eugeni Dodonov 
> Signed-Off-by: Daniel Vetter 
>

This voodoo magic works here, no more missed irqs on my IVBs.
Tested-by: Eugeni Dodonov 
Reviewed-by: Eugeni Dodonov 


>  static u32
> +gen6_ring_get_seqno(struct intel_ring_buffer *ring)
> +{
> +   /* Workaround for force correct ordering between irq and seqno
> writes on
> +* ivb (and maybe also on snb). */
> +   intel_ring_get_active_head(ring);
> +   return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
>


 - the comment could be rewritten to "Workaround for force correct ordering
between irq and seqno writes on ivb (and maybe also on snb) by reading a CS
register, like ACTHD, prior reading status page".
 - we could do a 'return ring_get_seqno(ring);' instead of 'return
intel_read_status_page(ring, I915_GEM_HWS_INDEX);' to reduce the Universe
entropy :).


-- 
Eugeni Dodonov

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: paper over missed irq issues with force wake vodoo

[Daniel Vetter]

Two things seem to do the trick on my ivb machine here:
- prevent the gt from powering down while waiting for seqno
  notification interrupts by grabbing the force_wake in get_irq (and
  dropping it in put_irq again).
- ordering writes from the ring's CS by reading a CS register, ACTHD
  seems to work.

Only the blt&bsd ring on ivb seem to be massively affected by this,
but for paranoia do this dance also on the render ring and on snb
(i.e. all gpus with forcewake).

Tested with Eric's glCopyPixels loop which without this patch scores a
missed irq every few seconds.

This patch needs my forcewake rework to use a spinlock instead of
dev->struct_mutex.

v2: Improve the comment per Eugeni Dodonov's suggestion.

Cc: sta...@kernel.org
Cc: Eric Anholt 
Cc: Kenneth Graunke 
Cc: Eugeni Dodonov 
Tested-by: Eugeni Dodonov 
Reviewed-by: Eugeni Dodonov 
Signed-Off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   22 --
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cf6e159..88499a0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -631,6 +631,16 @@ render_ring_add_request(struct intel_ring_buffer *ring,
 }
 
 static u32
+gen6_ring_get_seqno(struct intel_ring_buffer *ring)
+{
+   /* Workaround to force correct ordering between irq and seqno writes on
+* ivb (and maybe also on snb) by reading from a CS register (like
+* ACTHD) before reading the status page. */
+   intel_ring_get_active_head(ring);
+   return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
+}
+
+static u32
 ring_get_seqno(struct intel_ring_buffer *ring)
 {
return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
@@ -795,6 +805,11 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 
gflag, u32 rflag)
if (!dev->irq_enabled)
   return false;
 
+   /* It looks like we need to prevent the gt from suspending while waiting
+* for an notifiy irq, otherwise irqs seem to get lost on at least the
+* blt/bsd rings on ivb. */
+   gen6_gt_force_wake_get(dev_priv);
+
spin_lock(&ring->irq_lock);
if (ring->irq_refcount++ == 0) {
ring->irq_mask &= ~rflag;
@@ -819,6 +834,8 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 
gflag, u32 rflag)
ironlake_disable_irq(dev_priv, gflag);
}
spin_unlock(&ring->irq_lock);
+
+   gen6_gt_force_wake_put(dev_priv);
 }
 
 static bool
@@ -1329,7 +1346,7 @@ static const struct intel_ring_buffer gen6_bsd_ring = {
.write_tail = gen6_bsd_ring_write_tail,
.flush  = gen6_ring_flush,
.add_request= gen6_add_request,
-   .get_seqno  = ring_get_seqno,
+   .get_seqno  = gen6_ring_get_seqno,
.irq_get= gen6_bsd_ring_get_irq,
.irq_put= gen6_bsd_ring_put_irq,
.dispatch_execbuffer= gen6_ring_dispatch_execbuffer,
@@ -1388,7 +1405,7 @@ static const struct intel_ring_buffer gen6_blt_ring = {
.write_tail = ring_write_tail,
.flush  = blt_ring_flush,
.add_request= gen6_add_request,
-   .get_seqno  = ring_get_seqno,
+   .get_seqno  = gen6_ring_get_seqno,
.irq_get= blt_ring_get_irq,
.irq_put= blt_ring_put_irq,
.dispatch_execbuffer= gen6_ring_dispatch_execbuffer,
@@ -1410,6 +1427,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
ring->flush = gen6_render_ring_flush;
ring->irq_get = gen6_render_ring_get_irq;
ring->irq_put = gen6_render_ring_put_irq;
+   ring->get_seqno = gen6_ring_get_seqno;
} else if (IS_GEN5(dev)) {
ring->add_request = pc_render_add_request;
ring->get_seqno = pc_render_get_seqno;
-- 
1.7.7.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] Drivers for gma 4500m?

[jedim]

Are the graphic drivers for gma 4500m included in the intel-gfx release?
If no there is a future develop for it?


-- 
Luca Stancapiano
javaee consultant
skype: flashboss62
mobile: +393381584484
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] agp/intel: Add pci id for hostbridge from has/qemu

[Ben Widawsky]

This is needed to run the simulator.

Cc: Jesse Barnes 
Signed-off-by: Ben Widawsky 
---
 drivers/char/agp/intel-agp.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index b427711..f853514 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -850,6 +850,7 @@ static struct pci_device_id agp_intel_pci_table[] = {
.subvendor  = PCI_ANY_ID,   \
.subdevice  = PCI_ANY_ID,   \
}
+   ID(PCI_DEVICE_ID_INTEL_82441),
ID(PCI_DEVICE_ID_INTEL_82443LX_0),
ID(PCI_DEVICE_ID_INTEL_82443BX_0),
ID(PCI_DEVICE_ID_INTEL_82443GX_0),
-- 
1.7.8.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] agp/intel: Add pci id for hostbridge from has/qemu

[Daniel Vetter]

On Wed, Jan 04, 2012 at 02:04:33PM -0800, Ben Widawsky wrote:
> This is needed to run the simulator.
> 
> Cc: Jesse Barnes 
> Signed-off-by: Ben Widawsky 

I'll refrain from asking how you got here ...
Acked-by: Daniel Vetter 
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel: Update for new i915_drm.h defines.

[Eric Anholt]

On Wed, 4 Jan 2012 08:52:34 -0800, Jesse Barnes  
wrote:
> On Tue,  3 Jan 2012 19:10:00 -0800
> Eric Anholt  wrote:
> 
> > ---
> > 
> > This syncs the code up to the kernel as of the gen7 SOL changes.  It
> > would be nice if doing this was just a straight copy of the kernel
> > code -- there are two diffs left out.  One is this hunk:
> > 
> > -#ifdef __KERNEL__
> > -/* For use by IPS driver */
> > -extern unsigned long i915_read_mch_val(void);
> > -extern bool i915_gpu_raise(void);
> > -extern bool i915_gpu_lower(void);
> > -extern bool i915_gpu_busy(void);
> > -extern bool i915_gpu_turbo_disable(void);
> > -#endif
> > 
> > Does anyone see any real reason to be dropping this?
> > 
> > The other is removing the sparse __user annotations on the structs.
> > Could we just patch the kernel to #define it away for userland, so we
> > could make updating this file just a matter of "cp"?
> 
> Having just done this locally, I'm in favor.  The stuff under
> __KERNEL__ should be harmless; don't see why we can't just copy it.
> 
> For the __user define, we must be missing a compiler.h include
> somewhere in the libdrm case.  If you explicitly include it do the
> errors go away?

What version of POSIX is compiler.h part of?  Remember that this is
userland code built across OSes (as much as we try to forget that).


pgp4PZrWcmpAW.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] intel_decode for gen7 (finally!)

[Eric Anholt]

There was a logical split in this series at patch 8-9, but I figured I
should share the final result as well since people are probably
wondering why I care, and I'm hoping to land this all for the next
libdrm release (which we need to do for the updated header, which we
need for mesa).

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 03/17] intel: Drop the code for counting parsing failures.

[Eric Anholt]

Nothing was consuming it.  If something wants this in the future,
would be done using the decode context anyway.
---
 intel/intel_decode.c |   74 -
 1 files changed, 24 insertions(+), 50 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 61797e1..6345e87 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -81,7 +81,6 @@ static uint32_t tail_offset = 0x; /* undefined */
 #define BUFFER_FAIL(_count, _len, _name) do {  \
 fprintf(out, "Buffer size too small in %s (%d < %d)\n",\
(_name), (_count), (_len)); \
-(*failures)++; \
 return count;  \
 } while (0)
 
@@ -123,7 +122,7 @@ instr_out(uint32_t *data, uint32_t hw_offset, unsigned int 
index,
 }
 
 static int
-decode_mi(uint32_t *data, uint32_t count, uint32_t hw_offset, int *failures)
+decode_mi(uint32_t *data, uint32_t count, uint32_t hw_offset)
 {
unsigned int opcode, len = -1;
const char *post_sync_op = "";
@@ -264,7 +263,6 @@ decode_mi(uint32_t *data, uint32_t count, uint32_t 
hw_offset, int *failures)
}
 
instr_out(data, hw_offset, 0, "MI UNKNOWN\n");
-   (*failures)++;
return 1;
 }
 
@@ -311,7 +309,7 @@ static void decode_2d_br01(uint32_t *data, uint32_t count, 
uint32_t hw_offset)
 }
 
 static int
-decode_2d(uint32_t *data, uint32_t count, uint32_t hw_offset, int *failures)
+decode_2d(uint32_t *data, uint32_t count, uint32_t hw_offset)
 {
unsigned int opcode, len;
 
@@ -492,12 +490,11 @@ decode_2d(uint32_t *data, uint32_t count, uint32_t 
hw_offset, int *failures)
}
 
instr_out(data, hw_offset, 0, "2D UNKNOWN\n");
-   (*failures)++;
return 1;
 }
 
 static int
-decode_3d_1c(uint32_t *data, uint32_t count, uint32_t hw_offset, int *failures)
+decode_3d_1c(uint32_t *data, uint32_t count, uint32_t hw_offset)
 {
uint32_t opcode;
 
@@ -525,7 +522,6 @@ decode_3d_1c(uint32_t *data, uint32_t count, uint32_t 
hw_offset, int *failures)
 
instr_out(data, hw_offset, 0, "3D UNKNOWN: 3d_1c opcode = 0x%x\n",
  opcode);
-   (*failures)++;
return 1;
 }
 
@@ -1175,8 +1171,7 @@ decode_sample_filter(uint32_t mode)
 }
 
 static int
-decode_3d_1d(uint32_t *data, uint32_t count,
-uint32_t hw_offset, uint32_t devid, int *failures)
+decode_3d_1d(uint32_t *data, uint32_t count, uint32_t hw_offset, uint32_t 
devid)
 {
unsigned int len, i, c, idx, word, map, sampler, instr;
const char *format, *zformat, *type;
@@ -1263,7 +1258,6 @@ decode_3d_1d(uint32_t *data, uint32_t count,
}
if (len != i) {
fprintf(out, "Bad count in 3DSTATE_LOAD_INDIRECT\n");
-   (*failures)++;
return len;
}
return len;
@@ -1658,7 +1652,6 @@ decode_3d_1d(uint32_t *data, uint32_t count,
if (len != i) {
fprintf(out,
"Bad count in 
3DSTATE_LOAD_STATE_IMMEDIATE_1\n");
-   (*failures)++;
}
return len;
case 0x03:
@@ -1713,7 +1706,6 @@ decode_3d_1d(uint32_t *data, uint32_t count,
if (len != i) {
fprintf(out,
"Bad count in 
3DSTATE_LOAD_STATE_IMMEDIATE_2\n");
-   (*failures)++;
}
return len;
case 0x00:
@@ -1919,7 +1911,6 @@ decode_3d_1d(uint32_t *data, uint32_t count,
}
if (len != i) {
fprintf(out, "Bad count in 3DSTATE_MAP_STATE\n");
-   (*failures)++;
return len;
}
return len;
@@ -1951,7 +1942,6 @@ decode_3d_1d(uint32_t *data, uint32_t count,
if (len != i) {
fprintf(out,
"Bad count in 
3DSTATE_PIXEL_SHADER_CONSTANTS\n");
-   (*failures)++;
}
return len;
case 0x05:
@@ -1960,7 +1950,6 @@ decode_3d_1d(uint32_t *data, uint32_t count,
if ((len - 1) % 3 != 0 || len > 370) {
fprintf(out,
"Bad count in 3DSTATE_PIXEL_SHADER_PROGRAM\n");
-   (*failures)++;
}
i = 1;
for (instr = 0; instr < (len - 1) / 3; instr++) {
@@ -2043,7 +2032,6 @@ decode_3d_1d(uint32_t *data, uint32_t count,
}
if (len != i) {
fprintf(out, "Bad count in 3DSTATE_SAMPLER_STATE\n");
-   (*failures)++;
}
return len;
case 0x85:
@@ -2229,7 +2217,6 @@ decode_3d_1d(u

[Intel-gfx] [PATCH 02/17] intel: Track the current packet location in the decode context.

[Eric Anholt]

This is the start of plumbing the context through the decode
callchain instead of the current 4 arguments.
---
 intel/intel_decode.c  |   69 +++--
 intel/tests/gen7-3d.batch-ref.txt |   18 +-
 2 files changed, 52 insertions(+), 35 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 81ef712..61797e1 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -39,13 +39,20 @@ struct drm_intel_decode {
/** PCI device ID. */
uint32_t devid;
 
-   /** GPU address of the start of the batchbuffer data. */
+   /** GPU address of the start of the current packet. */
uint32_t hw_offset;
-   /** CPU Virtual address of the start of the batchbuffer data. */
+   /** CPU virtual address of the start of the current packet. */
uint32_t *data;
-   /** Number of DWORDs of batchbuffer data. */
+   /** DWORDs of remaining batchbuffer data starting from the packet. */
uint32_t count;
 
+   /** GPU address of the start of the batchbuffer data. */
+   uint32_t base_hw_offset;
+   /** CPU Virtual address of the start of the batchbuffer data. */
+   uint32_t *base_data;
+   /** Number of DWORDs of batchbuffer data. */
+   uint32_t base_count;
+
/** @{
 * GPU head and tail pointers, which will be noted in the dump, or ~0.
 */
@@ -3583,9 +3590,9 @@ void
 drm_intel_decode_set_batch_pointer(struct drm_intel_decode *ctx,
   void *data, uint32_t hw_offset, int count)
 {
-   ctx->data = data;
-   ctx->hw_offset = hw_offset;
-   ctx->count = count;
+   ctx->base_data = data;
+   ctx->base_hw_offset = hw_offset;
+   ctx->base_count = count;
 }
 
 void
@@ -3616,16 +3623,15 @@ drm_intel_decode(struct drm_intel_decode *ctx)
int ret;
unsigned int index = 0;
int failures = 0;
-   uint32_t *data;
-   uint32_t count, hw_offset;
uint32_t devid;
 
if (!ctx)
return;
 
-   data = ctx->data;
-   count = ctx->count;
-   hw_offset = ctx->hw_offset;
+   ctx->data = ctx->base_data;
+   ctx->hw_offset = ctx->base_hw_offset;
+   ctx->count = ctx->base_count;
+
devid = ctx->devid;
head_offset = ctx->head;
tail_offset = ctx->tail;
@@ -3634,11 +3640,13 @@ drm_intel_decode(struct drm_intel_decode *ctx)
saved_s2_set = 0;
saved_s4_set = 1;
 
-   while (index < count) {
-   switch ((data[index] & 0xe000) >> 29) {
+   while (ctx->count > 0) {
+   index = 0;
+
+   switch ((ctx->data[index] & 0xe000) >> 29) {
case 0x0:
-   ret = decode_mi(data + index, count - index,
-   hw_offset + index * 4, &failures);
+   ret = decode_mi(ctx->data, ctx->count,
+   ctx->hw_offset, &failures);
 
/* If MI_BATCHBUFFER_END happened, then dump
 * the rest of the output in case we some day
@@ -3650,9 +3658,10 @@ drm_intel_decode(struct drm_intel_decode *ctx)
if (ctx->dump_past_end) {
index++;
} else {
-   for (index = index + 1; index < count;
+   for (index = index + 1; index < 
ctx->count;
 index++) {
-   instr_out(data, hw_offset,
+   instr_out(ctx->data,
+ ctx->hw_offset,
  index, "\n");
}
}
@@ -3660,32 +3669,40 @@ drm_intel_decode(struct drm_intel_decode *ctx)
index += ret;
break;
case 0x2:
-   index += decode_2d(data + index, count - index,
-  hw_offset + index * 4, &failures);
+   index += decode_2d(ctx->data, ctx->count,
+  ctx->hw_offset, &failures);
break;
case 0x3:
if (IS_9XX(devid) && !IS_GEN3(devid)) {
index +=
-   decode_3d_965(data + index, count - index,
- hw_offset + index * 4, devid,
+   decode_3d_965(ctx->data, ctx->count,
+ ctx->hw_offset, devid,
  &failures);
} else if (IS_GEN3(devi

[Intel-gfx] [PATCH 04/17] intel: Plumb the context through the decode callchain.

[Eric Anholt]

We still deref the context at the start of every call, but that will
change next.
---
 intel/intel_decode.c |   70 +++--
 1 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 6345e87..be49b84 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -122,10 +122,13 @@ instr_out(uint32_t *data, uint32_t hw_offset, unsigned 
int index,
 }
 
 static int
-decode_mi(uint32_t *data, uint32_t count, uint32_t hw_offset)
+decode_mi(struct drm_intel_decode *ctx)
 {
unsigned int opcode, len = -1;
const char *post_sync_op = "";
+   uint32_t *data = ctx->data;
+   uint32_t hw_offset = ctx->hw_offset;
+   uint32_t count = ctx->count;
 
struct {
uint32_t opcode;
@@ -309,9 +312,12 @@ static void decode_2d_br01(uint32_t *data, uint32_t count, 
uint32_t hw_offset)
 }
 
 static int
-decode_2d(uint32_t *data, uint32_t count, uint32_t hw_offset)
+decode_2d(struct drm_intel_decode *ctx)
 {
unsigned int opcode, len;
+   uint32_t *data = ctx->data;
+   uint32_t hw_offset = ctx->hw_offset;
+   uint32_t count = ctx->count;
 
struct {
uint32_t opcode;
@@ -494,8 +500,10 @@ decode_2d(uint32_t *data, uint32_t count, uint32_t 
hw_offset)
 }
 
 static int
-decode_3d_1c(uint32_t *data, uint32_t count, uint32_t hw_offset)
+decode_3d_1c(struct drm_intel_decode *ctx)
 {
+   uint32_t *data = ctx->data;
+   uint32_t hw_offset = ctx->hw_offset;
uint32_t opcode;
 
opcode = (data[0] & 0x00f8) >> 19;
@@ -1171,11 +1179,15 @@ decode_sample_filter(uint32_t mode)
 }
 
 static int
-decode_3d_1d(uint32_t *data, uint32_t count, uint32_t hw_offset, uint32_t 
devid)
+decode_3d_1d(struct drm_intel_decode *ctx)
 {
unsigned int len, i, c, idx, word, map, sampler, instr;
const char *format, *zformat, *type;
uint32_t opcode;
+   uint32_t hw_offset = ctx->hw_offset;
+   uint32_t *data = ctx->data;
+   uint32_t count = ctx->count;
+   uint32_t devid = ctx->devid;
 
struct {
uint32_t opcode;
@@ -2237,8 +2249,11 @@ decode_3d_1d(uint32_t *data, uint32_t count, uint32_t 
hw_offset, uint32_t devid)
 }
 
 static int
-decode_3d_primitive(uint32_t *data, uint32_t count, uint32_t hw_offset)
+decode_3d_primitive(struct drm_intel_decode *ctx)
 {
+   uint32_t *data = ctx->data;
+   uint32_t hw_offset = ctx->hw_offset;
+   uint32_t count = ctx->count;
char immediate = (data[0] & (1 << 23)) == 0;
unsigned int len, i, j, ret;
const char *primtype;
@@ -2498,10 +2513,13 @@ out:
 }
 
 static int
-decode_3d(uint32_t *data, uint32_t count, uint32_t hw_offset, uint32_t devid)
+decode_3d(struct drm_intel_decode *ctx)
 {
uint32_t opcode;
unsigned int idx;
+   uint32_t *data = ctx->data;
+   uint32_t hw_offset = ctx->hw_offset;
+   uint32_t count = ctx->count;
 
struct {
uint32_t opcode;
@@ -2524,11 +2542,11 @@ decode_3d(uint32_t *data, uint32_t count, uint32_t 
hw_offset, uint32_t devid)
 
switch (opcode) {
case 0x1f:
-   return decode_3d_primitive(data, count, hw_offset);
+   return decode_3d_primitive(ctx);
case 0x1d:
-   return decode_3d_1d(data, count, hw_offset, devid);
+   return decode_3d_1d(ctx);
case 0x1c:
-   return decode_3d_1c(data, count, hw_offset);
+   return decode_3d_1c(ctx);
}
 
for (idx = 0; idx < ARRAY_SIZE(opcodes_3d); idx++) {
@@ -2758,13 +2776,16 @@ state_max_out(uint32_t *data, uint32_t hw_offset, 
unsigned int index,
 }
 
 static int
-decode_3d_965(uint32_t *data, uint32_t count, uint32_t hw_offset,
- uint32_t devid)
+decode_3d_965(struct drm_intel_decode *ctx)
 {
uint32_t opcode;
unsigned int idx, len;
unsigned int i, j, sba_len;
const char *desc1 = NULL;
+   uint32_t *data = ctx->data;
+   uint32_t hw_offset = ctx->hw_offset;
+   uint32_t count = ctx->count;
+   uint32_t devid = ctx->devid;
 
struct {
uint32_t opcode;
@@ -3466,11 +3487,13 @@ decode_3d_965(uint32_t *data, uint32_t count, uint32_t 
hw_offset,
 }
 
 static int
-decode_3d_i830(uint32_t *data, uint32_t count, uint32_t hw_offset,
-  uint32_t devid)
+decode_3d_i830(struct drm_intel_decode *ctx)
 {
unsigned int idx;
uint32_t opcode;
+   uint32_t *data = ctx->data;
+   uint32_t hw_offset = ctx->hw_offset;
+   uint32_t count = ctx->count;
 
struct {
uint32_t opcode;
@@ -3500,11 +3523,11 @@ decode_3d_i830(uint32_t *data, uint32_t count, uint32_t 
hw_offset,
 
switch (opcode) {
case 0x1f:
-   return decode_3d_primitive(data, count, hw_offset);
+   return decode_3d_primitive(ctx);
case 0x1d:
-   return 

[Intel-gfx] [PATCH 01/17] intel: Add a regression test for 2D decode, which I'm about to refactor.

[Eric Anholt]

---
 intel/Makefile.am  |1 +
 intel/tests/gen7-2d-copy.batch |  Bin 0 -> 56 bytes
 intel/tests/gen7-2d-copy.batch-ref.txt |   14 ++
 intel/tests/gen7-2d-copy.batch.sh  |1 +
 4 files changed, 16 insertions(+), 0 deletions(-)
 create mode 100644 intel/tests/gen7-2d-copy.batch
 create mode 100644 intel/tests/gen7-2d-copy.batch-ref.txt
 create mode 12 intel/tests/gen7-2d-copy.batch.sh

diff --git a/intel/Makefile.am b/intel/Makefile.am
index 581c8c0..7225a78 100644
--- a/intel/Makefile.am
+++ b/intel/Makefile.am
@@ -62,6 +62,7 @@ BATCHES = \
tests/gm45-3d.batch \
tests/gen5-3d.batch \
tests/gen6-3d.batch \
+   tests/gen7-2d-copy.batch \
tests/gen7-3d.batch
 
 TESTS = \
diff --git a/intel/tests/gen7-2d-copy.batch b/intel/tests/gen7-2d-copy.batch
new file mode 100644
index 
..ce7fc292b5d01010762d64d44098ae3c6dbdafc4
GIT binary patch
literal 56
qcmZQ;_z*II@eDHq5TpPh!vsAc5W4|H2>fSaVqg#kF_8c(hz$T0j|Of4

literal 0
HcmV?d1

diff --git a/intel/tests/gen7-2d-copy.batch-ref.txt 
b/intel/tests/gen7-2d-copy.batch-ref.txt
new file mode 100644
index 000..0d621d3
--- /dev/null
+++ b/intel/tests/gen7-2d-copy.batch-ref.txt
@@ -0,0 +1,14 @@
+0x1230:  0x54f08006: XY_SRC_COPY_BLT (rgb enabled, alpha enabled, src 
tile 1, dst tile 0)
+0x1234:  0x03cc0190:format , pitch 400, rop 0xcc, clipping 
disabled,  
+0x1238:  0x:dst (0,0)
+0x123c:  0x00640064:dst (100,100)
+0x12300010:  0x122e9000:dst offset 0x122e9000
+0x12300014:  0x:src (0,0)
+0x12300018:  0x0080:src pitch 128
+0x1230001c:  0x02ff1000:src offset 0x02ff1000
+0x12300020:  0x1302: MI_FLUSH_DW post_sync_op='no write' 
+0x12300024:  0x:address
+0x12300028:  0x:dword
+0x1230002c:  0x:upper dword
+0x12300030:  0x0500: MI_BATCH_BUFFER_END
+0x12300034:  0x:
diff --git a/intel/tests/gen7-2d-copy.batch.sh 
b/intel/tests/gen7-2d-copy.batch.sh
new file mode 12
index 000..796ca5f
--- /dev/null
+++ b/intel/tests/gen7-2d-copy.batch.sh
@@ -0,0 +1 @@
+test-batch.sh
\ No newline at end of file
-- 
1.7.7.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 05/17] intel: Use the context to simplify BR00 decode.

[Eric Anholt]

The count (actually index) was always 0, because BR00 is dword 0.
---
 intel/intel_decode.c |   23 +++
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index be49b84..5a4246d 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -270,15 +270,15 @@ decode_mi(struct drm_intel_decode *ctx)
 }
 
 static void
-decode_2d_br00(uint32_t *data, uint32_t count, uint32_t hw_offset,
-  const char *cmd)
+decode_2d_br00(struct drm_intel_decode *ctx, const char *cmd)
 {
-   instr_out(data, hw_offset, 0,
+   instr_out(ctx->data, ctx->hw_offset, 0,
  "%s (rgb %sabled, alpha %sabled, src tile %d, dst tile %d)\n",
  cmd,
- (data[count] & (1 << 20)) ? "en" : "dis",
- (data[count] & (1 << 21)) ? "en" : "dis",
- (data[count] >> 15) & 1, (data[count] >> 11) & 1);
+ (ctx->data[0] & (1 << 20)) ? "en" : "dis",
+ (ctx->data[0] & (1 << 21)) ? "en" : "dis",
+ (ctx->data[0] >> 15) & 1,
+ (ctx->data[0] >> 11) & 1);
 }
 
 static void decode_2d_br01(uint32_t *data, uint32_t count, uint32_t hw_offset)
@@ -371,7 +371,7 @@ decode_2d(struct drm_intel_decode *ctx)
  data[2] & 0x, data[2] >> 16);
return len;
case 0x01:
-   decode_2d_br00(data, 0, hw_offset, "XY_SETUP_BLT");
+   decode_2d_br00(ctx, "XY_SETUP_BLT");
 
len = (data[0] & 0x00ff) + 2;
if (len != 8)
@@ -391,7 +391,7 @@ decode_2d(struct drm_intel_decode *ctx)
instr_out(data, hw_offset, 7, "color pattern offset\n");
return len;
case 0x03:
-   decode_2d_br00(data, 0, hw_offset, "XY_SETUP_CLIP_BLT");
+   decode_2d_br00(ctx, "XY_SETUP_CLIP_BLT");
 
len = (data[0] & 0x00ff) + 2;
if (len != 3)
@@ -405,8 +405,7 @@ decode_2d(struct drm_intel_decode *ctx)
  data[2] & 0x, data[3] >> 16);
return len;
case 0x11:
-   decode_2d_br00(data, 0, hw_offset,
-  "XY_SETUP_MONO_PATTERN_SL_BLT");
+   decode_2d_br00(ctx, "XY_SETUP_MONO_PATTERN_SL_BLT");
 
len = (data[0] & 0x00ff) + 2;
if (len != 9)
@@ -428,7 +427,7 @@ decode_2d(struct drm_intel_decode *ctx)
instr_out(data, hw_offset, 8, "mono pattern dw1\n");
return len;
case 0x50:
-   decode_2d_br00(data, 0, hw_offset, "XY_COLOR_BLT");
+   decode_2d_br00(ctx, "XY_COLOR_BLT");
 
len = (data[0] & 0x00ff) + 2;
if (len != 6)
@@ -445,7 +444,7 @@ decode_2d(struct drm_intel_decode *ctx)
instr_out(data, hw_offset, 5, "color\n");
return len;
case 0x53:
-   decode_2d_br00(data, 0, hw_offset, "XY_SRC_COPY_BLT");
+   decode_2d_br00(ctx, "XY_SRC_COPY_BLT");
 
len = (data[0] & 0x00ff) + 2;
if (len != 8)
-- 
1.7.7.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 06/17] intel: Use the context to simplify BR01 decode.

[Eric Anholt]

Similar to BR00, count was always 1 and was always an index, not a count.
---
 intel/intel_decode.c |   26 ++
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 5a4246d..0c3ec3c 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -281,10 +281,11 @@ decode_2d_br00(struct drm_intel_decode *ctx, const char 
*cmd)
  (ctx->data[0] >> 11) & 1);
 }
 
-static void decode_2d_br01(uint32_t *data, uint32_t count, uint32_t hw_offset)
+static void
+decode_2d_br01(struct drm_intel_decode *ctx)
 {
const char *format;
-   switch ((data[count] >> 24) & 0x3) {
+   switch ((ctx->data[1] >> 24) & 0x3) {
case 0:
format = "8";
break;
@@ -299,14 +300,15 @@ static void decode_2d_br01(uint32_t *data, uint32_t 
count, uint32_t hw_offset)
break;
}
 
-   instr_out(data, hw_offset, count, "format %s, pitch %d, rop 0x%02x, "
+   instr_out(ctx->data, ctx->hw_offset, 1,
+ "format %s, pitch %d, rop 0x%02x, "
  "clipping %sabled, %s%s \n",
  format,
- (short)(data[count] & 0x),
- (data[count] >> 16) & 0xff,
- data[count] & (1 << 30) ? "en" : "dis",
- data[count] & (1 << 31) ? "solid pattern enabled, " : "",
- data[count] & (1 << 31) ?
+ (short)(ctx->data[1] & 0x),
+ (ctx->data[1] >> 16) & 0xff,
+ ctx->data[1] & (1 << 30) ? "en" : "dis",
+ ctx->data[1] & (1 << 31) ? "solid pattern enabled, " : "",
+ ctx->data[1] & (1 << 31) ?
  "mono pattern transparency enabled, " : "");
 
 }
@@ -379,7 +381,7 @@ decode_2d(struct drm_intel_decode *ctx)
if (count < 8)
BUFFER_FAIL(count, len, "XY_SETUP_BLT");
 
-   decode_2d_br01(data, 1, hw_offset);
+   decode_2d_br01(ctx);
instr_out(data, hw_offset, 2, "cliprect (%d,%d)\n",
  data[2] & 0x, data[2] >> 16);
instr_out(data, hw_offset, 3, "cliprect (%d,%d)\n",
@@ -414,7 +416,7 @@ decode_2d(struct drm_intel_decode *ctx)
if (count < 9)
BUFFER_FAIL(count, len, "XY_SETUP_MONO_PATTERN_SL_BLT");
 
-   decode_2d_br01(data, 1, hw_offset);
+   decode_2d_br01(ctx);
instr_out(data, hw_offset, 2, "cliprect (%d,%d)\n",
  data[2] & 0x, data[2] >> 16);
instr_out(data, hw_offset, 3, "cliprect (%d,%d)\n",
@@ -435,7 +437,7 @@ decode_2d(struct drm_intel_decode *ctx)
if (count < 6)
BUFFER_FAIL(count, len, "XY_COLOR_BLT");
 
-   decode_2d_br01(data, 1, hw_offset);
+   decode_2d_br01(ctx);
instr_out(data, hw_offset, 2, "(%d,%d)\n",
  data[2] & 0x, data[2] >> 16);
instr_out(data, hw_offset, 3, "(%d,%d)\n",
@@ -452,7 +454,7 @@ decode_2d(struct drm_intel_decode *ctx)
if (count < 8)
BUFFER_FAIL(count, len, "XY_SRC_COPY_BLT");
 
-   decode_2d_br01(data, 1, hw_offset);
+   decode_2d_br01(ctx);
instr_out(data, hw_offset, 2, "dst (%d,%d)\n",
  data[2] & 0x, data[2] >> 16);
instr_out(data, hw_offset, 3, "dst (%d,%d)\n",
-- 
1.7.7.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 11/17] intel: Add support for parsing 965 3d packets using helper functions.

[Eric Anholt]

I want to add packets, without contributing to the switch statement of
doom.
---
 intel/intel_decode.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 864d802..1f01b11 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -2719,6 +2719,7 @@ decode_3d_965(struct drm_intel_decode *ctx)
int unsigned max_len;
const char *name;
int gen;
+   int (*func)(struct drm_intel_decode *ctx);
} opcodes_3d[] = {
{ 0x6000, 3, 3, "URB_FENCE" },
{ 0x6001, 2, 2, "CS_URB_STATE" },
@@ -3341,7 +3342,12 @@ decode_3d_965(struct drm_intel_decode *ctx)
if (opcode_3d->gen && opcode_3d->gen != ctx->gen)
continue;
 
-   if ((data[0] & 0x) >> 16 == opcode_3d->opcode) {
+   if ((data[0] & 0x) >> 16 != opcode_3d->opcode)
+   continue;
+
+   if (opcode_3d->func) {
+   return opcode_3d->func(ctx);
+   } else {
unsigned int i;
len = 1;
 
-- 
1.7.7.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 12/17] intel: Move the logic for getting 965 3d packet length to the packet table.

[Eric Anholt]

While I'm touching every line of the table, sort it by opcode.
---
 intel/intel_decode.c |  156 +++--
 1 files changed, 73 insertions(+), 83 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 1f01b11..1519f6a 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -2707,7 +2707,7 @@ static int
 decode_3d_965(struct drm_intel_decode *ctx)
 {
uint32_t opcode;
-   unsigned int idx, len;
+   unsigned int len;
unsigned int i, j, sba_len;
const char *desc1 = NULL;
uint32_t *data = ctx->data;
@@ -2715,62 +2715,83 @@ decode_3d_965(struct drm_intel_decode *ctx)
 
struct {
uint32_t opcode;
+   uint32_t len_mask;
int unsigned min_len;
int unsigned max_len;
const char *name;
int gen;
int (*func)(struct drm_intel_decode *ctx);
} opcodes_3d[] = {
-   { 0x6000, 3, 3, "URB_FENCE" },
-   { 0x6001, 2, 2, "CS_URB_STATE" },
-   { 0x6002, 2, 2, "CONSTANT_BUFFER" },
-   { 0x6101, 6, 6, "STATE_BASE_ADDRESS" },
-   { 0x6102, 2, 2, "STATE_SIP" },
-   { 0x6104, 1, 1, "3DSTATE_PIPELINE_SELECT" },
-   { 0x680b, 1, 1, "3DSTATE_VF_STATISTICS" },
-   { 0x6904, 1, 1, "3DSTATE_PIPELINE_SELECT" },
-   { 0x7800, 7, 7, "3DSTATE_PIPELINED_POINTERS" },
-   { 0x7801, 6, 6, "3DSTATE_BINDING_TABLE_POINTERS" },
-   { 0x7808, 5, 257, "3DSTATE_VERTEX_BUFFERS" },
-   { 0x7809, 3, 256, "3DSTATE_VERTEX_ELEMENTS" },
-   { 0x780a, 3, 3, "3DSTATE_INDEX_BUFFER" },
-   { 0x780b, 1, 1, "3DSTATE_VF_STATISTICS" },
-   { 0x7900, 4, 4, "3DSTATE_DRAWING_RECTANGLE" },
-   { 0x7901, 5, 5, "3DSTATE_CONSTANT_COLOR" },
-   { 0x7905, 5, 7, "3DSTATE_DEPTH_BUFFER" },
-   { 0x7906, 2, 2, "3DSTATE_POLY_STIPPLE_OFFSET" },
-   { 0x7907, 33, 33, "3DSTATE_POLY_STIPPLE_PATTERN" },
-   { 0x7908, 3, 3, "3DSTATE_LINE_STIPPLE" },
-   { 0x7909, 2, 2, "3DSTATE_GLOBAL_DEPTH_OFFSET_CLAMP" },
-   { 0x7909, 2, 2, "3DSTATE_CLEAR_PARAMS" },
-   { 0x790a, 3, 3, "3DSTATE_AA_LINE_PARAMETERS" },
-   { 0x790b, 4, 4, "3DSTATE_GS_SVB_INDEX" },
-   { 0x790d, 3, 3, "3DSTATE_MULTISAMPLE", 6 },
-   { 0x790d, 4, 4, "3DSTATE_MULTISAMPLE", 7 },
-   { 0x7910, 2, 2, "3DSTATE_CLEAR_PARAMS" },
-   { 0x7b00, 6, 6, "3DPRIMITIVE" },
-   { 0x7802, 4, 4, "3DSTATE_SAMPLER_STATE_POINTERS" },
-   { 0x7805, 3, 3, "3DSTATE_URB" },
-   { 0x780d, 4, 4, "3DSTATE_VIEWPORT_STATE_POINTERS" },
-   { 0x780e, 4, 4, "3DSTATE_CC_STATE_POINTERS" },
-   { 0x780f, 2, 2, "3DSTATE_SCISSOR_STATE_POINTERS" },
-   { 0x7810, 6, 6, "3DSTATE_VS_STATE" },
-   { 0x7811, 7, 7, "3DSTATE_GS_STATE" },
-   { 0x7812, 4, 4, "3DSTATE_CLIP_STATE" },
-   { 0x7813, 20, 20, "3DSTATE_SF_STATE" },
-   { 0x7814, 9, 9, "3DSTATE_WM_STATE" },
-   { 0x7815, 5, 5, "3DSTATE_CONSTANT_VS_STATE" },
-   { 0x7816, 5, 5, "3DSTATE_CONSTANT_GS_STATE" },
-   { 0x7817, 5, 5, "3DSTATE_CONSTANT_PS_STATE" },
-   { 0x7818, 2, 2, "3DSTATE_SAMPLE_MASK"},
-   }, *opcode_3d;
-
-   len = (data[0] & 0x) + 2;
+   { 0x6000, 0x00ff, 3, 3, "URB_FENCE" },
+   { 0x6001, 0x, 2, 2, "CS_URB_STATE" },
+   { 0x6002, 0x00ff, 2, 2, "CONSTANT_BUFFER" },
+   { 0x6101, 0x, 6, 6, "STATE_BASE_ADDRESS" },
+   { 0x6102, 0x, 2, 2, "STATE_SIP" },
+   { 0x6104, 0x, 1, 1, "3DSTATE_PIPELINE_SELECT" },
+   { 0x680b, 0x, 1, 1, "3DSTATE_VF_STATISTICS" },
+   { 0x6904, 0x, 1, 1, "3DSTATE_PIPELINE_SELECT" },
+   { 0x7800, 0x, 7, 7, "3DSTATE_PIPELINED_POINTERS" },
+   { 0x7801, 0x00ff, 6, 6, "3DSTATE_BINDING_TABLE_POINTERS" },
+   { 0x7802, 0x00ff, 4, 4, "3DSTATE_SAMPLER_STATE_POINTERS" },
+   { 0x7805, 0x00ff, 3, 3, "3DSTATE_URB" },
+   { 0x7808, 0x00ff, 5, 257, "3DSTATE_VERTEX_BUFFERS" },
+   { 0x7809, 0x00ff, 3, 256, "3DSTATE_VERTEX_ELEMENTS" },
+   { 0x780a, 0x00ff, 3, 3, "3DSTATE_INDEX_BUFFER" },
+   { 0x780b, 0x, 1, 1, "3DSTATE_VF_STATISTICS" },
+   { 0x780d, 0x00ff, 4, 4, "3DSTATE_VIEWPORT_STATE_POINTERS" },
+   { 0x780e, 0x, 4, 4, "3DSTATE_CC_STATE_POINTERS" },
+   { 0x780f, 0x00ff, 2, 2, "3DSTATE_SCISSOR_POINTERS" },
+   { 0x7810, 0x00ff, 6, 6, "3DSTATE_VS" },
+   { 0x7811, 0x00ff, 7, 7, "3DSTATE_GS" },
+   { 0x7812, 0x00ff, 4, 4, "3DSTATE_CLIP" },
+   

[Intel-gfx] [PATCH 10/17] intel: Parse the correct length for gen7 3DSTATE_MULTISAMPLE.

[Eric Anholt]

---
 intel/intel_decode.c  |3 ++-
 intel/tests/gen7-3d.batch-ref.txt |1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 3548f57..864d802 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -2744,7 +2744,8 @@ decode_3d_965(struct drm_intel_decode *ctx)
{ 0x7909, 2, 2, "3DSTATE_CLEAR_PARAMS" },
{ 0x790a, 3, 3, "3DSTATE_AA_LINE_PARAMETERS" },
{ 0x790b, 4, 4, "3DSTATE_GS_SVB_INDEX" },
-   { 0x790d, 3, 3, "3DSTATE_MULTISAMPLE" },
+   { 0x790d, 3, 3, "3DSTATE_MULTISAMPLE", 6 },
+   { 0x790d, 4, 4, "3DSTATE_MULTISAMPLE", 7 },
{ 0x7910, 2, 2, "3DSTATE_CLEAR_PARAMS" },
{ 0x7b00, 6, 6, "3DPRIMITIVE" },
{ 0x7802, 4, 4, "3DSTATE_SAMPLER_STATE_POINTERS" },
diff --git a/intel/tests/gen7-3d.batch-ref.txt 
b/intel/tests/gen7-3d.batch-ref.txt
index 953b7bd..1f68a4b 100644
--- a/intel/tests/gen7-3d.batch-ref.txt
+++ b/intel/tests/gen7-3d.batch-ref.txt
@@ -1,6 +1,5 @@
 0x1230:  0x6904: 3DSTATE_PIPELINE_SELECT
 0x1234:  0x790d0002: 3DSTATE_MULTISAMPLE
-Bad count in 3DSTATE_MULTISAMPLE
 0x1238:  0x:dword 1
 0x123c:  0x:dword 2
 0x12300010:  0x:dword 3
-- 
1.7.7.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 09/17] intel: Put the "gen" shorthand chipset identifier in the context.

[Eric Anholt]

It's a lot nicer than using IS_WHATEVER(devid) all over the place, and
we have this in our other projects too.
---
 intel/intel_decode.c |   28 
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 97820b3..3548f57 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -21,6 +21,7 @@
  * IN THE SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -39,6 +40,12 @@ struct drm_intel_decode {
/** PCI device ID. */
uint32_t devid;
 
+   /**
+* Shorthand device identifier: 3 is 915, 4 is 965, 5 is
+* Ironlake, etc.
+*/
+   int gen;
+
/** GPU address of the start of the current packet. */
uint32_t hw_offset;
/** CPU virtual address of the start of the current packet. */
@@ -2711,6 +2718,7 @@ decode_3d_965(struct drm_intel_decode *ctx)
int unsigned min_len;
int unsigned max_len;
const char *name;
+   int gen;
} opcodes_3d[] = {
{ 0x6000, 3, 3, "URB_FENCE" },
{ 0x6001, 2, 2, "CS_URB_STATE" },
@@ -3327,6 +3335,11 @@ decode_3d_965(struct drm_intel_decode *ctx)
 
for (idx = 0; idx < ARRAY_SIZE(opcodes_3d); idx++) {
opcode_3d = &opcodes_3d[idx];
+
+   /* If it's marked as only for a specific gen, skip. */
+   if (opcode_3d->gen && opcode_3d->gen != ctx->gen)
+   continue;
+
if ((data[0] & 0x) >> 16 == opcode_3d->opcode) {
unsigned int i;
len = 1;
@@ -3434,6 +3447,21 @@ drm_intel_decode_context_alloc(uint32_t devid)
ctx->devid = devid;
ctx->out = stdout;
 
+   if (IS_GEN7(devid))
+   ctx->gen = 7;
+   else if (IS_GEN6(devid))
+   ctx->gen = 6;
+   else if (IS_GEN5(devid))
+   ctx->gen = 5;
+   else if (IS_GEN4(devid))
+   ctx->gen = 4;
+   else if (IS_9XX(devid))
+   ctx->gen = 3;
+   else {
+   assert(IS_GEN2(devid));
+   ctx->gen = 2;
+   }
+
return ctx;
 }
 
-- 
1.7.7.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 08/17] intel: Avoid the need for most overflow checks by using a scratch page.

[Eric Anholt]

The overflow checks were all thoroughly untested, and a bunch of the
ones I'm deleting were pretty broken.  Now, in the case of overflow,
you just decode data of 0xd0d0d0d0, and instr_out prints the warning
message instead.  Note that this still has the same issue of being
under-tested, but at least it's one place instead of per-packet.

A couple of BUFFER_FAIL uses are left where the length to be decoded
could be (significantly) larger than a page, and the decode didn't
just call instr_out (which doesn't dereference data itself unless it's
safe).
---
 intel/intel_decode.c |  165 +++--
 1 files changed, 24 insertions(+), 141 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 36caba7..97820b3 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -66,6 +66,8 @@ struct drm_intel_decode {
 * and is used by the intel-gpu-tools.
 */
bool dump_past_end;
+
+   bool overflowed;
 };
 
 static FILE *out;
@@ -107,6 +109,14 @@ instr_out(struct drm_intel_decode *ctx, unsigned int index,
const char *parseinfo;
uint32_t offset = ctx->hw_offset + index * 4;
 
+   if (index > ctx->count) {
+   if (!ctx->overflowed) {
+   fprintf(out, "ERROR: Decode attempted to continue 
beyond end of batchbuffer\n");
+   ctx->overflowed = true;
+   }
+   return;
+   }
+
if (offset == head_offset)
parseinfo = "HEAD";
else if (offset == tail_offset)
@@ -127,7 +137,6 @@ decode_mi(struct drm_intel_decode *ctx)
unsigned int opcode, len = -1;
const char *post_sync_op = "";
uint32_t *data = ctx->data;
-   uint32_t count = ctx->count;
 
struct {
uint32_t opcode;
@@ -254,9 +263,6 @@ decode_mi(struct drm_intel_decode *ctx)
instr_out(ctx, 0, "%s\n",
  opcodes_mi[opcode].name);
for (i = 1; i < len; i++) {
-   if (i >= count)
-   BUFFER_FAIL(count, len,
-   opcodes_mi[opcode].name);
instr_out(ctx, i, "dword %d\n", i);
}
 
@@ -317,7 +323,6 @@ decode_2d(struct drm_intel_decode *ctx)
 {
unsigned int opcode, len;
uint32_t *data = ctx->data;
-   uint32_t count = ctx->count;
 
struct {
uint32_t opcode;
@@ -362,8 +367,6 @@ decode_2d(struct drm_intel_decode *ctx)
len = (data[0] & 0x00ff) + 2;
if (len != 3)
fprintf(out, "Bad count in XY_SCANLINES_BLT\n");
-   if (count < 3)
-   BUFFER_FAIL(count, len, "XY_SCANLINES_BLT");
 
instr_out(ctx, 1, "dest (%d,%d)\n",
  data[1] & 0x, data[1] >> 16);
@@ -376,8 +379,6 @@ decode_2d(struct drm_intel_decode *ctx)
len = (data[0] & 0x00ff) + 2;
if (len != 8)
fprintf(out, "Bad count in XY_SETUP_BLT\n");
-   if (count < 8)
-   BUFFER_FAIL(count, len, "XY_SETUP_BLT");
 
decode_2d_br01(ctx);
instr_out(ctx, 2, "cliprect (%d,%d)\n",
@@ -396,8 +397,6 @@ decode_2d(struct drm_intel_decode *ctx)
len = (data[0] & 0x00ff) + 2;
if (len != 3)
fprintf(out, "Bad count in XY_SETUP_CLIP_BLT\n");
-   if (count < 3)
-   BUFFER_FAIL(count, len, "XY_SETUP_CLIP_BLT");
 
instr_out(ctx, 1, "cliprect (%d,%d)\n",
  data[1] & 0x, data[2] >> 16);
@@ -411,8 +410,6 @@ decode_2d(struct drm_intel_decode *ctx)
if (len != 9)
fprintf(out,
"Bad count in XY_SETUP_MONO_PATTERN_SL_BLT\n");
-   if (count < 9)
-   BUFFER_FAIL(count, len, "XY_SETUP_MONO_PATTERN_SL_BLT");
 
decode_2d_br01(ctx);
instr_out(ctx, 2, "cliprect (%d,%d)\n",
@@ -432,8 +429,6 @@ decode_2d(struct drm_intel_decode *ctx)
len = (data[0] & 0x00ff) + 2;
if (len != 6)
fprintf(out, "Bad count in XY_COLOR_BLT\n");
-   if (count < 6)
-   BUFFER_FAIL(count, len, "XY_COLOR_BLT");
 
decode_2d_br01(ctx);
instr_out(ctx, 2, "(%d,%d)\n",
@@ -449,8 +444,6 @@ decode_2d(struct drm_intel_decode *ctx)
len = (data[0] & 0x00ff) + 2;
if (len != 8)
fprintf(out, "Bad count in XY_SRC_COPY_BLT\n");
-   if (count < 8)
-   BUFFER_FAIL(count, len, "XY_SRC_COPY_BLT");
 
decode_2d_br01(ctx);
   

[Intel-gfx] [PATCH 13/17] intel: Make most of the logic for 965 3d packet length checks table-driven.

[Eric Anholt]

This puts the error message in a consistent location relative to the
packet, and while I'm here I made the error message a bit more
informative.

Now, most static length packets need to just declare their length in
the table and not worry.
---
 intel/intel_decode.c  |   55 +++-
 intel/tests/gen7-3d.batch-ref.txt |  128 ++--
 2 files changed, 73 insertions(+), 110 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 1519f6a..e1243ee 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -2725,13 +2725,13 @@ decode_3d_965(struct drm_intel_decode *ctx)
{ 0x6000, 0x00ff, 3, 3, "URB_FENCE" },
{ 0x6001, 0x, 2, 2, "CS_URB_STATE" },
{ 0x6002, 0x00ff, 2, 2, "CONSTANT_BUFFER" },
-   { 0x6101, 0x, 6, 6, "STATE_BASE_ADDRESS" },
+   { 0x6101, 0x, 6, 10, "STATE_BASE_ADDRESS" },
{ 0x6102, 0x, 2, 2, "STATE_SIP" },
{ 0x6104, 0x, 1, 1, "3DSTATE_PIPELINE_SELECT" },
{ 0x680b, 0x, 1, 1, "3DSTATE_VF_STATISTICS" },
{ 0x6904, 0x, 1, 1, "3DSTATE_PIPELINE_SELECT" },
{ 0x7800, 0x, 7, 7, "3DSTATE_PIPELINED_POINTERS" },
-   { 0x7801, 0x00ff, 6, 6, "3DSTATE_BINDING_TABLE_POINTERS" },
+   { 0x7801, 0x00ff, 4, 6, "3DSTATE_BINDING_TABLE_POINTERS" },
{ 0x7802, 0x00ff, 4, 4, "3DSTATE_SAMPLER_STATE_POINTERS" },
{ 0x7805, 0x00ff, 3, 3, "3DSTATE_URB" },
{ 0x7808, 0x00ff, 5, 257, "3DSTATE_VERTEX_BUFFERS" },
@@ -2786,6 +2786,13 @@ decode_3d_965(struct drm_intel_decode *ctx)
len = 1;
else
len = (data[0] & opcode_3d->len_mask) + 2;
+
+   if (len < opcode_3d->min_len ||
+   len > opcode_3d->max_len) {
+   fprintf(out, "Bad length %d in %s, expeted %d-%d\n",
+   len, opcode_3d->name,
+   opcode_3d->min_len, opcode_3d->max_len);
+   }
} else {
len = (data[0] & 0x) + 2;
}
@@ -2838,10 +2845,6 @@ decode_3d_965(struct drm_intel_decode *ctx)
 
return len;
case 0x7800:
-   if (len != 7)
-   fprintf(out,
-   "Bad count in 3DSTATE_PIPELINED_POINTERS\n");
-
instr_out(ctx, 0, "3DSTATE_PIPELINED_POINTERS\n");
instr_out(ctx, 1, "VS state\n");
instr_out(ctx, 2, "GS state\n");
@@ -2876,9 +2879,6 @@ decode_3d_965(struct drm_intel_decode *ctx)
 
return len;
case 0x7802:
-   if (len != 4)
-   fprintf(out,
-   "Bad count in 
3DSTATE_SAMPLER_STATE_POINTERS\n");
instr_out(ctx, 0,
  "3DSTATE_SAMPLER_STATE_POINTERS: VS mod %d, "
  "GS mod %d, PS mod %d\n", (data[0] & (1 << 8)) != 0,
@@ -2889,8 +2889,6 @@ decode_3d_965(struct drm_intel_decode *ctx)
instr_out(ctx, 3, "WM sampler state\n");
return len;
case 0x7805:
-   if (len != 3)
-   fprintf(out, "Bad count in 3DSTATE_URB\n");
instr_out(ctx, 0, "3DSTATE_URB\n");
instr_out(ctx, 1,
  "VS entries %d, alloc size %d (1024bit row)\n",
@@ -2951,9 +2949,6 @@ decode_3d_965(struct drm_intel_decode *ctx)
return len;
 
case 0x780d:
-   if (len != 4)
-   fprintf(out,
-   "Bad count in 
3DSTATE_VIEWPORT_STATE_POINTERS\n");
instr_out(ctx, 0,
  "3DSTATE_VIEWPORT_STATE_POINTERS\n");
instr_out(ctx, 1, "clip\n");
@@ -2962,17 +2957,12 @@ decode_3d_965(struct drm_intel_decode *ctx)
return len;
 
case 0x780a:
-   if (len != 3)
-   fprintf(out, "Bad count in 3DSTATE_INDEX_BUFFER\n");
instr_out(ctx, 0, "3DSTATE_INDEX_BUFFER\n");
instr_out(ctx, 1, "beginning buffer address\n");
instr_out(ctx, 2, "ending buffer address\n");
return len;
 
case 0x780e:
-   if (len != 4)
-   fprintf(out,
-   "Bad count in 3DSTATE_CC_STATE_POINTERS\n");
instr_out(ctx, 0, "3DSTATE_CC_STATE_POINTERS\n");
instr_out(ctx, 1, "blend change %d\n", data[1] & 1);
instr_out(ctx, 2, "depth stencil change %d\n",
@@ -2981,15 +2971,11 @@ decode_3d_965(struct drm_intel_decode *ctx)
return len;
 
case 0x780f:
-   if (len != 2)
-   fprintf(out, "Bad count in 3DSTATE_SCISSOR_POINTERS\n");
ins

[Intel-gfx] [PATCH 14/17] intel: Add support for parsing gen7 URB packets.

[Eric Anholt]

---
 intel/intel_decode.c  |   44 ++
 intel/tests/gen7-3d.batch-ref.txt |  112 ++--
 2 files changed, 100 insertions(+), 56 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index e1243ee..7b4ed42 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -2704,6 +2704,46 @@ state_max_out(struct drm_intel_decode *ctx, unsigned int 
index,
 }
 
 static int
+gen7_3DSTATE_URB_unit(struct drm_intel_decode *ctx, const char *unit)
+{
+int start_kb = ((ctx->data[1] >> 25) & 0x3f) * 8;
+/* the field is # of 512-bit rows - 1, we print bytes */
+int entry_size = (((ctx->data[1] >> 16) & 0x1ff) + 1);
+int nr_entries = ctx->data[1] & 0x;
+
+instr_out(ctx, 0, "3DSTATE_URB_%s\n", unit);
+instr_out(ctx, 1,
+ "%dKB start, size=%d 64B rows, nr_entries=%d, total size %dB\n",
+ start_kb, entry_size, nr_entries, nr_entries * 64 * entry_size);
+
+return 2;
+}
+
+static int
+gen7_3DSTATE_URB_VS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_URB_unit(ctx, "VS");
+}
+
+static int
+gen7_3DSTATE_URB_HS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_URB_unit(ctx, "HS");
+}
+
+static int
+gen7_3DSTATE_URB_DS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_URB_unit(ctx, "DS");
+}
+
+static int
+gen7_3DSTATE_URB_GS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_URB_unit(ctx, "GS");
+}
+
+static int
 decode_3d_965(struct drm_intel_decode *ctx)
 {
uint32_t opcode;
@@ -2750,6 +2790,10 @@ decode_3d_965(struct drm_intel_decode *ctx)
{ 0x7816, 0x00ff, 5, 5, "3DSTATE_CONSTANT_GS_STATE" },
{ 0x7817, 0x00ff, 5, 5, "3DSTATE_CONSTANT_PS_STATE" },
{ 0x7818, 0x, 2, 2, "3DSTATE_SAMPLE_MASK" },
+   { 0x7830, 0x00ff, 2, 2, NULL, 7, gen7_3DSTATE_URB_VS },
+   { 0x7831, 0x00ff, 2, 2, NULL, 7, gen7_3DSTATE_URB_HS },
+   { 0x7832, 0x00ff, 2, 2, NULL, 7, gen7_3DSTATE_URB_DS },
+   { 0x7833, 0x00ff, 2, 2, NULL, 7, gen7_3DSTATE_URB_GS },
{ 0x7900, 0x, 4, 4, "3DSTATE_DRAWING_RECTANGLE" },
{ 0x7901, 0x, 5, 5, "3DSTATE_CONSTANT_COLOR" },
{ 0x7905, 0x, 5, 7, "3DSTATE_DEPTH_BUFFER" },
diff --git a/intel/tests/gen7-3d.batch-ref.txt 
b/intel/tests/gen7-3d.batch-ref.txt
index 9c7f7ef..cde910f 100644
--- a/intel/tests/gen7-3d.batch-ref.txt
+++ b/intel/tests/gen7-3d.batch-ref.txt
@@ -26,14 +26,14 @@
 0x12300064:  0x0008: MI_NOOP
 0x12300068:  0x7916: 3D UNKNOWN: 3d_965 opcode = 0x7916
 0x1230006c:  0x00080008: MI_NOOP
-0x12300070:  0x7830: 3D UNKNOWN: 3d_965 opcode = 0x7830
-0x12300074:  0x040002c0: MI_ARB_ON_OFF
-0x12300078:  0x7833: 3D UNKNOWN: 3d_965 opcode = 0x7833
-0x1230007c:  0x0400: MI_ARB_ON_OFF
-0x12300080:  0x7831: 3D UNKNOWN: 3d_965 opcode = 0x7831
-0x12300084:  0x0400: MI_ARB_ON_OFF
-0x12300088:  0x7832: 3D UNKNOWN: 3d_965 opcode = 0x7832
-0x1230008c:  0x0400: MI_ARB_ON_OFF
+0x12300070:  0x7830: 3DSTATE_URB_VS
+0x12300074:  0x040002c0:16KB start, size=1 64B rows, nr_entries=704, 
total size 45056B
+0x12300078:  0x7833: 3DSTATE_URB_GS
+0x1230007c:  0x0400:16KB start, size=1 64B rows, nr_entries=0, 
total size 0B
+0x12300080:  0x7831: 3DSTATE_URB_HS
+0x12300084:  0x0400:16KB start, size=1 64B rows, nr_entries=0, 
total size 0B
+0x12300088:  0x7832: 3DSTATE_URB_DS
+0x1230008c:  0x0400:16KB start, size=1 64B rows, nr_entries=0, 
total size 0B
 0x12300090:  0x7824: 3D UNKNOWN: 3d_965 opcode = 0x7824
 0x12300094:  0x7f81: MI_NOOP
 Bad length 2 in 3DSTATE_CC_STATE_POINTERS, expeted 4-4
@@ -237,14 +237,14 @@ Bad length 7 in 3DPRIMITIVE, expeted 6-6
 0x1230034c:  0x0008: MI_NOOP
 0x12300350:  0x7916: 3D UNKNOWN: 3d_965 opcode = 0x7916
 0x12300354:  0x00080008: MI_NOOP
-0x12300358:  0x7830: 3D UNKNOWN: 3d_965 opcode = 0x7830
-0x1230035c:  0x040002c0: MI_ARB_ON_OFF
-0x12300360:  0x7833: 3D UNKNOWN: 3d_965 opcode = 0x7833
-0x12300364:  0x0400: MI_ARB_ON_OFF
-0x12300368:  0x7831: 3D UNKNOWN: 3d_965 opcode = 0x7831
-0x1230036c:  0x0400: MI_ARB_ON_OFF
-0x12300370:  0x7832: 3D UNKNOWN: 3d_965 opcode = 0x7832
-0x12300374:  0x0400: MI_ARB_ON_OFF
+0x12300358:  0x7830: 3DSTATE_URB_VS
+0x1230035c:  0x040002c0:16KB start, size=1 64B rows, nr_entries=704, 
total size 45056B
+0x12300360:  0x7833: 3DSTATE_URB_GS
+0x12300364:  0x0400:16KB start, size=1 64B rows, nr_entries=0, 
total size 0B
+0x12300368:  0x7831: 3DSTATE_URB_HS
+0x1230036c:  0x0400:16KB start, size=1 64B rows, nr_entries=0, 
total size 0B
+0x12300370:  0x7832: 3DSTATE_URB_DS
+0x12300374:  0x0400:16KB start, size=1 64B rows, nr_entries=0, 

[Intel-gfx] [PATCH 15/17] intel: Add decode for gen7 state pointers.

[Eric Anholt]

Since CC_STATE_POINTERS for gen6 and 7 are quite different but use the
same opcode, move gen6 out to a helper function too, so we can use a
helper function for gen7.
---
 intel/intel_decode.c  |   80 
 intel/tests/gen7-3d.batch-ref.txt |   30 ++
 2 files changed, 83 insertions(+), 27 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 7b4ed42..49242dc 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -2704,6 +2704,71 @@ state_max_out(struct drm_intel_decode *ctx, unsigned int 
index,
 }
 
 static int
+gen7_3DSTATE_VIEWPORT_STATE_POINTERS_CC(struct drm_intel_decode *ctx)
+{
+   instr_out(ctx, 0, "3DSTATE_VIEWPORT_STATE_POINTERS_CC\n");
+   instr_out(ctx, 1, "pointer to CC viewport\n");
+
+   return 2;
+}
+
+static int
+gen7_3DSTATE_VIEWPORT_STATE_POINTERS_SF_CLIP(struct drm_intel_decode *ctx)
+{
+   instr_out(ctx, 0, "3DSTATE_VIEWPORT_STATE_POINTERS_SF_CLIP\n");
+   instr_out(ctx, 1, "pointer to SF_CLIP viewport\n");
+
+   return 2;
+}
+
+static int
+gen7_3DSTATE_BLEND_STATE_POINTERS(struct drm_intel_decode *ctx)
+{
+   instr_out(ctx, 0, "3DSTATE_BLEND_STATE_POINTERS\n");
+   instr_out(ctx, 1, "pointer to BLEND_STATE at 0x%08x (%s)\n",
+ ctx->data[1] & ~1,
+ (ctx->data[1] & 1) ? "changed" : "unchanged");
+
+   return 2;
+}
+
+static int
+gen7_3DSTATE_DEPTH_STENCIL_STATE_POINTERS(struct drm_intel_decode *ctx)
+{
+   instr_out(ctx, 0, "3DSTATE_DEPTH_STENCIL_STATE_POINTERS\n");
+   instr_out(ctx, 1,
+ "pointer to DEPTH_STENCIL_STATE at 0x%08x (%s)\n",
+ ctx->data[1] & ~1,
+ (ctx->data[1] & 1) ? "changed" : "unchanged");
+
+   return 2;
+}
+
+static int
+gen6_3DSTATE_CC_STATE_POINTERS(struct drm_intel_decode *ctx)
+{
+   instr_out(ctx, 0, "3DSTATE_CC_STATE_POINTERS\n");
+   instr_out(ctx, 1, "blend change %d\n", ctx->data[1] & 1);
+   instr_out(ctx, 2, "depth stencil change %d\n",
+ ctx->data[2] & 1);
+   instr_out(ctx, 3, "cc change %d\n", ctx->data[3] & 1);
+
+   return 4;
+}
+
+static int
+gen7_3DSTATE_CC_STATE_POINTERS(struct drm_intel_decode *ctx)
+{
+   instr_out(ctx, 0, "3DSTATE_CC_STATE_POINTERS\n");
+   instr_out(ctx, 1, "pointer to COLOR_CALC_STATE at 0x%08x "
+ "(%s)\n",
+ ctx->data[1] & ~1,
+ (ctx->data[1] & 1) ? "changed" : "unchanged");
+
+   return 2;
+}
+
+static int
 gen7_3DSTATE_URB_unit(struct drm_intel_decode *ctx, const char *unit)
 {
 int start_kb = ((ctx->data[1] >> 25) & 0x3f) * 8;
@@ -2779,7 +2844,8 @@ decode_3d_965(struct drm_intel_decode *ctx)
{ 0x780a, 0x00ff, 3, 3, "3DSTATE_INDEX_BUFFER" },
{ 0x780b, 0x, 1, 1, "3DSTATE_VF_STATISTICS" },
{ 0x780d, 0x00ff, 4, 4, "3DSTATE_VIEWPORT_STATE_POINTERS" },
-   { 0x780e, 0x, 4, 4, "3DSTATE_CC_STATE_POINTERS" },
+   { 0x780e, 0x, 4, 4, NULL, 6, gen6_3DSTATE_CC_STATE_POINTERS 
},
+   { 0x780e, 0x00ff, 2, 2, NULL, 7, gen7_3DSTATE_CC_STATE_POINTERS 
},
{ 0x780f, 0x00ff, 2, 2, "3DSTATE_SCISSOR_POINTERS" },
{ 0x7810, 0x00ff, 6, 6, "3DSTATE_VS" },
{ 0x7811, 0x00ff, 7, 7, "3DSTATE_GS" },
@@ -2790,6 +2856,10 @@ decode_3d_965(struct drm_intel_decode *ctx)
{ 0x7816, 0x00ff, 5, 5, "3DSTATE_CONSTANT_GS_STATE" },
{ 0x7817, 0x00ff, 5, 5, "3DSTATE_CONSTANT_PS_STATE" },
{ 0x7818, 0x, 2, 2, "3DSTATE_SAMPLE_MASK" },
+   { 0x7821, 0x00ff, 2, 2, NULL, 7, 
gen7_3DSTATE_VIEWPORT_STATE_POINTERS_SF_CLIP },
+   { 0x7823, 0x00ff, 2, 2, NULL, 7, 
gen7_3DSTATE_VIEWPORT_STATE_POINTERS_CC },
+   { 0x7824, 0x00ff, 2, 2, NULL, 7, 
gen7_3DSTATE_BLEND_STATE_POINTERS },
+   { 0x7825, 0x00ff, 2, 2, NULL, 7, 
gen7_3DSTATE_DEPTH_STENCIL_STATE_POINTERS },
{ 0x7830, 0x00ff, 2, 2, NULL, 7, gen7_3DSTATE_URB_VS },
{ 0x7831, 0x00ff, 2, 2, NULL, 7, gen7_3DSTATE_URB_HS },
{ 0x7832, 0x00ff, 2, 2, NULL, 7, gen7_3DSTATE_URB_DS },
@@ -3006,14 +3076,6 @@ decode_3d_965(struct drm_intel_decode *ctx)
instr_out(ctx, 2, "ending buffer address\n");
return len;
 
-   case 0x780e:
-   instr_out(ctx, 0, "3DSTATE_CC_STATE_POINTERS\n");
-   instr_out(ctx, 1, "blend change %d\n", data[1] & 1);
-   instr_out(ctx, 2, "depth stencil change %d\n",
- data[2] & 1);
-   instr_out(ctx, 3, "cc change %d\n", data[3] & 1);
-   return len;
-
case 0x780f:
instr_out(ctx, 0, "3DSTATE_SCISSOR_POINTERS\n");
instr_out(ctx, 1, "scissor rect offset\n");
diff --git a/intel/tests/gen7-3d.batch-ref.txt 
b/intel/tests/gen7-3d.batch-ref.txt
index cde910f..6b2a0d4 100

[Intel-gfx] [PATCH 16/17] intel: Add decode for gen7 constant buffer packets.

[Eric Anholt]

---
 intel/intel_decode.c  |   62 ++-
 intel/tests/gen7-3d.batch-ref.txt |  388 +
 2 files changed, 241 insertions(+), 209 deletions(-)

diff --git a/intel/intel_decode.c b/intel/intel_decode.c
index 49242dc..d9001d4 100644
--- a/intel/intel_decode.c
+++ b/intel/intel_decode.c
@@ -2809,6 +2809,57 @@ gen7_3DSTATE_URB_GS(struct drm_intel_decode *ctx)
 }
 
 static int
+gen7_3DSTATE_CONSTANT(struct drm_intel_decode *ctx, const char *unit)
+{
+   int rlen[4];
+
+   rlen[0] = (ctx->data[1] >> 0) & 0x;
+   rlen[1] = (ctx->data[1] >> 16) & 0x;
+   rlen[2] = (ctx->data[2] >> 0) & 0x;
+   rlen[3] = (ctx->data[2] >> 16) & 0x;
+
+   instr_out(ctx, 0, "3DSTATE_CONSTANT_%s\n", unit);
+   instr_out(ctx, 1, "len 0 = %d, len 1 = %d\n", rlen[0], rlen[1]);
+   instr_out(ctx, 2, "len 2 = %d, len 3 = %d\n", rlen[2], rlen[3]);
+   instr_out(ctx, 3, "pointer to constbuf 0\n");
+   instr_out(ctx, 4, "pointer to constbuf 1\n");
+   instr_out(ctx, 5, "pointer to constbuf 2\n");
+   instr_out(ctx, 6, "pointer to constbuf 3\n");
+
+   return 7;
+}
+
+static int
+gen7_3DSTATE_CONSTANT_VS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_CONSTANT(ctx, "VS");
+}
+
+static int
+gen7_3DSTATE_CONSTANT_GS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_CONSTANT(ctx, "GS");
+}
+
+static int
+gen7_3DSTATE_CONSTANT_PS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_CONSTANT(ctx, "PS");
+}
+
+static int
+gen7_3DSTATE_CONSTANT_DS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_CONSTANT(ctx, "DS");
+}
+
+static int
+gen7_3DSTATE_CONSTANT_HS(struct drm_intel_decode *ctx)
+{
+   return gen7_3DSTATE_CONSTANT(ctx, "HS");
+}
+
+static int
 decode_3d_965(struct drm_intel_decode *ctx)
 {
uint32_t opcode;
@@ -2852,10 +2903,15 @@ decode_3d_965(struct drm_intel_decode *ctx)
{ 0x7812, 0x00ff, 4, 4, "3DSTATE_CLIP" },
{ 0x7813, 0x00ff, 20, 20, "3DSTATE_SF" },
{ 0x7814, 0x00ff, 9, 9, "3DSTATE_WM" },
-   { 0x7815, 0x00ff, 5, 5, "3DSTATE_CONSTANT_VS_STATE" },
-   { 0x7816, 0x00ff, 5, 5, "3DSTATE_CONSTANT_GS_STATE" },
-   { 0x7817, 0x00ff, 5, 5, "3DSTATE_CONSTANT_PS_STATE" },
+   { 0x7815, 0x00ff, 5, 5, "3DSTATE_CONSTANT_VS_STATE", 6 },
+   { 0x7815, 0x00ff, 7, 7, "3DSTATE_CONSTANT_VS", 7, 
gen7_3DSTATE_CONSTANT_VS },
+   { 0x7816, 0x00ff, 5, 5, "3DSTATE_CONSTANT_GS_STATE", 6 },
+   { 0x7816, 0x00ff, 7, 7, "3DSTATE_CONSTANT_GS", 7, 
gen7_3DSTATE_CONSTANT_GS },
+   { 0x7817, 0x00ff, 5, 5, "3DSTATE_CONSTANT_PS_STATE", 6 },
+   { 0x7817, 0x00ff, 7, 7, "3DSTATE_CONSTANT_PS", 7, 
gen7_3DSTATE_CONSTANT_PS },
{ 0x7818, 0x, 2, 2, "3DSTATE_SAMPLE_MASK" },
+   { 0x7819, 0x00ff, 7, 7, "3DSTATE_CONSTANT_HS", 7, 
gen7_3DSTATE_CONSTANT_HS },
+   { 0x781a, 0x00ff, 7, 7, "3DSTATE_CONSTANT_DS", 7, 
gen7_3DSTATE_CONSTANT_DS },
{ 0x7821, 0x00ff, 2, 2, NULL, 7, 
gen7_3DSTATE_VIEWPORT_STATE_POINTERS_SF_CLIP },
{ 0x7823, 0x00ff, 2, 2, NULL, 7, 
gen7_3DSTATE_VIEWPORT_STATE_POINTERS_CC },
{ 0x7824, 0x00ff, 2, 2, NULL, 7, 
gen7_3DSTATE_BLEND_STATE_POINTERS },
diff --git a/intel/tests/gen7-3d.batch-ref.txt 
b/intel/tests/gen7-3d.batch-ref.txt
index 6b2a0d4..3baf524 100644
--- a/intel/tests/gen7-3d.batch-ref.txt
+++ b/intel/tests/gen7-3d.batch-ref.txt
@@ -40,14 +40,13 @@
 0x1230009c:  0x7f41:pointer to COLOR_CALC_STATE at 0x7f40 
(changed)
 0x123000a0:  0x7825: 3DSTATE_DEPTH_STENCIL_STATE_POINTERS
 0x123000a4:  0x7f01:pointer to DEPTH_STENCIL_STATE at 0x7f00 
(changed)
-Bad length 7 in 3DSTATE_CONSTANT_GS_STATE, expeted 5-5
-0x123000a8:  0x78160005: 3DSTATE_CONSTANT_GS_STATE
-0x123000ac:  0x:dword 1
-0x123000b0:  0x:dword 2
-0x123000b4:  0x:dword 3
-0x123000b8:  0x:dword 4
-0x123000bc:  0x:dword 5
-0x123000c0:  0x:dword 6
+0x123000a8:  0x78160005: 3DSTATE_CONSTANT_GS
+0x123000ac:  0x:len 0 = 0, len 1 = 0
+0x123000b0:  0x:len 2 = 0, len 3 = 0
+0x123000b4:  0x:pointer to constbuf 0
+0x123000b8:  0x:pointer to constbuf 1
+0x123000bc:  0x:pointer to constbuf 2
+0x123000c0:  0x:pointer to constbuf 3
 0x123000c4:  0x78110005: 3DSTATE_GS
 0x123000c8:  0x:kernel pointer
 0x123000cc:  0x:SPF=0, VME=0, Sampler Count 0, Binding table 
count 0
@@ -57,13 +56,13 @@ Bad length 7 in 3DSTATE_CONSTANT_GS_STATE, expeted 5-5
 0x123000dc:  0x:Reorder disable, Discard Adjaceny disable, GS 
disable
 0x123000e0:  0x7829: 3D UNKNOWN: 3d_965 opcode = 0x7829
 0x123000e4:  0x: MI_NOOP
-0x1

Re: [Intel-gfx] [PATCH] intel: Update for new i915_drm.h defines.

[Jesse Barnes]

On Wed, 04 Jan 2012 14:48:37 -0800
Eric Anholt  wrote:

> On Wed, 4 Jan 2012 08:52:34 -0800, Jesse Barnes  
> wrote:
> > On Tue,  3 Jan 2012 19:10:00 -0800
> > Eric Anholt  wrote:
> > 
> > > ---
> > > 
> > > This syncs the code up to the kernel as of the gen7 SOL changes.  It
> > > would be nice if doing this was just a straight copy of the kernel
> > > code -- there are two diffs left out.  One is this hunk:
> > > 
> > > -#ifdef __KERNEL__
> > > -/* For use by IPS driver */
> > > -extern unsigned long i915_read_mch_val(void);
> > > -extern bool i915_gpu_raise(void);
> > > -extern bool i915_gpu_lower(void);
> > > -extern bool i915_gpu_busy(void);
> > > -extern bool i915_gpu_turbo_disable(void);
> > > -#endif
> > > 
> > > Does anyone see any real reason to be dropping this?
> > > 
> > > The other is removing the sparse __user annotations on the structs.
> > > Could we just patch the kernel to #define it away for userland, so we
> > > could make updating this file just a matter of "cp"?
> > 
> > Having just done this locally, I'm in favor.  The stuff under
> > __KERNEL__ should be harmless; don't see why we can't just copy it.
> > 
> > For the __user define, we must be missing a compiler.h include
> > somewhere in the libdrm case.  If you explicitly include it do the
> > errors go away?
> 
> What version of POSIX is compiler.h part of?  Remember that this is
> userland code built across OSes (as much as we try to forget that).

It's one of the Linux kernel headers.  I'm such a Linux-ist I didn't
even consider an OS that might not have it.  We can just #define away
__user in drm.h or something to avoid adding OS deps.  Or use the
header install target like Daniel said and copy from that.

-- 
Jesse Barnes, Intel Open Source Technology Center


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Drivers for gma 4500m?

[Kenneth Graunke]

On 01/04/2012 11:23 AM, je...@vige.it wrote:
> Are the graphic drivers for gma 4500m included in the intel-gfx release?
> If no there is a future develop for it?

Yes, that's been supported for ages, and the drivers should already ship
with basically any Linux distribution.  You shouldn't have to download
anything special.

For reference, the drivers are 'i915' in the kernel (display), 'i965' in
Mesa (3D), and 'xf86-video-intel' for X.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/43] drm/i915: protect force_wake_(get|put) with the gt_lock

[Keith Packard]

On Wed, 4 Jan 2012 19:12:57 +0100, Daniel Vetter  wrote:

> The "Correct?" was just to check my understanding of your concern, I still
> think its invalid. You've cut away the second part of my mail where I
> explain why and I don't see you responding to that here. Also
> micro-optimizing the gpu reset code sounds a bit strange.

Sorry, I didn't explain things very well.

Right now, our register access looks like:

get(struct_mutex);
if (++forcewake_count == 1)
force_wake_get()

value = read32(reg) or  write32(reg, val)

if (--forcewake_count == 0)
force_wake_put();

/* more register accesses may follow ... */
put(struct_mutex);

All very sensible, the whole register sequence is covered by
struct_mutex, which ensures that the forcewake is set across the
register access.

The patch does:

get(spin_lock)
if (++forcewake_count == 1)
force_wake_get()
put(spin_lock)
value = read32(reg) or write32(reg, val)
get(spin_lock)
if (--forcewake_count == 0)
force_wake_put()
put(spin_lock)

I realize that all current users hold the struct_mutex across this whole
sequence, aside from the reset path, but we're removing that requirement
explicitly (the patch removes the WARN_ON calls).

Without a lock held across the whole sequence, it's easy to see how a
race could occur. We're also dropping and re-acquiring a spinlock with a
single instruction between, which seems wasteful. Instead, we should be
doing:

get(spin_lock)
force_wake_get();
value = read32(reg) or  write32(reg,val)
force_wake_put();
put(spin_lock);

No need here to deal with the wake lock at reset time; the whole
operation is atomic wrt interrupts. It's more efficient *and* correct,
without depending on the old struct_mutex locking.

If you want to continue to expose the user-mode wake lock stuff, you
could add:

get(spin_lock);
if (!forcewake_count)
force_wake_get();
value = read32(reg) or  write32(reg,val)
if (!forcewake_count)
force_wake_put();
put(spin_lock);

This would require that the reset code also deal with the
forcewake_count to restore the user-mode force wake.

A further optimization would hold the force_wake active for 'a while' to
avoid the extra bus cycles required, but we'd want to see a performance
problem from this before doing that.


-- 
keith.pack...@intel.com


pgphx3pZKZy5W.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: paper over missed irq issues with force wake vodoo

[Keith Packard]

On Wed,  4 Jan 2012 19:40:45 +0100, Daniel Vetter  
wrote:

> Two things seem to do the trick on my ivb machine here:
> - prevent the gt from powering down while waiting for seqno
>   notification interrupts by grabbing the force_wake in get_irq (and
>   dropping it in put_irq again).
> - ordering writes from the ring's CS by reading a CS register, ACTHD
>   seems to work.

If this works reliably, you'll deserve a medal...

-- 
keith.pack...@intel.com


pgpjiETxybTmP.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PULL] drm-intel-next

[Keith Packard]

Here are the rest of the 3.3 pending changes.

This has a bunch of small bug fixes and overlay plane support for i915.

The following changes since commit 7a7e8734ac3235efafd34819b27fbdf5417e6d60:

  Merge branch 'drm-radeon-testing' of ../drm-radeon-next into drm-core-next 
(2012-01-03 09:45:12 +)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/keithp/linux drm-intel-next

Ben Widawsky (4):
  drm/i915: relative_constants_mode race fix
  drm/i915: Force sync command ordering (Gen6+)
  drm/i915: Update GEN6_RP_CONTROL definitions
  drm/i915: drpc debugfs update for gen6

Daniel Vetter (2):
  drm/i915: kicking rings stuck on semaphores considered harmful
  drm/i915: don't bail out of intel_wait_ring_buffer too early

Eric Anholt (5):
  drm/i915: Set two chicken bits implicated in missed IRQs on Ivybridge.
  drm/i915: Do the fallback non-IRQ wait in ring throttle, too.
  drm/i915: Work around gen7 BLT ring synchronization issues.
  drm/i915: Make the fallback IRQ wait not sleep.
  drm/i915: Add support for resetting the SO write pointers on gen7.

Jesse Barnes (5):
  drm/i915: add SNB and IVB video sprite support v6
  drm/i915: track sprite coverage and disable primary plane if possible
  drm/i915: add color key support v4
  drm/i915: don't disable a PCH DPLL that's in use
  drm/i915: only set the intel_crtc DPMS mode to on if the mode set 
succeeded

Keith Packard (1):
  drm/i915: Clean up multi-threaded forcewake patch

 drivers/gpu/drm/i915/Makefile  |1 +
 drivers/gpu/drm/i915/i915_debugfs.c|   86 -
 drivers/gpu/drm/i915/i915_dma.c|5 +
 drivers/gpu/drm/i915/i915_drv.c|7 -
 drivers/gpu/drm/i915/i915_drv.h|6 +-
 drivers/gpu/drm/i915/i915_gem.c|   10 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   63 +++-
 drivers/gpu/drm/i915/i915_irq.c|7 -
 drivers/gpu/drm/i915/i915_reg.h|  161 +++-
 drivers/gpu/drm/i915/intel_display.c   |  206 -
 drivers/gpu/drm/i915/intel_drv.h   |   40 ++
 drivers/gpu/drm/i915/intel_fb.c|6 +
 drivers/gpu/drm/i915/intel_ringbuffer.c|   30 ++-
 drivers/gpu/drm/i915/intel_sprite.c|  668 
 include/drm/i915_drm.h |   40 ++
 15 files changed, 1290 insertions(+), 46 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_sprite.c

-- 
keith.pack...@intel.com


pgphVYScKHxaG.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] drm/i915 kernel source available at kernel.org now

[Keith Packard]

I've had a working kernel.org account for a couple months now, and have
been pushing code to both there and fdo for the last release. I'll
continue to push to both, but I'll be sending pull requests from
kernel.org from now on.

git:/git.kernel.org/pub/scm/linux/kernel/git/keithp/linux 

Nothing has changed in the basic management plan, and the same branches
should be available:

drm-intel-fixes Fixes for post-RC1 merging
drm-intel-next  Changes for current or next merge window

-- 
keith.pack...@intel.com


pgprJe2eFvQfI.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx