[PATCH 0/2] ddx/dri: Add support for hiz and separate stencil buffers

[PATCH 0/2] ddx/dri: Add support for hiz and separate stencil buffers

[Chad Versace]

These patches belong to my effort to enable hiz and separate stencil for IVB. 

Patch 1 belongs to dri2proto.
Patch 2 belongs to xf86-video-intel.

There is a related patch series on the mesa-dev list that uses this
functionality. See the thread with subject:
    i965: Implement hiz and separate stencil for window framebuffer
-- 
1.7.5.2

[PATCH 1/2] Add attachment token DRI2BufferHiz

[Chad Versace]

... and bump version to 2.6.

Signed-off-by: Chad Versace <chad@chad-versace.us>
---
 configure.ac  |    2 +-
 dri2proto.txt |    6 +++++-
 dri2tokens.h  |    1 +
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 297be0e..d671f5a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.60])
-AC_INIT([DRI2Proto], [2.5], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([DRI2Proto], [2.6], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 AM_MAINTAINER_MODE
 
diff --git a/dri2proto.txt b/dri2proto.txt
index dc46e58..df763c7 100644
--- a/dri2proto.txt
+++ b/dri2proto.txt
@@ -178,7 +178,8 @@ DRI2ATTACHMENT { DRI2BufferFrontLeft
 	       	 DRI2BufferAccum
 		 DRI2BufferFakeFrontLeft
 		 DRI2BufferFakeFrontRight
-		 DRI2BufferDepthStencil }
+		 DRI2BufferDepthStencil
+		 DRI2BufferHiz }
 
 	These values describe various attachment points for DRI2
 	buffers.
@@ -509,6 +510,8 @@ The DRI2 extension has undergone a number of revisions before
 
 	2.3: Added the DRI2InvalidateBuffers event.
 
+	2.6: Enlightenment attained.  Added the DRI2BufferHiz attachment.
+
 Compatibility up to 2.0 is not preserved, but was also never released.
 
 
@@ -569,6 +572,7 @@ A.1 Common Types
 	0x7	DRI2BufferFakeFrontLeft
 	0x8	DRI2BufferFakeFrontRight
 	0x9	DRI2BufferDepthStencil
+	0xa	DRI2BufferHiz
 └───
 	Used to encode the possible attachment points.  The attachment
 	DRI2BufferDepthStencil is only available with protocol version 1.1 or
diff --git a/dri2tokens.h b/dri2tokens.h
index 7804e4d..16c9008 100644
--- a/dri2tokens.h
+++ b/dri2tokens.h
@@ -43,6 +43,7 @@
 #define DRI2BufferFakeFrontLeft		7
 #define DRI2BufferFakeFrontRight	8
 #define DRI2BufferDepthStencil		9
+#define DRI2BufferHiz			10
 
 #define DRI2DriverDRI			0
 #define DRI2DriverVDPAU			1
-- 
1.7.5.2

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

Re: [PATCH 1/2] Add attachment token DRI2BufferHiz

[Kenneth Graunke]

On 06/04/2011 05:06 PM, Chad Versace wrote:
> ... and bump version to 2.6.
>
> Signed-off-by: Chad Versace<chad@chad-versace.us>
> ---
>   configure.ac  |    2 +-
>   dri2proto.txt |    6 +++++-
>   dri2tokens.h  |    1 +
>   3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 297be0e..d671f5a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1,5 +1,5 @@
>   AC_PREREQ([2.60])
> -AC_INIT([DRI2Proto], [2.5], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> +AC_INIT([DRI2Proto], [2.6], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
>   AM_INIT_AUTOMAKE([foreign dist-bzip2])
>   AM_MAINTAINER_MODE
>
> diff --git a/dri2proto.txt b/dri2proto.txt
> index dc46e58..df763c7 100644
> --- a/dri2proto.txt
> +++ b/dri2proto.txt
> @@ -178,7 +178,8 @@ DRI2ATTACHMENT { DRI2BufferFrontLeft
>   	       	 DRI2BufferAccum
>   		 DRI2BufferFakeFrontLeft
>   		 DRI2BufferFakeFrontRight
> -		 DRI2BufferDepthStencil }
> +		 DRI2BufferDepthStencil
> +		 DRI2BufferHiz }
>
>   	These values describe various attachment points for DRI2
>   	buffers.
> @@ -509,6 +510,8 @@ The DRI2 extension has undergone a number of revisions before
>
>   	2.3: Added the DRI2InvalidateBuffers event.
>
> +	2.6: Enlightenment attained.  Added the DRI2BufferHiz attachment.
> +

I hate to be a killjoy, but I'd leave out the first sentence lest 
someone think this is to support Enlightenment/EFL (enlightenment.org).

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

>   Compatibility up to 2.0 is not preserved, but was also never released.
>
>
> @@ -569,6 +572,7 @@ A.1 Common Types
>   	0x7	DRI2BufferFakeFrontLeft
>   	0x8	DRI2BufferFakeFrontRight
>   	0x9	DRI2BufferDepthStencil
> +	0xa	DRI2BufferHiz
>   └───
>   	Used to encode the possible attachment points.  The attachment
>   	DRI2BufferDepthStencil is only available with protocol version 1.1 or
> diff --git a/dri2tokens.h b/dri2tokens.h
> index 7804e4d..16c9008 100644
> --- a/dri2tokens.h
> +++ b/dri2tokens.h
> @@ -43,6 +43,7 @@
>   #define DRI2BufferFakeFrontLeft		7
>   #define DRI2BufferFakeFrontRight	8
>   #define DRI2BufferDepthStencil		9
> +#define DRI2BufferHiz			10
>
>   #define DRI2DriverDRI			0
>   #define DRI2DriverVDPAU			1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] Add attachment token DRI2BufferHiz

[Chad Versace]

On Sun, 05 Jun 2011 01:53:29 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote:
> On 06/04/2011 05:06 PM, Chad Versace wrote:
> > ... and bump version to 2.6.
> >
> > Signed-off-by: Chad Versace<chad@chad-versace.us>
> > ---
> >   configure.ac  |    2 +-
> >   dri2proto.txt |    6 +++++-
> >   dri2tokens.h  |    1 +
> >   3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 297be0e..d671f5a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1,5 +1,5 @@
> >   AC_PREREQ([2.60])
> > -AC_INIT([DRI2Proto], [2.5], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> > +AC_INIT([DRI2Proto], [2.6], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> >   AM_INIT_AUTOMAKE([foreign dist-bzip2])
> >   AM_MAINTAINER_MODE
> >
> > diff --git a/dri2proto.txt b/dri2proto.txt
> > index dc46e58..df763c7 100644
> > --- a/dri2proto.txt
> > +++ b/dri2proto.txt
> > @@ -178,7 +178,8 @@ DRI2ATTACHMENT { DRI2BufferFrontLeft
> >   	       	 DRI2BufferAccum
> >   		 DRI2BufferFakeFrontLeft
> >   		 DRI2BufferFakeFrontRight
> > -		 DRI2BufferDepthStencil }
> > +		 DRI2BufferDepthStencil
> > +		 DRI2BufferHiz }
> >
> >   	These values describe various attachment points for DRI2
> >   	buffers.
> > @@ -509,6 +510,8 @@ The DRI2 extension has undergone a number of revisions before
> >
> >   	2.3: Added the DRI2InvalidateBuffers event.
> >
> > +	2.6: Enlightenment attained.  Added the DRI2BufferHiz attachment.
> > +
> 
> I hate to be a killjoy, but I'd leave out the first sentence lest 
> someone think this is to support Enlightenment/EFL (enlightenment.org).
> 
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

It's not evident from the hunk, but I was just following the silly precedent
in previous version bumps.

>	2.0: Awesomeness!
>
>	2.1: True excellence.  Added DRI2GetBuffersWithFormat to allow
>             more flexible object creation.
>
>	2.2: Approaching perfection.  Added requests for swapbuffers,
>             MSC and SBC related requests, and events.

Re: [PATCH 1/2] Add attachment token DRI2BufferHiz

[Kenneth Graunke]

On 06/05/2011 07:32 PM, Chad Versace wrote:
> On Sun, 05 Jun 2011 01:53:29 -0700, Kenneth Graunke<kenneth@whitecape.org>  wrote:
>> I hate to be a killjoy, but I'd leave out the first sentence lest
>> someone think this is to support Enlightenment/EFL (enlightenment.org).
>>
>> Reviewed-by: Kenneth Graunke<kenneth@whitecape.org>
>
> It's not evident from the hunk, but I was just following the silly precedent
> in previous version bumps.
>
>> 	2.0: Awesomeness!
>>
>> 	2.1: True excellence.  Added DRI2GetBuffersWithFormat to allow
>>              more flexible object creation.
>>
>> 	2.2: Approaching perfection.  Added requests for swapbuffers,
>>              MSC and SBC related requests, and events.

Oh :) I should have checked first...by all means, go ahead then!

[PATCH 2/2] dri: Add support for DRI2BufferStencil and DRI2BufferHiz

[Chad Versace]

Before this commit, if a client were to request a stencil or hiz buffer, then
I830DRI2CreateBuffer() allocated and returned an X-tiled buffer by
accident. (DRI2BufferStencil and DRI2BufferHiz were unintentionally caught
by the default case of a switch statement.)

Now, I830DRI2CreateBuffer() correctly returns a Y-tiled buffer and handles
the stencil buffer as a special case due its quirky pitch requirements.

This shouldn't break older Mesa versions, because they never query (via
DRI2GetBuffersWithFormat) for the drawable's DRI2BufferStencil.

Signed-off-by: Chad Versace <chad@chad-versace.us>
---
 src/intel_dri.c |   30 ++++++++++++++++++++++++++----
 1 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/intel_dri.c b/src/intel_dri.c
index 48d0f56..4bcec45 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -329,11 +329,16 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 		pixmap = get_front_buffer(drawable);
 	if (pixmap == NULL) {
 		unsigned int hint = INTEL_CREATE_PIXMAP_DRI2;
+		int pixmap_width = drawable->width;
+		int pixmap_height = drawable->height;
+		int pixmap_cpp = (format != 0) ? format : drawable->depth;
 
 		if (intel->tiling & INTEL_TILING_3D) {
 			switch (attachment) {
 			case DRI2BufferDepth:
 			case DRI2BufferDepthStencil:
+			case DRI2BufferStencil:
+			case DRI2BufferHiz:
 				if (SUPPORTS_YTILING(intel)) {
 					hint |= INTEL_CREATE_PIXMAP_TILING_Y;
 					break;
@@ -344,11 +349,28 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 			}
 		}
 
+		/*
+		 * The stencil buffer has quirky pitch requirements.  From Vol
+		 * 2a, 11.5.6.2.1 3DSTATE_STENCIL_BUFFER, field "Surface
+		 * Pitch":
+		 *    The pitch must be set to 2x the value computed based on
+		 *    width, as the stencil buffer is stored with two rows
+		 *    interleaved.
+		 * To accomplish this, we resort to the nasty hack of doubling
+		 * the drm region's cpp and halving its height.
+		 *
+		 * If we neglect to double the pitch, then
+		 * drm_intel_gem_bo_map_gtt() maps the memory incorrectly.
+		 */
+		if (attachment == DRI2BufferStencil) {
+			pixmap_height /= 2;
+			pixmap_cpp *= 2;
+		}
+
 		pixmap = screen->CreatePixmap(screen,
-					      drawable->width,
-					      drawable->height,
-					      (format != 0) ? format :
-							      drawable->depth,
+					      pixmap_width,
+					      pixmap_height,
+					      pixmap_cpp,
 					      hint);
 		if (pixmap == NULL || intel_get_pixmap_bo(pixmap) == NULL) {
 			if (pixmap)
-- 
1.7.5.2

Re: [PATCH 2/2] dri: Add support for DRI2BufferStencil and DRI2BufferHiz

[Kenneth Graunke]

On 06/04/2011 05:06 PM, Chad Versace wrote:
> Before this commit, if a client were to request a stencil or hiz buffer, then
> I830DRI2CreateBuffer() allocated and returned an X-tiled buffer by
> accident. (DRI2BufferStencil and DRI2BufferHiz were unintentionally caught
> by the default case of a switch statement.)
>
> Now, I830DRI2CreateBuffer() correctly returns a Y-tiled buffer and handles
> the stencil buffer as a special case due its quirky pitch requirements.
>
> This shouldn't break older Mesa versions, because they never query (via
> DRI2GetBuffersWithFormat) for the drawable's DRI2BufferStencil.
>
> Signed-off-by: Chad Versace<chad@chad-versace.us>
> ---
>   src/intel_dri.c |   30 ++++++++++++++++++++++++++----
>   1 files changed, 26 insertions(+), 4 deletions(-)

This looks good to me, though I'm not too familiar with the DDX code.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

Re: [PATCH 2/2] dri: Add support for DRI2BufferStencil and DRI2BufferHiz

[Chris Wilson]

On Sat,  4 Jun 2011 17:06:31 -0700, Chad Versace <chad@chad-versace.us> wrote:
> Before this commit, if a client were to request a stencil or hiz buffer, then
> I830DRI2CreateBuffer() allocated and returned an X-tiled buffer by
> accident. (DRI2BufferStencil and DRI2BufferHiz were unintentionally caught
> by the default case of a switch statement.)
> 
> Now, I830DRI2CreateBuffer() correctly returns a Y-tiled buffer and handles
> the stencil buffer as a special case due its quirky pitch requirements.

Yeah, this really highlights how broken the ddx is by conflating Pixmaps
and RenderBuffer attachments.

However as a short-term feature addition, it looks fine.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/2] dri: Add support for DRI2BufferStencil and DRI2BufferHiz

[Eric Anholt]

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

On Sat,  4 Jun 2011 17:06:31 -0700, Chad Versace <chad@chad-versace.us> wrote:
> Before this commit, if a client were to request a stencil or hiz buffer, then
> I830DRI2CreateBuffer() allocated and returned an X-tiled buffer by
> accident. (DRI2BufferStencil and DRI2BufferHiz were unintentionally caught
> by the default case of a switch statement.)
> 
> Now, I830DRI2CreateBuffer() correctly returns a Y-tiled buffer and handles
> the stencil buffer as a special case due its quirky pitch requirements.
> 
> This shouldn't break older Mesa versions, because they never query (via
> DRI2GetBuffersWithFormat) for the drawable's DRI2BufferStencil.
> 
> Signed-off-by: Chad Versace <chad@chad-versace.us>

This should make configure.ac depend on the bumped version of
dri2proto.h, so people get an explanation instead of just build failure.

Also, I'm surprised to see you didn't fix the "we'll allocate a buffer
for *anything*" behavior that made the compat path in Mesa strange this
time around.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 2/2] dri: Add support for DRI2BufferStencil and DRI2BufferHiz

