On Mon, Jan 23, 2017 at 10:21:46PM -0800, Ben Widawsky wrote:
> This patch provides the support (and comments) for allocating the BO
> with space for the CCS buffer just underneath it.
>
> This patch was originally titled:
> "i965: Create correctly sized mcs for an image"
>
> In order to make things more bisectable, reviewable, and to have the
> CCS_MODIFIER token saved for the last patch, this patch now does less so
> it was renamed.
>
> v2: Leave "image+mod" (Topi)
>
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> Acked-by: Daniel Stone <dani...@collabora.com>
> Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> ---
> src/mesa/drivers/dri/i965/intel_screen.c | 34
++++++++++++++++++++++++++++----
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
b/src/mesa/drivers/dri/i965/intel_screen.c
> index 8ec33ce5df..971013f2dd 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -607,6 +607,7 @@ create_image_with_modifier(struct intel_screen
*screen,
> uint32_t requested_tiling = 0, tiling = I915_TILING_X;
> unsigned long pitch;
> unsigned tiled_height = 0;
> + unsigned ccs_height = 0;
>
> switch (modifier) {
> case I915_FORMAT_MOD_Y_TILED:
> @@ -628,9 +629,33 @@ create_image_with_modifier(struct intel_screen
*screen,
> break;
> }
>
> - image->bo = drm_intel_bo_alloc_tiled(screen->bufmgr, "image+mod",
> - width, tiled_height, cpp,
&tiling,
> - &pitch, 0);
> + /*
> + * CCS width is always going to be less than or equal to the image's
width.
> + * All we need to do is make sure we add extra rows (height) for the
CCS.
> + *
> + * A pair of CCS bits correspond to 8x4 pixels, and must be cacheline
> + * granularity. Each CCS tile is laid out in 8b strips, which
corresponds to
> + * 1024x512 pixel region. In memory, it looks like the following:
> + *
> + * ?????????????????????????????????????????????????????????
> + * ??? ???
> + * ??? ???
> + * ??? ???
> + * ??? Image ???
> + * ??? ???
> + * ??? ???
> + * ???xxxxxxxxxxxxxxxxx???
> + * ?????????????????????????????????????????????????????????
> + * ??? ??? |
> + * ???ccs ??? unused |
> + * ?????????????????????-----------???
> + * <------pitch------>
> + */
> + cpp = _mesa_get_format_bytes(image->format);
> + image->bo = drm_intel_bo_alloc_tiled(screen->bufmgr,
> + ccs_height ? "image+ccs" :
"image+mod",
> + width, tiled_height +
ccs_height,
> + cpp, &tiling, &pitch, 0);
> if (image->bo == NULL)
> return false;
>
> @@ -647,7 +672,8 @@ create_image_with_modifier(struct intel_screen
*screen,
> if (image->planar_format)
> assert(image->planar_format->nplanes == 1);
>
> - image->aux_offset = 0; /* y_tiled_height * pitch; */
> + if (ccs_height)
> + image->aux_offset = tiled_height * pitch /* + mt->offset */;
I think it would be clearer to drop the comment about mt->offset and
assert here also. How do you feel?
if (ccs_height) {
assert(mt->offset == 0);
image->aux_offset = tiled_height * pitch;
}
