On Mon, 06 Jun 2011 17:50:06 -0700, Kenneth Graunke <kenn...@whitecape.org>
wrote:
> On 06/04/2011 05:45 PM, Chad Versace wrote:
> > ... which indicates if the X driver supports DRI2BufferHiz and
> > DRI2BufferStencil.
> >
> > I'm placing this in its own commit due to the large comment block.
> >
> > CC: Eric Anholt<e...@anholt.net>
> > CC: Ian Romanick<i...@freedesktop.org>
> > CC: Kenneth Graunke<kenn...@whitecape.org>
> > CC: Kristian Høgsberg<k...@bitplanet.net>
> > Signed-off-by: Chad Versace<c...@chad-versace.us>
> > ---
> > src/mesa/drivers/dri/intel/intel_screen.h | 54
> > +++++++++++++++++++++++++++++
> > 1 files changed, 54 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/intel/intel_screen.h
> > b/src/mesa/drivers/dri/intel/intel_screen.h
> > index 4613c98..d4add6a 100644
> > --- a/src/mesa/drivers/dri/intel/intel_screen.h
> > +++ b/src/mesa/drivers/dri/intel/intel_screen.h
> > @@ -34,6 +34,60 @@
> > #include "i915_drm.h"
> > #include "xmlconfig.h"
> >
> > +/**
> > + * \brief Does the X driver support DRI2BufferHiz and DRI2BufferStencil?
> > + *
> > + * The DRI2 protocol does not allow us to query the X driver's version nor
> > + * query for a list of buffer formats that the driver supports. So, to
> > + * determine if the X driver supports DRI2BufferHiz and DRI2BufferStencil
> > we
> > + * must resort to a handshake.
> > + *
> > + * If the hardware lacks support for separate stencil (and consequently,
> > lacks
> > + * support for hiz also), then the X driver's separate stencil and hiz
> > support
> > + * is irrelevant and the handshake never occurs.
> > + *
> > + * Complications
> > + * -------------
> > + * The handshake is complicated by a bug in the current driver version.
> > Even
>
> I might say "xf86-video-intel 2.15" rather than "current driver
> version". It's future-proof (saves us from reassociating this commit's
> date with DDX versions) and more specific (the issue is the DDX driver,
> not the Mesa driver).
You're right: "current driver version" is too ambiguous. It's now
replaced with "xf86-video-intel 2.15".
To eliminate any ambiguity, I also placed this near the top of the comment:
> (Here, "X driver" referes to the DDX driver, xf86-video-intel).
>
> > + * though the driver currently does not support requests for DRI2BufferHiz
> > or
> > + * DRI2BufferStencil, if requested one it still allocates and returns one.
> > The
> > + * returned buffer, however, is incorrectly X tiled.
> > + *
> > + * How the handshake works
> > + * -----------------------
> > + * (TODO: To be implemented on a future commit).
> > + *
> > + * Initially, intel_screen.dri2_has_hiz is set to unknown. The first time
> > the
> > + * user requests a depth and stencil buffer, intelCreateBuffers() creates a
> > + * framebuffer with separate depth and stencil attachments (with formats
> > + * x8_z24 and s8).
> > + *
>
> As I mentioned in person today, this isn't necessary for Ivybridge---the
> released DDX driver doesn't include Ivybridge PCI IDs.
>
> So we could possibly do:
>
> if (intel->gen >= 7)
> intel_screen.dri2_has_hiz = INTEL_DRI2_HAS_HIZ_TRUE;
>
> But your handshake should already work that out anyway, so I suppose
> it's pointless to add that.
>
> > + * Eventually, intel_update_renderbuffers() makes a DRI2 request for
> > + * DRI2BufferStencil and DRI2BufferHiz. If the returned buffers are Y
> > tiled,
> > + * then we joyfully set intel_screen.dri2_has_hiz to true and continue as
> > if
> > + * nothing happend.
> > + *
> > + * If the buffers are X tiled, however, the handshake has failed and we
> > must
> > + * clean up.
> > + * 1. Angrily set intel_screen.dri2_has_hiz to false.
> > + * 2. Discard the framebuffer's depth and stencil attachments.
> > + * 3. Attach a packed depth/stencil buffer to the framebuffer (with
> > format
> > + * s8_z24).
> > + * 4. Make a DRI2 request for the new buffer, using attachment type
> > + * DRI2BufferDepthStencil).
> > + *
> > + * Future Considerations
> > + * ---------------------
> > + * On a sunny day in the far future, when we are certain that no one has an
> > + * X driver installed without hiz and separate stencil support, then this
> > + * enumerant and the handshake should die.
> > + */
> > +enum intel_dri2_has_hiz {
> > + INTEL_DRI2_HAS_HIZ_UNKNOWN,
> > + INTEL_DRI2_HAS_HIZ_TRUE,
> > + INTEL_DRI2_HAS_HIZ_FALSE,
> > +};
> > +
>
> I was going to suggest doing the following:
>
> +enum intel_dri2_has_hiz {
> + INTEL_DRI2_HAS_HIZ_UNKNOWN = -1,
> + INTEL_DRI2_HAS_HIZ_TRUE = 1
> + INTEL_DRI2_HAS_HIZ_FALSE = 0,
> +};
>
> That way, TRUE == true and FALSE == false, which is nice if you forget
> to use the enums. But of course that would make UNKNOWN == true too,
> which is bullshit. So not using the enums is bad. What you have should
> be fine. :)
It's shame that there is no #include <ternary_logic.h>.
I was worried about the same issues you mention here, and came to the same
conclusions. UKNOWN == true is bullshit, and we just have to use the
enums everywhere. Since that's the case, it doesn't matter what their values
are.
----
Chad Versace
c...@chad-versace.us
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
