Re: [PATCH v2 1/2] drm: Add DRM_GEM_FOPS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 07.06.22 um 16:58 schrieb Rob Clark:
On Mon, Jun 6, 2022 at 11:56 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:

Hi

Am 06.06.22 um 21:54 schrieb Rob Clark:
From: Rob Clark <robdclark@xxxxxxxxxxxx>

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

Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
---
   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..dc88d4a2cdf6 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.

I would appreciate a reference to DEFINE_DRM_GEM_FOPS. Something along
the lines of 'if all you need are the default ops, use DEFINE_DRM_GEM_FOPS'.

+ */
+#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
+
+

Only one empty line please.

   /**
    * 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,\

Is there a specific reason why .owner is still set here? I suspect that
DRM_GEM_FOPS is strictly for callback functions?

I was on the fence about that one, but it seemed better to not mix
"magic" and the callbacks.. but I could be convinced in either
direction

I think you made the right choice. It's cleaner and most drivers will want to use DEFINE_DRM_GEM_FOPS, which includes the magic.

Best regards
Thomas


In any case

Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx>

thx, I'll fixup the other nits in v3.


Best regards
Thomas

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

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

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux