[Mesa-dev] [PATCH 1/2] isl: remove the cache line size alignment requirement

[Lionel Landwerlin]

On 15/02/2019 14:43, Samuel Iglesias Gonsálvez wrote:
> There are formats which bpp are not aligned to a power-of-two and
> that can cause problems in the checks we do.
>
> The cacheline size was a requirement for using the BLT engine, which
> we don't use anymore except for a few things on old HW, so we drop it.
>
> Fixes CTS's CL#3500 test:
>
> dEQP-VK.api.image_clearing.core.clear_color_image.2d.linear.single_layer.r8g8b8_unorm
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>


That looks good to me :
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

I'm doing a CI run just to convince myself, so if you can wait for that.

Thanks,

-Lionel


> ---
>   src/intel/isl/isl.c | 21 ++++-----------------
>   1 file changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index eaaa28014a3..7f1f2339931 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -1381,20 +1381,6 @@ isl_calc_row_pitch(const struct isl_device *dev,
>      uint32_t alignment_B =
>         isl_calc_row_pitch_alignment(surf_info, tile_info);
>   
> -   /* If pitch isn't given and it can be chosen freely, align it by cache line
> -    * allowing one to use blit engine on the surface.
> -    */
> -   if (surf_info->row_pitch_B == 0 && tile_info->tiling == ISL_TILING_LINEAR) {
> -      /* From the Broadwell PRM docs for XY_SRC_COPY_BLT::SourceBaseAddress:
> -       *
> -       *    "Base address of the destination surface: X=0, Y=0. Lower 32bits
> -       *    of the 48bit addressing. When Src Tiling is enabled (Bit_15
> -       *    enabled), this address must be 4KB-aligned. When Tiling is not
> -       *    enabled, this address should be CL (64byte) aligned."
> -       */
> -      alignment_B = MAX2(alignment_B, 64);
> -   }
> -
>      const uint32_t min_row_pitch_B =
>         isl_calc_min_row_pitch(dev, surf_info, tile_info, phys_total_el,
>                                alignment_B);
> @@ -1527,12 +1513,13 @@ isl_surf_init_s(const struct isl_device *dev,
>         base_alignment_B = MAX(1, info->min_alignment_B);
>         if (info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) {
>            if (isl_format_is_yuv(info->format)) {
> -            base_alignment_B = MAX(base_alignment_B, fmtl->bpb / 4);
> +            base_alignment_B = isl_align_npot(base_alignment_B, fmtl->bpb / 4);
>            } else {
> -            base_alignment_B = MAX(base_alignment_B, fmtl->bpb / 8);
> +            base_alignment_B = isl_align_npot(base_alignment_B, fmtl->bpb / 8);
>            }
> +      } else {
> +         base_alignment_B = isl_round_up_to_power_of_two(base_alignment_B);
>         }
> -      base_alignment_B = isl_round_up_to_power_of_two(base_alignment_B);
>      } else {
>         const uint32_t total_h_tl =
>            isl_align_div(phys_total_el.h, tile_info.logical_extent_el.height);