Re: [PATCH 01/21] drm/cma-helper: Rework DRM_GEM_CMA_VMAP_DRIVER_OPS macro

Re: [PATCH 01/21] drm/cma-helper: Rework DRM_GEM_CMA_VMAP_DRIVER_OPS macro

[Sam Ravnborg]

Hi Thomas.

On Fri, May 22, 2020 at 03:52:26PM +0200, Thomas Zimmermann wrote:
> Rename the macro to DRM_GEM_CMA_DRIVER_OPS to align with SHMEM
> helpers.
This part is fine, I like that the naming is somehow consistent.

> An internal version is provided for drivers that override
> the default .dumb_create callback. Adapt drivers to the changes.
I loathe anything named __foo or __FOO. This __ signals to me
that the author was clueless in naming - or some sort.
I know that __ is used in some lib headers - but thats not the case
here.

But I love that we have a variant that takes a create function.
So we do not have to escape from the nice macro.
The macro is another way to tell me as rewiewer that this
drivers uses all the default helpers for this.


So critizising the name I better suggest something that
I personally like better:

DRM_GEM_CMA_DRIVER_OPS_CREATE()

It would look like this:
	/* GEM Operations */
-	DRM_GEM_CMA_VMAP_DRIVER_OPS,
-	.dumb_create            = drm_sun4i_gem_dumb_create,
+	DRM_GEM_CMA_DRIVER_OPS_CREATE(drm_sun4i_gem_dumb_create),



Please fix zte/zx_drm_drv.c which also uses DRM_GEM_CMA_VMAP_DRIVER_OPS.


The naming is a bikeshedding topic that we may not agree on, soo..

With zte fixed the patch is:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam


> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_gem_cma_helper.c |  2 +-
>  drivers/gpu/drm/sun4i/sun4i_drv.c    |  3 +--
>  drivers/gpu/drm/tidss/tidss_drv.c    |  2 +-
>  drivers/gpu/drm/tiny/hx8357d.c       |  2 +-
>  drivers/gpu/drm/tiny/ili9225.c       |  2 +-
>  drivers/gpu/drm/tiny/ili9341.c       |  2 +-
>  drivers/gpu/drm/tiny/ili9486.c       |  2 +-
>  drivers/gpu/drm/tiny/mi0283qt.c      |  2 +-
>  drivers/gpu/drm/tiny/repaper.c       |  2 +-
>  drivers/gpu/drm/tiny/st7586.c        |  2 +-
>  drivers/gpu/drm/tiny/st7735r.c       |  2 +-
>  include/drm/drm_gem_cma_helper.h     | 24 ++++++++++++++++++++----
>  12 files changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 12e98fb28229d..6fa4d2f2e3987 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -620,7 +620,7 @@ EXPORT_SYMBOL(drm_cma_gem_create_object_default_funcs);
>   * address set. This address is released when the object is freed.
>   *
>   * This function can be used as the &drm_driver.gem_prime_import_sg_table
> - * callback. The DRM_GEM_CMA_VMAP_DRIVER_OPS() macro provides a shortcut to set
> + * callback. The &DRM_GEM_CMA_DRIVER_OPS macro provides a shortcut to set
>   * the necessary DRM driver operations.
>   *
>   * Returns:
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 328272ff77d84..012855fd89c24 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -52,8 +52,7 @@ static struct drm_driver sun4i_drv_driver = {
>  	.minor			= 0,
>  
>  	/* GEM Operations */
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> -	.dumb_create		= drm_sun4i_gem_dumb_create,
> +	__DRM_GEM_CMA_DRIVER_OPS(drm_sun4i_gem_dumb_create),
>  };
>  
>  static int sun4i_drv_bind(struct device *dev)
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
> index 99edc66ebdef2..1753cdc74ebda 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.c
> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> @@ -112,7 +112,7 @@ static struct drm_driver tidss_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &tidss_fops,
>  	.release		= tidss_release,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.name			= "tidss",
>  	.desc			= "TI Keystone DSS",
>  	.date			= "20180215",
> diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
> index b4bc358a3269a..592da71d7ca70 100644
> --- a/drivers/gpu/drm/tiny/hx8357d.c
> +++ b/drivers/gpu/drm/tiny/hx8357d.c
> @@ -196,7 +196,7 @@ DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
>  static struct drm_driver hx8357d_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &hx8357d_fops,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "hx8357d",
>  	.desc			= "HX8357D",
> diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
> index d1a5ab6747d5c..368ff6c8a1efb 100644
> --- a/drivers/gpu/drm/tiny/ili9225.c
> +++ b/drivers/gpu/drm/tiny/ili9225.c
> @@ -346,7 +346,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
>  static struct drm_driver ili9225_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &ili9225_fops,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.name			= "ili9225",
>  	.desc			= "Ilitek ILI9225",
>  	.date			= "20171106",
> diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
> index bb819f45a5d3b..e1b9043ef7a0a 100644
> --- a/drivers/gpu/drm/tiny/ili9341.c
> +++ b/drivers/gpu/drm/tiny/ili9341.c
> @@ -152,7 +152,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
>  static struct drm_driver ili9341_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &ili9341_fops,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "ili9341",
>  	.desc			= "Ilitek ILI9341",
> diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
> index 2702ea557d297..90a17f40fdf0c 100644
> --- a/drivers/gpu/drm/tiny/ili9486.c
> +++ b/drivers/gpu/drm/tiny/ili9486.c
> @@ -165,7 +165,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9486_fops);
>  static struct drm_driver ili9486_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &ili9486_fops,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "ili9486",
>  	.desc			= "Ilitek ILI9486",
> diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
> index 08ac549ab0f7f..6624c2098fba2 100644
> --- a/drivers/gpu/drm/tiny/mi0283qt.c
> +++ b/drivers/gpu/drm/tiny/mi0283qt.c
> @@ -156,7 +156,7 @@ DEFINE_DRM_GEM_CMA_FOPS(mi0283qt_fops);
>  static struct drm_driver mi0283qt_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &mi0283qt_fops,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "mi0283qt",
>  	.desc			= "Multi-Inno MI0283QT",
> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> index 1c0e7169545b4..877dcece25828 100644
> --- a/drivers/gpu/drm/tiny/repaper.c
> +++ b/drivers/gpu/drm/tiny/repaper.c
> @@ -946,7 +946,7 @@ DEFINE_DRM_GEM_CMA_FOPS(repaper_fops);
>  static struct drm_driver repaper_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &repaper_fops,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.name			= "repaper",
>  	.desc			= "Pervasive Displays RePaper e-ink panels",
>  	.date			= "20170405",
> diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
> index 2a1fae422f7a2..ec84bdc51f60d 100644
> --- a/drivers/gpu/drm/tiny/st7586.c
> +++ b/drivers/gpu/drm/tiny/st7586.c
> @@ -285,7 +285,7 @@ DEFINE_DRM_GEM_CMA_FOPS(st7586_fops);
>  static struct drm_driver st7586_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &st7586_fops,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "st7586",
>  	.desc			= "Sitronix ST7586",
> diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
> index 0af1b15efdf8a..cfd4933f3b30c 100644
> --- a/drivers/gpu/drm/tiny/st7735r.c
> +++ b/drivers/gpu/drm/tiny/st7735r.c
> @@ -157,7 +157,7 @@ DEFINE_DRM_GEM_CMA_FOPS(st7735r_fops);
>  static struct drm_driver st7735r_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.fops			= &st7735r_fops,
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "st7735r",
>  	.desc			= "Sitronix ST7735R",
> diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
> index 947ac95eb24a9..917d42603db06 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -110,21 +110,37 @@ struct drm_gem_object *
>  drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size);
>  
>  /**
> - * DRM_GEM_CMA_VMAP_DRIVER_OPS - CMA GEM driver operations ensuring a virtual
> - *                               address on the buffer
> + * __DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations ensuring a
> + *                            virtual address on the buffer
> + * @__dumb_create: callback function for .dumb_create
>   *
>   * This macro provides a shortcut for setting the default GEM operations in the
>   * &drm_driver structure for drivers that need the virtual address also on
>   * imported buffers.
> + *
> + * This macro is a variant of DRM_GEM_CMA_DRIVER_OPS for drivers that
> + * override the default implementation of .dumb_create. Use
> + * DRM_GEM_CMA_DRIVER_OPS if possible.
>   */
> -#define DRM_GEM_CMA_VMAP_DRIVER_OPS \
> +#define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
>  	.gem_create_object	= drm_cma_gem_create_object_default_funcs, \
> -	.dumb_create		= drm_gem_cma_dumb_create, \
> +	.dumb_create		= (__dumb_create), \
>  	.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 = drm_gem_cma_prime_import_sg_table_vmap, \
>  	.gem_prime_mmap		= drm_gem_prime_mmap
>  
> +/**
> + * DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations ensuring a virtual
> + *                          address on the buffer
> + *
> + * This macro provides a shortcut for setting the default GEM operations in the
> + * &drm_driver structure for drivers that need the virtual address also on
> + * imported buffers.
> + */
> +#define DRM_GEM_CMA_DRIVER_OPS \
> +	__DRM_GEM_CMA_DRIVER_OPS(drm_gem_cma_dumb_create)
> +
>  struct drm_gem_object *
>  drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *drm,
>  				       struct dma_buf_attachment *attach,
> -- 
> 2.26.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/21] drm/cma-helper: Rework DRM_GEM_CMA_VMAP_DRIVER_OPS macro

