[Mesa-dev] [PATCH 11/11] clover: Use PIPE_TRANSFER_MAP_DIRECTLY when writing/reading buffers

[Bruno Jimenez]

On Thu, 2014-06-19 at 10:23 +0900, Michel Dänzer wrote:
> On 19.06.2014 00:02, Bruno Jiménez wrote:
> > Note: This is just a proof of concept.
> > ---
> >  src/gallium/state_trackers/clover/api/transfer.cpp  | 4 ++--
> >  src/gallium/state_trackers/clover/core/object.hpp   | 4 ++++
> >  src/gallium/state_trackers/clover/core/resource.cpp | 2 ++
> >  3 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/gallium/state_trackers/clover/api/transfer.cpp b/src/gallium/state_trackers/clover/api/transfer.cpp
> > index 404ceb0..f34ae8b 100644
> > --- a/src/gallium/state_trackers/clover/api/transfer.cpp
> > +++ b/src/gallium/state_trackers/clover/api/transfer.cpp
> > @@ -174,8 +174,8 @@ namespace {
> >        static mapping
> >        get(command_queue &q, T obj, cl_map_flags flags,
> >            size_t offset, size_t size) {
> > -         return { q, obj->resource(q), flags, true,
> > -                  {{ offset }}, {{ size, 1, 1 }} };
> > +         return { q, obj->resource(q), flags | CLOVER_TRANSFER_MAP_DIRECTLY,
> > +                  true, {{ offset }}, {{ size, 1, 1 }} };
> >        }
> >     };
> >  
> > diff --git a/src/gallium/state_trackers/clover/core/object.hpp b/src/gallium/state_trackers/clover/core/object.hpp
> > index 697565c..7d5adf9 100644
> > --- a/src/gallium/state_trackers/clover/core/object.hpp
> > +++ b/src/gallium/state_trackers/clover/core/object.hpp
> > @@ -33,6 +33,10 @@
> >  #include "core/property.hpp"
> >  #include "api/dispatch.hpp"
> >  
> > +#ifndef CLOVER_TRANSFER_MAP_DIRECTLY
> > +#define CLOVER_TRANSFER_MAP_DIRECTLY (1<<8)
> > +#endif
> > +
> >  ///
> >  /// Main namespace of the CL state tracker.
> >  ///
> > diff --git a/src/gallium/state_trackers/clover/core/resource.cpp b/src/gallium/state_trackers/clover/core/resource.cpp
> > index 7b8a40a..c8e97db 100644
> > --- a/src/gallium/state_trackers/clover/core/resource.cpp
> > +++ b/src/gallium/state_trackers/clover/core/resource.cpp
> > @@ -174,6 +174,8 @@ mapping::mapping(command_queue &q, resource &r,
> >     pctx(q.pipe) {
> >     unsigned usage = ((flags & CL_MAP_WRITE ? PIPE_TRANSFER_WRITE : 0 ) |
> >                       (flags & CL_MAP_READ ? PIPE_TRANSFER_READ : 0 ) |
> > +                     (flags & CLOVER_TRANSFER_MAP_DIRECTLY ?
> > +                              PIPE_TRANSFER_MAP_DIRECTLY : 0 ) |
> >                       (!blocking ? PIPE_TRANSFER_UNSYNCHRONIZED : 0));
> >  
> >     p = pctx->transfer_map(pctx, r.pipe, 0, usage,
> > 
> 
> The driver can return NULL when PIPE_TRANSFER_MAP_DIRECTLY is set (if
> the driver can't actually map the resource directly), so you'd need to
> add code to try again without PIPE_TRANSFER_MAP_DIRECTLY in that case.

Hi,

Thanks for the information. The r600g driver can handle this with the
previous patch. But you are completely right about other drivers.

Anyway, as I have writen in the commit message, these two last patches
are just proofs of concept for a way to avoid unnecesary transfers
GPU<->GPU and aren't needed to solve the mapping bug.

Thanks!
Bruno