<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Mar 21, 2017 at 8:04 AM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Mon, Mar 20, 2017 at 8:35 PM, Ben Widawsky <span dir="ltr"><<a href="mailto:ben@bwidawsk.net" target="_blank">ben@bwidawsk.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>This patch begins introducing how we'll actually handle the potentially<br>
many modifiers coming in from the API, how we'll store them, and the<br>
structure in the code to support it.<br>
<br>
Prior to this patch, the Y-tiled modifier would be entirely ignored. It<br>
shouldn't actually be used until this point because we've not bumped the<br>
DRIimage extension version (which is a requirement to use modifiers).<br>
<br>
</span>Measuring later in the series with kmscbe:<br>
Linear:<br>
Read bandwidth: 1048.44 MiB/s<br>
Write bandwidth: 1483.17 MiB/s<br>
<br>
Y-tiled:<br>
Read bandwidth: 471.13 MiB/s<br>
Write bandwidth: 589.10 MiB/s<br>
<span><br>
Similar functionality was introduced and then reverted here:<br>
<br>
commit 6a0d036483caf87d43ebe2edd19058<wbr>73446c9589<br>
Author: Ben Widawsky <<a href="mailto:ben@bwidawsk.net" target="_blank">ben@bwidawsk.net</a>><br>
Date: Thu Apr 21 20:14:58 2016 -0700<br>
<br>
i965: Always use Y-tiled buffers on SKL+<br>
<br>
v2: Use last set bit instead of first set bit in modifiers to address<br>
bug found by Daniel Stone.<br>
<br>
</span>v3: Use the new priority modifier selection thing. This nullifies the<br>
bug fixed by v2 also.<br>
<span><br>
Signed-off-by: Ben Widawsky <<a href="mailto:benjamin.widawsky@intel.com" target="_blank">benjamin.widawsky@intel.com</a>><br>
Reviewed-by: Eric Engestrom <<a href="mailto:eric.engestrom@imgtec.com" target="_blank">eric.engestrom@imgtec.com</a>><br>
Acked-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>><br>
---<br>
</span> src/gbm/backends/dri/gbm_dri.<wbr>c | 18 ++++++++++++++++<br>
src/mesa/drivers/dri/i965/int<wbr>el_screen.c | 36 ++++++++++++++++++++++++++++--<wbr>--<br>
2 files changed, 50 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/src/gbm/backends/dri/gbm_dri<wbr>.c b/src/gbm/backends/dri/gbm_dri<wbr>.c<br>
index a7ac149365..a78ea89fca 100644<br>
--- a/src/gbm/backends/dri/gbm_dri<wbr>.c<br>
+++ b/src/gbm/backends/dri/gbm_dri<wbr>.c<br>
@@ -1143,6 +1143,15 @@ gbm_dri_bo_create(struct gbm_device *gbm,<br>
goto failed;<br>
}<br>
<br>
+ if (modifiers) {<br>
<span>+ for (int i = 0; i < count; i++)<br>
</span>+ if (modifiers[i] == DRM_FORMAT_MOD_INVALID) {<br>
+ fprintf(stderr, "Invalid modifier passed in DRM_FORMAT_MOD_INVALID");<br>
+ errno = EINVAL;<br>
+ goto failed;<br>
+ }<br>
+ }<br>
+<br>
bo->image =<br>
dri->image->createImageWithMod<wbr>ifiers(dri->screen,<br>
width, height,<br>
@@ -1240,6 +1249,15 @@ gbm_dri_surface_create(struct gbm_device *gbm,<br>
return NULL;<br>
}<br>
<br>
+ if (modifiers) {<br>
<span>+ for (int i = 0; i < count; i++)<br>
</span>+ if (modifiers[i] == DRM_FORMAT_MOD_INVALID) {<br>
+ fprintf(stderr, "Invalid modifier passed in DRM_FORMAT_MOD_INVALID");<br>
+ errno = EINVAL;<br>
+ return NULL;<br>
+ }<br>
+ }<br></blockquote><div><br></div></div></div><div>This could be its own patch but I don't care too much.<br></div></div></div></div></blockquote><div><br></div><div>Also, you don't need this anymore now that MOD_INVALID just falls through and doesn't assert below. Feel free to drop it, pull it into its own patch, or leave it here. (I have a mild preference for dropping it and just ignoring MOD_INVALID but don't care too much). With that, series is<br><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
surf = calloc(1, sizeof *surf);<br>
if (surf == NULL) {<br>
errno = ENOMEM;<br>
diff --git a/src/mesa/drivers/dri/i965/in<wbr>tel_screen.c b/src/mesa/drivers/dri/i965/in<wbr>tel_screen.c<br>
index 14e60ef1a1..e4f858ed33 100644<br>
<span>--- a/src/mesa/drivers/dri/i965/in<wbr>tel_screen.c<br>
+++ b/src/mesa/drivers/dri/i965/in<wbr>tel_screen.c<br>
@@ -23,6 +23,7 @@<br>
* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.<br>
*/<br>
<br>
+#include <drm_fourcc.h><br>
#include <errno.h><br>
#include <time.h><br>
#include <unistd.h><br>
</span>@@ -518,11 +519,13 @@ intel_destroy_image(__DRIimage *image)<br>
enum modifier_priority {<br>
MODIFIER_PRIORITY_INVALID = 0,<br>
MODIFIER_PRIORITY_LINEAR,<br>
+ MODIFIER_PRIORITY_Y,<br>
};<br>
<br>
const uint64_t priority_to_modifier[] = {<br>
[MODIFIER_PRIORITY_INVALID] = DRM_FORMAT_MOD_INVALID,<br>
[MODIFIER_PRIORITY_LINEAR] = DRM_FORMAT_MOD_LINEAR,<br>
+ [MODIFIER_PRIORITY_Y] = I915_FORMAT_MOD_Y_TILED,<br>
};<br>
<br>
static uint64_t<br>
@@ -530,11 +533,13 @@ select_best_modifier(struct gen_device_info *devinfo,<br>
<span> const uint64_t *modifiers,<br>
const unsigned count)<br>
{<br>
-<br>
</span> enum modifier_priority prio = MODIFIER_PRIORITY_INVALID;<br>
<span><br>
for (int i = 0; i < count; i++) {<br>
switch (modifiers[i]) {<br>
</span>+ case I915_FORMAT_MOD_Y_TILED:<br>
+ prio = MAX2(prio, MODIFIER_PRIORITY_Y);<br>
+ break;<br>
<span> case DRM_FORMAT_MOD_LINEAR:<br>
prio = MAX2(prio, MODIFIER_PRIORITY_LINEAR);<br>
break;<br>
</span>@@ -575,6 +580,9 @@ intel_create_image_common(__DR<wbr>Iscreen *dri_screen,<br>
<span> case DRM_FORMAT_MOD_LINEAR:<br>
tiling = I915_TILING_NONE;<br>
break;<br>
+ case I915_FORMAT_MOD_Y_TILED:<br>
+ tiling = I915_TILING_Y;<br>
+ break;<br>
case DRM_FORMAT_MOD_INVALID:<br>
default:<br>
break;<br>
</span>@@ -626,8 +634,26 @@ intel_create_image_with_modifi<wbr>ers(__DRIscreen *dri_screen,<br>
<div><div class="m_-7776364685084415466h5"> const unsigned count,<br>
void *loaderPrivate)<br>
{<br>
- return intel_create_image_common(dri_<wbr>screen, width, height, format, 0, NULL,<br>
- 0, loaderPrivate);<br>
+ uint64_t local_mods[count];<br>
+ int local_count = 0;<br>
+<br>
+ /* This compacts the actual modifiers to the ones we know how to handle */<br>
+ for (int i = 0; i < count; i++) {<br>
+ switch (modifiers[i]) {<br>
+ case I915_FORMAT_MOD_Y_TILED:<br>
+ case DRM_FORMAT_MOD_LINEAR:<br>
+ local_mods[local_count++] = modifiers[i];<br>
+ break;<br>
+ case DRM_FORMAT_MOD_INVALID:<br>
+ unreachable("Invalid modifiers specified\n");<br>
+ default:<br>
+ /* Modifiers from other vendors would land here. */<br>
+ break;<br>
+ }<br>
+ }<br>
+<br>
+ return intel_create_image_common(dri_<wbr>screen, width, height, format, 0,<br>
+ local_mods, local_count, loaderPrivate);<br>
}<br>
<br>
static GLboolean<br>
</div></div>@@ -1997,7 +2023,9 @@ intelAllocateBuffer(__DRIscree<wbr>n *dri_screen,<br>
<div class="m_-7776364685084415466HOEnZb"><div class="m_-7776364685084415466h5"> if (intelBuffer == NULL)<br>
return NULL;<br>
<br>
- /* The front and back buffers are color buffers, which are X tiled. */<br>
+ /* The front and back buffers are color buffers, which are X tiled. GEN9+<br>
+ * supports Y tiled and compressed buffers, but there is no way to plumb that<br>
+ * through to here. */<br>
uint32_t tiling = I915_TILING_X;<br>
unsigned long pitch;<br>
int cpp = format / 8;<br>
--<br>
2.12.0<br>
<br>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>