[Emil Velikov]

On Fri, 22 May 2020 at 18:48, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Thomas.
>
> On Fri, May 22, 2020 at 03:52:26PM +0200, Thomas Zimmermann wrote:
> > Rename the macro to DRM_GEM_CMA_DRIVER_OPS to align with SHMEM
> > helpers.
> This part is fine, I like that the naming is somehow consistent.
>
> > An internal version is provided for drivers that override
> > the default .dumb_create callback. Adapt drivers to the changes.
> I loathe anything named __foo or __FOO. This __ signals to me
> that the author was clueless in naming - or some sort.
> I know that __ is used in some lib headers - but thats not the case
> here.
>
> But I love that we have a variant that takes a create function.
> So we do not have to escape from the nice macro.
> The macro is another way to tell me as rewiewer that this
> drivers uses all the default helpers for this.
>
Fwiw I share the sentiment, although I fear we're a little late. __
prefixed functions are widely common in core drm and it's helpers.

>
> So critizising the name I better suggest something that
> I personally like better:
>
> DRM_GEM_CMA_DRIVER_OPS_CREATE()
>
> It would look like this:
>         /* GEM Operations */
> -       DRM_GEM_CMA_VMAP_DRIVER_OPS,
> -       .dumb_create            = drm_sun4i_gem_dumb_create,
> +       DRM_GEM_CMA_DRIVER_OPS_CREATE(drm_sun4i_gem_dumb_create),
>
>
>
> Please fix zte/zx_drm_drv.c which also uses DRM_GEM_CMA_VMAP_DRIVER_OPS.
>
Isn't DRM_GEM_CMA_VMAP_DRIVER_OPS introduced to zte with the last
patch in the series?

-Emil

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/21] drm/cma-helper: Rework DRM_GEM_CMA_VMAP_DRIVER_OPS macro

[Sam Ravnborg]

Hi Emil.

> > Please fix zte/zx_drm_drv.c which also uses DRM_GEM_CMA_VMAP_DRIVER_OPS.
> >
> Isn't DRM_GEM_CMA_VMAP_DRIVER_OPS introduced to zte with the last
> patch in the series?

You are right, I just applied all patches and throw them after my
build script and zte failed.

	Sam

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/21] drm/cma-helper: Rework DRM_GEM_CMA_VMAP_DRIVER_OPS macro

[Laurent Pinchart]

Hello,

On Fri, May 22, 2020 at 07:48:35PM +0200, Sam Ravnborg wrote:
> On Fri, May 22, 2020 at 03:52:26PM +0200, Thomas Zimmermann wrote:
> > Rename the macro to DRM_GEM_CMA_DRIVER_OPS to align with SHMEM
> > helpers.
>
> This part is fine, I like that the naming is somehow consistent.
> 
> > An internal version is provided for drivers that override
> > the default .dumb_create callback. Adapt drivers to the changes.
> I loathe anything named __foo or __FOO. This __ signals to me
> that the author was clueless in naming - or some sort.
> I know that __ is used in some lib headers - but thats not the case
> here.
> 
> But I love that we have a variant that takes a create function.
> So we do not have to escape from the nice macro.
> The macro is another way to tell me as rewiewer that this
> drivers uses all the default helpers for this.
> 
> 
> So critizising the name I better suggest something that
> I personally like better:
> 
> DRM_GEM_CMA_DRIVER_OPS_CREATE()
> 
> It would look like this:
> 	/* GEM Operations */
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> -	.dumb_create            = drm_sun4i_gem_dumb_create,
> +	DRM_GEM_CMA_DRIVER_OPS_CREATE(drm_sun4i_gem_dumb_create),

I agree with Sam here. With this change,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Please fix zte/zx_drm_drv.c which also uses DRM_GEM_CMA_VMAP_DRIVER_OPS.
> 
> The naming is a bikeshedding topic that we may not agree on, soo..
> 
> With zte fixed the patch is:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > ---
> >  drivers/gpu/drm/drm_gem_cma_helper.c |  2 +-
> >  drivers/gpu/drm/sun4i/sun4i_drv.c    |  3 +--
> >  drivers/gpu/drm/tidss/tidss_drv.c    |  2 +-
> >  drivers/gpu/drm/tiny/hx8357d.c       |  2 +-
> >  drivers/gpu/drm/tiny/ili9225.c       |  2 +-
> >  drivers/gpu/drm/tiny/ili9341.c       |  2 +-
> >  drivers/gpu/drm/tiny/ili9486.c       |  2 +-
> >  drivers/gpu/drm/tiny/mi0283qt.c      |  2 +-
> >  drivers/gpu/drm/tiny/repaper.c       |  2 +-
> >  drivers/gpu/drm/tiny/st7586.c        |  2 +-
> >  drivers/gpu/drm/tiny/st7735r.c       |  2 +-
> >  include/drm/drm_gem_cma_helper.h     | 24 ++++++++++++++++++++----
> >  12 files changed, 31 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> > index 12e98fb28229d..6fa4d2f2e3987 100644
> > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > @@ -620,7 +620,7 @@ EXPORT_SYMBOL(drm_cma_gem_create_object_default_funcs);
> >   * address set. This address is released when the object is freed.
> >   *
> >   * This function can be used as the &drm_driver.gem_prime_import_sg_table
> > - * callback. The DRM_GEM_CMA_VMAP_DRIVER_OPS() macro provides a shortcut to set
> > + * callback. The &DRM_GEM_CMA_DRIVER_OPS macro provides a shortcut to set
> >   * the necessary DRM driver operations.
> >   *
> >   * Returns:
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > index 328272ff77d84..012855fd89c24 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > @@ -52,8 +52,7 @@ static struct drm_driver sun4i_drv_driver = {
> >  	.minor			= 0,
> >  
> >  	/* GEM Operations */
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > -	.dumb_create		= drm_sun4i_gem_dumb_create,
> > +	__DRM_GEM_CMA_DRIVER_OPS(drm_sun4i_gem_dumb_create),
> >  };
> >  
> >  static int sun4i_drv_bind(struct device *dev)
> > diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
> > index 99edc66ebdef2..1753cdc74ebda 100644
> > --- a/drivers/gpu/drm/tidss/tidss_drv.c
> > +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> > @@ -112,7 +112,7 @@ static struct drm_driver tidss_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &tidss_fops,
> >  	.release		= tidss_release,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.name			= "tidss",
> >  	.desc			= "TI Keystone DSS",
> >  	.date			= "20180215",
> > diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
> > index b4bc358a3269a..592da71d7ca70 100644
> > --- a/drivers/gpu/drm/tiny/hx8357d.c
> > +++ b/drivers/gpu/drm/tiny/hx8357d.c
> > @@ -196,7 +196,7 @@ DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
> >  static struct drm_driver hx8357d_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &hx8357d_fops,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.debugfs_init		= mipi_dbi_debugfs_init,
> >  	.name			= "hx8357d",
> >  	.desc			= "HX8357D",
> > diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
> > index d1a5ab6747d5c..368ff6c8a1efb 100644
> > --- a/drivers/gpu/drm/tiny/ili9225.c
> > +++ b/drivers/gpu/drm/tiny/ili9225.c
> > @@ -346,7 +346,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
> >  static struct drm_driver ili9225_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &ili9225_fops,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.name			= "ili9225",
> >  	.desc			= "Ilitek ILI9225",
> >  	.date			= "20171106",
> > diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
> > index bb819f45a5d3b..e1b9043ef7a0a 100644
> > --- a/drivers/gpu/drm/tiny/ili9341.c
> > +++ b/drivers/gpu/drm/tiny/ili9341.c
> > @@ -152,7 +152,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
> >  static struct drm_driver ili9341_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &ili9341_fops,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.debugfs_init		= mipi_dbi_debugfs_init,
> >  	.name			= "ili9341",
> >  	.desc			= "Ilitek ILI9341",
> > diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
> > index 2702ea557d297..90a17f40fdf0c 100644
> > --- a/drivers/gpu/drm/tiny/ili9486.c
> > +++ b/drivers/gpu/drm/tiny/ili9486.c
> > @@ -165,7 +165,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9486_fops);
> >  static struct drm_driver ili9486_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &ili9486_fops,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.debugfs_init		= mipi_dbi_debugfs_init,
> >  	.name			= "ili9486",
> >  	.desc			= "Ilitek ILI9486",
> > diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
> > index 08ac549ab0f7f..6624c2098fba2 100644
> > --- a/drivers/gpu/drm/tiny/mi0283qt.c
> > +++ b/drivers/gpu/drm/tiny/mi0283qt.c
> > @@ -156,7 +156,7 @@ DEFINE_DRM_GEM_CMA_FOPS(mi0283qt_fops);
> >  static struct drm_driver mi0283qt_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &mi0283qt_fops,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.debugfs_init		= mipi_dbi_debugfs_init,
> >  	.name			= "mi0283qt",
> >  	.desc			= "Multi-Inno MI0283QT",
> > diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> > index 1c0e7169545b4..877dcece25828 100644
> > --- a/drivers/gpu/drm/tiny/repaper.c
> > +++ b/drivers/gpu/drm/tiny/repaper.c
> > @@ -946,7 +946,7 @@ DEFINE_DRM_GEM_CMA_FOPS(repaper_fops);
> >  static struct drm_driver repaper_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &repaper_fops,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.name			= "repaper",
> >  	.desc			= "Pervasive Displays RePaper e-ink panels",
> >  	.date			= "20170405",
> > diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
> > index 2a1fae422f7a2..ec84bdc51f60d 100644
> > --- a/drivers/gpu/drm/tiny/st7586.c
> > +++ b/drivers/gpu/drm/tiny/st7586.c
> > @@ -285,7 +285,7 @@ DEFINE_DRM_GEM_CMA_FOPS(st7586_fops);
> >  static struct drm_driver st7586_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &st7586_fops,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.debugfs_init		= mipi_dbi_debugfs_init,
> >  	.name			= "st7586",
> >  	.desc			= "Sitronix ST7586",
> > diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
> > index 0af1b15efdf8a..cfd4933f3b30c 100644
> > --- a/drivers/gpu/drm/tiny/st7735r.c
> > +++ b/drivers/gpu/drm/tiny/st7735r.c
> > @@ -157,7 +157,7 @@ DEFINE_DRM_GEM_CMA_FOPS(st7735r_fops);
> >  static struct drm_driver st7735r_driver = {
> >  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >  	.fops			= &st7735r_fops,
> > -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> > +	DRM_GEM_CMA_DRIVER_OPS,
> >  	.debugfs_init		= mipi_dbi_debugfs_init,
> >  	.name			= "st7735r",
> >  	.desc			= "Sitronix ST7735R",
> > diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
> > index 947ac95eb24a9..917d42603db06 100644
> > --- a/include/drm/drm_gem_cma_helper.h
> > +++ b/include/drm/drm_gem_cma_helper.h
> > @@ -110,21 +110,37 @@ struct drm_gem_object *
> >  drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size);
> >  
> >  /**
> > - * DRM_GEM_CMA_VMAP_DRIVER_OPS - CMA GEM driver operations ensuring a virtual
> > - *                               address on the buffer
> > + * __DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations ensuring a
> > + *                            virtual address on the buffer
> > + * @__dumb_create: callback function for .dumb_create
> >   *
> >   * This macro provides a shortcut for setting the default GEM operations in the
> >   * &drm_driver structure for drivers that need the virtual address also on
> >   * imported buffers.
> > + *
> > + * This macro is a variant of DRM_GEM_CMA_DRIVER_OPS for drivers that
> > + * override the default implementation of .dumb_create. Use
> > + * DRM_GEM_CMA_DRIVER_OPS if possible.
> >   */
> > -#define DRM_GEM_CMA_VMAP_DRIVER_OPS \
> > +#define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
> >  	.gem_create_object	= drm_cma_gem_create_object_default_funcs, \
> > -	.dumb_create		= drm_gem_cma_dumb_create, \
> > +	.dumb_create		= (__dumb_create), \
> >  	.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 = drm_gem_cma_prime_import_sg_table_vmap, \
> >  	.gem_prime_mmap		= drm_gem_prime_mmap
> >  
> > +/**
> > + * DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations ensuring a virtual
> > + *                          address on the buffer
> > + *
> > + * This macro provides a shortcut for setting the default GEM operations in the
> > + * &drm_driver structure for drivers that need the virtual address also on
> > + * imported buffers.
> > + */
> > +#define DRM_GEM_CMA_DRIVER_OPS \
> > +	__DRM_GEM_CMA_DRIVER_OPS(drm_gem_cma_dumb_create)
> > +
> >  struct drm_gem_object *
> >  drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *drm,
> >  				       struct dma_buf_attachment *attach,

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/21] drm/cma-helper: Rework DRM_GEM_CMA_VMAP_DRIVER_OPS macro

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 12225 bytes --]

