[Mesa-dev] [PATCH 15/21] intel/blorp: Use isl_surf_get_uncompressed_surf

[Jason Ekstrand]

On February 28, 2018 09:23:59 "Pohjolainen, Topi" 
<topi.pohjolainen at gmail.com> wrote:

> On Thu, Feb 22, 2018 at 11:06:55PM -0800, Jason Ekstrand wrote:
>> ---
>>  src/intel/blorp/blorp_blit.c | 58 +++++++++++++-------------------------------
>>  1 file changed, 17 insertions(+), 41 deletions(-)
>>
>> diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
>> index 876498d..5b63754 100644
>> --- a/src/intel/blorp/blorp_blit.c
>> +++ b/src/intel/blorp/blorp_blit.c
>> @@ -2349,50 +2349,26 @@ blorp_surf_convert_to_uncompressed(const struct 
>> isl_device *isl_dev,
>>
>>     assert(fmtl->bw > 1 || fmtl->bh > 1);
>>
>> -   /* This is a compressed surface.  We need to convert it to a single
>> -    * slice (because compressed layouts don't perfectly match uncompressed
>> -    * ones with the same bpb) and divide x, y, width, and height by the
>> -    * block size.
>> -    */
>> -   blorp_surf_convert_to_single_slice(isl_dev, info);
>> -
>> -   if (width && height) {
>> -#ifndef NDEBUG
>> -      uint32_t right_edge_px = info->tile_x_sa + *x + *width;
>> -      uint32_t bottom_edge_px = info->tile_y_sa + *y + *height;
>> -      assert(*width % fmtl->bw == 0 ||
>> -             right_edge_px == info->surf.logical_level0_px.width);
>> -      assert(*height % fmtl->bh == 0 ||
>> -             bottom_edge_px == info->surf.logical_level0_px.height);
>> -#endif
>> -      *width = DIV_ROUND_UP(*width, fmtl->bw);
>> -      *height = DIV_ROUND_UP(*height, fmtl->bh);
>> -   }
>> -
>> -   if (x && y) {
>> -      assert(*x % fmtl->bw == 0);
>> -      assert(*y % fmtl->bh == 0);
>> -      *x /= fmtl->bw;
>> -      *y /= fmtl->bh;
>> -   }
>> -
>> -   info->surf.logical_level0_px.width =
>> -      DIV_ROUND_UP(info->surf.logical_level0_px.width, fmtl->bw);
>> -   info->surf.logical_level0_px.height =
>> -      DIV_ROUND_UP(info->surf.logical_level0_px.height, fmtl->bh);
>> +   /* It's now an uncompressed surface so we need an uncompressed format */
>> +   info->view.format = get_copy_format_for_bpb(isl_dev, fmtl->bpb);
>>
>> -   assert(info->surf.phys_level0_sa.width % fmtl->bw == 0);
>> -   assert(info->surf.phys_level0_sa.height % fmtl->bh == 0);
>> -   info->surf.phys_level0_sa.width /= fmtl->bw;
>> -   info->surf.phys_level0_sa.height /= fmtl->bh;
>> +   /* We only one one level and slice */
>> +   info->view.levels = 1;
>> +   info->view.array_len = 1;
>>
>> -   assert(info->tile_x_sa % fmtl->bw == 0);
>> -   assert(info->tile_y_sa % fmtl->bh == 0);
>> -   info->tile_x_sa /= fmtl->bw;
>> -   info->tile_y_sa /= fmtl->bh;
>> +   uint32_t offset_B;
>> +   isl_surf_get_uncompressed_surf(isl_dev, &info->surf, &info->view,
>> +                                  &info->surf, &info->view, &offset_B,
>> +                                  &info->tile_x_sa, &info->tile_y_sa);
>
> Here we pass info->view in separate arguments for reading and writing. Having
> just read the implementation I know that all the reading takes place before
> any writing. But somebody might later on change
> isl_surf_get_uncompressed_surf() in way that breaks here.

The documentation for isl_surf_get_uncompressed_surf explicitly says that 
this is a supported usage model.  My expectation was that we would continue 
to provide that guarantee.

> I was happy when you took away one or two such constructs earlier in the
> series.

Are you saying that you would rather BLORP explicitly make a copy?  We 
could do that if you wanted.

--Jason

>> +   info->addr.offset += offset_B;
>>
>> -   /* It's now an uncompressed surface so we need an uncompressed format */
>> -   info->surf.format = get_copy_format_for_bpb(isl_dev, fmtl->bpb);
>> +   /* BLORP doesn't use the actual intratile offsets.  Instead, it needs the
>> +    * surface to be a bit bigger and we offset the vertices instead.
>> +    */
>> +   info->surf.logical_level0_px.w += info->tile_x_sa;
>> +   info->surf.logical_level0_px.h += info->tile_y_sa;
>> +   info->surf.phys_level0_sa.w += info->tile_x_sa;
>> +   info->surf.phys_level0_sa.h += info->tile_y_sa;
>>  }
>>
>>  void
>> --
>> 2.5.0.400.gff86faf
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev