<div dir="ltr"><div>The way I see it, pipe_surface is a render target view for pipe_framebuffer_state. The fact that other functions also accept pipe_surface is a historic artifact. resource_copy_region used to accept 2 pipe_surfaces before it was changed to 2 pipe_resources + parameters. Somebody might decide to change invalidate_subresource to take pipe_resource in the future. </div><div> </div><div>It seems to me that you really want e.g. invalidate_framebuffer(ctx, PIPE_FRAMEBUFFER_COLOR0 | PIPE_FRAMEBUFFER_COLOR1), not pipe_surface.</div><div> </div><div>I'm not too crazy about this, just pointing out the way I understand the conventions. </div><div> </div><div>Marek </div> <div class="gmail_quote"><div dir="ltr">On Fri, Dec 14, 2018 at 3:51 PM Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I don't mind renaming it to invalidate_subsurface() if you prefer that, but I really prefer that it takes a surface rather than a resource... pipe_resource is the wrong level to do this. Even though surfaces are transient, tracking the invalidate state in the surface works out much easier in the driver. And I don't want to have to map resource+params back to a surface. If another driver wants to track this at the resource level, it is easier for them to go from surface to resource+params, compared to the opposite direction. BR, -R On Fri, Dec 14, 2018 at 3:38 PM Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank">maraeo@gmail.com</a>> wrote: > > Can you please call it invalidate_subresource and inline relevant pipe_surface variables inside the parameters? > > Thanks, > Marek > > On Wed, Dec 12, 2018 at 10:48 AM Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>> wrote: >> >> A new API to implement glInvalidateFramebuffer() and friends. It is >> similar to invalidate_resource() but can be used to invalidate a >> specific layer/level, so it is suitable to use for user FBOs in addition >> to window system framebuffer. >> >> Signed-off-by: Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>> >> --- >> src/gallium/docs/source/context.rst | 18 ++++++++++++++++++ >> src/gallium/include/pipe/p_context.h | 18 ++++++++++++++++++ >> 2 files changed, 36 insertions(+) >> >> diff --git a/src/gallium/docs/source/context.rst b/src/gallium/docs/source/context.rst >> index 20d0df79312..9f3cd49ee31 100644 >> --- a/src/gallium/docs/source/context.rst >> +++ b/src/gallium/docs/source/context.rst >> @@ -740,7 +740,25 @@ of the buffer is not a multiple of the page size, changing the commit state of >> the last (partial) page requires a box that ends at the end of the buffer >> (i.e., box->x + box->width == buffer->width0). >> >> +.. _invalidate_surface: >> >> +invalidate_surface >> +%%%%%%%%%%%%%%%%%% >> + >> +This optional function marks a surface, or a portion of a surface, as invalid, >> +for example to implement glInvalidateFramebuffer() (and friends). It is >> +useful in particular for tiler GPUs, as a way to avoid unnecessary transfers >> +between system memory and tile buffer. >> + >> +For example, invalidating a surface after rendering, but before flush, can >> +be used to avoid tile to system memory transfer (for example, if zs can be >> +discarded). And an invalidate after flush before rendering can be used to >> +avoid a system memory to tile buffer transfer. >> + >> +Invalidating a partial surface can also be used to avoid unnecessary transfer >> +from system memory to tile buffer in the case of a scissored clear (which is >> +implemented via ->draw_vbo() by the state tracker) by invalidating the >> +scissored region. >> >> .. _pipe_transfer: >> >> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h >> index e07b76d4f03..4dfc87faff6 100644 >> --- a/src/gallium/include/pipe/p_context.h >> +++ b/src/gallium/include/pipe/p_context.h >> @@ -803,6 +803,24 @@ struct pipe_context { >> void (*invalidate_resource)(struct pipe_context *ctx, >> struct pipe_resource *resource); >> >> + /** >> + * Like ->invalidate_resource, but can invalidate a specific layer and level >> + * of a resource, and optionally a specific sub-region of the resource (if >> + * region is not NULL). >> + * >> + * If the backing surf->texture has just a single layer and level (like >> + * window system buffers), and region is NULL, it is equivalent to >> + * ->invalidate_resource(). >> + * >> + * \param ctx pipe context >> + * \param surf surface to invalidate >> + * \param region NULL to invalidate whole surface, otherwise specifies which >> + * portion of the surface is invalidated >> + */ >> + void (*invalidate_surface)(struct pipe_context *ctx, >> + struct pipe_surface *surf, >> + const struct pipe_box *region); >> + >> /** >> * Return information about unexpected device resets. >> */ >> -- >> 2.19.2 >> >> _______________________________________________ >> mesa-dev mailing list >> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div></div>