Am Donnerstag, dem 24.06.2021 um 12:47 +0200 schrieb Thomas Zimmermann:
> Hi
>
> Am 24.06.21 um 11:11 schrieb Lucas Stach:
> > Am Donnerstag, dem 24.06.2021 um 10:58 +0200 schrieb Thomas Zimmermann:
> > > Moving the driver-specific mmap code into a GEM object function allows
> > > for using DRM helpers for various mmap callbacks.
> > >
> > > The respective etnaviv functions are being removed. The file_operations
> > > structure fops is now being created by the helper macro
> > > DEFINE_DRM_GEM_FOPS().
> > >
> > > Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
> > > ---
> > > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 14 ++------------
> > > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 3 ---
> > > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 18 +++++-------------
> > > drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 -------------
> > > 4 files changed, 7 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > > index f0a07278ad04..7dcc6392792d 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > > @@ -468,17 +468,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[]
> > > = {
> > > ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> > > };
> > >
> > > -static const struct file_operations fops = {
> > > - .owner = THIS_MODULE,
> > > - .open = drm_open,
> > > - .release = drm_release,
> > > - .unlocked_ioctl = drm_ioctl,
> > > - .compat_ioctl = drm_compat_ioctl,
> > > - .poll = drm_poll,
> > > - .read = drm_read,
> > > - .llseek = no_llseek,
> > > - .mmap = etnaviv_gem_mmap,
> > > -};
> > > +DEFINE_DRM_GEM_FOPS(fops);
> > >
> > > static const struct drm_driver etnaviv_drm_driver = {
> > > .driver_features = DRIVER_GEM | DRIVER_RENDER,
> > > @@ -487,7 +477,7 @@ static const struct drm_driver etnaviv_drm_driver = {
> > > .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > > .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > > .gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
> > > - .gem_prime_mmap = etnaviv_gem_prime_mmap,
> > > + .gem_prime_mmap = drm_gem_prime_mmap,
> > > #ifdef CONFIG_DEBUG_FS
> > > .debugfs_init = etnaviv_debugfs_init,
> > > #endif
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > > b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > > index 003288ebd896..049ae87de9be 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > > @@ -47,12 +47,9 @@ struct etnaviv_drm_private {
> > > int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> > > struct drm_file *file);
> > >
> > > -int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> > > int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
> > > struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object
> > > *obj);
> > > int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct
> > > dma_buf_map *map);
> > > -int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
> > > - struct vm_area_struct *vma);
> > > struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct
> > > drm_device *dev,
> > > struct dma_buf_attachment *attach, struct sg_table *sg);
> > > int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > > b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > > index b8fa6ed3dd73..8f1b5af47dd6 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> > > @@ -130,8 +130,7 @@ static int etnaviv_gem_mmap_obj(struct
> > > etnaviv_gem_object *etnaviv_obj,
> > > {
> > > pgprot_t vm_page_prot;
> > >
> > > - vma->vm_flags &= ~VM_PFNMAP;
> > > - vma->vm_flags |= VM_MIXEDMAP;
> > > + vma->vm_flags |= VM_IO | VM_MIXEDMAP | VM_DONTEXPAND | VM_DONTDUMP;
> >
> > I don't fully understand why this change is needed and the commit log
> > is silent about it. Excuse my ignorance, but can you please explain the
> > reasoning behind this change?
>
> Sure, sorry for being brief.
>
> I worked on cleaning up the deprecated gem_prime_* callbacks in struct
> drm_driver. These are supposed to be GEM object functions. The only
> obsolete gem prime callback in drm_driver is gem_prime_mmap.
Sorry, that's a misunderstanding. I see the justification for the patch
as a whole. I was asking specifically about the hunk above my comment.
Why are the vm_flags changed and how did you come up with this exact
combination of flags?
Regards,
Lucas
