[Mesa-dev] [PATCH 1/3] intel/blorp: Support map/unmap of blorp_address

[Jordan Justen]

On 2016-11-22 15:06:25, Jason Ekstrand wrote:
>    On Wed, Nov 16, 2016 at 12:56 AM, Jordan Justen
>    <jordan.l.justen at intel.com> wrote:
> 
>      Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>      ---
>       src/intel/blorp/blorp.h               |  8 +++++++
>       src/mesa/drivers/dri/i965/brw_blorp.c | 39
>      +++++++++++++++++++++++++++++++++++
>       2 files changed, 47 insertions(+)
> 
>      diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
>      index 4351cb1..d891ba3 100644
>      --- a/src/intel/blorp/blorp.h
>      +++ b/src/intel/blorp/blorp.h
>      @@ -38,6 +38,7 @@ extern "C" {
> 
>       struct blorp_batch;
>       struct blorp_params;
>      +struct blorp_address;
> 
>       struct blorp_context {
>          void *driver_ctx;
>      @@ -52,6 +53,13 @@ struct blorp_context {
>             uint32_t vb;
>          } mocs;
> 
>      +   void (*map)(const struct blorp_context *blorp,
>      +               const struct blorp_address *blorp_addr,
>      +               void **addr,
>      +               unsigned int *map_size,
>      +               bool *mapped_previously);
>      +   void (*unmap)(const struct blorp_context *blorp,
>      +                 const struct blorp_address *blorp_addr);
>          bool (*lookup_shader)(struct blorp_context *blorp,
>                                const void *key, uint32_t key_size,
>                                uint32_t *kernel_out, void *prog_data_out);
>      diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>      b/src/mesa/drivers/dri/i965/brw_blorp.c
>      index f0ad074..41b6214 100644
>      --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>      +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>      @@ -34,10 +34,47 @@
>       #include "brw_meta_util.h"
>       #include "brw_state.h"
>       #include "intel_fbo.h"
>      +#include "intel_batchbuffer.h"
>       #include "intel_debug.h"
> 
>       #define FILE_DEBUG_FLAG DEBUG_BLORP
> 
>      +static void
>      +brw_blorp_map(const struct blorp_context *blorp,
>      +              const struct blorp_address *blorp_addr,
>      +              void **addr,
>      +              unsigned int *size,
>      +              bool *mapped_previously)
>      +{
>      +   struct brw_context *brw = blorp->driver_ctx;
>      +   drm_intel_bo *bo = (drm_intel_bo *)blorp_addr->buffer;
>      +
>      +   *mapped_previously = bo->virtual != NULL;
>      +
>      +   if (!*mapped_previously) {
>      +      if (drm_intel_bo_references(brw->batch.bo, bo))
>      +         intel_batchbuffer_flush(brw);
>      +
>      +      int ret = drm_intel_bo_map(bo, /*write_enable*/ false);
>      +      if (ret == -1) {
>      +         assert(bo->virtual == NULL);
>      +         *addr = NULL;
>      +         return;
>      +      }
>      +   }
>      +
>      +   *addr = bo->virtual;
>      +   *size = bo->size;
>      +}
>      +
>      +static void
>      +brw_blorp_unmap(const struct blorp_context *blorp,
>      +                const struct blorp_address *blorp_addr)
>      +{
>      +   drm_intel_bo *bo = (drm_intel_bo *)blorp_addr->buffer;
>      +   drm_intel_bo_unmap(bo);
> 
>    In the above code you don't map again if it's already got a map but here
>    you unmap regardless.  Is this going to result in some sort of nasty
>    double-umapping?
>

I expected the caller would use the mapped_previously return from the
map call to decide if unmap should be called. Maybe there is a better
interface?

-Jordan

>
>      +}
>      +
>       static bool
>       brw_blorp_lookup_shader(struct blorp_context *blorp,
>                               const void *key, uint32_t key_size,
>      @@ -102,6 +139,8 @@ brw_blorp_init(struct brw_context *brw)
>             unreachable("Invalid gen");
>          }
> 
>      +   brw->blorp.map = brw_blorp_map;
>      +   brw->blorp.unmap = brw_blorp_unmap;
>          brw->blorp.lookup_shader = brw_blorp_lookup_shader;
>          brw->blorp.upload_shader = brw_blorp_upload_shader;
>       }
>      --
>      2.10.2
> 
>      _______________________________________________
>      mesa-dev mailing list
>      mesa-dev at lists.freedesktop.org
>      https://lists.freedesktop.org/mailman/listinfo/mesa-dev