[virtio-dev] Re: [PATCH v2 1/1] video_video: Add the Virtio Video V4L2 driver
> > diff --git a/include/uapi/linux/virtio_video.h > > b/include/uapi/linux/virtio_video.h > > new file mode 100644 > > index ..0dd98a2237c6 > > --- /dev/null > > +++ b/include/uapi/linux/virtio_video.h > > @@ -0,0 +1,469 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > I suspect it's not expected to use GPL licence without exceptions in a > UAPI header file: > https://www.kernel.org/doc/html/v5.4/process/license-rules.html > > If GPL is used here, only GPL user programs can include this header > file, can't they? > So, can we use BSD licence like other virtio headers (e.g. virtio_gpu.h)? > > Note that I found this program when running > /scripts/headers_install.sh. Though it suggested to add "WITH > Linux-syscall-note", it shouldn't be the case because this header > doesn't provide syscall interface. > > Best regards, > Keiichi > > > +/* > > + * Virtio Video Device > > + * > > + * This header is BSD licensed so anyone can use the definitions > > + * to implement compatible drivers/servers: > > + * Ah, this line says this header is BSD licensed. So, the SPDX-License-Identifier above should be simply wrong. Best regards, Keiichi - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [PATCH v7] virtio-snd: add virtio sound device specification
Hi, On 16.03.2020 11:19, Gerd Hoffmann wrote: Hi, 2. Rework jack configuration structure, now it's more aligned with the HDA spec. Yep, that looks good. +\item[\field{hda_fn_nid}] indicates a functional node identifier +(see \hyperref[intro:HDA]{HDA}, section 7.1.2). How is the nid used? Since a functional group in HDA links entities of different kinds together, as an example, the linux driver can provide different groups as different PCM devices (and this well aligns with hardware case when streams with different capabilities live in different groups). This may be necessary to reassign the jack, because you need to know which streams are attached to the jacks. In addition, it will allow you to associate device controls with the stream/PCM device. In general some of the defconf and caps fields might need some clarification if and how they are used, given we just cherry-pick some items from the HDA spec ... Yes, I also thought about it. But will this lead to a direct dependence on the HDA specification? If something changes there, we should update here. Therefore, I left these elements transparent and allowed the device/driver to decide on the wise use of these fields. +\item[\field{hda_reg_caps}] indicates a pin capabilities value +(see \hyperref[intro:HDA]{HDA}, section 7.3.4.9). ... for example: If the "Presence Detect Capable" bit is set the status is available in the "connected" field. "VRef Control" is not supported and must be zero. +\item[\field{connected}] indicates the current jack connection status +(1 - connected, 0 - disconnected). cheers, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org -- Anton Yakovlev Senior Software Engineer OpenSynergy GmbH Rotherstr. 20, 10245 Berlin - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v2 1/1] video_video: Add the Virtio Video V4L2 driver
Hi Tomasz, On Freitag, 13. März 2020 12:11:51 CET Tomasz Figa wrote: > On Fri, Mar 13, 2020 at 11:27 AM Dmitry Sepp > > wrote: > > Hi Tomasz, > > > > On Freitag, 13. März 2020 11:05:35 CET Tomasz Figa wrote: > > > On Thu, Mar 12, 2020 at 12:48 PM Dmitry Sepp > > > > > > wrote: > > > > Hi Hans, > > > > > > > > One more thing: > > > > > GFP_DMA? That's unusual. I'd expect GFP_DMA32. All V4L2 drivers use > > > > > that. > > > > > > > > GFP_DMA32 had no effect for me on arm64. Probably I need to recheck. > > > > > > What's the reason to use any specific GFP flags at all? GFP_DMA(32) > > > memory in the guest would typically correspond to host pages without > > > any specific location guarantee. > > > > Typically, but not always, especially for non x86. Say, some platforms > > don't have IOMMUs for codec devices and those devices require physically > > contig low memory. We had to find a way to handle that. > > So basically your hypervisor guarantees that the guest pages inside > the GFP_DMA zone are contiguous and DMA-able on the host as well? > Given the Linux-specific aspect of GFP flags and differences in the > implementation across architectures, perhaps it would be a better idea > to use the DMA mask instead? That wouldn't currently affect vb2_dma_sg > allocations, but in that case the host decoder would have some IOMMU > anyway, right? > DMA mask has no effect for vb2_dma_sg, but GFP has. Unfortunately we need to support both of the two: low mem phys contig and low mem sg. So DMA mask cannot be an option. No, there are use-cases with obsolutely no iommus. Best regards, Dmitry. > > Best regards, > > Dmitry. > > > > > Best regards, > > > Tomasz > > > > > > > Best regards, > > > > Dmitry. > > > > > > > > On Donnerstag, 12. März 2020 11:18:26 CET Hans Verkuil wrote: > > > > > On 3/12/20 11:15 AM, Dmitry Sepp wrote: > > > > > > Hi Hans, > > > > > > > > > > > > Thank you for your great detailed review! > > > > > > > > > > > > I won't provide inline answers as your comments totally make > > > > > > sense. > > > > > > There > > > > > > is> > > > > > > > > > > > > only one thing I want to mention: > > > > > >>> + struct video_plane_format > > > > > >>> plane_format[VIRTIO_VIDEO_MAX_PLANES]; > > > > > >> > > > > > >> Why is this virtio specific? Any reason for not using > > > > > >> VIDEO_MAX_PLANES? > > > > > > > > > > > > I'd say this is because VIDEO_MAX_PLANES does not exist outside of > > > > > > the > > > > > > Linux OS, so for whatever other system we need a virtio specific > > > > > > definition. > > > > > > > > > > OK, good reason :-) > > > > > > > > > > It's probably a good thing to add a comment where > > > > > VIRTIO_VIDEO_MAX_PLANES is defined that explains this. > > > > > > > > > > Regards, > > > > > > > > > > Hans - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v7] virtio-snd: add virtio sound device specification
Hi, > 2. Rework jack configuration structure, now it's more aligned with the HDA > spec. Yep, that looks good. > +\item[\field{hda_fn_nid}] indicates a functional node identifier > +(see \hyperref[intro:HDA]{HDA}, section 7.1.2). How is the nid used? In general some of the defconf and caps fields might need some clarification if and how they are used, given we just cherry-pick some items from the HDA spec ... > +\item[\field{hda_reg_caps}] indicates a pin capabilities value > +(see \hyperref[intro:HDA]{HDA}, section 7.3.4.9). ... for example: If the "Presence Detect Capable" bit is set the status is available in the "connected" field. "VRef Control" is not supported and must be zero. > +\item[\field{connected}] indicates the current jack connection status > +(1 - connected, 0 - disconnected). cheers, Gerd - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org