| Message ID | 20191121103807.18424-2-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: mmap fixups | expand |
On Thu, Nov 21, 2019 at 11:38:06AM +0100, Gerd Hoffmann wrote: > The fake offset is going to stay, so change the calling convention for > drm_gem_object_funcs.mmap to include the fake offset. Update all users > accordingly. Please add to the commit message: Note that this reverts 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap") and on top then adds the fake offset to drm_gem_prime_mmap to make sure all paths leading to obj->funcs->mmap are consistent. Fixes: 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap") Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Rob Herring <robh@kernel.org> With that also Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> -Daniel > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > include/drm/drm_gem.h | 4 +--- > drivers/gpu/drm/drm_gem.c | 3 --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ > drivers/gpu/drm/drm_prime.c | 3 +++ > drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 ------- > 5 files changed, 7 insertions(+), 13 deletions(-) > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index 97a48165642c..0b375069cd48 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -159,9 +159,7 @@ struct drm_gem_object_funcs { > * > * The callback is used by by both drm_gem_mmap_obj() and > * drm_gem_prime_mmap(). When @mmap is present @vm_ops is not > - * used, the @mmap callback must set vma->vm_ops instead. The @mmap > - * callback is always called with a 0 offset. The caller will remove > - * the fake offset as necessary. > + * used, the @mmap callback must set vma->vm_ops instead. > */ > int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 2f2b889096b0..56f42e0f2584 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, > return -EINVAL; > > if (obj->funcs && obj->funcs->mmap) { > - /* Remove the fake offset */ > - vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); > - > ret = obj->funcs->mmap(obj, vma); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 0810d3ef6961..a421a2eed48a 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) > struct drm_gem_shmem_object *shmem; > int ret; > > + /* Remove the fake offset */ > + vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); > + > shmem = to_drm_gem_shmem_obj(obj); > > ret = drm_gem_shmem_get_pages(shmem); > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 0814211b0f3f..a9633bd241bb 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) > int ret; > > if (obj->funcs && obj->funcs->mmap) { > + /* Add the fake offset */ > + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node); > + > ret = obj->funcs->mmap(obj, vma); > if (ret) > return ret; > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index e6495ca2630b..3e8c3de91ae4 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap); > int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo) > { > ttm_bo_get(bo); > - > - /* > - * FIXME: &drm_gem_object_funcs.mmap is called with the fake offset > - * removed. Add it back here until the rest of TTM works without it. > - */ > - vma->vm_pgoff += drm_vma_node_start(&bo->base.vma_node); > - > ttm_bo_mmap_vma_setup(bo, vma); > return 0; > } > -- > 2.18.1 >
On Thu, Nov 21, 2019 at 02:52:59PM +0100, Daniel Vetter wrote: > On Thu, Nov 21, 2019 at 11:38:06AM +0100, Gerd Hoffmann wrote: > > The fake offset is going to stay, so change the calling convention for > > drm_gem_object_funcs.mmap to include the fake offset. Update all users > > accordingly. > > Please add to the commit message: > > Note that this reverts 83b8a6f242ea ("drm/gem: Fix mmap fake offset > handling for drm_gem_object_funcs.mmap") and on top then adds the fake > offset to drm_gem_prime_mmap to make sure all paths leading to > obj->funcs->mmap are consistent. > > Fixes: 83b8a6f242ea ("drm/gem: Fix mmap fake offset handling for drm_gem_object_funcs.mmap") > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Rob Herring <robh@kernel.org> > > With that also Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Also added Rob to cc here. Rob, can you pls take a look an ack? The sage took another turn :-) -Daniel > -Daniel > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > --- > > include/drm/drm_gem.h | 4 +--- > > drivers/gpu/drm/drm_gem.c | 3 --- > > drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ > > drivers/gpu/drm/drm_prime.c | 3 +++ > > drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 ------- > > 5 files changed, 7 insertions(+), 13 deletions(-) > > > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > > index 97a48165642c..0b375069cd48 100644 > > --- a/include/drm/drm_gem.h > > +++ b/include/drm/drm_gem.h > > @@ -159,9 +159,7 @@ struct drm_gem_object_funcs { > > * > > * The callback is used by by both drm_gem_mmap_obj() and > > * drm_gem_prime_mmap(). When @mmap is present @vm_ops is not > > - * used, the @mmap callback must set vma->vm_ops instead. The @mmap > > - * callback is always called with a 0 offset. The caller will remove > > - * the fake offset as necessary. > > + * used, the @mmap callback must set vma->vm_ops instead. > > */ > > int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > index 2f2b889096b0..56f42e0f2584 100644 > > --- a/drivers/gpu/drm/drm_gem.c > > +++ b/drivers/gpu/drm/drm_gem.c > > @@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, > > return -EINVAL; > > > > if (obj->funcs && obj->funcs->mmap) { > > - /* Remove the fake offset */ > > - vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); > > - > > ret = obj->funcs->mmap(obj, vma); > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > > index 0810d3ef6961..a421a2eed48a 100644 > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > @@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) > > struct drm_gem_shmem_object *shmem; > > int ret; > > > > + /* Remove the fake offset */ > > + vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); > > + > > shmem = to_drm_gem_shmem_obj(obj); > > > > ret = drm_gem_shmem_get_pages(shmem); > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > index 0814211b0f3f..a9633bd241bb 100644 > > --- a/drivers/gpu/drm/drm_prime.c > > +++ b/drivers/gpu/drm/drm_prime.c > > @@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) > > int ret; > > > > if (obj->funcs && obj->funcs->mmap) { > > + /* Add the fake offset */ > > + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node); > > + > > ret = obj->funcs->mmap(obj, vma); > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > > index e6495ca2630b..3e8c3de91ae4 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > > @@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap); > > int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo) > > { > > ttm_bo_get(bo); > > - > > - /* > > - * FIXME: &drm_gem_object_funcs.mmap is called with the fake offset > > - * removed. Add it back here until the rest of TTM works without it. > > - */ > > - vma->vm_pgoff += drm_vma_node_start(&bo->base.vma_node); > > - > > ttm_bo_mmap_vma_setup(bo, vma); > > return 0; > > } > > -- > > 2.18.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 97a48165642c..0b375069cd48 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -159,9 +159,7 @@ struct drm_gem_object_funcs { * * The callback is used by by both drm_gem_mmap_obj() and * drm_gem_prime_mmap(). When @mmap is present @vm_ops is not - * used, the @mmap callback must set vma->vm_ops instead. The @mmap - * callback is always called with a 0 offset. The caller will remove - * the fake offset as necessary. + * used, the @mmap callback must set vma->vm_ops instead. */ int (*mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 2f2b889096b0..56f42e0f2584 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1106,9 +1106,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, return -EINVAL; if (obj->funcs && obj->funcs->mmap) { - /* Remove the fake offset */ - vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); - ret = obj->funcs->mmap(obj, vma); if (ret) return ret; diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 0810d3ef6961..a421a2eed48a 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -528,6 +528,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) struct drm_gem_shmem_object *shmem; int ret; + /* Remove the fake offset */ + vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node); + shmem = to_drm_gem_shmem_obj(obj); ret = drm_gem_shmem_get_pages(shmem); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 0814211b0f3f..a9633bd241bb 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -714,6 +714,9 @@ int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma) int ret; if (obj->funcs && obj->funcs->mmap) { + /* Add the fake offset */ + vma->vm_pgoff += drm_vma_node_start(&obj->vma_node); + ret = obj->funcs->mmap(obj, vma); if (ret) return ret; diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index e6495ca2630b..3e8c3de91ae4 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -514,13 +514,6 @@ EXPORT_SYMBOL(ttm_bo_mmap); int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo) { ttm_bo_get(bo); - - /* - * FIXME: &drm_gem_object_funcs.mmap is called with the fake offset - * removed. Add it back here until the rest of TTM works without it. - */ - vma->vm_pgoff += drm_vma_node_start(&bo->base.vma_node); - ttm_bo_mmap_vma_setup(bo, vma); return 0; }
The fake offset is going to stay, so change the calling convention for drm_gem_object_funcs.mmap to include the fake offset. Update all users accordingly. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- include/drm/drm_gem.h | 4 +--- drivers/gpu/drm/drm_gem.c | 3 --- drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ drivers/gpu/drm/drm_prime.c | 3 +++ drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 ------- 5 files changed, 7 insertions(+), 13 deletions(-)