[Intel-gfx] Requesting proposals for a 2.7.1 maintenance release

[Carl Worth]

On Fri, 2009-05-08 at 12:48 -0700, Keith Packard wrote:
> On Fri, 2009-05-08 at 11:50 -0700, Eric Anholt wrote:
> 
> > Why?  They appear to be cleanups rather than real bugfixes.
> 
> The first one is pure cleanup, but the second one corrects a request to
> include the specified number of dwords instead of being one short. This
> didn't appear to cause any problems, but it seems best to conform to the
> hardware spec.

Now you got me to go review the actual patch and I noticed a minor
issue:

@@ -1127,15 +1127,16 @@ I965DisplayVideoTextured(ScrnInfoPtr pScrn, I830PortPriv
 
        i965_emit_video_setup(pScrn, bind_bo, n_src_surf);
 
-       BEGIN_BATCH(10);
+       BEGIN_BATCH(12);
        /* Set up the pointer to our vertex buffer */
-       OUT_BATCH(BRW_3DSTATE_VERTEX_BUFFERS | 2);
+       OUT_BATCH(BRW_3DSTATE_VERTEX_BUFFERS | 3);
        /* four 32-bit floats per vertex */
        OUT_BATCH((0 << VB0_BUFFER_INDEX_SHIFT) |
                  VB0_VERTEXDATA |
                  ((4 * 4) << VB0_BUFFER_PITCH_SHIFT));
        OUT_RELOC(vb_bo, I915_GEM_DOMAIN_VERTEX, 0, 0);
        OUT_BATCH(3); /* four corners to our rectangle */
+       OUT_BATCH(0); /* reserved */
 
        OUT_BATCH(BRW_3DPRIMITIVE |
                  BRW_3DPRIMITIVE_VERTEX_SEQUENTIAL |
@@ -1147,6 +1148,7 @@ I965DisplayVideoTextured(ScrnInfoPtr pScrn, I830PortPrivPt
        OUT_BATCH(1); /* single instance */
        OUT_BATCH(0); /* start instance location */
        OUT_BATCH(0); /* index buffer offset, ignored */
+       OUT_BATCH(MI_NOOP);
        ADVANCE_BATCH();
 
        intel_batch_end_atomic(pScrn);

That looks like the traditional style of ensuring that BEGIN_BATCH is
always called with an even number and padding with NOOP before
ADVANCE_BATCH. If I'm not mistaken, this style is no longer necessary,
since the lower-level batch submission code will actually ensure the
even-number constraint is met.

That's mostly just an FYI. I think I'll go ahead and cherry-pick this
patch as is for now. (It's just a judgment call whether trying to
conform closer to the specification like this is more likely to cause
new problems in unknown ways or to fix bugs in unknown ways. I'm fairly
conservative about these things in general, but I think the number of
known cases of failures for unknown reasons tips the balance toward
sending possible fixes out onto the maintenance branch.)

-Carl

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20090508/9d580fb2/attachment.sig>