| Message ID | 1307232391-26973-3-git-send-email-chad@chad-versace.us (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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
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.
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.
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)
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(-)