[Chad Versace]

On Sun, 05 Jun 2011 10:10:39 -0700, Eric Anholt <eric@anholt.net> wrote:
> On Sat,  4 Jun 2011 17:06:31 -0700, Chad Versace <chad@chad-versace.us> wrote:
> > Before this commit, if a client were to request a stencil or hiz buffer, then
> > I830DRI2CreateBuffer() allocated and returned an X-tiled buffer by
> > accident. (DRI2BufferStencil and DRI2BufferHiz were unintentionally caught
> > by the default case of a switch statement.)
> > 
> > Now, I830DRI2CreateBuffer() correctly returns a Y-tiled buffer and handles
> > the stencil buffer as a special case due its quirky pitch requirements.
> > 
> > This shouldn't break older Mesa versions, because they never query (via
> > DRI2GetBuffersWithFormat) for the drawable's DRI2BufferStencil.
> > 
> > Signed-off-by: Chad Versace <chad@chad-versace.us>
> 
> This should make configure.ac depend on the bumped version of
> dri2proto.h, so people get an explanation instead of just build failure.
> 
> Also, I'm surprised to see you didn't fix the "we'll allocate a buffer
> for *anything*" behavior that made the compat path in Mesa strange this
> time around.

Eric, I've fixed the patch series according to your comments and resubmitted.

I don't know why I didn't fix the "we'll allocate anything you ask for"
problem; guess I was just too busy digging in other things.