2022-02-26 01:45:20

by Rob Clark

Subject: [PATCH 1/3] drm: Extend DEFINE_DRM_GEM_FOPS() for optional fops

From: Rob Clark <[email protected]>

Extend the helper macro so we don't have to throw it away if driver adds
support for optional fops, like show_fdinfo().

Signed-off-by: Rob Clark <[email protected]>
---
include/drm/drm_gem.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 35e7f44c2a75..987e78b18244 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -327,7 +327,7 @@ struct drm_gem_object {
* non-static version of this you're probably doing it wrong and will break the
* THIS_MODULE reference by accident.
*/
-#define DEFINE_DRM_GEM_FOPS(name) \
+#define DEFINE_DRM_GEM_FOPS(name, ...) \
static const struct file_operations name = {\
.owner = THIS_MODULE,\
.open = drm_open,\
@@ -338,6 +338,7 @@ struct drm_gem_object {
.read = drm_read,\
.llseek = noop_llseek,\
.mmap = drm_gem_mmap,\
+ ##__VA_ARGS__\
}

void drm_gem_object_release(struct drm_gem_object *obj);
--
2.35.1

2022-02-26 02:08:15

by Ville Syrjälä

Subject: Re: [PATCH 1/3] drm: Extend DEFINE_DRM_GEM_FOPS() for optional fops

On Fri, Feb 25, 2022 at 12:26:12PM -0800, Rob Clark wrote:
> From: Rob Clark <[email protected]>
>
> Extend the helper macro so we don't have to throw it away if driver adds
> support for optional fops, like show_fdinfo().
>
> Signed-off-by: Rob Clark <[email protected]>
> ---
> include/drm/drm_gem.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 35e7f44c2a75..987e78b18244 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -327,7 +327,7 @@ struct drm_gem_object {
> * non-static version of this you're probably doing it wrong and will break the
> * THIS_MODULE reference by accident.
> */
> -#define DEFINE_DRM_GEM_FOPS(name) \
> +#define DEFINE_DRM_GEM_FOPS(name, ...) \
> static const struct file_operations name = {\
> .owner = THIS_MODULE,\
> .open = drm_open,\
> @@ -338,6 +338,7 @@ struct drm_gem_object {
> .read = drm_read,\
> .llseek = noop_llseek,\
> .mmap = drm_gem_mmap,\
> + ##__VA_ARGS__\
> }

Would it not be less convoluted to make the macro only provide
the initializers? So you'd get something like:

static const struct file_operations foo = {
DRM_GEM_FOPS,
.my_stuff = whatever,
};

>
> void drm_gem_object_release(struct drm_gem_object *obj);
> --
> 2.35.1

--
Ville Syrj?l?
Intel

2022-02-26 02:15:52

by Rob Clark

Subject: Re: [Freedreno] [PATCH 1/3] drm: Extend DEFINE_DRM_GEM_FOPS() for optional fops

On Fri, Feb 25, 2022 at 12:36 PM Ville Syrjälä
<[email protected]> wrote:
>
> On Fri, Feb 25, 2022 at 12:26:12PM -0800, Rob Clark wrote:
> > From: Rob Clark <[email protected]>
> >
> > Extend the helper macro so we don't have to throw it away if driver adds
> > support for optional fops, like show_fdinfo().
> >
> > Signed-off-by: Rob Clark <[email protected]>
> > ---
> > include/drm/drm_gem.h | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > index 35e7f44c2a75..987e78b18244 100644
> > --- a/include/drm/drm_gem.h
> > +++ b/include/drm/drm_gem.h
> > @@ -327,7 +327,7 @@ struct drm_gem_object {
> > * non-static version of this you're probably doing it wrong and will break the
> > * THIS_MODULE reference by accident.
> > */
> > -#define DEFINE_DRM_GEM_FOPS(name) \
> > +#define DEFINE_DRM_GEM_FOPS(name, ...) \
> > static const struct file_operations name = {\
> > .owner = THIS_MODULE,\
> > .open = drm_open,\
> > @@ -338,6 +338,7 @@ struct drm_gem_object {
> > .read = drm_read,\
> > .llseek = noop_llseek,\
> > .mmap = drm_gem_mmap,\
> > + ##__VA_ARGS__\
> > }
>
> Would it not be less convoluted to make the macro only provide
> the initializers? So you'd get something like:
>
> static const struct file_operations foo = {
> DRM_GEM_FOPS,
> .my_stuff = whatever,
> };
>

Hmm, I like my color of the bikeshed, but I guess it is a matter of opinion ;-)

BR,
-R

2022-02-26 02:29:47

by Sam Ravnborg

Subject: Re: [Freedreno] [PATCH 1/3] drm: Extend DEFINE_DRM_GEM_FOPS() for optional fops

Hi Rob,

> > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > > index 35e7f44c2a75..987e78b18244 100644
> > > --- a/include/drm/drm_gem.h
> > > +++ b/include/drm/drm_gem.h
> > > @@ -327,7 +327,7 @@ struct drm_gem_object {
> > > * non-static version of this you're probably doing it wrong and will break the
> > > * THIS_MODULE reference by accident.
> > > */
> > > -#define DEFINE_DRM_GEM_FOPS(name) \
> > > +#define DEFINE_DRM_GEM_FOPS(name, ...) \
> > > static const struct file_operations name = {\
> > > .owner = THIS_MODULE,\
> > > .open = drm_open,\
> > > @@ -338,6 +338,7 @@ struct drm_gem_object {
> > > .read = drm_read,\
> > > .llseek = noop_llseek,\
> > > .mmap = drm_gem_mmap,\
> > > + ##__VA_ARGS__\
> > > }
> >
> > Would it not be less convoluted to make the macro only provide
> > the initializers? So you'd get something like:
> >
> > static const struct file_operations foo = {
> > DRM_GEM_FOPS,
> > .my_stuff = whatever,
> > };
> >
>
> Hmm, I like my color of the bikeshed, but I guess it is a matter of opinion ;-)
Or less surprise. Most similar macros provides initializers only.

Try "git grep DRM_.*OPS | grep define" in include/drm
and take a look at the hits.

Sam