On Thu, Sep 08, 2011 at 08:39:46AM -0700, Eric Anholt wrote:
> On Thu,  8 Sep 2011 11:00:52 +0800, Yuanhan Liu <yuanhan....@linux.intel.com> 
> wrote:
> > This patch is just for RFC, as I am not sure it's the right way to setup
> > the edge flag enable bit in Vertex Element struture. Setting up this
> > bit, according to Bspec, need do:
> >  1. Edge flags are supported for the following primitives
> >             3DPRIM_TRILIST*
> >     3DPRIM_TRISTRIP*
> >     3DPRIM_TRIFAN*
> >     3DPRIM_POLYGON
> >  2. This bit must only be ENABLED on the last valid VERTEX_ELEMENT
> >     structure.
> > 
> >  3. When set, Component 0 Control must be set to VFCOMP_STORE_SRC, and
> >     Component 1-3 Control must be set to VFCOMP_NOSTORE.
> > 
> >  4. The Source Element Format must be set to the UINT format.
> > 
> > This patch did 1, 2, but didn't do 3, 4. As it simply seems wrong to me
> > just change the last vetex element's component setting and source
> > element format.
> > 
> > Thoughts?
> > 
> > BTW, this patch fix the oglc pntrast fail on SNB(haven't tested it on
> > IVB yet).
> 
> Could you include a piglit test for the failure?

Actually, this is an issue of edgeflag. You will also find this patch
will fix the oglc edgeflag test fail. pntrast test case would also use
polygonmode with edgeflag to render a point. Thus it failed.

I simply grep-ed the piglit repo, didn't find any references on
glEdgeFlag. Seems that current piglit doesn't include this test?

> 
> > +      /*
> > +       * According to Bspec, the edge flag enable bit should be set
> > +       * at the last valid vertex element structure
> > +       */
> 
> Instead of saying "According to the Bspec", could you actually cite the
> text from the public PRM?  For example, in one of my previous patches:

Thanks for the suggestions. Yeah, I should and will do this. 

Besides this, any ideas on how to setup the edge flag enable bit rightly
to match the Bspec? Or, does this patch make sense to you?

Thanks,
Yuanhan Liu
> 
> ...
> + * And this last workaround is tricky because of the requirements on
> + * that bit.  From section 1.4.7.2.3 "Stall" of the Sandy Bridge PRM
> + * volume 2 part 1:
> + *
> + *     "1 of the following must also be set:
> + *      - Render Target Cache Flush Enable ([12] of DW1)
> + *      - Depth Cache Flush Enable ([0] of DW1)
> + *      - Stall at Pixel Scoreboard ([1] of DW1)
> + *      - Depth Stall ([13] of DW1)
> + *      - Post-Sync Operation ([13] of DW1)
> + *      - Notify Enable ([8] of DW1)"
> 
> It means that someone else stumbling on this code later gets a pointer
> to where to start reading up on what's going on in the code.


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to