From 0f460e8063754e25bf6889a80cae2fdc974fc7f9 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 10 Jan 2022 01:26:06 +0000 Subject: [PATCH 1/3] video(4): Allow drivers to pass the softc explicitly. This way one device driver can have multiple video0, video1, &c., interfaces attached, using independent state and a common parent. --- sys/dev/video.c | 15 ++++++++++++++- sys/dev/video_if.h | 2 ++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/sys/dev/video.c b/sys/dev/video.c index 3fc6c1ed822f..96923f25c849 100644 --- a/sys/dev/video.c +++ b/sys/dev/video.c @@ -351,7 +351,7 @@ video_attach(device_t parent, device_t self, void *aux) sc->sc_dev = self; sc->hw_dev = parent; sc->hw_if = args->hw_if; - sc->hw_softc = device_private(parent); + sc->hw_softc = args->hw_softc; sc->sc_open = 0; sc->sc_refcnt = 0; @@ -433,6 +433,19 @@ video_attach_mi(const struct video_hw_if *hw_if, device_t parent) struct video_attach_args args; args.hw_if = hw_if; + args.hw_softc = device_private(parent); + return config_found(parent, &args, video_print, + CFARGS(.iattr = "videobus")); +} + +device_t +video_attach_mi_softc(const struct video_hw_if *hw_if, device_t parent, + void *sc) +{ + struct video_attach_args args; + + args.hw_if = hw_if; + args.hw_softc = sc; return config_found(parent, &args, video_print, CFARGS(.iattr = "videobus")); } diff --git a/sys/dev/video_if.h b/sys/dev/video_if.h index 932f477a8675..e731d1d2331e 100644 --- a/sys/dev/video_if.h +++ b/sys/dev/video_if.h @@ -501,9 +501,11 @@ struct video_hw_if { struct video_attach_args { const struct video_hw_if *hw_if; + void *hw_softc; }; device_t video_attach_mi(const struct video_hw_if *, device_t); +device_t video_attach_mi_softc(const struct video_hw_if *, device_t, void *); void video_submit_payload(device_t, const struct video_payload *); #endif /* _SYS_DEV_VIDEO_IF_H_ */ From f6382a1365276db792a348a69ae2907206140a79 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 10 Jan 2022 01:31:04 +0000 Subject: [PATCH 2/3] uvideo(4): Fix zero initialization of uvideo_stream. Just use kmem_zalloc; don't memset it to zero, especially not after we just inserted it into the list, with the side effect of deleting the rest of the list! --- sys/dev/usb/uvideo.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sys/dev/usb/uvideo.c b/sys/dev/usb/uvideo.c index 6331609d6e83..75cfbd6c68a0 100644 --- a/sys/dev/usb/uvideo.c +++ b/sys/dev/usb/uvideo.c @@ -752,7 +752,7 @@ uvideo_stream_guess_format(struct uvideo_stream *vs, static struct uvideo_stream * uvideo_stream_alloc(void) { - return kmem_alloc(sizeof(struct uvideo_stream), KM_SLEEP); + return kmem_zalloc(sizeof(struct uvideo_stream), KM_SLEEP); } @@ -1031,7 +1031,6 @@ uvideo_stream_init(struct uvideo_stream *vs, vs)); SLIST_INSERT_HEAD(&sc->sc_stream_list, vs, entries); - memset(vs, 0, sizeof(*vs)); vs->vs_parent = sc; vs->vs_ifaceno = ifdesc->bInterfaceNumber; vs->vs_subtype = 0; From e13b9d159c8d08157c5a4fcce892d2a0f2976980 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 10 Jan 2022 01:36:50 +0000 Subject: [PATCH 3/3] uvideo(4): Attach one video(4) per independent stream. --- sys/dev/usb/uvideo.c | 115 +++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 64 deletions(-) diff --git a/sys/dev/usb/uvideo.c b/sys/dev/usb/uvideo.c index 75cfbd6c68a0..f70ad0c517db 100644 --- a/sys/dev/usb/uvideo.c +++ b/sys/dev/usb/uvideo.c @@ -210,6 +210,7 @@ struct uvideo_bulk_xfer { }; struct uvideo_stream { + device_t vs_videodev; struct uvideo_softc *vs_parent; struct usbd_interface *vs_iface; uint8_t vs_ifaceno; @@ -236,6 +237,8 @@ struct uvideo_stream { uint32_t vs_max_payload_size; uint32_t vs_frame_interval; SLIST_ENTRY(uvideo_stream) entries; + + uvideo_state vs_state; }; SLIST_HEAD(uvideo_stream_list, uvideo_stream); @@ -246,16 +249,11 @@ struct uvideo_softc { int sc_ifaceno; /* interface number */ char *sc_devname; - device_t sc_videodev; - int sc_dying; - uvideo_state sc_state; uint8_t sc_nunits; struct uvideo_unit **sc_unit; - struct uvideo_stream *sc_stream_in; - struct uvideo_stream_list sc_stream_list; char sc_businfo[32]; @@ -501,7 +499,6 @@ uvideo_attach(device_t parent, device_t self, void *aux) sc->sc_iface = uiaa->uiaa_iface; sc->sc_ifaceno = uiaa->uiaa_ifaceno; sc->sc_dying = 0; - sc->sc_state = UVIDEO_STATE_CLOSED; SLIST_INIT(&sc->sc_stream_list); snprintf(sc->sc_businfo, sizeof(sc->sc_businfo), "usb:%08x", sc->sc_udev->ud_cookie.cookie); @@ -574,8 +571,6 @@ uvideo_attach(device_t parent, device_t self, void *aux) usbd_errstr(err), err)); goto bad; } - /* TODO: for now, set (each) stream to stream_in. */ - sc->sc_stream_in = vs; break; case UISUBCLASS_VIDEOCOLLECTION: err = uvideo_init_collection(sc, ifdesc, &iter); @@ -606,9 +601,11 @@ uvideo_attach(device_t parent, device_t self, void *aux) if (!pmf_device_register(self, NULL, NULL)) aprint_error_dev(self, "couldn't establish power handler\n"); - sc->sc_videodev = video_attach_mi(&uvideo_hw_if, sc->sc_dev); - DPRINTF(("uvideo_attach: attached video driver at %p\n", - sc->sc_videodev)); + SLIST_FOREACH(vs, &sc->sc_stream_list, entries) { + /* XXX initialization of vs_videodev is racy */ + vs->vs_videodev = video_attach_mi_softc(&uvideo_hw_if, + sc->sc_dev, vs); + } return; @@ -642,21 +639,29 @@ static void uvideo_childdet(device_t self, device_t child) { struct uvideo_softc *sc = device_private(self); + struct uvideo_stream *vs; - KASSERT(sc->sc_videodev == child); - sc->sc_videodev = NULL; + SLIST_FOREACH(vs, &sc->sc_stream_list, entries) { + if (child == vs->vs_videodev) { + vs->vs_videodev = NULL; + break; + } + } + KASSERTMSG(vs != NULL, "unknown child of %s detached: %s @ %p", + device_xname(self), device_xname(child), child); } static int uvideo_detach(device_t self, int flags) { - struct uvideo_softc *sc; + struct uvideo_softc *sc = device_private(self); struct uvideo_stream *vs; - int rv; + int error; - sc = device_private(self); - rv = 0; + error = config_detach_children(self, flags); + if (error) + return error; sc->sc_dying = 1; @@ -681,14 +686,11 @@ uvideo_detach(device_t self, int flags) DPRINTFN(15, ("uvideo: detaching from %s\n", device_xname(sc->sc_dev))); - if (sc->sc_videodev != NULL) - rv = config_detach(sc->sc_videodev, flags); - usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->sc_udev, sc->sc_dev); usbd_devinfo_free(sc->sc_devname); - return rv; + return 0; } /* Search the stream list for a stream matching the interface number. @@ -1039,6 +1041,7 @@ uvideo_stream_init(struct uvideo_stream *vs, vs->vs_default_format = NULL; vs->vs_current_format.priv = -1; vs->vs_xfer_type = 0; + vs->vs_state = UVIDEO_STATE_CLOSED; err = usbd_device2interface_handle(sc->sc_udev, vs->vs_ifaceno, &vs->vs_iface); @@ -1783,7 +1786,7 @@ uvideo_stream_recv_process(struct uvideo_stream *vs, uint8_t *buf, uint32_t len) payload.frameno = hdr->bmHeaderInfo & UV_FRAME_ID; payload.end_of_frame = hdr->bmHeaderInfo & UV_END_OF_FRAME; - video_submit_payload(vs->vs_parent->sc_videodev, &payload); + video_submit_payload(vs->vs_videodev, &payload); return USBD_NORMAL_COMPLETION; } @@ -1881,13 +1884,10 @@ uvideo_stream_recv_bulk_transfer(void *addr) static int uvideo_open(void *addr, int flags) { - struct uvideo_softc *sc; - struct uvideo_stream *vs; + struct uvideo_stream *vs = addr; + struct uvideo_softc *sc = vs->vs_parent; struct video_format fmt; - sc = addr; - vs = sc->sc_stream_in; - DPRINTF(("uvideo_open: sc=%p\n", sc)); if (sc->sc_dying) return EIO; @@ -1901,36 +1901,36 @@ uvideo_open(void *addr, int flags) static void uvideo_close(void *addr) { - struct uvideo_softc *sc; - - sc = addr; + struct uvideo_stream *vs = addr; uvideo_stop_transfer(addr); - if (sc->sc_state != UVIDEO_STATE_CLOSED) { - sc->sc_state = UVIDEO_STATE_CLOSED; + if (vs->vs_state != UVIDEO_STATE_CLOSED) { + vs->vs_state = UVIDEO_STATE_CLOSED; } } static const char * uvideo_get_devname(void *addr) { - struct uvideo_softc *sc = addr; - return sc->sc_devname; + struct uvideo_stream *vs = addr; + + return vs->vs_parent->sc_devname; } static const char * uvideo_get_businfo(void *addr) { - struct uvideo_softc *sc = addr; - return sc->sc_businfo; + struct uvideo_stream *vs = addr; + + return vs->vs_parent->sc_businfo; } static int uvideo_enum_format(void *addr, uint32_t index, struct video_format *format) { - struct uvideo_softc *sc = addr; - struct uvideo_stream *vs = sc->sc_stream_in; + struct uvideo_stream *vs = addr; + struct uvideo_softc *sc = vs->vs_parent; struct uvideo_format *video_format; int off; @@ -1956,8 +1956,8 @@ uvideo_enum_format(void *addr, uint32_t index, struct video_format *format) static int uvideo_get_format(void *addr, struct video_format *format) { - struct uvideo_softc *sc = addr; - struct uvideo_stream *vs = sc->sc_stream_in; + struct uvideo_stream *vs = addr; + struct uvideo_softc *sc = vs->vs_parent; if (sc->sc_dying) return EIO; @@ -1973,20 +1973,16 @@ uvideo_get_format(void *addr, struct video_format *format) static int uvideo_set_format(void *addr, struct video_format *format) { - struct uvideo_softc *sc; - struct uvideo_stream *vs; + struct uvideo_stream *vs = addr; + struct uvideo_softc *sc = vs->vs_parent; struct uvideo_format *uvfmt; uvideo_probe_and_commit_data_t probe, maxprobe; usbd_status err; - sc = addr; - DPRINTF(("uvideo_set_format: sc=%p\n", sc)); if (sc->sc_dying) return EIO; - vs = sc->sc_stream_in; - uvfmt = uvideo_stream_guess_format(vs, format->pixel_format, format->width, format->height); if (uvfmt == NULL) { @@ -2090,8 +2086,7 @@ uvideo_set_format(void *addr, struct video_format *format) static int uvideo_try_format(void *addr, struct video_format *format) { - struct uvideo_softc *sc = addr; - struct uvideo_stream *vs = sc->sc_stream_in; + struct uvideo_stream *vs = addr; struct uvideo_format *uvfmt; uvfmt = uvideo_stream_guess_format(vs, format->pixel_format, @@ -2106,8 +2101,7 @@ uvideo_try_format(void *addr, struct video_format *format) static int uvideo_get_framerate(void *addr, struct video_fract *fract) { - struct uvideo_softc *sc = addr; - struct uvideo_stream *vs = sc->sc_stream_in; + struct uvideo_stream *vs = addr; switch (vs->vs_frame_interval) { case 41666: /* 240 */ @@ -2149,12 +2143,9 @@ uvideo_set_framerate(void *addr, struct video_fract *fract) static int uvideo_start_transfer(void *addr) { - struct uvideo_softc *sc = addr; - struct uvideo_stream *vs; + struct uvideo_stream *vs = addr; int s, err; - /* FIXME: this function should be stream specific */ - vs = SLIST_FIRST(&sc->sc_stream_list); s = splusb(); err = uvideo_stream_start_xfer(vs); splx(s); @@ -2165,13 +2156,11 @@ uvideo_start_transfer(void *addr) static int uvideo_stop_transfer(void *addr) { - struct uvideo_softc *sc; + struct uvideo_stream *vs = addr; int err, s; - sc = addr; - s = splusb(); - err = uvideo_stream_stop_xfer(sc->sc_stream_in); + err = uvideo_stream_stop_xfer(vs); splx(s); return err; @@ -2181,15 +2170,14 @@ uvideo_stop_transfer(void *addr) static int uvideo_get_control_group(void *addr, struct video_control_group *group) { - struct uvideo_softc *sc; + struct uvideo_stream *vs = addr; + struct uvideo_softc *sc = vs->vs_parent; usb_device_request_t req; usbd_status err; uint8_t control_id, ent_id, data[16]; uint16_t len; int s; - sc = addr; - /* request setup */ switch (group->group_id) { case VIDEO_CONTROL_PANTILT_RELATIVE: @@ -2243,15 +2231,14 @@ uvideo_get_control_group(void *addr, struct video_control_group *group) static int uvideo_set_control_group(void *addr, const struct video_control_group *group) { - struct uvideo_softc *sc; + struct uvideo_stream *vs = addr; + struct uvideo_softc *sc = vs->vs_parent; usb_device_request_t req; usbd_status err; uint8_t control_id, ent_id, data[16]; /* long enough for all controls */ uint16_t len; int s; - sc = addr; - switch (group->group_id) { case VIDEO_CONTROL_PANTILT_RELATIVE: if (group->length != 4)