Revert "i965: Always use Y-tiled buffers on SKL+"

Submitted by Daniel Stone on May 2, 2016, 2:40 p.m.

Details

Message ID 1462200014-23888-1-git-send-email-daniels@collabora.com
State New
Headers show
Series "Revert "i965: Always use Y-tiled buffers on SKL+"" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Daniel Stone May 2, 2016, 2:40 p.m.
This commit broke Weston/KMS, and presumably also xf86-video-modesetting.

In order to use Y-tiled buffers, the kernel requires the tiling mode to
be explicitly named through the I915_FORMAT_MOD_Y_TILED AddFB2 modifier;
it disallows any attempt to infer the buffer's tiling mode.

As the GBM API does not have a way to extract modifiers for a buffer,
this commit broke all users of GBM on SKL+. Revert it for now, until we
get a way to extract modifier information from GBM, and also let GBM
users inform the implementation that it intends to use the modifiers.

This reverts commit 6a0d036483caf87d43ebe2edd1905873446c9589.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
---
 src/mesa/drivers/dri/i965/brw_meta_fast_clear.c |  4 ++--
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c   | 10 ++--------
 src/mesa/drivers/dri/i965/intel_mipmap_tree.h   |  3 +--
 src/mesa/drivers/dri/i965/intel_screen.c        | 21 +++------------------
 4 files changed, 8 insertions(+), 30 deletions(-)

Patch hide | download patch | download mbox

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 7760cce..76988bf 100644
--- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
+++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
@@ -244,7 +244,7 @@  brw_get_fast_clear_rect(const struct brw_context *brw,
        * 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(brw, mt, &x_align, &y_align);
+      intel_get_non_msrt_mcs_alignment(mt, &x_align, &y_align);
       x_align *= 16;
 
       /* SKL+ line alignment requirement for Y-tiled are half those of the prior
@@ -838,7 +838,7 @@  brw_get_resolve_rect(const struct brw_context *brw,
     * by a factor of 2.
     */
 
-   intel_get_non_msrt_mcs_alignment(brw, mt, &x_align, &y_align);
+   intel_get_non_msrt_mcs_alignment(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 3d8f48e..94f6333 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -144,8 +144,7 @@  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(const struct brw_context *brw,
-                                 const struct intel_mipmap_tree *mt,
+intel_get_non_msrt_mcs_alignment(const struct intel_mipmap_tree *mt,
                                  unsigned *width_px, unsigned *height)
 {
    switch (mt->tiling) {
@@ -157,11 +156,6 @@  intel_get_non_msrt_mcs_alignment(const struct brw_context *brw,
       *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."
-       */
-      assert(brw->gen < 9);
       *width_px = 64 / mt->cpp;
       *height = 2;
    }
@@ -1558,7 +1552,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(brw, mt, &block_width_px, &block_height);
+   intel_get_non_msrt_mcs_alignment(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 21e4718..7862152 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -663,8 +663,7 @@  struct intel_mipmap_tree
 };
 
 void
-intel_get_non_msrt_mcs_alignment(const struct brw_context *brw,
-                                 const struct intel_mipmap_tree *mt,
+intel_get_non_msrt_mcs_alignment(const 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 878901a..db9d94d 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -516,11 +516,7 @@  intel_create_image(__DRIscreen *screen,
    int cpp;
    unsigned long pitch;
 
-   if (intelScreen->devinfo->gen >= 9) {
-      tiling = I915_TILING_Y;
-   } else {
-      tiling = I915_TILING_X;
-   }
+   tiling = I915_TILING_X;
    if (use & __DRI_IMAGE_USE_CURSOR) {
       if (width != 64 || height != 64)
 	 return NULL;
@@ -1148,14 +1144,8 @@  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,
@@ -1581,12 +1571,7 @@  intelAllocateBuffer(__DRIscreen *screen,
       return NULL;
 
    /* The front and back buffers are color buffers, which are X tiled. */
-   uint32_t tiling;
-   if (intelScreen->devinfo->gen >= 9) {
-      tiling = I915_TILING_Y;
-   } else {
-      tiling = I915_TILING_X;
-   }
+   uint32_t tiling = I915_TILING_X;
    unsigned long pitch;
    int cpp = format / 8;
    intelBuffer->bo = drm_intel_bo_alloc_tiled(intelScreen->bufmgr,

Comments

On Monday, May 2, 2016 3:40:14 PM PDT Daniel Stone wrote:
> This commit broke Weston/KMS, and presumably also xf86-video-modesetting.

For me, xf86-video-modesetting, and xf86-video-intel/SNA+DRI3 both work
fine with Y-tiling enabled.  However, it does break UXA+DRI3.

I'm curious why xf86-video-modesetting would continue to work, while
Weston/KMS broke...it sounds like modesetting shouldn't work either...

--Ken
On Mon, May 02, 2016 at 11:38:15AM -0700, Kenneth Graunke wrote:
> On Monday, May 2, 2016 3:40:14 PM PDT Daniel Stone wrote:
> > This commit broke Weston/KMS, and presumably also xf86-video-modesetting.
> 
> For me, xf86-video-modesetting, and xf86-video-intel/SNA+DRI3 both work
> fine with Y-tiling enabled.  However, it does break UXA+DRI3.
> 
> I'm curious why xf86-video-modesetting would continue to work, while
> Weston/KMS broke...it sounds like modesetting shouldn't work either...

I think -modesetting should keep working as long as you copy from the
client buffer's to the screen. For frontbuffer flipping it /should/ blow
up I think too.
-Daniel
Hi,

On 9 May 2016 at 08:05, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, May 02, 2016 at 11:38:15AM -0700, Kenneth Graunke wrote:
>> On Monday, May 2, 2016 3:40:14 PM PDT Daniel Stone wrote:
>> > This commit broke Weston/KMS, and presumably also xf86-video-modesetting.
>>
>> For me, xf86-video-modesetting, and xf86-video-intel/SNA+DRI3 both work
>> fine with Y-tiling enabled.  However, it does break UXA+DRI3.
>>
>> I'm curious why xf86-video-modesetting would continue to work, while
>> Weston/KMS broke...it sounds like modesetting shouldn't work either...
>
> I think -modesetting should keep working as long as you copy from the
> client buffer's to the screen. For frontbuffer flipping it /should/ blow
> up I think too.

The screen buffer is allocated by GBM (when using Glamor for
acceleration), which as of this update would be Y-tiled as well. So,
doesn't really help at all.

Cheers,
Daniel