Am Mittwoch, den 14.08.2019, 14:29 +0200 schrieb Guido Günther:
> Hi,
> nOn Fri, Aug 09, 2019 at 02:05:14PM +0200, Lucas Stach wrote:
> > With softpin we allow the userspace to take control over the GPU virtual
> > address space. The new capability is relected by a bump of the minor DRM
> > version. There are a few restrictions for userspace to take into
> > account:
> > 
> > 1. The kernel reserves a bit of the address space to implement zero page
> > faulting and mapping of the kernel internal ring buffer. Userspace can
> > query the kernel for the first usable GPU VM address via
> > ETNAVIV_PARAM_SOFTPIN_START_ADDR.
> > 
> > 2. We only allow softpin on GPUs, which implement proper process
> > separation via PPAS. If softpin is not available the softpin start
> > address will be set to ~0.
> > 
> > 3. Softpin is all or nothing. A submit using softpin must not use any
> > address fixups via relocs.
> > 
> > > > Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
> > > > Reviewed-by: Philipp Zabel <p.za...@pengutronix.de>
> > ---
> > v2: Extend 'presumed' field description to match its softpin use.
> > ---
> >  drivers/gpu/drm/etnaviv/etnaviv_drv.c        |  2 +-
> >  drivers/gpu/drm/etnaviv/etnaviv_drv.h        |  2 ++
> >  drivers/gpu/drm/etnaviv/etnaviv_gem.c        |  5 ++--
> >  drivers/gpu/drm/etnaviv/etnaviv_gem.h        |  1 +
> >  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 26 +++++++++++++++++++-
> >  drivers/gpu/drm/etnaviv/etnaviv_gpu.c        |  9 +++++++
> >  include/uapi/drm/etnaviv_drm.h               | 10 +++++++-
> >  7 files changed, 50 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c 
> > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index 80f1edcbbea0..45851d510f62 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -529,7 +529,7 @@ static struct drm_driver etnaviv_drm_driver = {
> > > >         .desc               = "etnaviv DRM",
> > > >         .date               = "20151214",
> > > >         .major              = 1,
> > > > -       .minor              = 2,
> > > > +       .minor              = 3,
> >  };
> >  
> >  /*
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h 
> > b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > index a488cfdb6bbf..32cfa5a48d42 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > @@ -24,6 +24,8 @@ struct etnaviv_gem_object;
> >  struct etnaviv_gem_submit;
> >  struct etnaviv_iommu_global;
> >  
> > > > +#define ETNAVIV_SOFTPIN_START_ADDRESS  SZ_4M /* must be >=
> > SUBALLOC_SIZE */
> 
> what about doing this as code like
> 
> #if ETNAVIV_SOFTPIN_START_ADDRESS < SUBALLOC_SIZE
> # error "ETNAVIV_SOFTPIN_START_ADDRESS < SUBALLOC_SIZE"
> #endif
> 
> in e.g. entaviv_cmdbuf.c ?

Yes, validating this is a good idea. I've gone with BUILD_BUG_ON()
which is a bit more compact.

Regards,
Lucas

