[Mesa-dev] [PATCH 23/34] i965: Add logic for allocating BO with CCS

[Ben Widawsky]

On 17-01-31 13:03:52, Jason Ekstrand wrote:
>On Wed, Jan 25, 2017 at 10:36 AM, Pohjolainen, Topi <
>topi.pohjolainen at 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 <ben at bwidawsk.net>
>> > Acked-by: Daniel Stone <daniels at collabora.com>
>> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at 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.

The problem here is that I don't have a readily available mt AFAICT.