Re: [Freedreno] [PATCH v3 1/2] drm: Add DRM_GEM_FOPS
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
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
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
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
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