On 28/04/20(Tue) 11:56, Mark Kettenis wrote:
> > Date: Tue, 28 Apr 2020 11:40:17 +0200
> > From: Martin Pieuchot <[email protected]>
> >
> > On 27/04/20(Mon) 19:34, Martin Pieuchot wrote:
> > > On 28/04/20(Tue) 01:54, Jonathan Gray wrote:
> > > > On Mon, Apr 27, 2020 at 04:52:33PM +0200, Martin Pieuchot wrote:
> > > > > Diff below extends the existing drmkqfilter() to support EVFILT_READ.
> > > > > This makes drm(4)'s kqueue support in pair with poll().
> > > > >
> > > > > The event list queried in the filt_drmread() should be protected by
> > > > > the
> > > > > `event_lock' mutex. This could be done by using the `kdev'
> > > > > backpointer
> > > > > as shown in comment. However this would require some plumbing to make
> > > > > use of `minor'. The side effect of this approach would be to reduce
> > > > > the
> > > > > diff with Linux.
> > > >
> > > > You seem to be confusing kdev and dev in the drm_minor struct.
> > > > at least in linux kdev is 'struct device *' and dev is
> > > > 'struct drm_device *' (which has the event lock).
> > > > I have a tree which uses the drm_minor struct but it is part of a much
> > > > larger diff.
> > >
> > > Thanks for pointing that out! Do you see another way to comply to the
> > > locking then?
> >
> > If not or if we agree that this can be improved later on I'd like to
> > commit the fixed diff below, ok?
>
> Please don't commit anything with C++ style comments.
Sure, updated diff below!
> I'm also not convinced that this is the appropriate way to implement
> kqueue for drm devices. This is Linux-compat stuff and Linux doesn't
> have kqueue.
I'm in the process of unifying the behavior of all the pieces of code
supporting poll, select and kqueue. drm(4) is one of the exceptions,
video(4) is another, there might be more I haven't finished my audit :)
What would be the proper way to implement kqueue support for OpenBSD if
this is not it?
Index: sys/conf.h
===================================================================
RCS file: /cvs/src/sys/sys/conf.h,v
retrieving revision 1.150
diff -u -p -r1.150 conf.h
--- sys/conf.h 21 Apr 2020 08:29:27 -0000 1.150
+++ sys/conf.h 27 Apr 2020 14:43:48 -0000
@@ -439,7 +439,7 @@ extern struct cdevsw cdevsw[];
(dev_type_stop((*))) enodev, 0, selfalse, \
(dev_type_mmap((*))) enodev }
-/* open, close, read, ioctl, poll, mmap, nokqfilter */
+/* open, close, read, ioctl, poll, mmap, kqfilter */
#define cdev_drm_init(c,n) { \
dev_init(c,n,open), dev_init(c,n,close), dev_init(c, n, read), \
(dev_type_write((*))) enodev, dev_init(c,n,ioctl), \
Index: dev/pci/drm/drm_drv.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v
retrieving revision 1.174
diff -u -p -r1.174 drm_drv.c
--- dev/pci/drm/drm_drv.c 7 Apr 2020 13:27:51 -0000 1.174
+++ dev/pci/drm/drm_drv.c 28 Apr 2020 10:15:16 -0000
@@ -484,6 +484,34 @@ filt_drmkms(struct knote *kn, long hint)
return (kn->kn_fflags != 0);
}
+void
+filt_drmreaddetach(struct knote *kn)
+{
+ struct drm_file *file_priv = kn->kn_hook;
+ int s;
+
+ s = spltty();
+ klist_remove(&file_priv->rsel.si_note, kn);
+ splx(s);
+}
+
+int
+filt_drmread(struct knote *kn, long hint)
+{
+ struct drm_file *file_priv = kn->kn_hook;
+ int val = 0;
+
+#if notyet
+ mtx_enter(&file_priv->minor->dev->event_lock);
+#endif
+ val = !list_empty(&file_priv->event_list);
+#if notyet
+ mtx_leave(&file_priv->minor->dev->event_lock);
+#endif
+
+ return (val);
+}
+
const struct filterops drm_filtops = {
.f_flags = FILTEROP_ISFD,
.f_attach = NULL,
@@ -491,30 +519,51 @@ const struct filterops drm_filtops = {
.f_event = filt_drmkms,
};
+const struct filterops drmread_filtops = {
+ .f_flags = FILTEROP_ISFD,
+ .f_attach = NULL,
+ .f_detach = filt_drmreaddetach,
+ .f_event = filt_drmread,
+};
+
int
drmkqfilter(dev_t kdev, struct knote *kn)
{
struct drm_device *dev = NULL;
- int s;
+ struct drm_file *file_priv = NULL;
+ int s;
dev = drm_get_device_from_kdev(kdev);
if (dev == NULL || dev->dev_private == NULL)
return (ENXIO);
switch (kn->kn_filter) {
+ case EVFILT_READ:
+ mutex_lock(&dev->struct_mutex);
+ file_priv = drm_find_file_by_minor(dev, minor(kdev));
+ mutex_unlock(&dev->struct_mutex);
+ if (file_priv == NULL)
+ return (ENXIO);
+
+ kn->kn_fop = &drmread_filtops;
+ kn->kn_hook = file_priv;
+
+ s = spltty();
+ klist_insert(&file_priv->rsel.si_note, kn);
+ splx(s);
+ break;
case EVFILT_DEVICE:
kn->kn_fop = &drm_filtops;
+ kn->kn_hook = dev;
+
+ s = spltty();
+ klist_insert(&dev->note, kn);
+ splx(s);
break;
default:
return (EINVAL);
}
- kn->kn_hook = dev;
-
- s = spltty();
- klist_insert(&dev->note, kn);
- splx(s);
-
return (0);
}
@@ -772,7 +821,6 @@ out:
return (gotone);
}
-/* XXX kqfilter ... */
int
drmpoll(dev_t kdev, int events, struct proc *p)
{
