On Tue, Apr 20, 2021 at 09:51:27AM +0200, Thomas Zimmermann wrote: > Hi > > Am 16.04.21 um 15:51 schrieb Christian König: > > Am 16.04.21 um 15:46 schrieb Christian König: > > > Am 16.04.21 um 15:31 schrieb Thomas Zimmermann: > > > > The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline > > > > the code. The internal helper ttm_bo_vm_lookup() is now also part of > > > > vmwgfx as vmw_bo_vm_lookup(). > > > > > > > > v2: > > > > * replace pr_err() with drm_err() (Zack) > > > > > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > > > > Reviewed-by: Zack Rusin <zackr@xxxxxxxxxx> > > > > --- > > > > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++++++++++++++++++++++-- > > > > 1 file changed, 53 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c > > > > b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c > > > > index cb9975889e2f..c8b6543b4e39 100644 > > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c > > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c > > > > @@ -27,6 +27,32 @@ > > > > #include "vmwgfx_drv.h" > > > > +static struct ttm_buffer_object *vmw_bo_vm_lookup(struct > > > > ttm_device *bdev, > > > > + unsigned long offset, > > > > + unsigned long pages) > > > > +{ > > > > + struct vmw_private *dev_priv = container_of(bdev, struct > > > > vmw_private, bdev); > > > > + struct drm_device *drm = &dev_priv->drm; > > > > + struct drm_vma_offset_node *node; > > > > + struct ttm_buffer_object *bo = NULL; > > > > + > > > > + drm_vma_offset_lock_lookup(bdev->vma_manager); > > > > + > > > > + node = drm_vma_offset_lookup_locked(bdev->vma_manager, > > > > offset, pages); > > > > + if (likely(node)) { > > > > + bo = container_of(node, > struct ttm_buffer_object, > > > > + base.vma_node); > > > > + bo = ttm_bo_get_unless_zero(bo); > > > > + } > > > > + > > > > + drm_vma_offset_unlock_lookup(bdev->vma_manager); > > > > + > > > > + if (!bo) > > > > + drm_err(drm, "Could not find buffer object to map\n"); > > > > + > > > > + return bo; > > > > +} > > > > + > > > > int vmw_mmap(struct file *filp, struct vm_area_struct *vma) > > > > { > > > > static const struct vm_operations_struct vmw_vm_ops = { > > > > @@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct > > > > vm_area_struct *vma) > > > > }; > > > > struct drm_file *file_priv = filp->private_data; > > > > struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev); > > > > - int ret = ttm_bo_mmap(filp, vma, &dev_priv->bdev); > > > > + struct ttm_device *bdev = &dev_priv->bdev; > > > > + struct ttm_buffer_object *bo; > > > > + int ret; > > > > + > > > > + if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START)) > > > > + return -EINVAL; > > > > + > > > > + bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma)); > > > > + if (unlikely(!bo)) > > > > + return -EINVAL; > > > > - if (ret) > > > > - return ret; > > > > + if (unlikely(!bo->bdev->funcs->verify_access)) { > > > > + ret = -EPERM; > > > > + goto out_unref; > > > > + } > > > > + ret = bo->bdev->funcs->verify_access(bo, filp); > > > > > > Is there any reason we can't call vmw_verify_access() directly here? > > > > > > Would allow us to completely nuke the verify_access callback as well > > > as far as I can see. > > > > Forget what I said, couldn't see the next patch in my mailbox at time of > > writing. > > > > Whole series is Reviewed-by: Christian König <christian.koenig@xxxxxxx> > > Thanks a lot. If I'm not mistaken, the patches at [1] need to go in first. > So it could take a a bit until this lands. > > Otherwise, this series could go through the same tree as [1] if nouveau and > vmwgfx devs don't mind. I would land it all through drm-misc-next. Maybe check with Alex on irc for an ack for merging that way, but I don't think this will cause issues against the amdgpu tree. Lots of ttm cleanup has landed this way already past few months. Otherwise you could create a small topic branch with these patches here and send that to Alex, and he can sort out the interaction with Felix' series. -Daniel > > Best regards > Thomas > > [1] https://patchwork.freedesktop.org/series/88822/ > > > > > Thanks for the nice cleanup, > > Christian. > > > > > > > > Regards, > > > Christian. > > > > > > > + if (unlikely(ret != 0)) > > > > + goto out_unref; > > > > + > > > > + ret = ttm_bo_mmap_obj(vma, bo); > > > > + if (unlikely(ret != 0)) > > > > + goto out_unref; > > > > vma->vm_ops = &vmw_vm_ops; > > > > @@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct > > > > vm_area_struct *vma) > > > > if (!is_cow_mapping(vma->vm_flags)) > > > > vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP; > > > > + ttm_bo_put(bo); /* release extra ref taken > by > > > > ttm_bo_mmap_obj() */ > > > > + > > > > return 0; > > > > + > > > > +out_unref: > > > > + ttm_bo_put(bo); > > > > + return ret; > > > > } > > > > /* struct vmw_validation_mem callback */ > > > > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx