On Wed, Mar 08, 2017 at 03:12:57PM +0100, Daniel Vetter wrote:
> Sadly there's only 1 driver which can use it, everyone else is special
> for some reason:
>
> - gma500 has a horrible runtime PM ioctl wrapper that probably doesn't
> really work but meh.
> - i915 needs special compat_ioctl handler because regrets.
> - arcgpu needs to fixup the pgprot because (no idea why it can't do
> that in the fault handler like everyone else).
> - tegra does even worse stuff with pgprot
> - udl does something with vm_flags too ...
> - cma helpers, etnaviv, mtk, msm, rockchip, omap all implement some
> variation on prefaulting.
> - exynos is exynos, I got lost in the midlayers.
> - vc4 has to reinvent half of cma helpers because those are too much
> midlayer, plus vm_flags dances.
> - vgem also seems unhappy with the default vm_flags.
I wonder whether a todo entry to clean this up would be appropriate.
Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
>
> So pretty sad divergence and I'm sure we could do better, but not
> really an idea. Oh well, maybe this macro here helps to encourage more
> consistency at least going forward.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
> drivers/gpu/drm/armada/armada_drv.c | 11 +----------
> drivers/gpu/drm/drm_file.c | 8 +++-----
> include/drm/drm_gem.h | 26 ++++++++++++++++++++++++++
> 3 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 1952e8748fea..f986e9ff093d 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -60,16 +60,7 @@ static void armada_drm_lastclose(struct drm_device *dev)
> armada_fbdev_lastclose(dev);
> }
>
> -static const struct file_operations armada_drm_fops = {
> - .owner = THIS_MODULE,
> - .llseek = no_llseek,
> - .read = drm_read,
> - .poll = drm_poll,
> - .unlocked_ioctl = drm_ioctl,
> - .mmap = drm_gem_mmap,
> - .open = drm_open,
> - .release = drm_release,
> -};
> +DEFINE_DRM_GEM_FOPS(armada_drm_fops);
>
> static struct drm_driver armada_drm_driver = {
> .lastclose = armada_drm_lastclose,
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 260a83563976..0701362dcca1 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -89,11 +89,9 @@ DEFINE_MUTEX(drm_global_mutex);
> * .mmap = drm_gem_mmap,
> * };
> *
> - * For CMA based drivers there is the DEFINE_DRM_GEM_CMA_FOPS() macro to make
> - * this simpler.
> - *
> - * FIXME: We should have a macro for this (and the CMA version) so that drivers
> - * don't have to repeat it all the time.
> + * For plain GEM based drivers there is the DEFINE_DRM_GEM_FOPS() macro, and for
> + * CMA based drivers there is the DEFINE_DRM_GEM_CMA_FOPS() macro to make this
> + * simpler.
> */
>
> static int drm_open_helper(struct file *filp, struct drm_minor *minor);
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index b9ade75ecd82..663d80358057 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -178,6 +178,32 @@ struct drm_gem_object {
> struct dma_buf_attachment *import_attach;
> };
>
> +/**
> + * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
> + * @name: name for the generated structure
> + *
> + * This macro autogenerates a suitable &struct file_operations for GEM based
> + * drivers, which can be assigned to &drm_driver.fops. Note that this structure
> + * cannot be shared between drivers, because it contains a reference to the
> + * current module using THIS_MODULE.
> + *
> + * Note that the declaration is already marked as static - if you need a
> + * non-static version of this you're probably doing it wrong and will break the
> + * THIS_MODULE reference by accident.
> + */
> +#define DEFINE_DRM_GEM_FOPS(name) \
> + static const struct file_operations name = {\
> + .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 = noop_llseek,\
> + .mmap = drm_gem_mmap,\
> + }
> +
> void drm_gem_object_release(struct drm_gem_object *obj);
> void drm_gem_object_free(struct kref *kref);
> int drm_gem_object_init(struct drm_device *dev,
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
