[Mesa-dev] [PATCH 16/27] i965/miptree: Allocate mcs_buf for an image's CCS_E
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Sat Dec 10 13:26:02 UTC 2016
On Thu, Dec 01, 2016 at 02:09:57PM -0800, Ben Widawsky wrote:
> From: Ben Widawsky <ben at bwidawsk.net>
>
> This code will disable actually creating these buffers for the scanout,
> but it puts the allocation in place.
>
> Primarily this patch is split out for review, it can be squashed in
> later if preferred.
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 89 +++++++++++++++++++++++----
> 1 file changed, 77 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index cfa2dc0..d002546 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -58,6 +58,11 @@ intel_miptree_alloc_mcs(struct brw_context *brw,
> struct intel_mipmap_tree *mt,
> GLuint num_samples);
>
> +static void
> +intel_miptree_init_mcs(struct brw_context *brw,
> + struct intel_mipmap_tree *mt,
> + int init_value);
> +
> /**
> * Determine which MSAA layout should be used by the MSAA surface being
> * created, based on the chip generation and the surface type.
> @@ -152,6 +157,9 @@ intel_miptree_supports_non_msrt_fast_clear(struct brw_context *brw,
> if (mt->disable_aux_buffers)
> return false;
>
> + if (mt->is_scanout)
> + return false;
> +
> /* This function applies only to non-multisampled render targets. */
> if (mt->num_samples > 1)
> return false;
> @@ -744,6 +752,7 @@ intel_miptree_create(struct brw_context *brw,
> * resolves.
> */
> const bool lossless_compression_disabled = INTEL_DEBUG & DEBUG_NO_RBC;
> + assert(!mt->is_scanout);
> const bool is_lossless_compressed =
> unlikely(!lossless_compression_disabled) &&
> brw->gen >= 9 && !mt->is_scanout &&
> @@ -810,6 +819,36 @@ intel_miptree_create_for_bo(struct brw_context *brw,
> return mt;
> }
>
> +static bool
> +create_ccs_buf_for_image(struct brw_context *intel,
> + __DRIimage *image,
> + struct intel_mipmap_tree *mt)
> +{
> +
> + struct isl_surf temp_main_surf;
> + struct isl_surf temp_ccs_surf;
> + uint32_t offset = mt->offset + image->aux_offset;
I need to ask about this as I'm not sure myself. We usually use offset to
move to a slice other than the first. Here the offset to the start of the
auxiliary is taken as relative to that. This seems awkward to me. Moreover,
if we move to another slice, don't we need to move equally within the auxiliary
also.
Further down you assert for mipmap levels "assert(mt->last_level < 2)" just
before calling this. So shoudln't we actually assert here that:
assert(mt->offset == 0);
> +
> + intel_miptree_get_isl_surf(intel, mt, &temp_main_surf);
> + if (!isl_surf_get_ccs_surf(&intel->isl_dev, &temp_main_surf, &temp_ccs_surf))
> + return false;
> +
> + mt->mcs_buf = calloc(1, sizeof(*mt->mcs_buf));
> + mt->mcs_buf->bo = image->bo;
> + drm_intel_bo_reference(image->bo);
> +
> + mt->mcs_buf->offset = offset;
> + mt->mcs_buf->size = temp_ccs_surf.size;
> + mt->mcs_buf->pitch = temp_ccs_surf.row_pitch;
> + mt->mcs_buf->qpitch = isl_surf_get_array_pitch_sa_rows(&temp_ccs_surf);
> +
> + intel_miptree_init_mcs(intel, mt, 0);
> + mt->no_ccs = false;
> + mt->msaa_layout = INTEL_MSAA_LAYOUT_CMS;
> +
> + return true;
> +}
> +
> struct intel_mipmap_tree *
> intel_miptree_create_for_image(struct brw_context *intel,
> __DRIimage *image,
> @@ -820,17 +859,43 @@ intel_miptree_create_for_image(struct brw_context *intel,
> uint32_t pitch,
> uint32_t layout_flags)
> {
> - layout_flags = (layout_flags & MIPTREE_LAYOUT_FOR_SCANOUT) ?
> - MIPTREE_LAYOUT_FOR_SCANOUT : MIPTREE_LAYOUT_DISABLE_AUX;
> - return intel_miptree_create_for_bo(intel,
> - image->bo,
> - format,
> - offset,
> - width,
> - height,
> - 1,
> - pitch,
> - layout_flags);
> + struct intel_mipmap_tree *mt;
> +
> + /* Other flags will be ignored, so make sure the caller didn't pass any. */
> + assert((layout_flags & ~MIPTREE_LAYOUT_FOR_SCANOUT) == 0);
> +
> + if (!image->aux_offset)
> + layout_flags |= MIPTREE_LAYOUT_DISABLE_AUX;
> + else
> + layout_flags |= MIPTREE_LAYOUT_FORCE_HALIGN16;
> +
> + mt = intel_miptree_create_for_bo(intel,
> + image->bo,
> + format,
> + offset,
> + width,
> + height,
> + 1,
> + pitch,
> + layout_flags);
> +
> + if (!intel_tiling_supports_non_msrt_mcs(intel, mt->tiling)) {
> + assert(image->aux_offset == 0);
> + return mt;
> + }
> +
> + if (layout_flags & MIPTREE_LAYOUT_DISABLE_AUX)
> + return mt;
> +
> + layout_flags &= ~MIPTREE_LAYOUT_FOR_SCANOUT;
This isn't used in the rest of the function anymore.
> +
> + assert(image->aux_offset);
> + assert(mt->num_samples >= 0);
This is always true isn't it? Did you mean:
assert(mt->num_samples <= 1);
> + assert(mt->last_level < 2);
Should we also:
assert(mt->logical_depth0 == 1);
> +
> + create_ccs_buf_for_image(intel, image, mt);
> +
> + return mt;
> }
>
> /**
> @@ -991,7 +1056,7 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
> drm_intel_bo_unreference((*mt)->hiz_buf->aux_base.bo);
> free((*mt)->hiz_buf);
> }
> - if ((*mt)->mcs_buf) {
> + if ((*mt)->mcs_buf && !(*mt)->is_scanout) {
> drm_intel_bo_unreference((*mt)->mcs_buf->bo);
> free((*mt)->mcs_buf);
> }
> --
> 2.10.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list