Hi Sam

Am 22.05.20 um 19:48 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Fri, May 22, 2020 at 03:52:26PM +0200, Thomas Zimmermann wrote:
>> Rename the macro to DRM_GEM_CMA_DRIVER_OPS to align with SHMEM
>> helpers.
> This part is fine, I like that the naming is somehow consistent.
> 
>> An internal version is provided for drivers that override
>> the default .dumb_create callback. Adapt drivers to the changes.
> I loathe anything named __foo or __FOO. This __ signals to me
> that the author was clueless in naming - or some sort.
> I know that __ is used in some lib headers - but thats not the case
> here.

I agree with your comment and I've been trying to find a better name
before posting the patchset. I considered something like
DRM_GEM_CMA_DRIVER_OPS_INTERNAL(), but wasn't happy with that either. In
the end, I uses the double underscore to push driver authors towards the
other macro. This one's only for the special case of settings a separate
implementation for .dumb_create().

> 
> But I love that we have a variant that takes a create function.
> So we do not have to escape from the nice macro.
> The macro is another way to tell me as rewiewer that this
> drivers uses all the default helpers for this.
> 
> 
> So critizising the name I better suggest something that
> I personally like better:
> 
> DRM_GEM_CMA_DRIVER_OPS_CREATE()

Can we at least use DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE ? Because
there's also gem_object_create.

I'll update the patches accordingly.

I noticed that most of the affected drivers do some kind of alignment
calculation in their dumb_create code. IMHO in the long run, we should
move such calculations into the default implementation and put the
control paramters into struct drm_mode_config.

Best regards
Thomas

> 
> It would look like this:
> 	/* GEM Operations */
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> -	.dumb_create            = drm_sun4i_gem_dumb_create,
> +	DRM_GEM_CMA_DRIVER_OPS_CREATE(drm_sun4i_gem_dumb_create),
> 
> 
> 
> Please fix zte/zx_drm_drv.c which also uses DRM_GEM_CMA_VMAP_DRIVER_OPS.
> 
> 
> The naming is a bikeshedding topic that we may not agree on, soo..
> 
> With zte fixed the patch is:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> 	Sam
> 
> 
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/drm_gem_cma_helper.c |  2 +-
>>  drivers/gpu/drm/sun4i/sun4i_drv.c    |  3 +--
>>  drivers/gpu/drm/tidss/tidss_drv.c    |  2 +-
>>  drivers/gpu/drm/tiny/hx8357d.c       |  2 +-
>>  drivers/gpu/drm/tiny/ili9225.c       |  2 +-
>>  drivers/gpu/drm/tiny/ili9341.c       |  2 +-
>>  drivers/gpu/drm/tiny/ili9486.c       |  2 +-
>>  drivers/gpu/drm/tiny/mi0283qt.c      |  2 +-
>>  drivers/gpu/drm/tiny/repaper.c       |  2 +-
>>  drivers/gpu/drm/tiny/st7586.c        |  2 +-
>>  drivers/gpu/drm/tiny/st7735r.c       |  2 +-
>>  include/drm/drm_gem_cma_helper.h     | 24 ++++++++++++++++++++----
>>  12 files changed, 31 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
>> index 12e98fb28229d..6fa4d2f2e3987 100644
>> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
>> @@ -620,7 +620,7 @@ EXPORT_SYMBOL(drm_cma_gem_create_object_default_funcs);
>>   * address set. This address is released when the object is freed.
>>   *
>>   * This function can be used as the &drm_driver.gem_prime_import_sg_table
>> - * callback. The DRM_GEM_CMA_VMAP_DRIVER_OPS() macro provides a shortcut to set
>> + * callback. The &DRM_GEM_CMA_DRIVER_OPS macro provides a shortcut to set
>>   * the necessary DRM driver operations.
>>   *
>>   * Returns:
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
>> index 328272ff77d84..012855fd89c24 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
>> @@ -52,8 +52,7 @@ static struct drm_driver sun4i_drv_driver = {
>>  	.minor			= 0,
>>  
>>  	/* GEM Operations */
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> -	.dumb_create		= drm_sun4i_gem_dumb_create,
>> +	__DRM_GEM_CMA_DRIVER_OPS(drm_sun4i_gem_dumb_create),
>>  };
>>  
>>  static int sun4i_drv_bind(struct device *dev)
>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
>> index 99edc66ebdef2..1753cdc74ebda 100644
>> --- a/drivers/gpu/drm/tidss/tidss_drv.c
>> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
>> @@ -112,7 +112,7 @@ static struct drm_driver tidss_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &tidss_fops,
>>  	.release		= tidss_release,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.name			= "tidss",
>>  	.desc			= "TI Keystone DSS",
>>  	.date			= "20180215",
>> diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
>> index b4bc358a3269a..592da71d7ca70 100644
>> --- a/drivers/gpu/drm/tiny/hx8357d.c
>> +++ b/drivers/gpu/drm/tiny/hx8357d.c
>> @@ -196,7 +196,7 @@ DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
>>  static struct drm_driver hx8357d_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &hx8357d_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "hx8357d",
>>  	.desc			= "HX8357D",
>> diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
>> index d1a5ab6747d5c..368ff6c8a1efb 100644
>> --- a/drivers/gpu/drm/tiny/ili9225.c
>> +++ b/drivers/gpu/drm/tiny/ili9225.c
>> @@ -346,7 +346,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
>>  static struct drm_driver ili9225_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &ili9225_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.name			= "ili9225",
>>  	.desc			= "Ilitek ILI9225",
>>  	.date			= "20171106",
>> diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
>> index bb819f45a5d3b..e1b9043ef7a0a 100644
>> --- a/drivers/gpu/drm/tiny/ili9341.c
>> +++ b/drivers/gpu/drm/tiny/ili9341.c
>> @@ -152,7 +152,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
>>  static struct drm_driver ili9341_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &ili9341_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "ili9341",
>>  	.desc			= "Ilitek ILI9341",
>> diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
>> index 2702ea557d297..90a17f40fdf0c 100644
>> --- a/drivers/gpu/drm/tiny/ili9486.c
>> +++ b/drivers/gpu/drm/tiny/ili9486.c
>> @@ -165,7 +165,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9486_fops);
>>  static struct drm_driver ili9486_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &ili9486_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "ili9486",
>>  	.desc			= "Ilitek ILI9486",
>> diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
>> index 08ac549ab0f7f..6624c2098fba2 100644
>> --- a/drivers/gpu/drm/tiny/mi0283qt.c
>> +++ b/drivers/gpu/drm/tiny/mi0283qt.c
>> @@ -156,7 +156,7 @@ DEFINE_DRM_GEM_CMA_FOPS(mi0283qt_fops);
>>  static struct drm_driver mi0283qt_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &mi0283qt_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "mi0283qt",
>>  	.desc			= "Multi-Inno MI0283QT",
>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
>> index 1c0e7169545b4..877dcece25828 100644
>> --- a/drivers/gpu/drm/tiny/repaper.c
>> +++ b/drivers/gpu/drm/tiny/repaper.c
>> @@ -946,7 +946,7 @@ DEFINE_DRM_GEM_CMA_FOPS(repaper_fops);
>>  static struct drm_driver repaper_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &repaper_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.name			= "repaper",
>>  	.desc			= "Pervasive Displays RePaper e-ink panels",
>>  	.date			= "20170405",
>> diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
>> index 2a1fae422f7a2..ec84bdc51f60d 100644
>> --- a/drivers/gpu/drm/tiny/st7586.c
>> +++ b/drivers/gpu/drm/tiny/st7586.c
>> @@ -285,7 +285,7 @@ DEFINE_DRM_GEM_CMA_FOPS(st7586_fops);
>>  static struct drm_driver st7586_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &st7586_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "st7586",
>>  	.desc			= "Sitronix ST7586",
>> diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
>> index 0af1b15efdf8a..cfd4933f3b30c 100644
>> --- a/drivers/gpu/drm/tiny/st7735r.c
>> +++ b/drivers/gpu/drm/tiny/st7735r.c
>> @@ -157,7 +157,7 @@ DEFINE_DRM_GEM_CMA_FOPS(st7735r_fops);
>>  static struct drm_driver st7735r_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &st7735r_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "st7735r",
>>  	.desc			= "Sitronix ST7735R",
>> diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
>> index 947ac95eb24a9..917d42603db06 100644
>> --- a/include/drm/drm_gem_cma_helper.h
>> +++ b/include/drm/drm_gem_cma_helper.h
>> @@ -110,21 +110,37 @@ struct drm_gem_object *
>>  drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size);
>>  
>>  /**
>> - * DRM_GEM_CMA_VMAP_DRIVER_OPS - CMA GEM driver operations ensuring a virtual
>> - *                               address on the buffer
>> + * __DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations ensuring a
>> + *                            virtual address on the buffer
>> + * @__dumb_create: callback function for .dumb_create
>>   *
>>   * This macro provides a shortcut for setting the default GEM operations in the
>>   * &drm_driver structure for drivers that need the virtual address also on
>>   * imported buffers.
>> + *
>> + * This macro is a variant of DRM_GEM_CMA_DRIVER_OPS for drivers that
>> + * override the default implementation of .dumb_create. Use
>> + * DRM_GEM_CMA_DRIVER_OPS if possible.
>>   */
>> -#define DRM_GEM_CMA_VMAP_DRIVER_OPS \
>> +#define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
>>  	.gem_create_object	= drm_cma_gem_create_object_default_funcs, \
>> -	.dumb_create		= drm_gem_cma_dumb_create, \
>> +	.dumb_create		= (__dumb_create), \
>>  	.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 = drm_gem_cma_prime_import_sg_table_vmap, \
>>  	.gem_prime_mmap		= drm_gem_prime_mmap
>>  
>> +/**
>> + * DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations ensuring a virtual
>> + *                          address on the buffer
>> + *
>> + * This macro provides a shortcut for setting the default GEM operations in the
>> + * &drm_driver structure for drivers that need the virtual address also on
>> + * imported buffers.
>> + */
>> +#define DRM_GEM_CMA_DRIVER_OPS \
>> +	__DRM_GEM_CMA_DRIVER_OPS(drm_gem_cma_dumb_create)
>> +
>>  struct drm_gem_object *
>>  drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *drm,
>>  				       struct dma_buf_attachment *attach,
>> -- 
>> 2.26.2
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> 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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 05/21] drm/atmel-hlcdc: Use GEM CMA object functions

[Sam Ravnborg]

Hi Thomas.

On Fri, May 22, 2020 at 03:52:30PM +0200, Thomas Zimmermann wrote:
> The atmel-hlcdc driver uses the default implementation for CMA functions. The
> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
> All remaining operations are provided by CMA GEM object functions.

A nice side-effect of introducing the defualt implementation
of CMA functions is that this driver is now migrated over to use
drm_gem_object_funcs, which is why we can replace all these
helpers with a simple macro that defined only 6 helpers.

With an improved changelog:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

And as I said in the other mail, this is really nice.
It is now much more obvious that this drivers uses
all the default helpers for CMA.

And I can drop one item from my TODO list on top of that.

	Sam

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 112aa5066ceed..871293d1aeeba 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -821,16 +821,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
> -	.dumb_create = drm_gem_cma_dumb_create,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.fops = &fops,
>  	.name = "atmel-hlcdc",
>  	.desc = "Atmel HLCD Controller DRM",
> -- 
> 2.26.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 05/21] drm/atmel-hlcdc: Use GEM CMA object functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3182 bytes --]

Hi

Am 22.05.20 um 20:08 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Fri, May 22, 2020 at 03:52:30PM +0200, Thomas Zimmermann wrote:
>> The atmel-hlcdc driver uses the default implementation for CMA functions. The
>> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
>> All remaining operations are provided by CMA GEM object functions.
> 
> A nice side-effect of introducing the defualt implementation
> of CMA functions is that this driver is now migrated over to use
> drm_gem_object_funcs, which is why we can replace all these
> helpers with a simple macro that defined only 6 helpers.
> 
> With an improved changelog:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> And as I said in the other mail, this is really nice.
> It is now much more obvious that this drivers uses
> all the default helpers for CMA.
> 
> And I can drop one item from my TODO list on top of that.

There's still more to do. The current macro still sets
.gem_object_create to a CMA default. But that pointer is the interface
where drivers can override some of the CMA object defaults, so it should
not be set by CMA helpers.

After the other CMA drivers have been converted to GEM object functions,
this setting can be kept to zero and __drm_gem_cma_create() can set the
default object functions. SHMEM and VRAM helpers already work this way.

Best regards
Thomas

> 
> 	Sam
> 
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 11 +----------
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index 112aa5066ceed..871293d1aeeba 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -821,16 +821,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>>  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>>  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>>  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
>> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
>> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
>> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
>> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
>> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
>> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
>> -	.dumb_create = drm_gem_cma_dumb_create,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.fops = &fops,
>>  	.name = "atmel-hlcdc",
>>  	.desc = "Atmel HLCD Controller DRM",
>> -- 
>> 2.26.2
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> 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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 05/21] drm/atmel-hlcdc: Use GEM CMA object functions

[Sam Ravnborg]

Hi Thomas.

On Fri, May 22, 2020 at 03:52:30PM +0200, Thomas Zimmermann wrote:
> The atmel-hlcdc driver uses the default implementation for CMA functions. The
> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
> All remaining operations are provided by CMA GEM object functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 112aa5066ceed..871293d1aeeba 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -821,16 +821,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,

> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
When using DRM_GEM_CMA_DRIVER_OPS gem_prime_mmap is set to
drm_gem_prime_mmap.
Why is this the same as drm_gem_cma_prime_mmap?

Maybe this is all obvious when you know all the CMA stuff,
but this puzzeled me.

	Sam


> -	.dumb_create = drm_gem_cma_dumb_create,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  	.fops = &fops,
>  	.name = "atmel-hlcdc",
>  	.desc = "Atmel HLCD Controller DRM",
> -- 
> 2.26.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 05/21] drm/atmel-hlcdc: Use GEM CMA object functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2513 bytes --]

Hi

Am 22.05.20 um 21:25 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Fri, May 22, 2020 at 03:52:30PM +0200, Thomas Zimmermann wrote:
>> The atmel-hlcdc driver uses the default implementation for CMA functions. The
>> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
>> All remaining operations are provided by CMA GEM object functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 11 +----------
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index 112aa5066ceed..871293d1aeeba 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -821,16 +821,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>>  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>>  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>>  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
>> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
>> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
>> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
>> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
>> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> 
>> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
> When using DRM_GEM_CMA_DRIVER_OPS gem_prime_mmap is set to
> drm_gem_prime_mmap.
> Why is this the same as drm_gem_cma_prime_mmap?
> 
> Maybe this is all obvious when you know all the CMA stuff,
> but this puzzeled me.

Following through the calls is far from easy.

I took the macro from the aspeed driver. I had some doubts about the
mmap code, but expected the driver to be working correctly. Maybe we
should set that field to drm_gem_cma_prime_mmap or implement the mmap
object function.

Best regards
Thomas



> 
> 	Sam
> 
> 
>> -	.dumb_create = drm_gem_cma_dumb_create,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.fops = &fops,
>>  	.name = "atmel-hlcdc",
>>  	.desc = "Atmel HLCD Controller DRM",
>> -- 
>> 2.26.2

-- 
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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 21/21] drm/zte: Use GEM CMA object functions

[Sam Ravnborg]

Hi Thomas.

On Fri, May 22, 2020 at 03:52:46PM +0200, Thomas Zimmermann wrote:
> The zte driver uses the default implementation for CMA functions. The
> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
> All remaining operations are provided by CMA GEM object functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/zte/zx_drm_drv.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
> index 1141c1ed1ed04..42c59eae0ea03 100644
> --- a/drivers/gpu/drm/zte/zx_drm_drv.c
> +++ b/drivers/gpu/drm/zte/zx_drm_drv.c
> @@ -36,16 +36,7 @@ DEFINE_DRM_GEM_CMA_FOPS(zx_drm_fops);
>  
>  static struct drm_driver zx_drm_driver = {
>  	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
> -	.dumb_create = drm_gem_cma_dumb_create,
> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
> +	DRM_GEM_CMA_VMAP_DRIVER_OPS,
s/_VMAP// as pointed out by Emil.

	Sam

>  	.fops = &zx_drm_fops,
>  	.name = "zx-vou",
>  	.desc = "ZTE VOU Controller DRM",
> -- 
> 2.26.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions

[Laurent Pinchart]

Hi Thomas,

Thank you for the patch.

On Fri, May 22, 2020 at 03:52:40PM +0200, Thomas Zimmermann wrote:
> The rcar-du driver uses the default implementation for CMA functions;
> except for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro
> now sets these defaults and .dumb_create in struct drm_driver. All
> remaining operations are provided by CMA GEM object functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 3e67cf70f0402..3728038cec1d1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -476,16 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
>  
>  static struct drm_driver rcar_du_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> -	.gem_vm_ops		= &drm_gem_cma_vm_ops,
> -	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
> -	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
> -	.dumb_create		= rcar_du_dumb_create,
> +	__DRM_GEM_CMA_DRIVER_OPS(rcar_du_dumb_create),

Your __DRM_GEM_CMA_DRIVER_OPS is defined as

#define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
        .gem_create_object      = drm_cma_gem_create_object_default_funcs, \
        .dumb_create            = (__dumb_create), \
        .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 = drm_gem_cma_prime_import_sg_table_vmap, \
        .gem_prime_mmap         = drm_gem_prime_mmap

The patch thus introduces several changes:

- drm_gem_cma_prime_import_sg_table_vmap() is used instead of
  drm_gem_cma_prime_import_sg_table() combined with .gem_prime_vmap()
  and .gem_prime_vunmap(). I believe that's fine, but splitting that
  change in a separate commit, or at the very least explaining it in
  details in the commit message, would make review easier.

- .gem_create_object() is now set. That seems to be OK, but I'm not sure
  to grasp all the implications. This should also be explained in the
  commit message, and ideally split to a separate patch.

- drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same
  comments :-)

This patch hides way too many changes in what is documented as just
innocent refactoring. It seems other drivers are affected too.

>  	.fops			= &rcar_du_fops,
>  	.name			= "rcar-du",
>  	.desc			= "Renesas R-Car Display Unit",

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3887 bytes --]

Hi

Am 22.05.20 um 22:12 schrieb Laurent Pinchart:
> Hi Thomas,
> 
> Thank you for the patch.
> 
> On Fri, May 22, 2020 at 03:52:40PM +0200, Thomas Zimmermann wrote:
>> The rcar-du driver uses the default implementation for CMA functions;
>> except for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro
>> now sets these defaults and .dumb_create in struct drm_driver. All
>> remaining operations are provided by CMA GEM object functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 11 +----------
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> index 3e67cf70f0402..3728038cec1d1 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> @@ -476,16 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
>>  
>>  static struct drm_driver rcar_du_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
>> -	.gem_vm_ops		= &drm_gem_cma_vm_ops,
>> -	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
>> -	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
>> -	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
>> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>> -	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
>> -	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
>> -	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
>> -	.dumb_create		= rcar_du_dumb_create,
>> +	__DRM_GEM_CMA_DRIVER_OPS(rcar_du_dumb_create),
> 
> Your __DRM_GEM_CMA_DRIVER_OPS is defined as
> 
> #define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
>         .gem_create_object      = drm_cma_gem_create_object_default_funcs, \
>         .dumb_create            = (__dumb_create), \
>         .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 = drm_gem_cma_prime_import_sg_table_vmap, \
>         .gem_prime_mmap         = drm_gem_prime_mmap
> 
> The patch thus introduces several changes:
> 
> - drm_gem_cma_prime_import_sg_table_vmap() is used instead of
>   drm_gem_cma_prime_import_sg_table() combined with .gem_prime_vmap()
>   and .gem_prime_vunmap(). I believe that's fine, but splitting that
>   change in a separate commit, or at the very least explaining it in
>   details in the commit message, would make review easier.
> 
> - .gem_create_object() is now set. That seems to be OK, but I'm not sure
>   to grasp all the implications. This should also be explained in the
>   commit message, and ideally split to a separate patch.

That's relevant during object creation and sets the object functions.
See one of my other replies for how this can go away after all CMA
drivers have been updated to GEM object functions.


> 
> - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same
>   comments :-)

I relied on the aspeed driver to be correct. After Sam's comment on
that, I read the code once again several times. The original
implementation clears VM_PFNMAP. And I cannot find that code any longer.
Going back to the original function might be better.


> 
> This patch hides way too many changes in what is documented as just
> innocent refactoring. It seems other drivers are affected too.

Could you test the patchset? I don't have the HW.

Best regards
Thomas

> 
>>  	.fops			= &rcar_du_fops,
>>  	.name			= "rcar-du",
>>  	.desc			= "Renesas R-Car Display Unit",
> 

-- 
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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions

[Kieran Bingham]

On 25/05/2020 13:49, Thomas Zimmermann wrote:
> Hi
> 
> Am 22.05.20 um 22:12 schrieb Laurent Pinchart:
>> Hi Thomas,
>>
>> Thank you for the patch.
>>
>> On Fri, May 22, 2020 at 03:52:40PM +0200, Thomas Zimmermann wrote:
>>> The rcar-du driver uses the default implementation for CMA functions;
>>> except for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro
>>> now sets these defaults and .dumb_create in struct drm_driver. All
>>> remaining operations are provided by CMA GEM object functions.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 11 +----------
>>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>> index 3e67cf70f0402..3728038cec1d1 100644
>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>> @@ -476,16 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
>>>  
>>>  static struct drm_driver rcar_du_driver = {
>>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
>>> -	.gem_vm_ops		= &drm_gem_cma_vm_ops,
>>> -	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
>>> -	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
>>> -	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
>>> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>>> -	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
>>> -	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
>>> -	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
>>> -	.dumb_create		= rcar_du_dumb_create,
>>> +	__DRM_GEM_CMA_DRIVER_OPS(rcar_du_dumb_create),
>>
>> Your __DRM_GEM_CMA_DRIVER_OPS is defined as
>>
>> #define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
>>         .gem_create_object      = drm_cma_gem_create_object_default_funcs, \
>>         .dumb_create            = (__dumb_create), \
>>         .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 = drm_gem_cma_prime_import_sg_table_vmap, \
>>         .gem_prime_mmap         = drm_gem_prime_mmap
>>
>> The patch thus introduces several changes:
>>
>> - drm_gem_cma_prime_import_sg_table_vmap() is used instead of
>>   drm_gem_cma_prime_import_sg_table() combined with .gem_prime_vmap()
>>   and .gem_prime_vunmap(). I believe that's fine, but splitting that
>>   change in a separate commit, or at the very least explaining it in
>>   details in the commit message, would make review easier.
>>
>> - .gem_create_object() is now set. That seems to be OK, but I'm not sure
>>   to grasp all the implications. This should also be explained in the
>>   commit message, and ideally split to a separate patch.
> 
> That's relevant during object creation and sets the object functions.
> See one of my other replies for how this can go away after all CMA
> drivers have been updated to GEM object functions.
> 
> 
>>
>> - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same
>>   comments :-)
> 
> I relied on the aspeed driver to be correct. After Sam's comment on
> that, I read the code once again several times. The original
> implementation clears VM_PFNMAP. And I cannot find that code any longer.
> Going back to the original function might be better.
> 
> 
>>
>> This patch hides way too many changes in what is documented as just
>> innocent refactoring. It seems other drivers are affected too.
> 
> Could you test the patchset? I don't have the HW.

Digging out the branch you provided elsewhere in this thread:

>>> Could you boot-test with the patchset applied?
>>
>> Yes, if you have a git branch I can just build and boot I can
>> do it quickly!
>
> Fantastic! It's the cma-objfuncs branch of
>
> https://gitlab.freedesktop.org/tzimmermann/linux.git

I have successfully run our display tests with your patches here on an
R-Car H3 Salvator-XS(-es2).

Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>



> Best regards
> Thomas
> 
>>
>>>  	.fops			= &rcar_du_fops,
>>>  	.name			= "rcar-du",
>>>  	.desc			= "Renesas R-Car Display Unit",
>>
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 4722 bytes --]

Hi

Am 25.05.20 um 17:38 schrieb Kieran Bingham:
> On 25/05/2020 13:49, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 22.05.20 um 22:12 schrieb Laurent Pinchart:
>>> Hi Thomas,
>>>
>>> Thank you for the patch.
>>>
>>> On Fri, May 22, 2020 at 03:52:40PM +0200, Thomas Zimmermann wrote:
>>>> The rcar-du driver uses the default implementation for CMA functions;
>>>> except for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro
>>>> now sets these defaults and .dumb_create in struct drm_driver. All
>>>> remaining operations are provided by CMA GEM object functions.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> ---
>>>>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 11 +----------
>>>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>>> index 3e67cf70f0402..3728038cec1d1 100644
>>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>>>> @@ -476,16 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
>>>>  
>>>>  static struct drm_driver rcar_du_driver = {
>>>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>>> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
>>>> -	.gem_vm_ops		= &drm_gem_cma_vm_ops,
>>>> -	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
>>>> -	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
>>>> -	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
>>>> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>>>> -	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
>>>> -	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
>>>> -	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
>>>> -	.dumb_create		= rcar_du_dumb_create,
>>>> +	__DRM_GEM_CMA_DRIVER_OPS(rcar_du_dumb_create),
>>>
>>> Your __DRM_GEM_CMA_DRIVER_OPS is defined as
>>>
>>> #define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
>>>         .gem_create_object      = drm_cma_gem_create_object_default_funcs, \
>>>         .dumb_create            = (__dumb_create), \
>>>         .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 = drm_gem_cma_prime_import_sg_table_vmap, \
>>>         .gem_prime_mmap         = drm_gem_prime_mmap
>>>
>>> The patch thus introduces several changes:
>>>
>>> - drm_gem_cma_prime_import_sg_table_vmap() is used instead of
>>>   drm_gem_cma_prime_import_sg_table() combined with .gem_prime_vmap()
>>>   and .gem_prime_vunmap(). I believe that's fine, but splitting that
>>>   change in a separate commit, or at the very least explaining it in
>>>   details in the commit message, would make review easier.
>>>
>>> - .gem_create_object() is now set. That seems to be OK, but I'm not sure
>>>   to grasp all the implications. This should also be explained in the
>>>   commit message, and ideally split to a separate patch.
>>
>> That's relevant during object creation and sets the object functions.
>> See one of my other replies for how this can go away after all CMA
>> drivers have been updated to GEM object functions.
>>
>>
>>>
>>> - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same
>>>   comments :-)
>>
>> I relied on the aspeed driver to be correct. After Sam's comment on
>> that, I read the code once again several times. The original
>> implementation clears VM_PFNMAP. And I cannot find that code any longer.
>> Going back to the original function might be better.
>>
>>
>>>
>>> This patch hides way too many changes in what is documented as just
>>> innocent refactoring. It seems other drivers are affected too.
>>
>> Could you test the patchset? I don't have the HW.
> 
> Digging out the branch you provided elsewhere in this thread:
> 
>>>> Could you boot-test with the patchset applied?
>>>
>>> Yes, if you have a git branch I can just build and boot I can
>>> do it quickly!
>>
>> Fantastic! It's the cma-objfuncs branch of
>>
>> https://gitlab.freedesktop.org/tzimmermann/linux.git
> 
> I have successfully run our display tests with your patches here on an
> R-Car H3 Salvator-XS(-es2).
> 
> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Thanks for testing.

> 
> 
> 
>> Best regards
>> Thomas
>>
>>>
>>>>  	.fops			= &rcar_du_fops,
>>>>  	.name			= "rcar-du",
>>>>  	.desc			= "Renesas R-Car Display Unit",
>>>
>>
> 

-- 
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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions

[Laurent Pinchart]

Hi Thomas,

On Mon, May 25, 2020 at 02:49:46PM +0200, Thomas Zimmermann wrote:
> Am 22.05.20 um 22:12 schrieb Laurent Pinchart:
> > On Fri, May 22, 2020 at 03:52:40PM +0200, Thomas Zimmermann wrote:
> >> The rcar-du driver uses the default implementation for CMA functions;
> >> except for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro
> >> now sets these defaults and .dumb_create in struct drm_driver. All
> >> remaining operations are provided by CMA GEM object functions.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >> ---
> >>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 11 +----------
> >>  1 file changed, 1 insertion(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> >> index 3e67cf70f0402..3728038cec1d1 100644
> >> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> >> @@ -476,16 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
> >>  
> >>  static struct drm_driver rcar_du_driver = {
> >>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> >> -	.gem_vm_ops		= &drm_gem_cma_vm_ops,
> >> -	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
> >> -	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
> >> -	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
> >> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> >> -	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
> >> -	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
> >> -	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
> >> -	.dumb_create		= rcar_du_dumb_create,
> >> +	__DRM_GEM_CMA_DRIVER_OPS(rcar_du_dumb_create),
> > 
> > Your __DRM_GEM_CMA_DRIVER_OPS is defined as
> > 
> > #define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
> >         .gem_create_object      = drm_cma_gem_create_object_default_funcs, \
> >         .dumb_create            = (__dumb_create), \
> >         .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 = drm_gem_cma_prime_import_sg_table_vmap, \
> >         .gem_prime_mmap         = drm_gem_prime_mmap
> > 
> > The patch thus introduces several changes:
> > 
> > - drm_gem_cma_prime_import_sg_table_vmap() is used instead of
> >   drm_gem_cma_prime_import_sg_table() combined with .gem_prime_vmap()
> >   and .gem_prime_vunmap(). I believe that's fine, but splitting that
> >   change in a separate commit, or at the very least explaining it in
> >   details in the commit message, would make review easier.
> > 
> > - .gem_create_object() is now set. That seems to be OK, but I'm not sure
> >   to grasp all the implications. This should also be explained in the
> >   commit message, and ideally split to a separate patch.
> 
> That's relevant during object creation and sets the object functions.
> See one of my other replies for how this can go away after all CMA
> drivers have been updated to GEM object functions.

I don't dispute that's fine, but I think it should really be explained
in the commit message, and ideally split to a separate patch.

> > - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same
> >   comments :-)
> 
> I relied on the aspeed driver to be correct. After Sam's comment on
> that, I read the code once again several times. The original
> implementation clears VM_PFNMAP. And I cannot find that code any longer.
> Going back to the original function might be better.
> 
> > This patch hides way too many changes in what is documented as just
> > innocent refactoring. It seems other drivers are affected too.
> 
> Could you test the patchset? I don't have the HW.

Kieran has beaten me to it :-)

> >>  	.fops			= &rcar_du_fops,
> >>  	.name			= "rcar-du",
> >>  	.desc			= "Renesas R-Car Display Unit",

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/21] drm/mcde: Use GEM CMA object functions

[Linus Walleij]

On Fri, May 22, 2020 at 3:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:

> The mcde driver uses the default implementation for CMA functions. The
> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
> All remaining operations are provided by CMA GEM object functions.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/21] drm/mcde: Use GEM CMA object functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 797 bytes --]

Hi

Am 25.05.20 um 13:36 schrieb Linus Walleij:
> On Fri, May 22, 2020 at 3:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
>> The mcde driver uses the default implementation for CMA functions. The
>> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
>> All remaining operations are provided by CMA GEM object functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Could you boot-test with the patchset applied?

Best regards
Thomas

> 
> Yours,
> Linus Walleij
> 

-- 
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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/21] drm/mcde: Use GEM CMA object functions

[Linus Walleij]

On Mon, May 25, 2020 at 2:51 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Am 25.05.20 um 13:36 schrieb Linus Walleij:
> > On Fri, May 22, 2020 at 3:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> >> The mcde driver uses the default implementation for CMA functions. The
> >> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
> >> All remaining operations are provided by CMA GEM object functions.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Could you boot-test with the patchset applied?

Yes, if you have a git branch I can just build and boot I can
do it quickly!

I have no idea what this patch set is based on so it could be
hard to figure out the dependencies otherwise. Also many
patches.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 12/21] drm/mcde: Use GEM CMA object functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1551 bytes --]

Hi

Am 25.05.20 um 15:08 schrieb Linus Walleij:
> On Mon, May 25, 2020 at 2:51 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Am 25.05.20 um 13:36 schrieb Linus Walleij:
>>> On Fri, May 22, 2020 at 3:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>
>>>> The mcde driver uses the default implementation for CMA functions. The
>>>> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
>>>> All remaining operations are provided by CMA GEM object functions.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Could you boot-test with the patchset applied?
> 
> Yes, if you have a git branch I can just build and boot I can
> do it quickly!

Fantastic! It's the cma-objfuncs branch of

 https://gitlab.freedesktop.org/tzimmermann/linux.git

Here's the gitlab page:

 https://gitlab.freedesktop.org/tzimmermann/linux/-/tree/cma-objfuncs

Best regards
Thomas

> 
> I have no idea what this patch set is based on so it could be
> hard to figure out the dependencies otherwise. Also many
> patches.
> 
> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> 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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 20/21] drm/tv200: Use GEM CMA object functions

[Linus Walleij]

On Fri, May 22, 2020 at 3:53 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:

> The tve200 driver uses the default implementation for CMA functions. The
> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
> All remaining operations are provided by CMA GEM object functions.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 07/21] drm/hisilicon/kirin: Use GEM CMA object functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2335 bytes --]

Hi Emil

Am 22.05.20 um 20:11 schrieb Emil Velikov:
> Hi Thomas,
> 
> On Fri, 22 May 2020 at 14:53, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> The kirin driver uses the default implementation for CMA functions; except
>> for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro now sets
>> these defaults and .dumb_create in struct drm_driver. All remaining
>> operations are provided by CMA GEM object functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 12 +-----------
>>  1 file changed, 1 insertion(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
>> index c339e632522a9..b1ffd7d43e562 100644
>> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
>> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
>> @@ -921,17 +921,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ade_fops);
>>  static struct drm_driver ade_driver = {
>>         .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>         .fops = &ade_fops,
>> -       .gem_free_object_unlocked = drm_gem_cma_free_object,
>> -       .gem_vm_ops = &drm_gem_cma_vm_ops,
>> -       .dumb_create = drm_gem_cma_dumb_create_internal,
> 
> This doesn't seem right. The _internal documentation explicitly says
> that this should _not_ be used as .dumb_create. Instead drivers should
> use it to implement their callback.
> 
> Since it yields the same result as drm_gem_cma_dumb_create we can use
> the default macro below.

I noticed this and thought that the driver authors probably had their
reasons. Changing the driver to the default macro is probably still a
good idea.

Best regards
Thomas

> 
> Weather to the .dumb_create in separate patch, or squash it here -
> I'll leave to you.
> In case of the latter, please mentioned it in the commit message.
> 
> -Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> 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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 07/21] drm/hisilicon/kirin: Use GEM CMA object functions

[Emil Velikov]

On Mon, 25 May 2020 at 13:41, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi Emil
>
> Am 22.05.20 um 20:11 schrieb Emil Velikov:
> > Hi Thomas,
> >
> > On Fri, 22 May 2020 at 14:53, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> The kirin driver uses the default implementation for CMA functions; except
> >> for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro now sets
> >> these defaults and .dumb_create in struct drm_driver. All remaining
> >> operations are provided by CMA GEM object functions.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >> ---
> >>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 12 +-----------
> >>  1 file changed, 1 insertion(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> >> index c339e632522a9..b1ffd7d43e562 100644
> >> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> >> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> >> @@ -921,17 +921,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ade_fops);
> >>  static struct drm_driver ade_driver = {
> >>         .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >>         .fops = &ade_fops,
> >> -       .gem_free_object_unlocked = drm_gem_cma_free_object,
> >> -       .gem_vm_ops = &drm_gem_cma_vm_ops,
> >> -       .dumb_create = drm_gem_cma_dumb_create_internal,
> >
> > This doesn't seem right. The _internal documentation explicitly says
> > that this should _not_ be used as .dumb_create. Instead drivers should
> > use it to implement their callback.
> >
> > Since it yields the same result as drm_gem_cma_dumb_create we can use
> > the default macro below.
>
> I noticed this and thought that the driver authors probably had their
> reasons. Changing the driver to the default macro is probably still a
> good idea.
>
To be on the _extra_ safe side might want to keep that separate patch
explicitly CC-ing the author/reviewers of the offending commit.
Although as said before - it's your call, I'm fine either way.

HTH
Emil

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 17/21] drm/stm: Use GEM CMA object functions

[Philippe CORNU]

On 5/22/20 3:52 PM, Thomas Zimmermann wrote:
> The stm driver uses the default implementation for CMA functions; except
> for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro now sets
> these defaults and .dumb_create in struct drm_driver. All remaining
> operations are provided by CMA GEM object functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/stm/drv.c | 11 +----------
>   1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index 0f85dd86cafa7..d6238c71fabf6 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -62,16 +62,7 @@ static struct drm_driver drv_driver = {
>   	.minor = 0,
>   	.patchlevel = 0,
>   	.fops = &drv_driver_fops,
> -	.dumb_create = stm_gem_cma_dumb_create,
> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
> +	__DRM_GEM_CMA_DRIVER_OPS(stm_gem_cma_dumb_create),
>   };
>   
>   static int drv_load(struct drm_device *ddev)
> 
Hi Thomas,
Thank you for the patch.
Reviewed-by: Philippe Cornu <philippe.cornu@st.com>
Philippe :-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 03/21] drm/arm: Use GEM CMA object functions

[Liviu Dudau]

On Fri, May 22, 2020 at 03:52:28PM +0200, Thomas Zimmermann wrote:
> The arm driver uses the default implementation for CMA functions. The
> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
> All remaining operations are provided by CMA GEM object functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks!
Liviu

> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 194419f47c5e5..c83b81a3a582a 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -240,17 +240,7 @@ static struct drm_driver hdlcd_driver = {
>  	.irq_preinstall = hdlcd_irq_preinstall,
>  	.irq_postinstall = hdlcd_irq_postinstall,
>  	.irq_uninstall = hdlcd_irq_uninstall,
> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> -	.gem_print_info = drm_gem_cma_print_info,
> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
> -	.dumb_create = drm_gem_cma_dumb_create,
> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
> +	DRM_GEM_CMA_DRIVER_OPS,
>  #ifdef CONFIG_DEBUG_FS
>  	.debugfs_init = hdlcd_debugfs_init,
>  #endif
> -- 
> 2.26.2
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 10/21] drm/komeda: Use GEM CMA object functions

[Liviu Dudau]

On Fri, May 22, 2020 at 03:52:35PM +0200, Thomas Zimmermann wrote:
> The komeda driver uses the default implementation for CMA functions; except
> for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro now sets
> these defaults and .dumb_create in struct drm_driver. All remaining
> operations are provided by CMA GEM object functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index 6b85d5f4caa85..bdfbcbc416260 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -61,16 +61,7 @@ static irqreturn_t komeda_kms_irq_handler(int irq, void *data)
>  static struct drm_driver komeda_kms_driver = {
>  	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.lastclose			= drm_fb_helper_lastclose,
> -	.gem_free_object_unlocked	= drm_gem_cma_free_object,
> -	.gem_vm_ops			= &drm_gem_cma_vm_ops,
> -	.dumb_create			= komeda_gem_cma_dumb_create,
> -	.prime_handle_to_fd		= drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle		= drm_gem_prime_fd_to_handle,
> -	.gem_prime_get_sg_table		= drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table	= drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap			= drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap		= drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap			= drm_gem_cma_prime_mmap,
> +	__DRM_GEM_CMA_DRIVER_OPS(komeda_gem_cma_dumb_create),
>  	.fops = &komeda_cma_fops,
>  	.name = "komeda",
>  	.desc = "Arm Komeda Display Processor driver",
> -- 
> 2.26.2
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 11/21] drm/malidp: Use GEM CMA object functions

[Liviu Dudau]

On Fri, May 22, 2020 at 03:52:36PM +0200, Thomas Zimmermann wrote:
> The malidp driver uses the default implementation for CMA functions; except
> for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro now sets
> these defaults and .dumb_create in struct drm_driver. All remaining
> operations are provided by CMA GEM object functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index def8c9ffafcaf..92e0bca6aa2f4 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -563,16 +563,7 @@ static void malidp_debugfs_init(struct drm_minor *minor)
>  
>  static struct drm_driver malidp_driver = {
>  	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
> -	.dumb_create = malidp_dumb_create,
> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
> +	__DRM_GEM_CMA_DRIVER_OPS(malidp_dumb_create),
>  #ifdef CONFIG_DEBUG_FS
>  	.debugfs_init = malidp_debugfs_init,
>  #endif
> -- 
> 2.26.2
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 09/21] drm/ingenic: Use GEM CMA object functions

[Paul Cercueil]

Hi Thomas,

