[Mesa-dev] [PATCH 2/6] r600g: Add r600_rat_buffer_create()
Tom Stellard
tom at stellard.net
Mon Sep 24 07:06:19 PDT 2012
Hi Marek,
On Sat, Sep 22, 2012 at 05:31:51AM +0200, Marek Olšák wrote:
> Hi Tom,
>
> First I'd like to say that even though I don't agree with the
> direction you're taking, I'd be okay with it for initial bring-up of
> the compute support.
>
> I am not in favor of using radeon_surface for plain untyped 1D
> buffers, where the only thing that really matters is the buffer
> alignment, right? I am also curious about what is so special about
> radeon_surface that we need to use it to allocate the RAT buffers.
>
There are two reasons I went with the radeon_surface approach.
First, is that I wanted to reuse as much of the current r600g code
as possible. When I first tried to get RATs working I had trouble
manually programming the CB registers to satisfy the CS checker, and
since using radeon_surface for emitting the CB registers was already
implemented and well tested, it made sense to go with that approach.
It may not be ideal, but there are other more critical parts of the
compute stack that need work, so I wanted to come up with a working
solution as quickly as possible.
Second, is that I thought in the future we might be able to take advantage
of 2D tiling for compute buffers, so I wanted to keep that option open.
> The other issue is that this solution doesn't scale. It would be
> impossible to use an ordinary buffer created using resource_create as
> a RAT. Does the hardware really impose special restrictions on RATs
> such that the the current buffer allocation codepath cannot be used?
> Would such a restriction disallow interoperability with GL or even
> disallow implementing GL_ARB_shader_storage_buffer_object? That
> extension allows reads from and writes to ordinary GL buffers in all
> shader stages (though only the fragment and compute shader support is
> mandatory) and, of course, the buffers can be later used as a source
> for vertices, indices, uniforms, etc, and vice versa, any buffer can
> be used as a "shader storage buffer object".
>
> Now some comments about the code...
>
> I don't like allocating r600_texture for PIPE_BUFFER. The problem is
> if you got a pipe_resource, there would be no way to tell if it's
> actually r600_texture or r600_resource. If using radeon_surface is the
> way to go, let's move radeon_surface into r600_resource.
>
I think moving radeon_surface into r600_resource makes a lot of sense.
With this change it should be possible to use the current
buffer creation path for RATs. I'll work on a new patch set.
-Tom
> Also checking for R600_RESOURCE_FLAG_FLUSHED_DEPTH doesn't seem to be
> very useful there (it's not an allocation of a depth-stencil texture).
>
> Marek
>
> On Fri, Sep 21, 2012 at 10:39 PM, Tom Stellard <tom at stellard.net> wrote:
> > From: Tom Stellard <thomas.stellard at amd.com>
> >
> > This function creates a buffer with an associated radeon_surface that
> > can be used as a RAT.
> > ---
> > src/gallium/drivers/r600/r600_buffer.c | 69 ++++++++++++++++++++++++++++----
> > src/gallium/drivers/r600/r600_pipe.h | 2 +
> > src/gallium/drivers/r600/r600_resource.h | 5 +++
> > src/gallium/drivers/r600/r600_texture.c | 2 +-
> > 4 files changed, 69 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/gallium/drivers/r600/r600_buffer.c b/src/gallium/drivers/r600/r600_buffer.c
> > index 0b0ac34..5bc3d72 100644
> > --- a/src/gallium/drivers/r600/r600_buffer.c
> > +++ b/src/gallium/drivers/r600/r600_buffer.c
> > @@ -24,6 +24,7 @@
> > * Jerome Glisse
> > * Corbin Simpson <MostAwesomeDude at gmail.com>
> > */
> > +#include "r600d.h"
> > #include "r600_pipe.h"
> > #include "util/u_upload_mgr.h"
> > #include "util/u_memory.h"
> > @@ -239,6 +240,65 @@ bool r600_init_resource(struct r600_screen *rscreen,
> > return true;
> > }
> >
> > +static int r600_buffer_init(struct r600_resource *rbuffer,
> > + struct r600_screen *rscreen,
> > + const struct pipe_resource *templ,
> > + unsigned alignment)
> > +
> > +{
> > + rbuffer->b.b = *templ;
> > + pipe_reference_init(&rbuffer->b.b.reference, 1);
> > + rbuffer->b.b.screen = (struct pipe_screen *)rscreen;
> > + rbuffer->b.vtbl = &r600_buffer_vtbl;
> > +
> > + if (!r600_init_resource(rscreen, rbuffer, templ->width0, alignment, templ->bind, templ->usage)) {
> > + FREE(rbuffer);
> > + return -1;
> > + }
> > + return 0;
> > +}
> > +
> > +/**
> > + * Create a buffer that can be used as a RAT and also passed to
> > + * evergreen_init_color surface.
> > + */
> > +struct r600_texture *r600_rat_buffer_create(struct pipe_screen *screen,
> > + const struct pipe_resource *templ)
> > +{
> > + struct r600_screen *rscreen = (struct r600_screen*)screen;
> > + struct r600_texture *rtex;
> > + int r;
> > +
> > + rtex = CALLOC_STRUCT(r600_texture);
> > + if (!rtex)
> > + goto fail;
> > +
> > + r = r600_init_surface(rscreen, &rtex->surface, templ,
> > + V_038000_ARRAY_LINEAR_ALIGNED,
> > + templ->flags & R600_RESOURCE_FLAG_TRANSFER,
> > + templ->flags & R600_RESOURCE_FLAG_FLUSHED_DEPTH);
> > + if (r)
> > + goto fail;
> > +
> > + r = rscreen->ws->surface_best(rscreen->ws, &rtex->surface);
> > +
> > + if (r)
> > + goto fail;
> > +
> > + rtex->is_rat = 1;
> > +
> > + r = r600_buffer_init(&rtex->resource, rscreen, templ,
> > + rtex->surface.bo_alignment);
> > + if (r)
> > + goto fail;
> > +
> > + return rtex;
> > +
> > +fail:
> > + FREE(rtex);
> > + return NULL;
> > +}
> > +
> > struct pipe_resource *r600_buffer_create(struct pipe_screen *screen,
> > const struct pipe_resource *templ,
> > unsigned alignment)
> > @@ -248,14 +308,7 @@ struct pipe_resource *r600_buffer_create(struct pipe_screen *screen,
> >
> > rbuffer = MALLOC_STRUCT(r600_resource);
> >
> > - rbuffer->b.b = *templ;
> > - pipe_reference_init(&rbuffer->b.b.reference, 1);
> > - rbuffer->b.b.screen = screen;
> > - rbuffer->b.vtbl = &r600_buffer_vtbl;
> > + r600_buffer_init(rbuffer, rscreen, templ, alignment);
> >
> > - if (!r600_init_resource(rscreen, rbuffer, templ->width0, alignment, templ->bind, templ->usage)) {
> > - FREE(rbuffer);
> > - return NULL;
> > - }
> > return &rbuffer->b.b;
> > }
> > diff --git a/src/gallium/drivers/r600/r600_pipe.h b/src/gallium/drivers/r600/r600_pipe.h
> > index 1428e9a..80ac5d0 100644
> > --- a/src/gallium/drivers/r600/r600_pipe.h
> > +++ b/src/gallium/drivers/r600/r600_pipe.h
> > @@ -552,6 +552,8 @@ bool r600_init_resource(struct r600_screen *rscreen,
> > struct pipe_resource *r600_buffer_create(struct pipe_screen *screen,
> > const struct pipe_resource *templ,
> > unsigned alignment);
> > +struct r600_texture *r600_rat_buffer_create(struct pipe_screen *screen,
> > + const struct pipe_resource *templ);
> >
> > /* r600_pipe.c */
> > void r600_flush(struct pipe_context *ctx, struct pipe_fence_handle **fence,
> > diff --git a/src/gallium/drivers/r600/r600_resource.h b/src/gallium/drivers/r600/r600_resource.h
> > index a5a5404..28a72b0 100644
> > --- a/src/gallium/drivers/r600/r600_resource.h
> > +++ b/src/gallium/drivers/r600/r600_resource.h
> > @@ -125,6 +125,11 @@ void r600_texture_get_fmask_info(struct r600_screen *rscreen,
> > void r600_texture_get_cmask_info(struct r600_screen *rscreen,
> > struct r600_texture *rtex,
> > struct r600_cmask_info *out);
> > +int r600_init_surface(struct r600_screen *rscreen,
> > + struct radeon_surface *surface,
> > + const struct pipe_resource *ptex,
> > + unsigned array_mode,
> > + bool is_transfer, bool is_flushed_depth);
> > struct pipe_resource *r600_texture_create(struct pipe_screen *screen,
> > const struct pipe_resource *templ);
> > struct pipe_resource *r600_texture_from_handle(struct pipe_screen *screen,
> > diff --git a/src/gallium/drivers/r600/r600_texture.c b/src/gallium/drivers/r600/r600_texture.c
> > index 7dc3fa5..58bfa54 100644
> > --- a/src/gallium/drivers/r600/r600_texture.c
> > +++ b/src/gallium/drivers/r600/r600_texture.c
> > @@ -65,7 +65,7 @@ unsigned r600_texture_get_offset(struct r600_texture *rtex,
> > layer * rtex->surface.level[level].slice_size;
> > }
> >
> > -static int r600_init_surface(struct r600_screen *rscreen,
> > +int r600_init_surface(struct r600_screen *rscreen,
> > struct radeon_surface *surface,
> > const struct pipe_resource *ptex,
> > unsigned array_mode,
> > --
> > 1.7.11.4
> >
> > _______________________________________________
> > 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