On Wed, Apr 20, 2016 at 09:49:02PM -0700, Ben Widawsky wrote:
> From: Ben Widawsky <benjamin.widaw...@intel.com>
> 
> Starting with Skylake, the display engine is capable of scanning out from
> Y-tiled buffers. As such, we can and should use Y-tiling for better 
> efficiency.
> This also has the added benefit of being able to fast clear the winsys buffer.
> 
> Note that the buffer allocation done for mipmaps will already never allocate 
> an
> X-tiled buffer for GEN9.
> 
> Here is the statistically significant difference (percentage) with this 
> patch. I
> do not know why we did see these kind of results earlier. This is on a SKL 
> gt3e
> with a resolution of 3200x1800.

Yes, please. A few comments but:

Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>

> 
> Benchmark          a-master->b-yscanout
> OglBatch0          8.56
> OglBatch1          6.85
> OglBatch2          5.0
> OglBatch3          4.48
> OglBatch4          4.16
> OglBatch6          3.57
> OglBatch7          2.96
> OglFillPixel       1.91
> OglGeomPoint       4.43
> OglGeomTriList     2.83
> OglPSBump8         0.32
> OglShMapVsm        6.61
> OglTerrainFlyInst  5.2
> OglVSTangent       3.03
> OglZBuffer         8.88
> egypt              14.54
> manhattan          1.59
> manhattanoff       -0.25
> triangle           15.35
> warsow             2.26
> xonotic            7.9
> 
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
>  src/mesa/drivers/dri/i965/brw_meta_fast_clear.c |  4 ++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c   |  9 +++++++--
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h   |  3 ++-
>  src/mesa/drivers/dri/i965/intel_screen.c        | 21 ++++++++++++++++++---
>  4 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c 
> b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> index 1fb5dc8..cf634a2 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> @@ -240,7 +240,7 @@ get_fast_clear_rect(struct brw_context *brw, struct 
> gl_framebuffer *fb,
>         * alignment size returned by intel_get_non_msrt_mcs_alignment(), but
>         * with X alignment multiplied by 16 and Y alignment multiplied by 32.
>         */
> -      intel_get_non_msrt_mcs_alignment(irb->mt, &x_align, &y_align);
> +      intel_get_non_msrt_mcs_alignment(brw, irb->mt, &x_align, &y_align);
>        x_align *= 16;
>  
>        /* SKL+ line alignment requirement for Y-tiled are half those of the 
> prior
> @@ -821,7 +821,7 @@ get_resolve_rect(struct brw_context *brw,
>      * by a factor of 2.
>      */
>  
> -   intel_get_non_msrt_mcs_alignment(mt, &x_align, &y_align);
> +   intel_get_non_msrt_mcs_alignment(brw, mt, &x_align, &y_align);
>     if (brw->gen >= 9) {
>        x_scaledown = x_align * 8;
>        y_scaledown = y_align * 8;
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index d263ff8..a793370 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -144,7 +144,8 @@ compute_msaa_layout(struct brw_context *brw, mesa_format 
> format,
>   *   by half the block width, and Y coordinates by half the block height.
>   */
>  void
> -intel_get_non_msrt_mcs_alignment(struct intel_mipmap_tree *mt,
> +intel_get_non_msrt_mcs_alignment(const struct brw_context *brw,
> +                                 struct intel_mipmap_tree *mt,

I think you could mark mt as const while you are at it if you like.

>                                   unsigned *width_px, unsigned *height)
>  {
>     switch (mt->tiling) {
> @@ -156,6 +157,10 @@ intel_get_non_msrt_mcs_alignment(struct 
> intel_mipmap_tree *mt,
>        *height = 4;
>        break;
>     case I915_TILING_X:
> +      /* The docs are somewhat confusing with the way the tables are 
> displayed.
> +       * However, it does clearly state: "MCS and Lossless compression is
> +       * supported for TiledY/TileYs/TileYf non-MSRTs only." */

Comment closing on its own line. I'm also a little confused if this comment is
really related to this patch.

> +      assert(brw->gen < 9);
>        *width_px = 64 / mt->cpp;
>        *height = 2;
>     }
> @@ -1552,7 +1557,7 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context 
> *brw,
>     const mesa_format format = MESA_FORMAT_R_UINT32;
>     unsigned block_width_px;
>     unsigned block_height;
> -   intel_get_non_msrt_mcs_alignment(mt, &block_width_px, &block_height);
> +   intel_get_non_msrt_mcs_alignment(brw, mt, &block_width_px, &block_height);
>     unsigned width_divisor = block_width_px * 4;
>     unsigned height_divisor = block_height * 8;
>  
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 7cdfb37..826f572 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -663,7 +663,8 @@ struct intel_mipmap_tree
>  };
>  
>  void
> -intel_get_non_msrt_mcs_alignment(struct intel_mipmap_tree *mt,
> +intel_get_non_msrt_mcs_alignment(const struct brw_context *brw,
> +                                 struct intel_mipmap_tree *mt,
>                                   unsigned *width_px, unsigned *height);
>  
>  bool
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index db9d94d..878901a 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -516,7 +516,11 @@ intel_create_image(__DRIscreen *screen,
>     int cpp;
>     unsigned long pitch;
>  
> -   tiling = I915_TILING_X;
> +   if (intelScreen->devinfo->gen >= 9) {
> +      tiling = I915_TILING_Y;
> +   } else {
> +      tiling = I915_TILING_X;
> +   }
>     if (use & __DRI_IMAGE_USE_CURSOR) {
>        if (width != 64 || height != 64)
>        return NULL;
> @@ -1144,8 +1148,14 @@ intel_detect_swizzling(struct intel_screen *screen)
>     drm_intel_bo *buffer;
>     unsigned long flags = 0;
>     unsigned long aligned_pitch;
> -   uint32_t tiling = I915_TILING_X;
>     uint32_t swizzle_mode = 0;
> +   uint32_t tiling;
> +
> +   if (screen->devinfo->gen >= 9) {
> +      tiling = I915_TILING_Y;
> +   } else {
> +      tiling = I915_TILING_X;
> +   }
>  
>     buffer = drm_intel_bo_alloc_tiled(screen->bufmgr, "swizzle test",
>                                    64, 64, 4,
> @@ -1571,7 +1581,12 @@ intelAllocateBuffer(__DRIscreen *screen,
>        return NULL;
>  
>     /* The front and back buffers are color buffers, which are X tiled. */
> -   uint32_t tiling = I915_TILING_X;
> +   uint32_t tiling;
> +   if (intelScreen->devinfo->gen >= 9) {
> +      tiling = I915_TILING_Y;
> +   } else {
> +      tiling = I915_TILING_X;
> +   }
>     unsigned long pitch;
>     int cpp = format / 8;
>     intelBuffer->bo = drm_intel_bo_alloc_tiled(intelScreen->bufmgr,
> -- 
> 2.8.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to