> apart from that
> 
> > Reviewed-by: Guido Günther <a...@sigxcpu.org> 
> 
> > +
> >  struct etnaviv_file_private {
> > > > > >     struct etnaviv_iommu_context    *mmu;
> > > > > >     struct drm_sched_entity         sched_entity[ETNA_MAX_PIPES];
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
> > b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > index 1a64fad55740..be62f1368272 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > @@ -248,7 +248,8 @@ void etnaviv_gem_mapping_unreference(struct 
> > etnaviv_vram_mapping *mapping)
> >  }
> >  
> >  struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
> > > > -       struct drm_gem_object *obj, struct etnaviv_iommu_context 
> > > > *mmu_context)
> > > > +       struct drm_gem_object *obj, struct etnaviv_iommu_context 
> > > > *mmu_context,
> > > > +       u64 va)
> >  {
> > > >         struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> > > >         struct etnaviv_vram_mapping *mapping;
> > @@ -309,7 +310,7 @@ struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
> >  
> > > >         ret = etnaviv_iommu_map_gem(mmu_context, etnaviv_obj,
> > > >                                     mmu_context->global->memory_base,
> > > > -                                   mapping, 0);
> > > > +                                   mapping, va);
> > > >         if (ret < 0)
> > > >                 kfree(mapping);
> > > >         else
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h 
> > b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > index 2d01ee1f65c1..f1164934ecdf 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > @@ -77,6 +77,7 @@ static inline bool is_active(struct etnaviv_gem_object 
> > *etnaviv_obj)
> >  
> >  struct etnaviv_gem_submit_bo {
> > > >         u32 flags;
> > > > +       u64 va;
> > > >         struct etnaviv_gem_object *obj;
> > > >         struct etnaviv_vram_mapping *mapping;
> > > >         struct dma_fence *excl;
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c 
> > b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > index 63a1206492d2..75afce42921b 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > @@ -72,6 +72,14 @@ static int submit_lookup_objects(struct 
> > etnaviv_gem_submit *submit,
> > > >                 }
> >  
> > > >                 submit->bos[i].flags = bo->flags;
> > > > +               if (submit->flags & ETNA_SUBMIT_SOFTPIN) {
> > > > +                       if (bo->presumed < 
> > > > ETNAVIV_SOFTPIN_START_ADDRESS) {
> > > > +                               DRM_ERROR("invalid softpin address\n");
> > > > +                               ret = -EINVAL;
> > > > +                               goto out_unlock;
> > > > +                       }
> > > > +                       submit->bos[i].va = bo->presumed;
> > > > +               }
> >  
> > > >                 /* normally use drm_gem_object_lookup(), but for bulk 
> > > > lookup
> > > >                  * all under single table_lock just hit object_idr 
> > > > directly:
> > @@ -224,11 +232,17 @@ static int submit_pin_objects(struct 
> > etnaviv_gem_submit *submit)
> > > >                 struct etnaviv_vram_mapping *mapping;
> >  
> > > >                 mapping = etnaviv_gem_mapping_get(&etnaviv_obj->base,
> > > > -                                                 submit->mmu_context);
> > > > +                                                 submit->mmu_context,
> > > > +                                                 submit->bos[i].va);
> > > >                 if (IS_ERR(mapping)) {
> > > >                         ret = PTR_ERR(mapping);
> > > >                         break;
> > > >                 }
> > +
> > > > +               if ((submit->flags & ETNA_SUBMIT_SOFTPIN) &&
> > > > +                    submit->bos[i].va != mapping->iova)
> > > > +                       return -EINVAL;
> > +
> > > >                 atomic_inc(&etnaviv_obj->gpu_active);
> >  
> > > >                 submit->bos[i].flags |= BO_PINNED;
> > @@ -261,6 +275,10 @@ static int submit_reloc(struct etnaviv_gem_submit 
> > *submit, void *stream,
> > > >         u32 *ptr = stream;
> > > >         int ret;
> >  
> > > > +       /* Submits using softpin don't blend with relocs */
> > > > +       if ((submit->flags & ETNA_SUBMIT_SOFTPIN) && nr_relocs != 0)
> > > > +               return -EINVAL;
> > +
> > > >         for (i = 0; i < nr_relocs; i++) {
> > > >                 const struct drm_etnaviv_gem_submit_reloc *r = relocs + 
> > > > i;
> > > >                 struct etnaviv_gem_submit_bo *bo;
> > @@ -445,6 +463,12 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, 
> > void *data,
> > > >                 return -EINVAL;
> > > >         }
> >  
> > > > +       if ((args->flags & ETNA_SUBMIT_SOFTPIN) &&
> > > > +           priv->mmu_global->version != ETNAVIV_IOMMU_V2) {
> > > > +               DRM_ERROR("softpin requested on incompatible MMU\n");
> > > > +               return -EINVAL;
> > > > +       }
> > +
> > > >         /*
> > > >          * Copy the command submission and bo array to kernel space in
> > > >          * one go, and do this outside of any locks.
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
> > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > index d8a83ebfce47..d47d1a8e0219 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > @@ -42,6 +42,8 @@ static const struct platform_device_id gpu_ids[] = {
> >  
> >  int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
> >  {
> > > > +       struct etnaviv_drm_private *priv = gpu->drm->dev_private;
> > +
> > > >         switch (param) {
> > > >         case ETNAVIV_PARAM_GPU_MODEL:
> > > >                 *value = gpu->identity.model;
> > @@ -147,6 +149,13 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 
> > param, u64 *value)
> > > >                 *value = gpu->identity.varyings_count;
> > > >                 break;
> >  
> > > > +       case ETNAVIV_PARAM_SOFTPIN_START_ADDR:
> > > > +               if (priv->mmu_global->version == ETNAVIV_IOMMU_V2)
> > > > +                       *value = ETNAVIV_SOFTPIN_START_ADDRESS;
> > > > +               else
> > > > +                       *value = ~0ULL;
> > > > +               break;
> > +
> > > >         default:
> > > >                 DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
> > > >                 return -EINVAL;
> > diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> > index 0d5c49dc478c..09d0df8b71c5 100644
> > --- a/include/uapi/drm/etnaviv_drm.h
> > +++ b/include/uapi/drm/etnaviv_drm.h
> > @@ -73,6 +73,7 @@ struct drm_etnaviv_timespec {
> >  #define ETNAVIV_PARAM_GPU_INSTRUCTION_COUNT         0x18
> >  #define ETNAVIV_PARAM_GPU_NUM_CONSTANTS             0x19
> >  #define ETNAVIV_PARAM_GPU_NUM_VARYINGS              0x1a
> > +#define ETNAVIV_PARAM_SOFTPIN_START_ADDR            0x1b
> >  
> >  #define ETNA_MAX_PIPES 4
> >  
> > @@ -148,6 +149,11 @@ struct drm_etnaviv_gem_submit_reloc {
> >   * then patching the cmdstream for this entry is skipped.  This can
> >   * avoid kernel needing to map/access the cmdstream bo in the common
> >   * case.
> > + * If the submit is a softpin submit (ETNA_SUBMIT_SOFTPIN) the 'presumed'
> > + * field is interpreted as the fixed location to map the bo into the gpu
> > + * virtual address space. If the kernel is unable to map the buffer at
> > + * this location the submit will fail. This means userspace is responsible
> > + * for the whole gpu virtual address management.
> >   */
> >  #define ETNA_SUBMIT_BO_READ             0x0001
> >  #define ETNA_SUBMIT_BO_WRITE            0x0002
> > @@ -177,9 +183,11 @@ struct drm_etnaviv_gem_submit_pmr {
> >  #define ETNA_SUBMIT_NO_IMPLICIT         0x0001
> >  #define ETNA_SUBMIT_FENCE_FD_IN         0x0002
> >  #define ETNA_SUBMIT_FENCE_FD_OUT        0x0004
> > +#define ETNA_SUBMIT_SOFTPIN             0x0008
> > > >  #define ETNA_SUBMIT_FLAGS              (ETNA_SUBMIT_NO_IMPLICIT | \
> > > >                                          ETNA_SUBMIT_FENCE_FD_IN | \
> > > > -                                        ETNA_SUBMIT_FENCE_FD_OUT)
> > > > +                                        ETNA_SUBMIT_FENCE_FD_OUT| \
> > > > +                                        ETNA_SUBMIT_SOFTPIN)
> >  #define ETNA_PIPE_3D      0x00
> >  #define ETNA_PIPE_2D      0x01
> >  #define ETNA_PIPE_VG      0x02
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > etnaviv mailing list
> > etna...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/etnaviv
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to