[Mesa-dev] [PATCH 4/6] [v2] i965: Handle Y-tiled modifier
Jason Ekstrand
jason at jlekstrand.net
Tue Mar 21 15:04:49 UTC 2017
On Mon, Mar 20, 2017 at 8:35 PM, Ben Widawsky <ben at bwidawsk.net> 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 <ben at bwidawsk.net>
> 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 <benjamin.widawsky at intel.com>
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
> Acked-by: Daniel Stone <daniels at collabora.com>
> ---
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170321/6d57eb1a/attachment-0001.html>
More information about the mesa-dev
mailing list