| Message ID | 20220609174213.2265938-1-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] drm: Add DRM_GEM_FOPS | expand |
On 09/06/2022 20:42, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > 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 <robdclark@chromium.org> > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> 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 > + * &file_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);
Am 15.06.22 um 14:45 schrieb Dmitry Baryshkov: > On 09/06/2022 20:42, Rob Clark wrote: >> From: Rob Clark <robdclark@chromium.org> >> >> 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 <robdclark@chromium.org> >> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > 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 >> + * &file_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); > >
On Thu, Jun 09, 2022 at 10:42:11AM -0700, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > 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 <robdclark@chromium.org> > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> 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 > + * &file_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 >
On Fri, Jun 24, 2022 at 1:49 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Thu, Jun 09, 2022 at 10:42:11AM -0700, Rob Clark wrote: > > From: Rob Clark <robdclark@chromium.org> > > > > 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 <robdclark@chromium.org> > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > 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 > > + * &file_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
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 + * &file_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);