Re: [PATCH 23/24] drm: Create DEFINE_DRM_GEM_CMA_FOPS and roll it out to drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 13, 2017 at 03:31:40PM -0400, Sean Paul wrote:
> On Wed, Mar 08, 2017 at 03:12:56PM +0100, Daniel Vetter wrote:
> > Less code ftw.
> > 
> > This converts all drivers except the tinydrm helper module. That one
> > needs more work, since it gets the THIS_MODULE reference from
> > tinydrm.ko instead of the actual driver module like it should.
> > Probably needs a similar trick like I used here with generating the
> > entire struct with a macro.
> > 
> > Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/arm/hdlcd_drv.c                 | 12 +-----------
> >  drivers/gpu/drm/arm/malidp_drv.c                | 12 +-----------
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    | 12 +-----------
> >  drivers/gpu/drm/drm_file.c                      |  3 +++
> >  drivers/gpu/drm/drm_gem_cma_helper.c            |  3 +++
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c       | 12 +-----------
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 12 +-----------
> >  drivers/gpu/drm/imx/imx-drm-core.c              | 11 +----------
> >  drivers/gpu/drm/meson/meson_drv.c               | 14 +------------
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.c               | 14 +------------
> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c           | 12 +-----------
> >  drivers/gpu/drm/shmobile/shmob_drm_drv.c        | 12 +-----------
> >  drivers/gpu/drm/sti/sti_drv.c                   | 11 +----------
> >  drivers/gpu/drm/sun4i/sun4i_drv.c               | 12 +-----------
> >  drivers/gpu/drm/tilcdc/tilcdc_drv.c             | 12 +-----------
> >  drivers/gpu/drm/zte/zx_drm_drv.c                | 14 +------------
> >  include/drm/drm_gem_cma_helper.h                | 26 +++++++++++++++++++++++++
> >  17 files changed, 46 insertions(+), 158 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> > index 4840dc277339..0e74880b5e94 100644
> > --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> > @@ -239,17 +239,7 @@ static int hdlcd_debugfs_init(struct drm_minor *minor)
> >  }
> >  #endif
> >  
> > -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		= noop_llseek,
> > -	.mmap		= drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(fops);
> >  
> >  static struct drm_driver hdlcd_driver = {
> >  	.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> > index a9608a2e5a29..ea2546f766c2 100644
> > --- a/drivers/gpu/drm/arm/malidp_drv.c
> > +++ b/drivers/gpu/drm/arm/malidp_drv.c
> > @@ -178,17 +178,7 @@ static void malidp_lastclose(struct drm_device *drm)
> >  	drm_fbdev_cma_restore_mode(malidp->fbdev);
> >  }
> >  
> > -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 = noop_llseek,
> > -	.mmap = drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(fops);
> >  
> >  static struct drm_driver malidp_driver = {
> >  	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC |
> > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > index 970bd87d7cce..178acbf5f6c5 100644
> > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> > @@ -724,17 +724,7 @@ static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
> >  	regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_ISR, &isr);
> >  }
> >  
> > -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               = drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(fops);
> >  
> >  static struct drm_driver atmel_hlcdc_dc_driver = {
> >  	.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
> > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > index f8483fc6d3d7..260a83563976 100644
> > --- a/drivers/gpu/drm/drm_file.c
> > +++ b/drivers/gpu/drm/drm_file.c
> > @@ -89,6 +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
> 
> s_(and the CMA version)__

Whoops, just saw 24/24 removes this entirely. ¯\_(ツ)_/¯

> 
> Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> 
> >   * don't have to repeat it all the time.
> >   */
> > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> > index bb968e76779b..bc28e7575254 100644
> > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > @@ -338,6 +338,9 @@ static int drm_gem_cma_mmap_obj(struct drm_gem_cma_object *cma_obj,
> >   * as their ->mmap() handler in the DRM device file's file_operations
> >   * structure.
> >   *
> > + * Instead of directly referencing this function, drivers should use the
> > + * DEFINE_DRM_GEM_CMA_FOPS().macro.
> > + *
> >   * Returns:
> >   * 0 on success or a negative error code on failure.
> >   */
> > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > index b5391c124c64..6e00f4b267f1 100644
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > @@ -161,17 +161,7 @@ static void fsl_dcu_drm_lastclose(struct drm_device *dev)
> >  	drm_fbdev_cma_restore_mode(fsl_dev->fbdev);
> >  }
> >  
> > -static const struct file_operations fsl_dcu_drm_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		= drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
> >  
> >  static struct drm_driver fsl_dcu_drm_driver = {
> >  	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
> > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> > index 7ec93aec7e88..df4f50713e54 100644
> > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> > @@ -146,17 +146,7 @@ static int kirin_drm_kms_init(struct drm_device *dev)
> >  	return ret;
> >  }
> >  
> > -static const struct file_operations kirin_drm_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		= drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(kirin_drm_fops);
> >  
> >  static int kirin_gem_cma_dumb_create(struct drm_file *file,
> >  				     struct drm_device *dev,
> > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> > index 4b7b92a7bcf7..70736615983c 100644
> > --- a/drivers/gpu/drm/imx/imx-drm-core.c
> > +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> > @@ -57,16 +57,7 @@ static void imx_drm_driver_lastclose(struct drm_device *drm)
> >  	drm_fbdev_cma_restore_mode(imxdrm->fbhelper);
> >  }
> >  
> > -static const struct file_operations imx_drm_driver_fops = {
> > -	.owner = THIS_MODULE,
> > -	.open = drm_open,
> > -	.release = drm_release,
> > -	.unlocked_ioctl = drm_ioctl,
> > -	.mmap = drm_gem_cma_mmap,
> > -	.poll = drm_poll,
> > -	.read = drm_read,
> > -	.llseek = noop_llseek,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(imx_drm_driver_fops);
> >  
> >  void imx_drm_connector_destroy(struct drm_connector *connector)
> >  {
> > diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> > index 8d17d0e59cbe..bc562a07847b 100644
> > --- a/drivers/gpu/drm/meson/meson_drv.c
> > +++ b/drivers/gpu/drm/meson/meson_drv.c
> > @@ -91,19 +91,7 @@ static irqreturn_t meson_irq(int irq, void *arg)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static const struct file_operations fops = {
> > -	.owner		= THIS_MODULE,
> > -	.open		= drm_open,
> > -	.release	= drm_release,
> > -	.unlocked_ioctl	= drm_ioctl,
> > -#ifdef CONFIG_COMPAT
> > -	.compat_ioctl	= drm_compat_ioctl,
> > -#endif
> > -	.poll		= drm_poll,
> > -	.read		= drm_read,
> > -	.llseek		= no_llseek,
> > -	.mmap		= drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(fops);
> >  
> >  static struct drm_driver meson_driver = {
> >  	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM |
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > index a4633ada8429..5ac712325c75 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> > @@ -319,19 +319,7 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static const struct file_operations fops = {
> > -	.owner		= THIS_MODULE,
> > -	.open		= drm_open,
> > -	.release	= drm_release,
> > -	.unlocked_ioctl	= drm_ioctl,
> > -#ifdef CONFIG_COMPAT
> > -	.compat_ioctl	= drm_compat_ioctl,
> > -#endif
> > -	.poll		= drm_poll,
> > -	.read		= drm_read,
> > -	.llseek		= noop_llseek,
> > -	.mmap		= drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(fops);
> >  
> >  static struct drm_driver mxsfb_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET |
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > index 192346d4fb34..62a3b3e32153 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > @@ -226,17 +226,7 @@ static void rcar_du_lastclose(struct drm_device *dev)
> >  	drm_fbdev_cma_restore_mode(rcdu->fbdev);
> >  }
> >  
> > -static const struct file_operations rcar_du_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		= drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
> >  
> >  static struct drm_driver rcar_du_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME
> > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > index 34fefa0ba0f0..75f2e5e6d7fb 100644
> > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> > @@ -221,17 +221,7 @@ static irqreturn_t shmob_drm_irq(int irq, void *arg)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static const struct file_operations shmob_drm_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		= drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(shmob_drm_fops);
> >  
> >  static struct drm_driver shmob_drm_driver = {
> >  	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
> > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> > index 3b15c2cb2306..23da9bc4666a 100644
> > --- a/drivers/gpu/drm/sti/sti_drv.c
> > +++ b/drivers/gpu/drm/sti/sti_drv.c
> > @@ -167,16 +167,7 @@ static void sti_mode_config_init(struct drm_device *dev)
> >  	dev->mode_config.funcs = &sti_mode_config_funcs;
> >  }
> >  
> > -static const struct file_operations sti_driver_fops = {
> > -	.owner = THIS_MODULE,
> > -	.open = drm_open,
> > -	.mmap = drm_gem_cma_mmap,
> > -	.poll = drm_poll,
> > -	.read = drm_read,
> > -	.unlocked_ioctl = drm_ioctl,
> > -	.compat_ioctl = drm_compat_ioctl,
> > -	.release = drm_release,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(sti_driver_fops);
> >  
> >  static struct drm_driver sti_driver = {
> >  	.driver_features = DRIVER_MODESET |
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > index 9ccf7c4deb6d..329ea56106a5 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > @@ -25,17 +25,7 @@
> >  #include "sun4i_framebuffer.h"
> >  #include "sun4i_layer.h"
> >  
> > -static const struct file_operations sun4i_drv_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		= drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(sun4i_drv_fops);
> >  
> >  static struct drm_driver sun4i_drv_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > index 81d80a2ffeb1..480411c2e802 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> > @@ -530,17 +530,7 @@ static int tilcdc_debugfs_init(struct drm_minor *minor)
> >  }
> >  #endif
> >  
> > -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               = drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(fops);
> >  
> >  static struct drm_driver tilcdc_driver = {
> >  	.driver_features    = (DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET |
> > diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
> > index b24a70ba4b83..614e854f6be5 100644
> > --- a/drivers/gpu/drm/zte/zx_drm_drv.c
> > +++ b/drivers/gpu/drm/zte/zx_drm_drv.c
> > @@ -53,19 +53,7 @@ static void zx_drm_lastclose(struct drm_device *drm)
> >  	drm_fbdev_cma_restore_mode(priv->fbdev);
> >  }
> >  
> > -static const struct file_operations zx_drm_fops = {
> > -	.owner = THIS_MODULE,
> > -	.open = drm_open,
> > -	.release = drm_release,
> > -	.unlocked_ioctl = drm_ioctl,
> > -#ifdef CONFIG_COMPAT
> > -	.compat_ioctl = drm_compat_ioctl,
> > -#endif
> > -	.poll = drm_poll,
> > -	.read = drm_read,
> > -	.llseek = noop_llseek,
> > -	.mmap = drm_gem_cma_mmap,
> > -};
> > +DEFINE_DRM_GEM_CMA_FOPS(zx_drm_fops);
> >  
> >  static struct drm_driver zx_drm_driver = {
> >  	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
> > diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
> > index 2abcd5190cc1..f962d33667cf 100644
> > --- a/include/drm/drm_gem_cma_helper.h
> > +++ b/include/drm/drm_gem_cma_helper.h
> > @@ -26,6 +26,32 @@ to_drm_gem_cma_obj(struct drm_gem_object *gem_obj)
> >  	return container_of(gem_obj, struct drm_gem_cma_object, base);
> >  }
> >  
> > +/**
> > + * DEFINE_DRM_GEM_CMA_FOPS() - macro to generate file operations for CMA drivers
> > + * @name: name for the generated structure
> > + *
> > + * This macro autogenerates a suitable &struct file_operations for CMA 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_CMA_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_cma_mmap,\
> > +	}
> > +
> >  /* free GEM object */
> >  void drm_gem_cma_free_object(struct drm_gem_object *gem_obj);
> >  
> > -- 
> > 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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux