Re: [Freedreno] [PATCH v3 1/2] drm: Add DRM_GEM_FOPS

2022-06-24 Thread Rob Clark
On Fri, Jun 24, 2022 at 1:49 PM Daniel Vetter  wrote:
>
> On Thu, Jun 09, 2022 at 10:42:11AM -0700, Rob Clark wrote:
> > From: Rob Clark 
> >
> > The DEFINE_DRM_GEM_FOPS() helper is a bit limiting if a driver wants to
> > provide additional file ops, like show_fdinfo().
> >
> > v2: Split out DRM_GEM_FOPS instead of making DEFINE_DRM_GEM_FOPS
> > varardic
> > v3: nits
> >
> > Signed-off-by: Rob Clark 
> > Acked-by: Thomas Zimmermann 
>
> We're at three drivers, maybe it'd be better if this is more standardized?
> I feel like we're opening a bit a can of worms here where everyone just
> has some good odl fashioned fun. It's at least much better documented than
> the old property proliferation :-)

yeah, we could have a standardized drm_show_fdinfo() fop plus
drm_driver callback.. at this point the drm core fxn would be rather
boring, ie. only printing dev->driver->name, so I didn't pursue that
approach (yet?).. but perhaps that changes over time.  I think we
chose the right approach here, focusing on the documentation first so
that userspace has a standardized experience.  The kernel side of
things, we are free to refactor at any time ;-)

BR,
-R

> -Daniel
>
> > ---
> >  include/drm/drm_gem.h | 26 ++
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > index 9d7c61a122dc..87cffc9efa85 100644
> > --- a/include/drm/drm_gem.h
> > +++ b/include/drm/drm_gem.h
> > @@ -314,6 +314,23 @@ struct drm_gem_object {
> >   const struct drm_gem_object_funcs *funcs;
> >  };
> >
> > +/**
> > + * DRM_GEM_FOPS - Default drm GEM file operations
> > + *
> > + * This macro provides a shorthand for setting the GEM file ops in the
> > + * _operations structure.  If all you need are the default ops, use
> > + * DEFINE_DRM_GEM_FOPS instead.
> > + */
> > +#define DRM_GEM_FOPS \
> > + .open   = drm_open,\
> > + .release= drm_release,\
> > + .unlocked_ioctl = drm_ioctl,\
> > + .compat_ioctl   = drm_compat_ioctl,\
> > + .poll   = drm_poll,\
> > + .read   = drm_read,\
> > + .llseek = noop_llseek,\
> > + .mmap   = drm_gem_mmap
> > +
> >  /**
> >   * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM 
> > drivers
> >   * @name: name for the generated structure
> > @@ -330,14 +347,7 @@ struct drm_gem_object {
> >  #define DEFINE_DRM_GEM_FOPS(name) \
> >   static const struct file_operations name = {\
> >   .owner  = THIS_MODULE,\
> > - .open   = drm_open,\
> > - .release= drm_release,\
> > - .unlocked_ioctl = drm_ioctl,\
> > - .compat_ioctl   = drm_compat_ioctl,\
> > - .poll   = drm_poll,\
> > - .read   = drm_read,\
> > - .llseek = noop_llseek,\
> > - .mmap   = drm_gem_mmap,\
> > + DRM_GEM_FOPS,\
> >   }
> >
> >  void drm_gem_object_release(struct drm_gem_object *obj);
> > --
> > 2.36.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch


Re: [Freedreno] [PATCH v3 1/2] drm: Add DRM_GEM_FOPS

2022-06-24 Thread Daniel Vetter
On Thu, Jun 09, 2022 at 10:42:11AM -0700, Rob Clark wrote:
> From: Rob Clark 
> 
> The DEFINE_DRM_GEM_FOPS() helper is a bit limiting if a driver wants to
> provide additional file ops, like show_fdinfo().
> 
> v2: Split out DRM_GEM_FOPS instead of making DEFINE_DRM_GEM_FOPS
> varardic
> v3: nits
> 
> Signed-off-by: Rob Clark 
> Acked-by: Thomas Zimmermann 

We're at three drivers, maybe it'd be better if this is more standardized?
I feel like we're opening a bit a can of worms here where everyone just
has some good odl fashioned fun. It's at least much better documented than
the old property proliferation :-)
-Daniel

> ---
>  include/drm/drm_gem.h | 26 ++
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 9d7c61a122dc..87cffc9efa85 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -314,6 +314,23 @@ struct drm_gem_object {
>   const struct drm_gem_object_funcs *funcs;
>  };
>  
> +/**
> + * DRM_GEM_FOPS - Default drm GEM file operations
> + *
> + * This macro provides a shorthand for setting the GEM file ops in the
> + * _operations structure.  If all you need are the default ops, use
> + * DEFINE_DRM_GEM_FOPS instead.
> + */
> +#define DRM_GEM_FOPS \
> + .open   = drm_open,\
> + .release= drm_release,\
> + .unlocked_ioctl = drm_ioctl,\
> + .compat_ioctl   = drm_compat_ioctl,\
> + .poll   = drm_poll,\
> + .read   = drm_read,\
> + .llseek = noop_llseek,\
> + .mmap   = drm_gem_mmap
> +
>  /**
>   * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
>   * @name: name for the generated structure
> @@ -330,14 +347,7 @@ struct drm_gem_object {
>  #define DEFINE_DRM_GEM_FOPS(name) \
>   static const struct file_operations name = {\
>   .owner  = THIS_MODULE,\
> - .open   = drm_open,\
> - .release= drm_release,\
> - .unlocked_ioctl = drm_ioctl,\
> - .compat_ioctl   = drm_compat_ioctl,\
> - .poll   = drm_poll,\
> - .read   = drm_read,\
> - .llseek = noop_llseek,\
> - .mmap   = drm_gem_mmap,\
> + DRM_GEM_FOPS,\
>   }
>  
>  void drm_gem_object_release(struct drm_gem_object *obj);
> -- 
> 2.36.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [Freedreno] [PATCH v3 1/2] drm: Add DRM_GEM_FOPS

2022-06-15 Thread Thomas Zimmermann



Am 15.06.22 um 14:45 schrieb Dmitry Baryshkov:

On 09/06/2022 20:42, Rob Clark wrote:

From: Rob Clark 

The DEFINE_DRM_GEM_FOPS() helper is a bit limiting if a driver wants to
provide additional file ops, like show_fdinfo().

v2: Split out DRM_GEM_FOPS instead of making DEFINE_DRM_GEM_FOPS
 varardic
v3: nits

Signed-off-by: Rob Clark 
Acked-by: Thomas Zimmermann 


I suspect that with Tomas's ack we can pick this through the drm/msm. Is 
this correct? (I'll then pick it for the msm-lumag).


Sure, go ahead.




---
  include/drm/drm_gem.h | 26 ++
  1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 9d7c61a122dc..87cffc9efa85 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -314,6 +314,23 @@ struct drm_gem_object {
  const struct drm_gem_object_funcs *funcs;
  };
+/**
+ * DRM_GEM_FOPS - Default drm GEM file operations
+ *
+ * This macro provides a shorthand for setting the GEM file ops in the
+ * _operations structure.  If all you need are the default ops, use
+ * DEFINE_DRM_GEM_FOPS instead.
+ */
+#define DRM_GEM_FOPS \
+    .open    = drm_open,\
+    .release    = drm_release,\
+    .unlocked_ioctl    = drm_ioctl,\
+    .compat_ioctl    = drm_compat_ioctl,\
+    .poll    = drm_poll,\
+    .read    = drm_read,\
+    .llseek    = noop_llseek,\
+    .mmap    = drm_gem_mmap
+
  /**
   * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM 
drivers

   * @name: name for the generated structure
@@ -330,14 +347,7 @@ struct drm_gem_object {
  #define DEFINE_DRM_GEM_FOPS(name) \
  static const struct file_operations name = {\
  .owner    = THIS_MODULE,\
-    .open    = drm_open,\
-    .release    = drm_release,\
-    .unlocked_ioctl    = drm_ioctl,\
-    .compat_ioctl    = drm_compat_ioctl,\
-    .poll    = drm_poll,\
-    .read    = drm_read,\
-    .llseek    = noop_llseek,\
-    .mmap    = drm_gem_mmap,\
+    DRM_GEM_FOPS,\
  }
  void drm_gem_object_release(struct drm_gem_object *obj);





--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


OpenPGP_signature
Description: OpenPGP digital signature


Re: [Freedreno] [PATCH v3 1/2] drm: Add DRM_GEM_FOPS

2022-06-15 Thread Dmitry Baryshkov

On 09/06/2022 20:42, Rob Clark wrote:

From: Rob Clark 

The DEFINE_DRM_GEM_FOPS() helper is a bit limiting if a driver wants to
provide additional file ops, like show_fdinfo().

v2: Split out DRM_GEM_FOPS instead of making DEFINE_DRM_GEM_FOPS
 varardic
v3: nits

Signed-off-by: Rob Clark 
Acked-by: Thomas Zimmermann 


I suspect that with Tomas's ack we can pick this through the drm/msm. Is 
this correct? (I'll then pick it for the msm-lumag).



---
  include/drm/drm_gem.h | 26 ++
  1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 9d7c61a122dc..87cffc9efa85 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -314,6 +314,23 @@ struct drm_gem_object {
const struct drm_gem_object_funcs *funcs;
  };
  
+/**

+ * DRM_GEM_FOPS - Default drm GEM file operations
+ *
+ * This macro provides a shorthand for setting the GEM file ops in the
+ * _operations structure.  If all you need are the default ops, use
+ * DEFINE_DRM_GEM_FOPS instead.
+ */
+#define DRM_GEM_FOPS \
+   .open   = drm_open,\
+   .release= drm_release,\
+   .unlocked_ioctl = drm_ioctl,\
+   .compat_ioctl   = drm_compat_ioctl,\
+   .poll   = drm_poll,\
+   .read   = drm_read,\
+   .llseek = noop_llseek,\
+   .mmap   = drm_gem_mmap
+
  /**
   * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
   * @name: name for the generated structure
@@ -330,14 +347,7 @@ struct drm_gem_object {
  #define DEFINE_DRM_GEM_FOPS(name) \
static const struct file_operations name = {\
.owner  = THIS_MODULE,\
-   .open   = drm_open,\
-   .release= drm_release,\
-   .unlocked_ioctl = drm_ioctl,\
-   .compat_ioctl   = drm_compat_ioctl,\
-   .poll   = drm_poll,\
-   .read   = drm_read,\
-   .llseek = noop_llseek,\
-   .mmap   = drm_gem_mmap,\
+   DRM_GEM_FOPS,\
}
  
  void drm_gem_object_release(struct drm_gem_object *obj);



--
With best wishes
Dmitry


[Freedreno] [PATCH v3 1/2] drm: Add DRM_GEM_FOPS

2022-06-09 Thread Rob Clark
From: Rob Clark 

The DEFINE_DRM_GEM_FOPS() helper is a bit limiting if a driver wants to
provide additional file ops, like show_fdinfo().

v2: Split out DRM_GEM_FOPS instead of making DEFINE_DRM_GEM_FOPS
varardic
v3: nits

Signed-off-by: Rob Clark 
Acked-by: Thomas Zimmermann 
---
 include/drm/drm_gem.h | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 9d7c61a122dc..87cffc9efa85 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -314,6 +314,23 @@ struct drm_gem_object {
const struct drm_gem_object_funcs *funcs;
 };
 
+/**
+ * DRM_GEM_FOPS - Default drm GEM file operations
+ *
+ * This macro provides a shorthand for setting the GEM file ops in the
+ * _operations structure.  If all you need are the default ops, use
+ * DEFINE_DRM_GEM_FOPS instead.
+ */
+#define DRM_GEM_FOPS \
+   .open   = drm_open,\
+   .release= drm_release,\
+   .unlocked_ioctl = drm_ioctl,\
+   .compat_ioctl   = drm_compat_ioctl,\
+   .poll   = drm_poll,\
+   .read   = drm_read,\
+   .llseek = noop_llseek,\
+   .mmap   = drm_gem_mmap
+
 /**
  * DEFINE_DRM_GEM_FOPS() - macro to generate file operations for GEM drivers
  * @name: name for the generated structure
@@ -330,14 +347,7 @@ struct drm_gem_object {
 #define DEFINE_DRM_GEM_FOPS(name) \
static const struct file_operations name = {\
.owner  = THIS_MODULE,\
-   .open   = drm_open,\
-   .release= drm_release,\
-   .unlocked_ioctl = drm_ioctl,\
-   .compat_ioctl   = drm_compat_ioctl,\
-   .poll   = drm_poll,\
-   .read   = drm_read,\
-   .llseek = noop_llseek,\
-   .mmap   = drm_gem_mmap,\
+   DRM_GEM_FOPS,\
}
 
 void drm_gem_object_release(struct drm_gem_object *obj);
-- 
2.36.1