[Mesa-dev] [RFC PATCH 21/26] i965: add support for multisample textures
Paul Berry
stereotype441 at gmail.com
Fri Jan 4 13:42:30 PST 2013
On 29 December 2012 04:35, Chris Forbes <chrisf at ijw.co.nz> wrote:
> This is a bit nasty -- I've just (ab)used the existing multisample
> renderbuffer path.
>
> Also put the actual sample count from the mt back into the
> renderbuffer so core mesa can see what the driver actually gave it.
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 ++
> src/mesa/drivers/dri/intel/intel_fbo.c | 3 ++-
> src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 30
> ++++++++++++++++++++----
> src/mesa/drivers/dri/intel/intel_mipmap_tree.h | 2 ++
> src/mesa/drivers/dri/intel/intel_tex.c | 18 +++++++++++---
> 5 files changed, 47 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 94c01cb..1c54d59 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -59,6 +59,8 @@ translate_tex_target(GLenum target)
> case GL_TEXTURE_2D:
> case GL_TEXTURE_2D_ARRAY_EXT:
> case GL_TEXTURE_EXTERNAL_OES:
> + case GL_TEXTURE_2D_MULTISAMPLE:
> + case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
> return BRW_SURFACE_2D;
>
> case GL_TEXTURE_3D:
> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
> b/src/mesa/drivers/dri/intel/intel_fbo.c
> index 6a66521..4cc2c00 100644
> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
> @@ -270,7 +270,7 @@ intel_alloc_renderbuffer_storage(struct gl_context *
> ctx, struct gl_renderbuffer
> return true;
>
> irb->mt = intel_miptree_create_for_renderbuffer(intel, rb->Format,
> - width, height,
> + width, height, 1,
> rb->NumSamples);
> if (!irb->mt)
> return false;
> @@ -509,6 +509,7 @@ intel_renderbuffer_update_wrapper(struct intel_context
> *intel,
> rb->_BaseFormat = image->_BaseFormat;
> rb->Width = mt->level[level].width;
> rb->Height = mt->level[level].height;
> + rb->NumSamples = mt->num_samples;
>
> rb->Delete = intel_delete_renderbuffer;
> rb->AllocStorage = intel_nop_alloc_storage;
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> index 9ae7bcc..989c1f6 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> @@ -404,6 +404,7 @@ intel_miptree_create_for_dri2_buffer(struct
> intel_context *intel,
> format,
> region->width,
> region->height,
> + 1,
> num_samples);
> if (!multisample_mt) {
> intel_miptree_release(&singlesample_mt);
> @@ -427,10 +428,10 @@ intel_miptree_create_for_renderbuffer(struct
> intel_context *intel,
> gl_format format,
> uint32_t width,
> uint32_t height,
> + uint32_t depth,
> uint32_t num_samples)
> {
> struct intel_mipmap_tree *mt;
> - uint32_t depth = 1;
> enum intel_msaa_layout msaa_layout = INTEL_MSAA_LAYOUT_NONE;
> const uint32_t singlesample_width = width;
> const uint32_t singlesample_height = height;
> @@ -491,6 +492,7 @@ intel_miptree_create_for_renderbuffer(struct
> intel_context *intel,
> }
> } else {
> /* Non-interleaved */
> + assert(depth == 1); /* XXX: do we ever hit this path with
> multisample arrays? */
>
Yeah, this assertion isn't going to work for Gen7's non-interleaved
multisample arrays.
Note that the Intel hardware docs use the term "physical" to describe the
dimensions of the data structure that stores the multisampled data (i.e.
after width, height, and/or depth have been scaled up to account for the
number of samples) and "logical" to describe the dimensions of the image
from the client's point of view. I think I'm going to start adopting that
terminology for clarity.
Bearing that in mind, the way the format works for Gen7 is that for each
logical plane requested by the client using the depth parameter, N
contiguous physical planes are allocated by hardware, one for each sample.
So, for example, if the user allocates a buffer of depth 2 using 4x MSAA,
the surface we allocate will need to have a physical depth of 8, with the
first 4 physical planes corresponding to samples 0-3 of the logical plane
having z coordinate 0, and the next 4 physical planes corresponding to
samples 0-3 of the logical plane having z coordinate 1. So I think what we
need to do here is remove the assertion and update depth like so:
depth *= num_samples
(of course everything is going to look a little different assuming we move
this logic into common code that's shared by
intel_miptree_create_for{texture,renderbuffer}).
> depth = num_samples;
> }
> }
> @@ -508,6 +510,7 @@ intel_miptree_create_for_renderbuffer(struct
> intel_context *intel,
> }
>
> if (mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) {
> + assert(depth == 1); /* XXX: multisample arrays interaction with
> MCS? */
> ok = intel_miptree_alloc_mcs(intel, mt, num_samples);
>
I don't think you need the assertion here--intel_miptree_alloc_mcs() reads
the depth out of the miptree, so it should allocate the proper size buffer
without us having to do any extra work here.
Having said that, I think I just discovered a bug:
intel_miptree_alloc_mcs() allocates a surface whose depth is equal to
mt->depth0, which is the physical depth. But it should be using the
logical depth, since the MCS buffer stores data per pixel, not per sample.
It's a fairly benign bug--it just means we're allocating more memory than
we need--but embarrassing nonetheless.
I'm beginning to think that what we really ought to do is:
- Rename intel_mipmap_tree::{width,height,depth}0 to
intel_mipmap_tree::physical_{width,height,depth}0 to clarify that they
represent the width, height, and depth *after* appropriate multiplications
have been made to account for the number of samples.
- Rename intel_mipmap_tree::singlesample_{width,height}0 to
intel_mipmap_tree::logical_{width,height}0 to clarify that they represent
the width and height of the surface from the client's point of view.
- Add intel_mipmap_tree::logical_depth0 to represent the depth of the
surface from the client's point of view.
- Modify the code that initializes intel_mipmap_tree so that
logical_{width,heigt,depth}0 are always initialized (at the moment they're
only initialized for multisampled surfaces, and I think that leads to
confusion).
Unless someone has objections, I'll try to submit a patch to do it ASAP
(and fix the overallocation of MCS memory), and then hopefully we can
rebase Chris's patches on top of it.
> if (!ok)
> goto fail;
> @@ -624,9 +627,27 @@ intel_miptree_match_image(struct intel_mipmap_tree
> *mt,
> * minification. This will also catch images not present in the
> * tree, changed targets, etc.
> */
> - if (width != mt->level[level].width ||
> - height != mt->level[level].height ||
> - depth != mt->level[level].depth)
> + if (mt->target == GL_TEXTURE_2D_MULTISAMPLE ||
> + mt->target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY) {
> + /* nonzero level here is always bogus */
> + assert(level == 0);
> +
> + if (width != mt->singlesample_width0 ||
> + height != mt->singlesample_height0 ||
> + depth != mt->level[level].depth) {
> + return false;
> + }
> + }
> + else {
> + /* all normal textures, renderbuffers, etc */
> + if (width != mt->level[level].width ||
> + height != mt->level[level].height ||
> + depth != mt->level[level].depth) {
> + return false;
> + }
> + }
> +
> + if (image->TexObject->NumSamples != mt->num_samples)
> return false;
>
> return true;
> @@ -1644,6 +1665,7 @@ intel_miptree_map_multisample(struct intel_context
> *intel,
> mt->format,
> mt->singlesample_width0,
> mt->singlesample_height0,
> + 1,
> 0 /*num_samples*/);
> if (!mt->singlesample_mt)
> goto fail;
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> index eb4ad7f..3de0019 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> @@ -405,12 +405,14 @@ intel_miptree_create_for_dri2_buffer(struct
> intel_context *intel,
> * - The target is GL_TEXTURE_2D.
> * - There are no levels other than the base level 0.
> * - Depth is 1.
> + * XXX: this description is quite bogus now.
> */
> struct intel_mipmap_tree*
> intel_miptree_create_for_renderbuffer(struct intel_context *intel,
> gl_format format,
> uint32_t width,
> uint32_t height,
> + uint32_t depth,
> uint32_t num_samples);
>
> /** \brief Assert that the level and layer are valid for the miptree. */
> diff --git a/src/mesa/drivers/dri/intel/intel_tex.c
> b/src/mesa/drivers/dri/intel/intel_tex.c
> index fdf5812..5f0301c 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex.c
> @@ -71,6 +71,7 @@ intel_alloc_texture_image_buffer(struct gl_context *ctx,
> switch (texobj->Target) {
> case GL_TEXTURE_3D:
> case GL_TEXTURE_2D_ARRAY:
> + case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
> slices = image->Depth;
> break;
> case GL_TEXTURE_1D_ARRAY:
> @@ -91,9 +92,20 @@ intel_alloc_texture_image_buffer(struct gl_context *ctx,
> __FUNCTION__, texobj, image->Level,
> image->Width, image->Height, image->Depth, intel_texobj->mt);
> } else {
> - intel_image->mt = intel_miptree_create_for_teximage(intel,
> intel_texobj,
> - intel_image,
> - false);
> +
> + if (texobj->NumSamples) {
> + /* Treat multisample textures exactly like renderbuffers */
> + intel_image->mt = intel_miptree_create_for_renderbuffer(intel,
> + image->TexFormat, image->Width, image->Height,
> image->Depth,
> + texobj->NumSamples);
> + /* Renderbuffer path doesn't bother to set the mt's target, so
> do it here */
> + intel_image->mt->target = texobj->Target;
> + }
> + else {
> + intel_image->mt = intel_miptree_create_for_teximage(intel,
> intel_texobj,
> + intel_image,
> + false);
> + }
>
> /* Even if the object currently has a mipmap tree associated
> * with it, this one is a more likely candidate to represent the
> --
> 1.8.0.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130104/1cab999c/attachment.html>
More information about the mesa-dev
mailing list