[virtio-dev] Re: [PATCH v2 1/1] video_video: Add the Virtio Video V4L2 driver

2020-03-16 Thread Keiichi Watanabe
> > 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

2020-03-16 Thread Anton Yakovlev

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

2020-03-16 Thread Dmitry Sepp
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

2020-03-16 Thread Gerd Hoffmann
  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