From d7008a6d7bc8d894be9c51269418bc0a4d52920b Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Tue, 13 Apr 2021 12:24:17 +0200 Subject: [PATCH] panfrost: Fix partial update The KHR_partial_update spec says: " If EGL_EXT_buffer_age is supported, the contents of the buffer inside the damage region may also be relied upon to contain the same content as the last time they were defined for the current back buffer. " but we currently assume that everything inside the damage region will be overwritten by new data and that the previous content doesn't need to be reloaded. Let's get rid of the damage rect inversion logic for now and reload everything inside the damage extent. We will optimize things further down the line, using pre-frame DCDs on Bifrost, and a tile enable map on Midgard. Signed-off-by: Boris Brezillon Reviewed-by: Alyssa Rosenzweig Part-of: --- src/gallium/drivers/panfrost/Makefile.sources | 2 - src/gallium/drivers/panfrost/meson.build | 1 - src/gallium/drivers/panfrost/pan_job.c | 38 ++--- .../drivers/panfrost/pan_partial_update.c | 135 ------------------ .../drivers/panfrost/pan_partial_update.h | 49 ------- src/gallium/drivers/panfrost/pan_resource.c | 8 -- src/gallium/drivers/panfrost/pan_resource.h | 3 - 7 files changed, 13 insertions(+), 223 deletions(-) delete mode 100644 src/gallium/drivers/panfrost/pan_partial_update.c delete mode 100644 src/gallium/drivers/panfrost/pan_partial_update.h diff --git a/src/gallium/drivers/panfrost/Makefile.sources b/src/gallium/drivers/panfrost/Makefile.sources index 097b49b71fe..ce613d6793f 100644 --- a/src/gallium/drivers/panfrost/Makefile.sources +++ b/src/gallium/drivers/panfrost/Makefile.sources @@ -12,8 +12,6 @@ C_SOURCES := \ pan_job.c \ pan_job.h \ pan_mfbd.c \ - pan_partial_update.c \ - pan_partial_update.h \ pan_public.h \ pan_resource.c \ pan_resource.h \ diff --git a/src/gallium/drivers/panfrost/meson.build b/src/gallium/drivers/panfrost/meson.build index 391dcc5d5f1..ccee3ce00d2 100644 --- a/src/gallium/drivers/panfrost/meson.build +++ b/src/gallium/drivers/panfrost/meson.build @@ -35,7 +35,6 @@ files_panfrost = files( 'pan_fragment.c', 'pan_sfbd.c', 'pan_mfbd.c', - 'pan_partial_update.c', ) panfrost_includes = [ diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c index ca58fb343f1..817ee6a9896 100644 --- a/src/gallium/drivers/panfrost/pan_job.c +++ b/src/gallium/drivers/panfrost/pan_job.c @@ -778,9 +778,6 @@ panfrost_load_surface(struct panfrost_batch *batch, struct pipe_surface *surf, u if (!rsrc->state.slices[level].data_valid) return; - if (!rsrc->damage.inverted_len) - return; - /* Clamp the rendering area to the damage extent. The * KHR_partial_update() spec states that trying to render outside of * the damage region is "undefined behavior", so we should be safe. @@ -875,42 +872,33 @@ panfrost_load_surface(struct panfrost_batch *batch, struct pipe_surface *surf, u pthread_mutex_unlock(&dev->blend_shaders.lock); } - struct panfrost_ptr transfer = panfrost_pool_alloc_aligned(&batch->pool, - 4 * 4 * 6 * rsrc->damage.inverted_len, 64); + float rect[] = { + 0.0, rsrc->base.height0, 0.0, 1.0, + rsrc->base.width0, rsrc->base.height0, 0.0, 1.0, + 0.0, 0.0, 0.0, 1.0, - for (unsigned i = 0; i < rsrc->damage.inverted_len; ++i) { - float *o = (float *) (transfer.cpu + (4 * 4 * 6 * i)); - struct pan_rect r = rsrc->damage.inverted_rects[i]; + rsrc->base.width0, rsrc->base.height0, 0.0, 1.0, + 0.0, 0.0, 0.0, 1.0, + rsrc->base.width0, 0.0, 0.0, 1.0, + }; - float rect[] = { - r.minx, rsrc->base.height0 - r.miny, 0.0, 1.0, - r.maxx, rsrc->base.height0 - r.miny, 0.0, 1.0, - r.minx, rsrc->base.height0 - r.maxy, 0.0, 1.0, + mali_ptr vertices = + panfrost_pool_upload_aligned(&batch->pool, rect, sizeof(rect), 64); - r.maxx, rsrc->base.height0 - r.miny, 0.0, 1.0, - r.minx, rsrc->base.height0 - r.maxy, 0.0, 1.0, - r.maxx, rsrc->base.height0 - r.maxy, 0.0, 1.0, - }; - - assert(sizeof(rect) == 4 * 4 * 6); - memcpy(o, rect, sizeof(rect)); - } - - unsigned vertex_count = rsrc->damage.inverted_len * 6; if (pan_is_bifrost(batch->pool.dev)) { mali_ptr tiler = - panfrost_batch_get_bifrost_tiler(batch, vertex_count); + panfrost_batch_get_bifrost_tiler(batch, 6); panfrost_load_bifrost(&batch->pool, &batch->scoreboard, blend_shader, batch->framebuffer.gpu, tiler, - transfer.gpu, vertex_count, + vertices, 6, &iview, loc); } else { panfrost_load_midg(&batch->pool, &batch->scoreboard, blend_shader, batch->framebuffer.gpu, - transfer.gpu, vertex_count, + vertices, 6, &iview, loc); } diff --git a/src/gallium/drivers/panfrost/pan_partial_update.c b/src/gallium/drivers/panfrost/pan_partial_update.c deleted file mode 100644 index b4b61b7a03c..00000000000 --- a/src/gallium/drivers/panfrost/pan_partial_update.c +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright (C) 2020 Collabora, Ltd. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - * - */ - -#include "util/ralloc.h" -#include "pan_partial_update.h" - -static struct pan_rect -pan_make_rect(unsigned minx, unsigned miny, unsigned maxx, unsigned maxy) -{ - struct pan_rect r = { - .minx = minx, - .miny = miny, - .maxx = maxx, - .maxy = maxy - }; - - return r; -} - -static struct pan_rect -pan_from_pipe(const struct pipe_box *p) -{ - return pan_make_rect(p->x, p->y, p->x + p->width, p->y + p->height); -} - -/* Helper to clip a rect inside another box */ - -static struct pan_rect -pan_clip_rect( - const struct pan_rect *r, - const struct pan_rect *clip) -{ - unsigned minx = MAX2(r->minx, clip->minx); - unsigned miny = MAX2(r->miny, clip->miny); - unsigned maxx = MAX2(MIN2(r->maxx, clip->maxx), minx); - unsigned maxy = MAX2(MIN2(r->maxy, clip->maxy), miny); - - return pan_make_rect(minx, miny, maxx, maxy); -} - -/* Subtract d from r, yielding four (possibly degenerate) rectangles for each - * bounding region */ - -static void -pan_subtract_from_rect( - struct pan_rect *out, - const struct pan_rect *r, - const struct pan_rect *d) -{ - struct pan_rect dc = pan_clip_rect(r, d); - - out[0] = pan_make_rect(r->minx, r->miny, dc.minx, r->maxy); - out[1] = pan_make_rect(dc.minx, r->miny, dc.maxx, dc.miny); - out[2] = pan_make_rect(dc.maxx, r->miny, r->maxx, r->maxy); - out[3] = pan_make_rect(dc.minx, dc.maxy, dc.maxx, r->maxy); -} - -/* Subtract d from the set of rects R, returning the number of - * (non-degenerate) rectangles returned. - * - * out must be at least sizeof(struct pan_rect) * R_len * 4 bytes - * - * Trivially satisfies: return value < (R_len * 4) */ - -static unsigned -pan_subtract_from_rects( - struct pan_rect *out, - const struct pan_rect *R, - unsigned R_len, - const struct pan_rect *d) -{ - unsigned count = 0; - struct pan_rect temp[4]; - - for (unsigned r = 0; r < R_len; ++r) { - pan_subtract_from_rect(temp, &R[r], d); - - /* Copy the rectangles with nonzero area */ - for (unsigned i = 0; i < 4; ++i) { - if (temp[i].maxx <= temp[i].minx) continue; - if (temp[i].maxy <= temp[i].miny) continue; - - out[count++] = temp[i]; - } - } - - return count; -} - -struct pan_rect * -pan_subtract_damage( - void *memctx, - unsigned initial_w, unsigned initial_h, - unsigned nrects, - const struct pipe_box *rects, - unsigned *out_len) -{ - struct pan_rect *R = ralloc(memctx, struct pan_rect); - *R = pan_make_rect(0, 0, initial_w, initial_h); - unsigned R_len = 1; - - for (unsigned d = 0; d < nrects; ++d) { - const struct pan_rect D = pan_from_pipe(&rects[d]); - struct pan_rect *out = rzalloc_array(memctx, struct pan_rect, - R_len * 4); - - R_len = pan_subtract_from_rects(out, R, R_len, &D); - ralloc_free(R); - R = out; - } - - *out_len = R_len; - return R; -} diff --git a/src/gallium/drivers/panfrost/pan_partial_update.h b/src/gallium/drivers/panfrost/pan_partial_update.h deleted file mode 100644 index 9d85be664b9..00000000000 --- a/src/gallium/drivers/panfrost/pan_partial_update.h +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright (C) 2019 Collabora, Ltd. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - * - * Authors (Collabora): - * Alyssa Rosenzweig - * - */ - -#ifndef __PAN_PARTIAL_UPDATE_H -#define __PAN_PARTIAL_UPDATE_H - -#include "pipe/p_state.h" - -/* Like pipe_box but in terms of maxx/maxy instead of w/h, which is much easier - * to work with for our purposes */ - -struct pan_rect { - unsigned minx, miny; - unsigned maxx, maxy; -}; - -struct pan_rect * -pan_subtract_damage( - void *memctx, - unsigned initial_w, unsigned initial_h, - unsigned nrects, - const struct pipe_box *rects, - unsigned *R_len); - -#endif diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c index 0cfb975369e..c331e0334f3 100644 --- a/src/gallium/drivers/panfrost/pan_resource.c +++ b/src/gallium/drivers/panfrost/pan_resource.c @@ -535,16 +535,8 @@ panfrost_resource_set_damage_region(struct pipe_screen *screen, struct pipe_scissor_state *damage_extent = &pres->damage.extent; unsigned int i; - if (pres->damage.inverted_rects) - ralloc_free(pres->damage.inverted_rects); - memset(&pres->damage, 0, sizeof(pres->damage)); - pres->damage.inverted_rects = - pan_subtract_damage(pres, - res->width0, res->height0, - nrects, rects, &pres->damage.inverted_len); - /* Track the damage extent: the quad including all damage regions. Will * be used restrict the rendering area */ diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h index ebaa5d2c458..5d34e2d9566 100644 --- a/src/gallium/drivers/panfrost/pan_resource.h +++ b/src/gallium/drivers/panfrost/pan_resource.h @@ -31,7 +31,6 @@ #include "pan_pool.h" #include "pan_minmax_cache.h" #include "pan_texture.h" -#include "pan_partial_update.h" #include "drm-uapi/drm.h" #include "util/u_range.h" @@ -41,8 +40,6 @@ struct panfrost_resource { struct pipe_resource base; struct { struct pipe_scissor_state extent; - struct pan_rect *inverted_rects; - unsigned inverted_len; } damage; struct renderonly_scanout *scanout;