Le ven. 22 mai 2020 à 15:52, Thomas Zimmermann <tzimmermann@suse.de> a 
écrit :
> The ingenic driver uses the default implementation for CMA functions. 
> The
> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct 
> drm_driver.
> All remaining operations are provided by CMA GEM object functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Tested-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/gpu/drm/ingenic/ingenic-drm.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index eff57a1f70fb0..1c1cee367b752 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -519,18 +519,7 @@ static struct drm_driver ingenic_drm_driver_data 
> = {
>  	.patchlevel		= 0,
> 
>  	.fops			= &ingenic_drm_fops,
> -
> -	.dumb_create		= drm_gem_cma_dumb_create,
> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
> -	.gem_vm_ops		= &drm_gem_cma_vm_ops,
> -
> -	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
> -	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
> -	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> -	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
> -	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
> -	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
> +	DRM_GEM_CMA_DRIVER_OPS,
> 
>  	.irq_handler		= ingenic_drm_irq_handler,
>  };
> --
> 2.26.2
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/21] drm/aspeed: Set driver CMA functions with DRM_GEM_CMA_DRIVER_OPS

[Joel Stanley]

On Fri, 22 May 2020 at 13:52, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver
> to their defaults. No functional changes are made.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

I just found this in my inbox. I assume it has not been applied as you
were after a review.

Reviewed-by: Joel Stanley <joel@jms.id.au>

I will apply it to drm-misc-next.

Cheers,

Joel

> ---
>  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> index 6b27242b9ee3c..1167ff78e24a3 100644
> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> @@ -188,12 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
>
>  static struct drm_driver aspeed_gfx_driver = {
>         .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> -       .gem_create_object      = drm_cma_gem_create_object_default_funcs,
> -       .dumb_create            = drm_gem_cma_dumb_create,
> -       .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 = drm_gem_cma_prime_import_sg_table,
> -       .gem_prime_mmap         = drm_gem_prime_mmap,
> +       DRM_GEM_CMA_DRIVER_OPS,
>         .fops = &fops,
>         .name = "aspeed-gfx-drm",
>         .desc = "ASPEED GFX DRM",
> --
> 2.26.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/21] drm/aspeed: Set driver CMA functions with DRM_GEM_CMA_DRIVER_OPS

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2022 bytes --]

Hi

Am 09.10.20 um 09:54 schrieb Joel Stanley:
> On Fri, 22 May 2020 at 13:52, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver
>> to their defaults. No functional changes are made.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> I just found this in my inbox. I assume it has not been applied as you
> were after a review.

The patchset as a whole got an A-b and was merged a while ago.

> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>

Nevertheless thanks for taking a look at the patch.

Best regards
Thomas

> 
> I will apply it to drm-misc-next.
> 
> Cheers,
> 
> Joel
> 
>> ---
>>  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 7 +------
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
>> index 6b27242b9ee3c..1167ff78e24a3 100644
>> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
>> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
>> @@ -188,12 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
>>
>>  static struct drm_driver aspeed_gfx_driver = {
>>         .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>> -       .gem_create_object      = drm_cma_gem_create_object_default_funcs,
>> -       .dumb_create            = drm_gem_cma_dumb_create,
>> -       .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 = drm_gem_cma_prime_import_sg_table,
>> -       .gem_prime_mmap         = drm_gem_prime_mmap,
>> +       DRM_GEM_CMA_DRIVER_OPS,
>>         .fops = &fops,
>>         .name = "aspeed-gfx-drm",
>>         .desc = "ASPEED GFX DRM",
>> --
>> 2.26.2
>>

-- 
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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/21] drm/aspeed: Set driver CMA functions with DRM_GEM_CMA_DRIVER_OPS

[Joel Stanley]

On Fri, 9 Oct 2020 at 08:01, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 09.10.20 um 09:54 schrieb Joel Stanley:
> > On Fri, 22 May 2020 at 13:52, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver
> >> to their defaults. No functional changes are made.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >
> > I just found this in my inbox. I assume it has not been applied as you
> > were after a review.
>
> The patchset as a whole got an A-b and was merged a while ago.

I'm a bit confused, I couldn't see it in any tree. The aspeed one
seemed to have been skipped when applying the series.

I looked at today's linux-next and drm-misc-next.

> >
> > Reviewed-by: Joel Stanley <joel@jms.id.au>
>
> Nevertheless thanks for taking a look at the patch.
>
> Best regards
> Thomas
>
> >
> > I will apply it to drm-misc-next.
> >
> > Cheers,
> >
> > Joel
> >
> >> ---
> >>  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 7 +------
> >>  1 file changed, 1 insertion(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> >> index 6b27242b9ee3c..1167ff78e24a3 100644
> >> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> >> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> >> @@ -188,12 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
> >>
> >>  static struct drm_driver aspeed_gfx_driver = {
> >>         .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> >> -       .gem_create_object      = drm_cma_gem_create_object_default_funcs,
> >> -       .dumb_create            = drm_gem_cma_dumb_create,
> >> -       .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 = drm_gem_cma_prime_import_sg_table,
> >> -       .gem_prime_mmap         = drm_gem_prime_mmap,
> >> +       DRM_GEM_CMA_DRIVER_OPS,
> >>         .fops = &fops,
> >>         .name = "aspeed-gfx-drm",
> >>         .desc = "ASPEED GFX DRM",
> >> --
> >> 2.26.2
> >>
>
> --
> 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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/21] drm/aspeed: Set driver CMA functions with DRM_GEM_CMA_DRIVER_OPS

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3212 bytes --]

Hi

Am 09.10.20 um 10:06 schrieb Joel Stanley:
> On Fri, 9 Oct 2020 at 08:01, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 09.10.20 um 09:54 schrieb Joel Stanley:
>>> On Fri, 22 May 2020 at 13:52, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>>
>>>> DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver
>>>> to their defaults. No functional changes are made.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>
>>> I just found this in my inbox. I assume it has not been applied as you
>>> were after a review.
>>
>> The patchset as a whole got an A-b and was merged a while ago.
> 
> I'm a bit confused, I couldn't see it in any tree. The aspeed one
> seemed to have been skipped when applying the series.
> 
> I looked at today's linux-next and drm-misc-next.

Indeed. The other patches are in drm-misc-next, but not this one. I must
have lost it during the merge process.

> 
>>>
>>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>>
>> Nevertheless thanks for taking a look at the patch.
>>
>> Best regards
>> Thomas
>>
>>>
>>> I will apply it to drm-misc-next.

So please ahead and apply it. There's also an ack by Emil, which you may
want to add as well.

  Acked-by: Emil Velikov <emil.velikov@collabora.com>

Sorry about missing the patch and thanks for taking care.

Best regards
Thomas

>>>
>>> Cheers,
>>>
>>> Joel
>>>
>>>> ---
>>>>  drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 7 +------
>>>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
>>>> index 6b27242b9ee3c..1167ff78e24a3 100644
>>>> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
>>>> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
>>>> @@ -188,12 +188,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
>>>>
>>>>  static struct drm_driver aspeed_gfx_driver = {
>>>>         .driver_features        = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>>> -       .gem_create_object      = drm_cma_gem_create_object_default_funcs,
>>>> -       .dumb_create            = drm_gem_cma_dumb_create,
>>>> -       .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 = drm_gem_cma_prime_import_sg_table,
>>>> -       .gem_prime_mmap         = drm_gem_prime_mmap,
>>>> +       DRM_GEM_CMA_DRIVER_OPS,
>>>>         .fops = &fops,
>>>>         .name = "aspeed-gfx-drm",
>>>>         .desc = "ASPEED GFX DRM",
>>>> --
>>>> 2.26.2
>>>>
>>
>> --
>> 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
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> 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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/21] drm/aspeed: Set driver CMA functions with DRM_GEM_CMA_DRIVER_OPS

[Joel Stanley]

On Fri, 9 Oct 2020 at 08:26, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 09.10.20 um 10:06 schrieb Joel Stanley:
> > On Fri, 9 Oct 2020 at 08:01, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>
> >> Hi
> >>
> >> Am 09.10.20 um 09:54 schrieb Joel Stanley:
> >>> On Fri, 22 May 2020 at 13:52, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>>>
> >>>> DRM_GEM_CMA_DRIVER_OPS sets the functions in struct drm_driver
> >>>> to their defaults. No functional changes are made.
> >>>>
> >>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >>>
> >>> I just found this in my inbox. I assume it has not been applied as you
> >>> were after a review.
> >>
> >> The patchset as a whole got an A-b and was merged a while ago.
> >
> > I'm a bit confused, I couldn't see it in any tree. The aspeed one
> > seemed to have been skipped when applying the series.
> >
> > I looked at today's linux-next and drm-misc-next.
>
> Indeed. The other patches are in drm-misc-next, but not this one. I must
> have lost it during the merge process.
>
> >
> >>>
> >>> Reviewed-by: Joel Stanley <joel@jms.id.au>
> >>
> >> Nevertheless thanks for taking a look at the patch.
> >>
> >> Best regards
> >> Thomas
> >>
> >>>
> >>> I will apply it to drm-misc-next.
>
> So please ahead and apply it. There's also an ack by Emil, which you may
> want to add as well.
>
>   Acked-by: Emil Velikov <emil.velikov@collabora.com>
>
> Sorry about missing the patch and thanks for taking care.

No problem. Thanks for the cleanup.

Cheers,

Joel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel