[Mesa-dev] [PATCH 1/3] gallium: add expand_resource interface
Jose Fonseca
jfonseca at vmware.com
Fri Jul 12 07:19:10 PDT 2013
I admit I haven't fully understood what's being proposed yet. But just a few quick words.
I always wanted to have a "present" method that ensures that the contents of a resource is made visible to whatever the consumer is (full-screen flip, blit to primary surface, a compositor, etc.). We have been using off-the-side interfaces on Windows to achieve this [1] but I believe that this is a cross-platform problem so there should be something in Gallium interfaces for that.
So I'd be supportive if there is a proposal that caters for everything.
I'd hate to have a new interface aimed at a single use case.
Jose
[1] See stw_winsys::present and stw_winsys::compose in http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/wgl/stw_winsys.h
----- Original Message -----
> Let's ignore the function name for now, that's easy to change and I am
> open to suggestions. flush_window_resource, flush_shared_resource, and
> sync_shared_resource are possible candidates as well as
> gonna_put_this_resource_on_the_screen_right_away. :) The idea is to
> expose a function which will ensure resource coherency between gallium
> and an external client (it could be another process or another
> device).
>
> Gallium drivers really have no way to know which resource is the
> window front or back buffer. It has always been like that since I
> joined Mesa (about 3.5 years ago). The bind flags won't work, because
> there can be multiple resources with that same flag. Even knowing
> which resource is front and back is useless, because there is only one
> resource which needs to be flushed at a given point and only st/dri
> knows which one it is.
>
> Well, there is at least something good about classic drivers: They
> don't have to share their DRI support code with everybody else.
>
> Marek
>
> On Thu, Jul 11, 2013 at 10:24 PM, Roland Scheidegger <sroland at vmware.com>
> wrote:
> > Am 11.07.2013 20:15, schrieb Marek Olšák:
> >> Hi Roland,
> >>
> >> The fast color clear on Radeon doesn't touch the memory of the texture
> >> resource. Instead, it changes some GPU meta data that say the resource
> >> is cleared (the location of the meta data is stored in pipe_resource).
> >> This works fine as long as the gallium pipe_resource structure is used
> >> for accessing the resource. That's not the case with the DDX, which is
> >> responsible for putting the resource on the screen and it obviously
> >> has no idea about the contents of pipe_resource, so it doesn't know
> >> that the resource is in a cleared state and a special "flush"
> >> operation must be done to actually write the "cleared" pixels (which
> >> haven't been overwritten by new geometry of course).
> >>
> >> The easiest way to solve this is to "flush" the cleared resource in
> >> SwapBuffers and where the front buffer is flushed. The Gallium driver
> >> can't do it automatically, because it has no notion of front and back
> >> buffers nor does it know which resource must be "flushed". That's why
> >> a new pipe_context function is being proposed, which was originally my
> >> idea.
> > Ok I see now what it's used for. I don't like "expand" though this is
> > more like a resource flush (i.e. do whatever is necessary to make the
> > contents of this resource accessible by some dumb interface which knows
> > nothing about resources).
> > It isn't quite true that the gallium driver doesn't know this is a
> > resource used as a window fb, as those should have the
> > PIPE_BIND_DISPLAY_TARGET flag set (or maybe PIPE_BIND_SCANOUT). Don't
> > ask me though how they work or if you could figure out what you'd need
> > to flush...
> > As Christoph has said, there pipe_screen.flush_front_buffer which could
> > supposedly do this but it would also present the resource to screen I
> > guess (only really used in sw winsys).
> > So I'm still not really sure if a new function is really needed, I'm not
> > familiar enough with all the interface stuff. If it is though I don't
> > like the name, and it also would need documentation.
> >
> > Roland
> >
> >
> >>
> >> This commit only fixes r600g for st/dri. Any other co-state tracker
> >> (like st/egl and st/xlib) will be broken if it's used with r600g. I
> >> think we can ignore st/xlib. Not sure how important st/egl is (not
> >> required for EGL under X).
> >>
> >> Marek
> >>
> >> On Wed, Jul 10, 2013 at 7:32 PM, Roland Scheidegger <sroland at vmware.com>
> >> wrote:
> >>> I don't quite understand what this should do, at first sight it looks
> >>> like a ugly hack (which should really not be part of gallium interface)
> >>> to make fast color clearing work better with window framebuffers.
> >>> Seems to go against the idea of resources (which are immutable, well not
> >>> the contents but the properties).
> >>> (If anything I wanted an interface to change bind flags for resources
> >>> after initialization, because they are near impossible to guarantee with
> >>> OpenGL's (or d3d9 for that matter) distinct texture/fb model, but that
> >>> would also be quite a hack.)
> >>> Could you elaborate with some example what that's supposed to do in
> >>> practice?
> >>>
> >>> Roland
> >>>
> >>>
> >>> Am 10.07.2013 18:20, schrieb Grigori Goronzy:
> >>>> This interface is used to expand fast-cleared window system
> >>>> colorbuffers.
> >>>> ---
> >>>> src/gallium/include/pipe/p_context.h | 8 ++++++++
> >>>> src/gallium/state_trackers/dri/common/dri_drawable.c | 4 ++++
> >>>> src/gallium/state_trackers/dri/drm/dri2.c | 8 ++++++--
> >>>> 3 files changed, 18 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/src/gallium/include/pipe/p_context.h
> >>>> b/src/gallium/include/pipe/p_context.h
> >>>> index aa18cbf..38d5ee6 100644
> >>>> --- a/src/gallium/include/pipe/p_context.h
> >>>> +++ b/src/gallium/include/pipe/p_context.h
> >>>> @@ -354,6 +354,14 @@ struct pipe_context {
> >>>> unsigned dstx, unsigned dsty,
> >>>> unsigned width, unsigned height);
> >>>>
> >>>> + /**
> >>>> + * Expand a color resource in-place.
> >>>> + *
> >>>> + * \return TRUE if resource was expanded, FALSE otherwise
> >>>> + */
> >>>> + boolean (*expand_resource)(struct pipe_context *pipe,
> >>>> + struct pipe_resource *dst);
> >>>> +
> >>>> /** Flush draw commands
> >>>> *
> >>>> * \param flags bitfield of enum pipe_flush_flags values.
> >>>> diff --git a/src/gallium/state_trackers/dri/common/dri_drawable.c
> >>>> b/src/gallium/state_trackers/dri/common/dri_drawable.c
> >>>> index 18d8d89..b67a497 100644
> >>>> --- a/src/gallium/state_trackers/dri/common/dri_drawable.c
> >>>> +++ b/src/gallium/state_trackers/dri/common/dri_drawable.c
> >>>> @@ -448,6 +448,10 @@ dri_flush(__DRIcontext *cPriv,
> >>>> }
> >>>>
> >>>> /* FRONT_LEFT is resolved in drawable->flush_frontbuffer. */
> >>>> + } else if (ctx->st->pipe->expand_resource) {
> >>>> + /* Expand fast-cleared framebuffer */
> >>>> + ctx->st->pipe->expand_resource(ctx->st->pipe,
> >>>> + drawable->textures[ST_ATTACHMENT_BACK_LEFT]);
> >>>> }
> >>>>
> >>>> dri_postprocessing(ctx, drawable, ST_ATTACHMENT_BACK_LEFT);
> >>>> diff --git a/src/gallium/state_trackers/dri/drm/dri2.c
> >>>> b/src/gallium/state_trackers/dri/drm/dri2.c
> >>>> index 1dcc1f7..97784ec 100644
> >>>> --- a/src/gallium/state_trackers/dri/drm/dri2.c
> >>>> +++ b/src/gallium/state_trackers/dri/drm/dri2.c
> >>>> @@ -490,18 +490,22 @@ dri2_flush_frontbuffer(struct dri_context *ctx,
> >>>> {
> >>>> __DRIdrawable *dri_drawable = drawable->dPriv;
> >>>> struct __DRIdri2LoaderExtensionRec *loader =
> >>>> drawable->sPriv->dri2.loader;
> >>>> + struct pipe_context *pipe = ctx->st->pipe;
> >>>>
> >>>> if (statt != ST_ATTACHMENT_FRONT_LEFT)
> >>>> return;
> >>>>
> >>>> if (drawable->stvis.samples > 1) {
> >>>> - struct pipe_context *pipe = ctx->st->pipe;
> >>>> -
> >>>> /* Resolve the front buffer. */
> >>>> dri_pipe_blit(ctx->st->pipe,
> >>>> drawable->textures[ST_ATTACHMENT_FRONT_LEFT],
> >>>> drawable->msaa_textures[ST_ATTACHMENT_FRONT_LEFT]);
> >>>> pipe->flush(pipe, NULL, 0);
> >>>> + } else if (pipe->expand_resource &&
> >>>> drawable->textures[ST_ATTACHMENT_FRONT_LEFT]) {
> >>>> + /* Expand fast-cleared framebuffer */
> >>>> + if (pipe->expand_resource(pipe,
> >>>> drawable->textures[ST_ATTACHMENT_FRONT_LEFT])) {
> >>>> + pipe->flush(pipe, NULL, 0);
> >>>> + }
> >>>> }
> >>>>
> >>>> if (loader->flushFrontBuffer) {
> >>>>
> >>> _______________________________________________
> >>> mesa-dev mailing list
> >>> mesa-dev at lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list