On Mon, Mar 20, 2017 at 8:35 PM, Ben Widawsky <[email protected]> wrote:
> This patch begins introducing how we'll actually handle the potentially > many modifiers coming in from the API, how we'll store them, and the > structure in the code to support it. > > Prior to this patch, the Y-tiled modifier would be entirely ignored. It > shouldn't actually be used until this point because we've not bumped the > DRIimage extension version (which is a requirement to use modifiers). > > Measuring later in the series with kmscbe: > Linear: > Read bandwidth: 1048.44 MiB/s > Write bandwidth: 1483.17 MiB/s > > Y-tiled: > Read bandwidth: 471.13 MiB/s > Write bandwidth: 589.10 MiB/s > > Similar functionality was introduced and then reverted here: > > commit 6a0d036483caf87d43ebe2edd1905873446c9589 > Author: Ben Widawsky <[email protected]> > Date: Thu Apr 21 20:14:58 2016 -0700 > > i965: Always use Y-tiled buffers on SKL+ > > v2: Use last set bit instead of first set bit in modifiers to address > bug found by Daniel Stone. > > v3: Use the new priority modifier selection thing. This nullifies the > bug fixed by v2 also. > > Signed-off-by: Ben Widawsky <[email protected]> > Reviewed-by: Eric Engestrom <[email protected]> > Acked-by: Daniel Stone <[email protected]> > --- > src/gbm/backends/dri/gbm_dri.c | 18 ++++++++++++++++ > src/mesa/drivers/dri/i965/intel_screen.c | 36 > ++++++++++++++++++++++++++++---- > 2 files changed, 50 insertions(+), 4 deletions(-) > > diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_ > dri.c > index a7ac149365..a78ea89fca 100644 > --- a/src/gbm/backends/dri/gbm_dri.c > +++ b/src/gbm/backends/dri/gbm_dri.c > @@ -1143,6 +1143,15 @@ gbm_dri_bo_create(struct gbm_device *gbm, > goto failed; > } > > + if (modifiers) { > + for (int i = 0; i < count; i++) > + if (modifiers[i] == DRM_FORMAT_MOD_INVALID) { > + fprintf(stderr, "Invalid modifier passed in > DRM_FORMAT_MOD_INVALID"); > + errno = EINVAL; > + goto failed; > + } > + } > + > bo->image = > dri->image->createImageWithModifiers(dri->screen, > width, height, > @@ -1240,6 +1249,15 @@ gbm_dri_surface_create(struct gbm_device *gbm, > return NULL; > } > > + if (modifiers) { > + for (int i = 0; i < count; i++) > + if (modifiers[i] == DRM_FORMAT_MOD_INVALID) { > + fprintf(stderr, "Invalid modifier passed in > DRM_FORMAT_MOD_INVALID"); > + errno = EINVAL; > + return NULL; > + } > + } > This could be its own patch but I don't care too much. > + > surf = calloc(1, sizeof *surf); > if (surf == NULL) { > errno = ENOMEM; > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c > b/src/mesa/drivers/dri/i965/intel_screen.c > index 14e60ef1a1..e4f858ed33 100644 > --- a/src/mesa/drivers/dri/i965/intel_screen.c > +++ b/src/mesa/drivers/dri/i965/intel_screen.c > @@ -23,6 +23,7 @@ > * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > */ > > +#include <drm_fourcc.h> > #include <errno.h> > #include <time.h> > #include <unistd.h> > @@ -518,11 +519,13 @@ intel_destroy_image(__DRIimage *image) > enum modifier_priority { > MODIFIER_PRIORITY_INVALID = 0, > MODIFIER_PRIORITY_LINEAR, > + MODIFIER_PRIORITY_Y, > }; > > const uint64_t priority_to_modifier[] = { > [MODIFIER_PRIORITY_INVALID] = DRM_FORMAT_MOD_INVALID, > [MODIFIER_PRIORITY_LINEAR] = DRM_FORMAT_MOD_LINEAR, > + [MODIFIER_PRIORITY_Y] = I915_FORMAT_MOD_Y_TILED, > }; > > static uint64_t > @@ -530,11 +533,13 @@ select_best_modifier(struct gen_device_info *devinfo, > const uint64_t *modifiers, > const unsigned count) > { > - > enum modifier_priority prio = MODIFIER_PRIORITY_INVALID; > > for (int i = 0; i < count; i++) { > switch (modifiers[i]) { > + case I915_FORMAT_MOD_Y_TILED: > + prio = MAX2(prio, MODIFIER_PRIORITY_Y); > + break; > case DRM_FORMAT_MOD_LINEAR: > prio = MAX2(prio, MODIFIER_PRIORITY_LINEAR); > break; > @@ -575,6 +580,9 @@ intel_create_image_common(__DRIscreen *dri_screen, > case DRM_FORMAT_MOD_LINEAR: > tiling = I915_TILING_NONE; > break; > + case I915_FORMAT_MOD_Y_TILED: > + tiling = I915_TILING_Y; > + break; > case DRM_FORMAT_MOD_INVALID: > default: > break; > @@ -626,8 +634,26 @@ intel_create_image_with_modifiers(__DRIscreen > *dri_screen, > const unsigned count, > void *loaderPrivate) > { > - return intel_create_image_common(dri_screen, width, height, format, > 0, NULL, > - 0, loaderPrivate); > + uint64_t local_mods[count]; > + int local_count = 0; > + > + /* This compacts the actual modifiers to the ones we know how to > handle */ > + for (int i = 0; i < count; i++) { > + switch (modifiers[i]) { > + case I915_FORMAT_MOD_Y_TILED: > + case DRM_FORMAT_MOD_LINEAR: > + local_mods[local_count++] = modifiers[i]; > + break; > + case DRM_FORMAT_MOD_INVALID: > + unreachable("Invalid modifiers specified\n"); > + default: > + /* Modifiers from other vendors would land here. */ > + break; > + } > + } > + > + return intel_create_image_common(dri_screen, width, height, format, 0, > + local_mods, local_count, > loaderPrivate); > } > > static GLboolean > @@ -1997,7 +2023,9 @@ intelAllocateBuffer(__DRIscreen *dri_screen, > if (intelBuffer == NULL) > return NULL; > > - /* The front and back buffers are color buffers, which are X tiled. */ > + /* The front and back buffers are color buffers, which are X tiled. > GEN9+ > + * supports Y tiled and compressed buffers, but there is no way to > plumb that > + * through to here. */ > uint32_t tiling = I915_TILING_X; > unsigned long pitch; > int cpp = format / 8; > -- > 2.12.0 > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
