On Wed, Jan 25, 2017 at 10:36 AM, Pohjolainen, Topi < topi.pohjolai...@gmail.com> wrote:
> 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; > } > That's nicer.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev