[RFC] V4L HDR Architecture Proposal

[RFC] V4L HDR Architecture Proposal

[Dylan Yip]

Hi All,

We are planning to add HDR10 and HDR10+ metadata support into the V4L framework and were hoping for some feedback before we started implementation.

For context, Xilinx HDMI RX IP currently uses a AXI LITE interface where HDR metadata is obtained from a hardware FIFO. To access these packets a CPU copy is required. 
We are in the process of migrating towards a AXI MM interface where the hardware will directly write HDR metadata into memory. 
Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l subdev. This is linked to a DMA IP which utilizes the DMA engine APIs and registers itself as a video node for video data. 

HDR10 will only consist of static metadata which will come once per stream. However, HDR10+ will have dynamic metadata which can potentially come once per frame and be up to ~4000 bytes. We would like V4L architecture to be flexible to support both. 

We have 2 different proposals that we believe will work:

A. 2 video node approach (1 for video, 1 for metadata) - This will align with current v4l metadata structure (i.e. uvc) but will require our HDMI RX driver to register a subdev and device node
	a. Our HDMI RX driver will register a v4l subdev (for video data) and a metadata node
		i. Is this acceptable?
	b. Applications will qbuf/dqbuf to both video and metadata nodes for each frame

B. 1 video node approach - This will avoid mixing v4l subdev and v4l device node functionality inside HDMI RX driver but it strays from current v4l metadata architecture and also changes v4l subdev functionality
	a. We would add a "read" function to v4l subdev's
		i. This will also require us to add some "capabilities" field to subdev or be able to query for the "read" function
	b. HDMI Rx driver will register a v4l subdev with "read" function/capability
	c. Application can directly pass a buffer in the "read" function to HDMI RX subdev to obtain HDR metadata
		i. We will need to pass subdev name from application or be able to query all subdevs for this "read" capability, is this acceptable?

Please let me know your opinions on which approach is best or propose another approach if these 2 are unfit. Thanks

Best,
Dylan Yip

Re: [RFC] V4L HDR Architecture Proposal

[Hans Verkuil]

Hi Dylan,

On 1/22/20 9:13 PM, Dylan Yip wrote:
> Hi All,
> 
> We are planning to add HDR10 and HDR10+ metadata support into the V4L framework and were hoping for some feedback before we started implementation.

Nice!

> 
> For context, Xilinx HDMI RX IP currently uses a AXI LITE interface where HDR metadata is obtained from a hardware FIFO. To access these packets a CPU copy is required. 
> We are in the process of migrating towards a AXI MM interface where the hardware will directly write HDR metadata into memory. 
> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l subdev. This is linked to a DMA IP which utilizes the DMA engine APIs and registers itself as a video node for video data. 
> 
> HDR10 will only consist of static metadata which will come once per stream. However, HDR10+ will have dynamic metadata which can potentially come once per frame and be up to ~4000 bytes. We would like V4L architecture to be flexible to support both. 

The key here is the difference between Extended InfoFrames that can be long and the others, that
have a maximum size. The latter should be handled by controls, the first is more difficult.

Can you tell a bit more about how the hardware operates? Are all InfoFrames obtained through the hw
fifo, or are some stored in registers and some go through the fifo?

Does the hardware set maximum sizes for specific InfoFrames or the total size of all InfoFrames
combined? Or can it be any size?

Does it accept any InfoFrame or only specific InfoFrame types? Or is this programmable?

Regards,

	Hans

> 
> We have 2 different proposals that we believe will work:
> 
> A. 2 video node approach (1 for video, 1 for metadata) - This will align with current v4l metadata structure (i.e. uvc) but will require our HDMI RX driver to register a subdev and device node
> 	a. Our HDMI RX driver will register a v4l subdev (for video data) and a metadata node
> 		i. Is this acceptable?
> 	b. Applications will qbuf/dqbuf to both video and metadata nodes for each frame
> 
> B. 1 video node approach - This will avoid mixing v4l subdev and v4l device node functionality inside HDMI RX driver but it strays from current v4l metadata architecture and also changes v4l subdev functionality
> 	a. We would add a "read" function to v4l subdev's
> 		i. This will also require us to add some "capabilities" field to subdev or be able to query for the "read" function
> 	b. HDMI Rx driver will register a v4l subdev with "read" function/capability
> 	c. Application can directly pass a buffer in the "read" function to HDMI RX subdev to obtain HDR metadata
> 		i. We will need to pass subdev name from application or be able to query all subdevs for this "read" capability, is this acceptable?
> 
> Please let me know your opinions on which approach is best or propose another approach if these 2 are unfit. Thanks
> 
> Best,
> Dylan Yip
> 

RE: [RFC] V4L HDR Architecture Proposal

[Vishal Sagar]

Hi Hans,

Thanks for your response!

> -----Original Message-----
> From: linux-media-owner@vger.kernel.org <linux-media-
> owner@vger.kernel.org> On Behalf Of Hans Verkuil
> Sent: Thursday, January 23, 2020 6:36 PM
> To: Dylan Yip <dylany@xilinx.com>; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>; linux-media@vger.kernel.org
> Cc: Varunkumar Allagadapa <VARUNKUM@xilinx.com>; Madhurkiran
> Harikrishnan <MADHURKI@xilinx.com>; Jianqiang Chen
> <jianqian@xilinx.com>; Hyun Kwon <hyunk@xilinx.com>; Cyril Chemparathy
> <cyrilc@xilinx.com>; Vishal Sagar <vsagar@xilinx.com>; Sandip Kothari
> <sandipk@xilinx.com>; Subhransu Sekhar Prusty <sprusty@xilinx.com>
> Subject: Re: [RFC] V4L HDR Architecture Proposal
> 
> Hi Dylan,
> 
> On 1/22/20 9:13 PM, Dylan Yip wrote:
> > Hi All,
> >
> > We are planning to add HDR10 and HDR10+ metadata support into the V4L
> framework and were hoping for some feedback before we started
> implementation.
> 
> Nice!
> 
> >
> > For context, Xilinx HDMI RX IP currently uses a AXI LITE interface where
> HDR metadata is obtained from a hardware FIFO. To access these packets a
> CPU copy is required.
> > We are in the process of migrating towards a AXI MM interface where the
> hardware will directly write HDR metadata into memory.
> > Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l subdev. This
> is linked to a DMA IP which utilizes the DMA engine APIs and registers itself
> as a video node for video data.
> >
> > HDR10 will only consist of static metadata which will come once per stream.
> However, HDR10+ will have dynamic metadata which can potentially come
> once per frame and be up to ~4000 bytes. We would like V4L architecture to
> be flexible to support both.
> 
> The key here is the difference between Extended InfoFrames that can be
> long and the others, that have a maximum size. The latter should be handled
> by controls, the first is more difficult.
> 

Are you suggesting to handle static HDR via read only v4l controls in a meta video node?

> Can you tell a bit more about how the hardware operates? Are all InfoFrames
> obtained through the hw fifo, or are some stored in registers and some go
> through the fifo?
> 

In the current implementation of the HDMI Rx IP, all InfoFrames are read from a register byte by byte which has FIFO at the back.
The register is accessible by an AXI Lite interface.
The FIFO can store maximum 8 packets. Each packet is 36 bytes in size (31 bytes data and 5 bytes ECC calculated by IP). 
InfoFrames are one type of packets. 
There are other types like General Control Packet, Audio Clock Regeneration Packet, etc. referred in Table 5-8 packet types in HDMI specification v1.4b)

In future we plan on adding an AXIMM interface in the IP to handle Dynamic HDR. The tentative behavior will be as below -
The driver will provide a buffer pointer to the IP via a register. The IP will dump the infoframes's extracted data into this buffer. 
With Frame sync, IP will return the length of the buffer in the provided buffer.

> Does the hardware set maximum sizes for specific InfoFrames or the total
> size of all InfoFrames combined? Or can it be any size?
>
Hope the above info about FIFO depth for current HDMI Rx IP answers this.
 
> Does it accept any InfoFrame or only specific InfoFrame types? Or is this
> programmable?
> 

HDMI Rx IP accepts all types of InfoFrames.

Regards
Vishal Sagar

> Regards,
> 
> 	Hans
> 
> >
> > We have 2 different proposals that we believe will work:
> >
> > A. 2 video node approach (1 for video, 1 for metadata) - This will align with
> current v4l metadata structure (i.e. uvc) but will require our HDMI RX driver
> to register a subdev and device node
> > 	a. Our HDMI RX driver will register a v4l subdev (for video data) and a
> metadata node
> > 		i. Is this acceptable?
> > 	b. Applications will qbuf/dqbuf to both video and metadata nodes for
> > each frame
> >
> > B. 1 video node approach - This will avoid mixing v4l subdev and v4l device
> node functionality inside HDMI RX driver but it strays from current v4l
> metadata architecture and also changes v4l subdev functionality
> > 	a. We would add a "read" function to v4l subdev's
> > 		i. This will also require us to add some "capabilities" field to
> subdev or be able to query for the "read" function
> > 	b. HDMI Rx driver will register a v4l subdev with "read"
> function/capability
> > 	c. Application can directly pass a buffer in the "read" function to
> HDMI RX subdev to obtain HDR metadata
> > 		i. We will need to pass subdev name from application or be
> able to query all subdevs for this "read" capability, is this acceptable?
> >
> > Please let me know your opinions on which approach is best or propose
> > another approach if these 2 are unfit. Thanks
> >
> > Best,
> > Dylan Yip
> >

Re: [RFC] V4L HDR Architecture Proposal

[Hans Verkuil]

On 1/24/20 10:04 AM, Vishal Sagar wrote:
> Hi Hans,
> 
> Thanks for your response!
> 
>> -----Original Message-----
>> From: linux-media-owner@vger.kernel.org <linux-media-
>> owner@vger.kernel.org> On Behalf Of Hans Verkuil
>> Sent: Thursday, January 23, 2020 6:36 PM
>> To: Dylan Yip <dylany@xilinx.com>; Laurent Pinchart
>> <laurent.pinchart@ideasonboard.com>; linux-media@vger.kernel.org
>> Cc: Varunkumar Allagadapa <VARUNKUM@xilinx.com>; Madhurkiran
>> Harikrishnan <MADHURKI@xilinx.com>; Jianqiang Chen
>> <jianqian@xilinx.com>; Hyun Kwon <hyunk@xilinx.com>; Cyril Chemparathy
>> <cyrilc@xilinx.com>; Vishal Sagar <vsagar@xilinx.com>; Sandip Kothari
>> <sandipk@xilinx.com>; Subhransu Sekhar Prusty <sprusty@xilinx.com>
>> Subject: Re: [RFC] V4L HDR Architecture Proposal
>>
>> Hi Dylan,
>>
>> On 1/22/20 9:13 PM, Dylan Yip wrote:
>>> Hi All,
>>>
>>> We are planning to add HDR10 and HDR10+ metadata support into the V4L
>> framework and were hoping for some feedback before we started
>> implementation.
>>
>> Nice!
>>
>>>
>>> For context, Xilinx HDMI RX IP currently uses a AXI LITE interface where
>> HDR metadata is obtained from a hardware FIFO. To access these packets a
>> CPU copy is required.
>>> We are in the process of migrating towards a AXI MM interface where the
>> hardware will directly write HDR metadata into memory.
>>> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
>> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l subdev. This
>> is linked to a DMA IP which utilizes the DMA engine APIs and registers itself
>> as a video node for video data.
>>>
>>> HDR10 will only consist of static metadata which will come once per stream.
>> However, HDR10+ will have dynamic metadata which can potentially come
>> once per frame and be up to ~4000 bytes. We would like V4L architecture to
>> be flexible to support both.
>>
>> The key here is the difference between Extended InfoFrames that can be
>> long and the others, that have a maximum size. The latter should be handled
>> by controls, the first is more difficult.
>>
> 
> Are you suggesting to handle static HDR via read only v4l controls in a meta video node?

Yes. It's very suitable for that purpose.

> 
>> Can you tell a bit more about how the hardware operates? Are all InfoFrames
>> obtained through the hw fifo, or are some stored in registers and some go
>> through the fifo?
>>
> 
> In the current implementation of the HDMI Rx IP, all InfoFrames are read from a register byte by byte which has FIFO at the back.
> The register is accessible by an AXI Lite interface.
> The FIFO can store maximum 8 packets. Each packet is 36 bytes in size (31 bytes data and 5 bytes ECC calculated by IP). 
> InfoFrames are one type of packets. 

Does one packet correspond to one InfoFrame? Or are they all concatenated and hacked up
into packets for the FIFO?

This probably won't work well for large Extended InfoFrames of 4kB or more: the driver
would have to be able to read from the FIFO very quickly in order to prevent data from
being lost, right? Hence the development of the AXIMM interface referred to below.

> There are other types like General Control Packet, Audio Clock Regeneration Packet, etc. referred in Table 5-8 packet types in HDMI specification v1.4b)
> 
> In future we plan on adding an AXIMM interface in the IP to handle Dynamic HDR. The tentative behavior will be as below -
> The driver will provide a buffer pointer to the IP via a register. The IP will dump the infoframes's extracted data into this buffer. 
> With Frame sync, IP will return the length of the buffer in the provided buffer.
> 
>> Does the hardware set maximum sizes for specific InfoFrames or the total
>> size of all InfoFrames combined? Or can it be any size?
>>
> Hope the above info about FIFO depth for current HDMI Rx IP answers this.

Right, so the driver will provide the maximum size for all InfoFrames that can occur
between two video frames.

And the driver will parse the received InfoFrames.

I am strongly leaning towards using a control for the HDR10+ InfoFrame as well: it fits
well with the Request API where controls can be cleanly associated with a specific video
frame, and the amount of data isn't that large.

That said, some work in the control framework is probably needed to streamline things a
bit:

1) it should be possible to increase the size of compound controls later if new fields are
   added. This is on the TODO list already since it is desired functionality for codecs.

2) tentative, needs research first: add some sort of mechanism to mmap the control
   payload to avoid mem copies. That would make controls much more useful for large metadata.

I'm not sure when I will have time to work on that, though.

Regards,

	Hans

>  
>> Does it accept any InfoFrame or only specific InfoFrame types? Or is this
>> programmable?
>>
> 
> HDMI Rx IP accepts all types of InfoFrames.
> 
> Regards
> Vishal Sagar
> 
>> Regards,
>>
>> 	Hans
>>
>>>
>>> We have 2 different proposals that we believe will work:
>>>
>>> A. 2 video node approach (1 for video, 1 for metadata) - This will align with
>> current v4l metadata structure (i.e. uvc) but will require our HDMI RX driver
>> to register a subdev and device node
>>> 	a. Our HDMI RX driver will register a v4l subdev (for video data) and a
>> metadata node
>>> 		i. Is this acceptable?
>>> 	b. Applications will qbuf/dqbuf to both video and metadata nodes for
>>> each frame
>>>
>>> B. 1 video node approach - This will avoid mixing v4l subdev and v4l device
>> node functionality inside HDMI RX driver but it strays from current v4l
>> metadata architecture and also changes v4l subdev functionality
>>> 	a. We would add a "read" function to v4l subdev's
>>> 		i. This will also require us to add some "capabilities" field to
>> subdev or be able to query for the "read" function
>>> 	b. HDMI Rx driver will register a v4l subdev with "read"
>> function/capability
>>> 	c. Application can directly pass a buffer in the "read" function to
>> HDMI RX subdev to obtain HDR metadata
>>> 		i. We will need to pass subdev name from application or be
>> able to query all subdevs for this "read" capability, is this acceptable?
>>>
>>> Please let me know your opinions on which approach is best or propose
>>> another approach if these 2 are unfit. Thanks
>>>
>>> Best,
>>> Dylan Yip
>>>
> 

Re: [RFC] V4L HDR Architecture Proposal

[Laurent Pinchart]

Hi Hans,

On Fri, Jan 24, 2020 at 11:10:06AM +0100, Hans Verkuil wrote:
> On 1/24/20 10:04 AM, Vishal Sagar wrote:
> > On Thursday, January 23, 2020 6:36 PM, Hans Verkuil wrote:
> >> On 1/22/20 9:13 PM, Dylan Yip wrote:
> >>> Hi All,
> >>>
> >>> We are planning to add HDR10 and HDR10+ metadata support into the V4L
> >> framework and were hoping for some feedback before we started
> >> implementation.
> >>
> >> Nice!
> >>
> >>>
> >>> For context, Xilinx HDMI RX IP currently uses a AXI LITE interface where
> >>> HDR metadata is obtained from a hardware FIFO. To access these packets a
> >>> CPU copy is required.
> >>> We are in the process of migrating towards a AXI MM interface where the
> >>> hardware will directly write HDR metadata into memory.
> >>> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
> >>> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l subdev. This
> >>> is linked to a DMA IP which utilizes the DMA engine APIs and registers itself
> >>> as a video node for video data.
> >>>
> >>> HDR10 will only consist of static metadata which will come once per stream.
> >>> However, HDR10+ will have dynamic metadata which can potentially come
> >>> once per frame and be up to ~4000 bytes. We would like V4L architecture to
> >>> be flexible to support both.
> >>
> >> The key here is the difference between Extended InfoFrames that can be
> >> long and the others, that have a maximum size. The latter should be handled
> >> by controls, the first is more difficult.
> > 
> > Are you suggesting to handle static HDR via read only v4l controls in a meta video node?
> 
> Yes. It's very suitable for that purpose.
> 
> >> Can you tell a bit more about how the hardware operates? Are all InfoFrames
> >> obtained through the hw fifo, or are some stored in registers and some go
> >> through the fifo?
> > 
> > In the current implementation of the HDMI Rx IP, all InfoFrames are read from a register byte by byte which has FIFO at the back.
> > The register is accessible by an AXI Lite interface.
> > The FIFO can store maximum 8 packets. Each packet is 36 bytes in size (31 bytes data and 5 bytes ECC calculated by IP). 
> > InfoFrames are one type of packets. 
> 
> Does one packet correspond to one InfoFrame? Or are they all concatenated and hacked up
> into packets for the FIFO?
> 
> This probably won't work well for large Extended InfoFrames of 4kB or more: the driver
> would have to be able to read from the FIFO very quickly in order to prevent data from
> being lost, right? Hence the development of the AXIMM interface referred to below.
> 
> > There are other types like General Control Packet, Audio Clock Regeneration Packet, etc. referred in Table 5-8 packet types in HDMI specification v1.4b)
> > 
> > In future we plan on adding an AXIMM interface in the IP to handle Dynamic HDR. The tentative behavior will be as below -
> > The driver will provide a buffer pointer to the IP via a register. The IP will dump the infoframes's extracted data into this buffer. 
> > With Frame sync, IP will return the length of the buffer in the provided buffer.
> > 
> >> Does the hardware set maximum sizes for specific InfoFrames or the total
> >> size of all InfoFrames combined? Or can it be any size?
> >>
> > Hope the above info about FIFO depth for current HDMI Rx IP answers this.
> 
> Right, so the driver will provide the maximum size for all InfoFrames that can occur
> between two video frames.
> 
> And the driver will parse the received InfoFrames.
> 
> I am strongly leaning towards using a control for the HDR10+ InfoFrame as well: it fits
> well with the Request API where controls can be cleanly associated with a specific video
> frame, and the amount of data isn't that large.

This however leads me to a simple question: why do we have a metadata
API in the first place if everything should go through controls ?

> That said, some work in the control framework is probably needed to streamline things a
> bit:
> 
> 1) it should be possible to increase the size of compound controls later if new fields are
>    added. This is on the TODO list already since it is desired functionality for codecs.
> 
> 2) tentative, needs research first: add some sort of mechanism to mmap the control
>    payload to avoid mem copies. That would make controls much more useful for large metadata.

Let's not forget that we would then also need to mmap the control per
request, which will become challenging if we want to be able to pre-map
everything like we do for buffers instead of mapping and unmapping for
every request.

> I'm not sure when I will have time to work on that, though.
> 
> >> Does it accept any InfoFrame or only specific InfoFrame types? Or is this
> >> programmable?
> > 
> > HDMI Rx IP accepts all types of InfoFrames.
> > 
> >>> We have 2 different proposals that we believe will work:
> >>>
> >>> A. 2 video node approach (1 for video, 1 for metadata) - This will align with
> >>> current v4l metadata structure (i.e. uvc) but will require our HDMI RX driver
> >>> to register a subdev and device node
> >>> 	a. Our HDMI RX driver will register a v4l subdev (for video data) and a
> >>> metadata node
> >>> 		i. Is this acceptable?
> >>> 	b. Applications will qbuf/dqbuf to both video and metadata nodes for
> >>> each frame
> >>>
> >>> B. 1 video node approach - This will avoid mixing v4l subdev and v4l device
> >>> node functionality inside HDMI RX driver but it strays from current v4l
> >>> metadata architecture and also changes v4l subdev functionality
> >>> 	a. We would add a "read" function to v4l subdev's
> >>> 		i. This will also require us to add some "capabilities" field to
> >>> subdev or be able to query for the "read" function
> >>> 	b. HDMI Rx driver will register a v4l subdev with "read"
> >>> function/capability
> >>> 	c. Application can directly pass a buffer in the "read" function to
> >>> HDMI RX subdev to obtain HDR metadata
> >>> 		i. We will need to pass subdev name from application or be
> >>> able to query all subdevs for this "read" capability, is this acceptable?
> >>>
> >>> Please let me know your opinions on which approach is best or propose
> >>> another approach if these 2 are unfit. Thanks

-- 
Regards,

Laurent Pinchart

RE: [RFC] V4L HDR Architecture Proposal

[Dylan Yip]

Hi Laurent, Hans, 

Thanks for the insights!

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Friday, January 24, 2020 4:09 AM
> To: Hans Verkuil <hverkuil@xs4all.nl>
> Cc: Vishal Sagar <vsagar@xilinx.com>; Dylan Yip <dylany@xilinx.com>; linux-
> media@vger.kernel.org; Varunkumar Allagadapa
> <VARUNKUM@xilinx.com>; Madhurkiran Harikrishnan
> <MADHURKI@xilinx.com>; Jianqiang Chen <jianqian@xilinx.com>; Hyun
> Kwon <hyunk@xilinx.com>; Cyril Chemparathy <cyrilc@xilinx.com>; Sandip
> Kothari <sandipk@xilinx.com>; Subhransu Sekhar Prusty
> <sprusty@xilinx.com>; Anil Kumar Chimbeti <anilchc@xilinx.com>
> Subject: Re: [RFC] V4L HDR Architecture Proposal
> 
> Hi Hans,
> 
> On Fri, Jan 24, 2020 at 11:10:06AM +0100, Hans Verkuil wrote:
> > On 1/24/20 10:04 AM, Vishal Sagar wrote:
> > > On Thursday, January 23, 2020 6:36 PM, Hans Verkuil wrote:
> > >> On 1/22/20 9:13 PM, Dylan Yip wrote:
> > >>> Hi All,
> > >>>
> > >>> We are planning to add HDR10 and HDR10+ metadata support into the
> > >>> V4L
> > >> framework and were hoping for some feedback before we started
> > >> implementation.
> > >>
> > >> Nice!
> > >>
> > >>>
> > >>> For context, Xilinx HDMI RX IP currently uses a AXI LITE interface
> > >>> where HDR metadata is obtained from a hardware FIFO. To access
> > >>> these packets a CPU copy is required.
> > >>> We are in the process of migrating towards a AXI MM interface
> > >>> where the hardware will directly write HDR metadata into memory.
> > >>> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
> > >>> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l
> > >>> subdev. This is linked to a DMA IP which utilizes the DMA engine
> > >>> APIs and registers itself as a video node for video data.
> > >>>
> > >>> HDR10 will only consist of static metadata which will come once per
> stream.
> > >>> However, HDR10+ will have dynamic metadata which can potentially
> > >>> come once per frame and be up to ~4000 bytes. We would like V4L
> > >>> architecture to be flexible to support both.
> > >>
> > >> The key here is the difference between Extended InfoFrames that can
> > >> be long and the others, that have a maximum size. The latter should
> > >> be handled by controls, the first is more difficult.
> > >
> > > Are you suggesting to handle static HDR via read only v4l controls in a
> meta video node?
> >
> > Yes. It's very suitable for that purpose.

So are you saying we should create a separate metadata node and add the v4l control there or would we add the v4l control to the existing video data node? If it is the former, what's the point of creating the metadata node since we won't qbuf/dqbuf to it? 

Best,
Dylan Yip

> >
> > >> Can you tell a bit more about how the hardware operates? Are all
> > >> InfoFrames obtained through the hw fifo, or are some stored in
> > >> registers and some go through the fifo?
> > >
> > > In the current implementation of the HDMI Rx IP, all InfoFrames are read
> from a register byte by byte which has FIFO at the back.
> > > The register is accessible by an AXI Lite interface.
> > > The FIFO can store maximum 8 packets. Each packet is 36 bytes in size (31
> bytes data and 5 bytes ECC calculated by IP).
> > > InfoFrames are one type of packets.
> >
> > Does one packet correspond to one InfoFrame? Or are they all
> > concatenated and hacked up into packets for the FIFO?
> >
> > This probably won't work well for large Extended InfoFrames of 4kB or
> > more: the driver would have to be able to read from the FIFO very
> > quickly in order to prevent data from being lost, right? Hence the
> development of the AXIMM interface referred to below.
> >
> > > There are other types like General Control Packet, Audio Clock
> > > Regeneration Packet, etc. referred in Table 5-8 packet types in HDMI
> > > specification v1.4b)
> > >
> > > In future we plan on adding an AXIMM interface in the IP to handle
> > > Dynamic HDR. The tentative behavior will be as below - The driver will
> provide a buffer pointer to the IP via a register. The IP will dump the
> infoframes's extracted data into this buffer.
> > > With Frame sync, IP will return the length of the buffer in the provided
> buffer.
> > >
> > >> Does the hardware set maximum sizes for specific InfoFrames or the
> > >> total size of all InfoFrames combined? Or can it be any size?
> > >>
> > > Hope the above info about FIFO depth for current HDMI Rx IP answers
> this.
> >
> > Right, so the driver will provide the maximum size for all InfoFrames
> > that can occur between two video frames.
> >
> > And the driver will parse the received InfoFrames.
> >
> > I am strongly leaning towards using a control for the HDR10+ InfoFrame
> > as well: it fits well with the Request API where controls can be
> > cleanly associated with a specific video frame, and the amount of data isn't
> that large.
> 
> This however leads me to a simple question: why do we have a metadata API
> in the first place if everything should go through controls ?
> 
> > That said, some work in the control framework is probably needed to
> > streamline things a
> > bit:
> >
> > 1) it should be possible to increase the size of compound controls later if
> new fields are
> >    added. This is on the TODO list already since it is desired functionality for
> codecs.
> >
> > 2) tentative, needs research first: add some sort of mechanism to mmap
> the control
> >    payload to avoid mem copies. That would make controls much more
> useful for large metadata.
> 
> Let's not forget that we would then also need to mmap the control per
> request, which will become challenging if we want to be able to pre-map
> everything like we do for buffers instead of mapping and unmapping for
> every request.
> 
> > I'm not sure when I will have time to work on that, though.
> >
> > >> Does it accept any InfoFrame or only specific InfoFrame types? Or
> > >> is this programmable?
> > >
> > > HDMI Rx IP accepts all types of InfoFrames.
> > >
> > >>> We have 2 different proposals that we believe will work:
> > >>>
> > >>> A. 2 video node approach (1 for video, 1 for metadata) - This will
> > >>> align with current v4l metadata structure (i.e. uvc) but will
> > >>> require our HDMI RX driver to register a subdev and device node
> > >>> 	a. Our HDMI RX driver will register a v4l subdev (for video data)
> > >>> and a metadata node
> > >>> 		i. Is this acceptable?
> > >>> 	b. Applications will qbuf/dqbuf to both video and metadata nodes
> > >>> for each frame
> > >>>
> > >>> B. 1 video node approach - This will avoid mixing v4l subdev and
> > >>> v4l device node functionality inside HDMI RX driver but it strays
> > >>> from current v4l metadata architecture and also changes v4l subdev
> functionality
> > >>> 	a. We would add a "read" function to v4l subdev's
> > >>> 		i. This will also require us to add some "capabilities" field to
> > >>> subdev or be able to query for the "read" function
> > >>> 	b. HDMI Rx driver will register a v4l subdev with "read"
> > >>> function/capability
> > >>> 	c. Application can directly pass a buffer in the "read" function
> > >>> to HDMI RX subdev to obtain HDR metadata
> > >>> 		i. We will need to pass subdev name from application or be
> able
> > >>> to query all subdevs for this "read" capability, is this acceptable?
> > >>>
> > >>> Please let me know your opinions on which approach is best or
> > >>> propose another approach if these 2 are unfit. Thanks
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [RFC] V4L HDR Architecture Proposal

[Hans Verkuil]

On 1/29/20 7:14 AM, Dylan Yip wrote:
> Hi Laurent, Hans, 
> 
> Thanks for the insights!
> 
>> -----Original Message-----
>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Sent: Friday, January 24, 2020 4:09 AM
>> To: Hans Verkuil <hverkuil@xs4all.nl>
>> Cc: Vishal Sagar <vsagar@xilinx.com>; Dylan Yip <dylany@xilinx.com>; linux-
>> media@vger.kernel.org; Varunkumar Allagadapa
>> <VARUNKUM@xilinx.com>; Madhurkiran Harikrishnan
>> <MADHURKI@xilinx.com>; Jianqiang Chen <jianqian@xilinx.com>; Hyun
>> Kwon <hyunk@xilinx.com>; Cyril Chemparathy <cyrilc@xilinx.com>; Sandip
>> Kothari <sandipk@xilinx.com>; Subhransu Sekhar Prusty
>> <sprusty@xilinx.com>; Anil Kumar Chimbeti <anilchc@xilinx.com>
>> Subject: Re: [RFC] V4L HDR Architecture Proposal
>>
>> Hi Hans,
>>
>> On Fri, Jan 24, 2020 at 11:10:06AM +0100, Hans Verkuil wrote:
>>> On 1/24/20 10:04 AM, Vishal Sagar wrote:
>>>> On Thursday, January 23, 2020 6:36 PM, Hans Verkuil wrote:
>>>>> On 1/22/20 9:13 PM, Dylan Yip wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> We are planning to add HDR10 and HDR10+ metadata support into the
>>>>>> V4L
>>>>> framework and were hoping for some feedback before we started
>>>>> implementation.
>>>>>
>>>>> Nice!
>>>>>
>>>>>>
>>>>>> For context, Xilinx HDMI RX IP currently uses a AXI LITE interface
>>>>>> where HDR metadata is obtained from a hardware FIFO. To access
>>>>>> these packets a CPU copy is required.
>>>>>> We are in the process of migrating towards a AXI MM interface
>>>>>> where the hardware will directly write HDR metadata into memory.
>>>>>> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
>>>>>> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l
>>>>>> subdev. This is linked to a DMA IP which utilizes the DMA engine
>>>>>> APIs and registers itself as a video node for video data.
>>>>>>
>>>>>> HDR10 will only consist of static metadata which will come once per
>> stream.
>>>>>> However, HDR10+ will have dynamic metadata which can potentially
>>>>>> come once per frame and be up to ~4000 bytes. We would like V4L
>>>>>> architecture to be flexible to support both.
>>>>>
>>>>> The key here is the difference between Extended InfoFrames that can
>>>>> be long and the others, that have a maximum size. The latter should
>>>>> be handled by controls, the first is more difficult.
>>>>
>>>> Are you suggesting to handle static HDR via read only v4l controls in a
>> meta video node?
>>>
>>> Yes. It's very suitable for that purpose.
> 
> So are you saying we should create a separate metadata node and add the v4l control there or would we add the v4l control to the existing video data node? If it is the former, what's the point of creating the metadata node since we won't qbuf/dqbuf to it? 

I'm sorry, I misread your original question. Static HDR should be handled via read only
v4l controls in the existing video node, not in a meta video node.

Regards,

	Hans

> 
> Best,
> Dylan Yip
> 
>>>
>>>>> Can you tell a bit more about how the hardware operates? Are all
>>>>> InfoFrames obtained through the hw fifo, or are some stored in
>>>>> registers and some go through the fifo?
>>>>
>>>> In the current implementation of the HDMI Rx IP, all InfoFrames are read
>> from a register byte by byte which has FIFO at the back.
>>>> The register is accessible by an AXI Lite interface.
>>>> The FIFO can store maximum 8 packets. Each packet is 36 bytes in size (31
>> bytes data and 5 bytes ECC calculated by IP).
>>>> InfoFrames are one type of packets.
>>>
>>> Does one packet correspond to one InfoFrame? Or are they all
>>> concatenated and hacked up into packets for the FIFO?
>>>
>>> This probably won't work well for large Extended InfoFrames of 4kB or
>>> more: the driver would have to be able to read from the FIFO very
>>> quickly in order to prevent data from being lost, right? Hence the
>> development of the AXIMM interface referred to below.
>>>
>>>> There are other types like General Control Packet, Audio Clock
>>>> Regeneration Packet, etc. referred in Table 5-8 packet types in HDMI
>>>> specification v1.4b)
>>>>
>>>> In future we plan on adding an AXIMM interface in the IP to handle
>>>> Dynamic HDR. The tentative behavior will be as below - The driver will
>> provide a buffer pointer to the IP via a register. The IP will dump the
>> infoframes's extracted data into this buffer.
>>>> With Frame sync, IP will return the length of the buffer in the provided
>> buffer.
>>>>
>>>>> Does the hardware set maximum sizes for specific InfoFrames or the
>>>>> total size of all InfoFrames combined? Or can it be any size?
>>>>>
>>>> Hope the above info about FIFO depth for current HDMI Rx IP answers
>> this.
>>>
>>> Right, so the driver will provide the maximum size for all InfoFrames
>>> that can occur between two video frames.
>>>
>>> And the driver will parse the received InfoFrames.
>>>
>>> I am strongly leaning towards using a control for the HDR10+ InfoFrame
>>> as well: it fits well with the Request API where controls can be
>>> cleanly associated with a specific video frame, and the amount of data isn't
>> that large.
>>
>> This however leads me to a simple question: why do we have a metadata API
>> in the first place if everything should go through controls ?
>>
>>> That said, some work in the control framework is probably needed to
>>> streamline things a
>>> bit:
>>>
>>> 1) it should be possible to increase the size of compound controls later if
>> new fields are
>>>    added. This is on the TODO list already since it is desired functionality for
>> codecs.
>>>
>>> 2) tentative, needs research first: add some sort of mechanism to mmap
>> the control
>>>    payload to avoid mem copies. That would make controls much more
>> useful for large metadata.
>>
>> Let's not forget that we would then also need to mmap the control per
>> request, which will become challenging if we want to be able to pre-map
>> everything like we do for buffers instead of mapping and unmapping for
>> every request.
>>
>>> I'm not sure when I will have time to work on that, though.
>>>
>>>>> Does it accept any InfoFrame or only specific InfoFrame types? Or
>>>>> is this programmable?
>>>>
>>>> HDMI Rx IP accepts all types of InfoFrames.
>>>>
>>>>>> We have 2 different proposals that we believe will work:
>>>>>>
>>>>>> A. 2 video node approach (1 for video, 1 for metadata) - This will
>>>>>> align with current v4l metadata structure (i.e. uvc) but will
>>>>>> require our HDMI RX driver to register a subdev and device node
>>>>>> 	a. Our HDMI RX driver will register a v4l subdev (for video data)
>>>>>> and a metadata node
>>>>>> 		i. Is this acceptable?
>>>>>> 	b. Applications will qbuf/dqbuf to both video and metadata nodes
>>>>>> for each frame
>>>>>>
>>>>>> B. 1 video node approach - This will avoid mixing v4l subdev and
>>>>>> v4l device node functionality inside HDMI RX driver but it strays
>>>>>> from current v4l metadata architecture and also changes v4l subdev
>> functionality
>>>>>> 	a. We would add a "read" function to v4l subdev's
>>>>>> 		i. This will also require us to add some "capabilities" field to
>>>>>> subdev or be able to query for the "read" function
>>>>>> 	b. HDMI Rx driver will register a v4l subdev with "read"
>>>>>> function/capability
>>>>>> 	c. Application can directly pass a buffer in the "read" function
>>>>>> to HDMI RX subdev to obtain HDR metadata
>>>>>> 		i. We will need to pass subdev name from application or be
>> able
>>>>>> to query all subdevs for this "read" capability, is this acceptable?
>>>>>>
>>>>>> Please let me know your opinions on which approach is best or
>>>>>> propose another approach if these 2 are unfit. Thanks
>>
>> --
>> Regards,
>>
>> Laurent Pinchart

RE: [RFC] V4L HDR Architecture Proposal

[Dylan Yip]

Hi Hans,

> -----Original Message-----
> From: linux-media-owner@vger.kernel.org <linux-media-
> owner@vger.kernel.org> On Behalf Of Hans Verkuil
> Sent: Tuesday, January 28, 2020 11:31 PM
> To: Dylan Yip <dylany@xilinx.com>; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>
> Cc: Vishal Sagar <vsagar@xilinx.com>; linux-media@vger.kernel.org;
> Varunkumar Allagadapa <VARUNKUM@xilinx.com>; Madhurkiran
> Harikrishnan <MADHURKI@xilinx.com>; Jianqiang Chen
> <jianqian@xilinx.com>; Hyun Kwon <hyunk@xilinx.com>; Cyril Chemparathy
> <cyrilc@xilinx.com>; Sandip Kothari <sandipk@xilinx.com>; Subhransu
> Sekhar Prusty <sprusty@xilinx.com>; Anil Kumar Chimbeti
> <anilchc@xilinx.com>
> Subject: Re: [RFC] V4L HDR Architecture Proposal
> 
> On 1/29/20 7:14 AM, Dylan Yip wrote:
> > Hi Laurent, Hans,
> >
> > Thanks for the insights!
> >
> >> -----Original Message-----
> >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Sent: Friday, January 24, 2020 4:09 AM
> >> To: Hans Verkuil <hverkuil@xs4all.nl>
> >> Cc: Vishal Sagar <vsagar@xilinx.com>; Dylan Yip <dylany@xilinx.com>;
> >> linux- media@vger.kernel.org; Varunkumar Allagadapa
> >> <VARUNKUM@xilinx.com>; Madhurkiran Harikrishnan
> >> <MADHURKI@xilinx.com>; Jianqiang Chen <jianqian@xilinx.com>; Hyun
> >> Kwon <hyunk@xilinx.com>; Cyril Chemparathy <cyrilc@xilinx.com>;
> >> Sandip Kothari <sandipk@xilinx.com>; Subhransu Sekhar Prusty
> >> <sprusty@xilinx.com>; Anil Kumar Chimbeti <anilchc@xilinx.com>
> >> Subject: Re: [RFC] V4L HDR Architecture Proposal
> >>
> >> Hi Hans,
> >>
> >> On Fri, Jan 24, 2020 at 11:10:06AM +0100, Hans Verkuil wrote:
> >>> On 1/24/20 10:04 AM, Vishal Sagar wrote:
> >>>> On Thursday, January 23, 2020 6:36 PM, Hans Verkuil wrote:
> >>>>> On 1/22/20 9:13 PM, Dylan Yip wrote:
> >>>>>> Hi All,
> >>>>>>
> >>>>>> We are planning to add HDR10 and HDR10+ metadata support into
> the
> >>>>>> V4L
> >>>>> framework and were hoping for some feedback before we started
> >>>>> implementation.
> >>>>>
> >>>>> Nice!
> >>>>>
> >>>>>>
> >>>>>> For context, Xilinx HDMI RX IP currently uses a AXI LITE
> >>>>>> interface where HDR metadata is obtained from a hardware FIFO. To
> >>>>>> access these packets a CPU copy is required.
> >>>>>> We are in the process of migrating towards a AXI MM interface
> >>>>>> where the hardware will directly write HDR metadata into memory.
> >>>>>> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
> >>>>>> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l
> >>>>>> subdev. This is linked to a DMA IP which utilizes the DMA engine
> >>>>>> APIs and registers itself as a video node for video data.
> >>>>>>
> >>>>>> HDR10 will only consist of static metadata which will come once
> >>>>>> per
> >> stream.
> >>>>>> However, HDR10+ will have dynamic metadata which can potentially
> >>>>>> come once per frame and be up to ~4000 bytes. We would like V4L
> >>>>>> architecture to be flexible to support both.
> >>>>>
> >>>>> The key here is the difference between Extended InfoFrames that
> >>>>> can be long and the others, that have a maximum size. The latter
> >>>>> should be handled by controls, the first is more difficult.
> >>>>
> >>>> Are you suggesting to handle static HDR via read only v4l controls
> >>>> in a
> >> meta video node?
> >>>
> >>> Yes. It's very suitable for that purpose.
> >
> > So are you saying we should create a separate metadata node and add the
> v4l control there or would we add the v4l control to the existing video data
> node? If it is the former, what's the point of creating the metadata node
> since we won't qbuf/dqbuf to it?
> 
> I'm sorry, I misread your original question. Static HDR should be handled via
> read only v4l controls in the existing video node, not in a meta video node.
> 

Ah thanks for the clarification.

> Regards,
> 
> 	Hans
> 
> >
> > Best,
> > Dylan Yip
> >
> >>>
> >>>>> Can you tell a bit more about how the hardware operates? Are all
> >>>>> InfoFrames obtained through the hw fifo, or are some stored in
> >>>>> registers and some go through the fifo?
> >>>>
> >>>> In the current implementation of the HDMI Rx IP, all InfoFrames are
> >>>> read
> >> from a register byte by byte which has FIFO at the back.
> >>>> The register is accessible by an AXI Lite interface.
> >>>> The FIFO can store maximum 8 packets. Each packet is 36 bytes in
> >>>> size (31
> >> bytes data and 5 bytes ECC calculated by IP).
> >>>> InfoFrames are one type of packets.
> >>>
> >>> Does one packet correspond to one InfoFrame? Or are they all
> >>> concatenated and hacked up into packets for the FIFO?
> >>>
> >>> This probably won't work well for large Extended InfoFrames of 4kB
> >>> or
> >>> more: the driver would have to be able to read from the FIFO very
> >>> quickly in order to prevent data from being lost, right? Hence the
> >> development of the AXIMM interface referred to below.
> >>>
> >>>> There are other types like General Control Packet, Audio Clock
> >>>> Regeneration Packet, etc. referred in Table 5-8 packet types in
> >>>> HDMI specification v1.4b)
> >>>>
> >>>> In future we plan on adding an AXIMM interface in the IP to handle
> >>>> Dynamic HDR. The tentative behavior will be as below - The driver
> >>>> will
> >> provide a buffer pointer to the IP via a register. The IP will dump
> >> the infoframes's extracted data into this buffer.
> >>>> With Frame sync, IP will return the length of the buffer in the
> >>>> provided
> >> buffer.
> >>>>
> >>>>> Does the hardware set maximum sizes for specific InfoFrames or the
> >>>>> total size of all InfoFrames combined? Or can it be any size?
> >>>>>
> >>>> Hope the above info about FIFO depth for current HDMI Rx IP answers
> >> this.
> >>>
> >>> Right, so the driver will provide the maximum size for all
> >>> InfoFrames that can occur between two video frames.
> >>>
> >>> And the driver will parse the received InfoFrames.
> >>>
> >>> I am strongly leaning towards using a control for the HDR10+
> >>> InfoFrame as well: it fits well with the Request API where controls
> >>> can be cleanly associated with a specific video frame, and the
> >>> amount of data isn't
> >> that large.
> >>
> >> This however leads me to a simple question: why do we have a metadata
> >> API in the first place if everything should go through controls ?

I have the same concern as Laurent here. Why are we supporting HDR metadata through controls but using the metadata API for other types of metadata? Wouldn't it be cleaner to follow the existing metadata API since HDR is a type of metadata?

This is why we were originally thinking that a 2 node approach (1 for video 1 for metadata) would have been cleaner. 

> >>
> >>> That said, some work in the control framework is probably needed to
> >>> streamline things a
> >>> bit: 
> >>>
> >>> 1) it should be possible to increase the size of compound controls
> >>> later if
> >> new fields are
> >>>    added. This is on the TODO list already since it is desired
> >>> functionality for
> >> codecs.
> >>>
> >>> 2) tentative, needs research first: add some sort of mechanism to
> >>> mmap
> >> the control
> >>>    payload to avoid mem copies. That would make controls much more
> >> useful for large metadata.
> >>
> >> Let's not forget that we would then also need to mmap the control per
> >> request, which will become challenging if we want to be able to
> >> pre-map everything like we do for buffers instead of mapping and
> >> unmapping for every request.

Same concern here. If we want to pre-map everything like buffers, wouldn't we essentially be replicating the behavior of buffers. Then the only difference would be that we are doing g_ctrl instead of qbuf/dqbuf right?

Best,
Dylan Yip

> >>
> >>> I'm not sure when I will have time to work on that, though.
> >>>
> >>>>> Does it accept any InfoFrame or only specific InfoFrame types? Or
> >>>>> is this programmable?
> >>>>
> >>>> HDMI Rx IP accepts all types of InfoFrames.
> >>>>
> >>>>>> We have 2 different proposals that we believe will work:
> >>>>>>
> >>>>>> A. 2 video node approach (1 for video, 1 for metadata) - This
> >>>>>> will align with current v4l metadata structure (i.e. uvc) but
> >>>>>> will require our HDMI RX driver to register a subdev and device node
> >>>>>> 	a. Our HDMI RX driver will register a v4l subdev (for video
> >>>>>> data) and a metadata node
> >>>>>> 		i. Is this acceptable?
> >>>>>> 	b. Applications will qbuf/dqbuf to both video and metadata
> nodes
> >>>>>> for each frame
> >>>>>>
> >>>>>> B. 1 video node approach - This will avoid mixing v4l subdev and
> >>>>>> v4l device node functionality inside HDMI RX driver but it strays
> >>>>>> from current v4l metadata architecture and also changes v4l
> >>>>>> subdev
> >> functionality
> >>>>>> 	a. We would add a "read" function to v4l subdev's
> >>>>>> 		i. This will also require us to add some "capabilities"
> field
> >>>>>> to subdev or be able to query for the "read" function
> >>>>>> 	b. HDMI Rx driver will register a v4l subdev with "read"
> >>>>>> function/capability
> >>>>>> 	c. Application can directly pass a buffer in the "read" function
> >>>>>> to HDMI RX subdev to obtain HDR metadata
> >>>>>> 		i. We will need to pass subdev name from application
> or be
> >> able
> >>>>>> to query all subdevs for this "read" capability, is this acceptable?
> >>>>>>
> >>>>>> Please let me know your opinions on which approach is best or
> >>>>>> propose another approach if these 2 are unfit. Thanks
> >>
> >> --
> >> Regards,
> >>
> >> Laurent Pinchart

Re: [RFC] V4L HDR Architecture Proposal

[Laurent Pinchart]

Hi Hans,

On Thu, Jan 30, 2020 at 07:00:12AM +0000, Dylan Yip wrote:
> On Tuesday, January 28, 2020 11:31 PM, Hans Verkuil wrote:
> > On 1/29/20 7:14 AM, Dylan Yip wrote:
> >> On Friday, January 24, 2020 4:09 AM, Laurent Pinchart wrote:
> >>> On Fri, Jan 24, 2020 at 11:10:06AM +0100, Hans Verkuil wrote:
> >>>> On 1/24/20 10:04 AM, Vishal Sagar wrote:
> >>>>> On Thursday, January 23, 2020 6:36 PM, Hans Verkuil wrote:
> >>>>>> On 1/22/20 9:13 PM, Dylan Yip wrote:
> >>>>>>> Hi All,
> >>>>>>>
> >>>>>>> We are planning to add HDR10 and HDR10+ metadata support into the
> >>>>>>> V4L framework and were hoping for some feedback before we started
> >>>>>>> implementation.
> >>>>>>
> >>>>>> Nice!
> >>>>>>
> >>>>>>> For context, Xilinx HDMI RX IP currently uses a AXI LITE
> >>>>>>> interface where HDR metadata is obtained from a hardware FIFO. To
> >>>>>>> access these packets a CPU copy is required.
> >>>>>>> We are in the process of migrating towards a AXI MM interface
> >>>>>>> where the hardware will directly write HDR metadata into memory.
> >>>>>>> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
> >>>>>>> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l
> >>>>>>> subdev. This is linked to a DMA IP which utilizes the DMA engine
> >>>>>>> APIs and registers itself as a video node for video data.
> >>>>>>>
> >>>>>>> HDR10 will only consist of static metadata which will come once
> >>>>>>> per stream.
> >>>>>>> However, HDR10+ will have dynamic metadata which can potentially
> >>>>>>> come once per frame and be up to ~4000 bytes. We would like V4L
> >>>>>>> architecture to be flexible to support both.
> >>>>>>
> >>>>>> The key here is the difference between Extended InfoFrames that
> >>>>>> can be long and the others, that have a maximum size. The latter
> >>>>>> should be handled by controls, the first is more difficult.
> >>>>>
> >>>>> Are you suggesting to handle static HDR via read only v4l controls
> >>>>> in a meta video node?
> >>>>
> >>>> Yes. It's very suitable for that purpose.
> >>
> >> So are you saying we should create a separate metadata node and add the
> >> v4l control there or would we add the v4l control to the existing video data
> >> node? If it is the former, what's the point of creating the metadata node
> >> since we won't qbuf/dqbuf to it?
> > 
> > I'm sorry, I misread your original question. Static HDR should be handled via
> > read only v4l controls in the existing video node, not in a meta video node.
> 
> Ah thanks for the clarification.
>
> >>>>>> Can you tell a bit more about how the hardware operates? Are all
> >>>>>> InfoFrames obtained through the hw fifo, or are some stored in
> >>>>>> registers and some go through the fifo?
> >>>>>
> >>>>> In the current implementation of the HDMI Rx IP, all InfoFrames are
> >>>>> read from a register byte by byte which has FIFO at the back.
> >>>>> The register is accessible by an AXI Lite interface.
> >>>>> The FIFO can store maximum 8 packets. Each packet is 36 bytes in
> >>>>> size (31 bytes data and 5 bytes ECC calculated by IP).
> >>>>> InfoFrames are one type of packets.
> >>>>
> >>>> Does one packet correspond to one InfoFrame? Or are they all
> >>>> concatenated and hacked up into packets for the FIFO?
> >>>>
> >>>> This probably won't work well for large Extended InfoFrames of 4kB
> >>>> or more: the driver would have to be able to read from the FIFO very
> >>>> quickly in order to prevent data from being lost, right? Hence the
> >>>> development of the AXIMM interface referred to below.
> >>>>
> >>>>> There are other types like General Control Packet, Audio Clock
> >>>>> Regeneration Packet, etc. referred in Table 5-8 packet types in
> >>>>> HDMI specification v1.4b)
> >>>>>
> >>>>> In future we plan on adding an AXIMM interface in the IP to handle
> >>>>> Dynamic HDR. The tentative behavior will be as below - The driver
> >>>>> will provide a buffer pointer to the IP via a register. The IP will dump
> >>>>> the infoframes's extracted data into this buffer.
> >>>>> With Frame sync, IP will return the length of the buffer in the
> >>>>> provided buffer.
> >>>>>
> >>>>>> Does the hardware set maximum sizes for specific InfoFrames or the
> >>>>>> total size of all InfoFrames combined? Or can it be any size?
> >>>>>>
> >>>>> Hope the above info about FIFO depth for current HDMI Rx IP answers
> >>>>> this.
> >>>>
> >>>> Right, so the driver will provide the maximum size for all
> >>>> InfoFrames that can occur between two video frames.
> >>>>
> >>>> And the driver will parse the received InfoFrames.
> >>>>
> >>>> I am strongly leaning towards using a control for the HDR10+
> >>>> InfoFrame as well: it fits well with the Request API where controls
> >>>> can be cleanly associated with a specific video frame, and the
> >>>> amount of data isn't that large.
> >>>
> >>> This however leads me to a simple question: why do we have a metadata
> >>> API in the first place if everything should go through controls ?
> 
> I have the same concern as Laurent here. Why are we supporting HDR
> metadata through controls but using the metadata API for other types
> of metadata? Wouldn't it be cleaner to follow the existing metadata
> API since HDR is a type of metadata?
> 
> This is why we were originally thinking that a 2 node approach (1 for
> video 1 for metadata) would have been cleaner. 

I would also like to add that the implementation of the request API is
currently not able to support inline devices (with a live source). This
could be changed, but would require a very significant amount of work if
we want to implement it properly. As we already have a metadata capture
API, I really don't see why this should go through the control
framework.

Regarding HDR10 static metadata, while I agree that controls should work
for this purpose, I think we also need to take into account API
consistency. If we use the metadata API for dynamic metadata, we will
end up with two different APIs to cary two different types of metadata.
Is this desirable, or, given that we already plan for HDR10+ support,
would implementing HDR10 support through the metadata API be a better
solution for userspace ?

> >>>> That said, some work in the control framework is probably needed to
> >>>> streamline things a bit: 
> >>>>
> >>>> 1) it should be possible to increase the size of compound controls
> >>>> later if new fields are added. This is on the TODO list already since
> >>>> it is desired functionality for codecs.
> >>>>
> >>>> 2) tentative, needs research first: add some sort of mechanism to
> >>>> mmap the control payload to avoid mem copies. That would make controls
> >>>> much more useful for large metadata.
> >>>
> >>> Let's not forget that we would then also need to mmap the control per
> >>> request, which will become challenging if we want to be able to
> >>> pre-map everything like we do for buffers instead of mapping and
> >>> unmapping for every request.
> 
> Same concern here. If we want to pre-map everything like buffers,
> wouldn't we essentially be replicating the behavior of buffers. Then
> the only difference would be that we are doing g_ctrl instead of
> qbuf/dqbuf right?

I couldn't have said that better :-)

> >>>> I'm not sure when I will have time to work on that, though.
> >>>>
> >>>>>> Does it accept any InfoFrame or only specific InfoFrame types? Or
> >>>>>> is this programmable?
> >>>>>
> >>>>> HDMI Rx IP accepts all types of InfoFrames.
> >>>>>
> >>>>>>> We have 2 different proposals that we believe will work:
> >>>>>>>
> >>>>>>> A. 2 video node approach (1 for video, 1 for metadata) - This will
> >>>>>>>    align with current v4l metadata structure (i.e. uvc) but
> >>>>>>>    will require our HDMI RX driver to register a subdev and
> >>>>>>>    device node
> >>>>>>> 	a. Our HDMI RX driver will register a v4l subdev (for
> >>>>>>> 	   video data) and a metadata node
> >>>>>>> 		i. Is this acceptable?
> >>>>>>> 	b. Applications will qbuf/dqbuf to both video and metadata nodes
> >>>>>>> 	   for each frame
> >>>>>>>
> >>>>>>> B. 1 video node approach - This will avoid mixing v4l subdev and
> >>>>>>>    v4l device node functionality inside HDMI RX driver but it
> >>>>>>>    strays from current v4l metadata architecture and also
> >>>>>>>    changes v4l subdev functionality
> >>>>>>> 	a. We would add a "read" function to v4l subdev's
> >>>>>>> 		i. This will also require us to add some "capabilities" field
> >>>>>>> 		   to subdev or be able to query for the "read" function
> >>>>>>> 	b. HDMI Rx driver will register a v4l subdev with "read"
> >>>>>>> 	   function/capability
> >>>>>>> 	c. Application can directly pass a buffer in the "read" function
> >>>>>>> 	   to HDMI RX subdev to obtain HDR metadata
> >>>>>>> 		i. We will need to pass subdev name from application or be able
> >>>>>>> 		   to query all subdevs for this "read"
> >>>>>>> 		   capability, is this acceptable?
> >>>>>>>
> >>>>>>> Please let me know your opinions on which approach is best or
> >>>>>>> propose another approach if these 2 are unfit. Thanks

-- 
Regards,

Laurent Pinchart

Re: [RFC] V4L HDR Architecture Proposal

[Hans Verkuil]

On 2/12/20 5:48 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Thu, Jan 30, 2020 at 07:00:12AM +0000, Dylan Yip wrote:
>> On Tuesday, January 28, 2020 11:31 PM, Hans Verkuil wrote:
>>> On 1/29/20 7:14 AM, Dylan Yip wrote:
>>>> On Friday, January 24, 2020 4:09 AM, Laurent Pinchart wrote:
>>>>> On Fri, Jan 24, 2020 at 11:10:06AM +0100, Hans Verkuil wrote:
>>>>>> On 1/24/20 10:04 AM, Vishal Sagar wrote:
>>>>>>> On Thursday, January 23, 2020 6:36 PM, Hans Verkuil wrote:
>>>>>>>> On 1/22/20 9:13 PM, Dylan Yip wrote:
>>>>>>>>> Hi All,
>>>>>>>>>
>>>>>>>>> We are planning to add HDR10 and HDR10+ metadata support into the
>>>>>>>>> V4L framework and were hoping for some feedback before we started
>>>>>>>>> implementation.
>>>>>>>>
>>>>>>>> Nice!
>>>>>>>>
>>>>>>>>> For context, Xilinx HDMI RX IP currently uses a AXI LITE
>>>>>>>>> interface where HDR metadata is obtained from a hardware FIFO. To
>>>>>>>>> access these packets a CPU copy is required.
>>>>>>>>> We are in the process of migrating towards a AXI MM interface
>>>>>>>>> where the hardware will directly write HDR metadata into memory.
>>>>>>>>> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
>>>>>>>>> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l
>>>>>>>>> subdev. This is linked to a DMA IP which utilizes the DMA engine
>>>>>>>>> APIs and registers itself as a video node for video data.
>>>>>>>>>
>>>>>>>>> HDR10 will only consist of static metadata which will come once
>>>>>>>>> per stream.
>>>>>>>>> However, HDR10+ will have dynamic metadata which can potentially
>>>>>>>>> come once per frame and be up to ~4000 bytes. We would like V4L
>>>>>>>>> architecture to be flexible to support both.
>>>>>>>>
>>>>>>>> The key here is the difference between Extended InfoFrames that
>>>>>>>> can be long and the others, that have a maximum size. The latter
>>>>>>>> should be handled by controls, the first is more difficult.
>>>>>>>
>>>>>>> Are you suggesting to handle static HDR via read only v4l controls
>>>>>>> in a meta video node?
>>>>>>
>>>>>> Yes. It's very suitable for that purpose.
>>>>
>>>> So are you saying we should create a separate metadata node and add the
>>>> v4l control there or would we add the v4l control to the existing video data
>>>> node? If it is the former, what's the point of creating the metadata node
>>>> since we won't qbuf/dqbuf to it?
>>>
>>> I'm sorry, I misread your original question. Static HDR should be handled via
>>> read only v4l controls in the existing video node, not in a meta video node.
>>
>> Ah thanks for the clarification.
>>
>>>>>>>> Can you tell a bit more about how the hardware operates? Are all
>>>>>>>> InfoFrames obtained through the hw fifo, or are some stored in
>>>>>>>> registers and some go through the fifo?
>>>>>>>
>>>>>>> In the current implementation of the HDMI Rx IP, all InfoFrames are
>>>>>>> read from a register byte by byte which has FIFO at the back.
>>>>>>> The register is accessible by an AXI Lite interface.
>>>>>>> The FIFO can store maximum 8 packets. Each packet is 36 bytes in
>>>>>>> size (31 bytes data and 5 bytes ECC calculated by IP).
>>>>>>> InfoFrames are one type of packets.
>>>>>>
>>>>>> Does one packet correspond to one InfoFrame? Or are they all
>>>>>> concatenated and hacked up into packets for the FIFO?
>>>>>>
>>>>>> This probably won't work well for large Extended InfoFrames of 4kB
>>>>>> or more: the driver would have to be able to read from the FIFO very
>>>>>> quickly in order to prevent data from being lost, right? Hence the
>>>>>> development of the AXIMM interface referred to below.
>>>>>>
>>>>>>> There are other types like General Control Packet, Audio Clock
>>>>>>> Regeneration Packet, etc. referred in Table 5-8 packet types in
>>>>>>> HDMI specification v1.4b)
>>>>>>>
>>>>>>> In future we plan on adding an AXIMM interface in the IP to handle
>>>>>>> Dynamic HDR. The tentative behavior will be as below - The driver
>>>>>>> will provide a buffer pointer to the IP via a register. The IP will dump
>>>>>>> the infoframes's extracted data into this buffer.
>>>>>>> With Frame sync, IP will return the length of the buffer in the
>>>>>>> provided buffer.
>>>>>>>
>>>>>>>> Does the hardware set maximum sizes for specific InfoFrames or the
>>>>>>>> total size of all InfoFrames combined? Or can it be any size?
>>>>>>>>
>>>>>>> Hope the above info about FIFO depth for current HDMI Rx IP answers
>>>>>>> this.
>>>>>>
>>>>>> Right, so the driver will provide the maximum size for all
>>>>>> InfoFrames that can occur between two video frames.
>>>>>>
>>>>>> And the driver will parse the received InfoFrames.
>>>>>>
>>>>>> I am strongly leaning towards using a control for the HDR10+
>>>>>> InfoFrame as well: it fits well with the Request API where controls
>>>>>> can be cleanly associated with a specific video frame, and the
>>>>>> amount of data isn't that large.
>>>>>
>>>>> This however leads me to a simple question: why do we have a metadata
>>>>> API in the first place if everything should go through controls ?
>>
>> I have the same concern as Laurent here. Why are we supporting HDR
>> metadata through controls but using the metadata API for other types
>> of metadata? Wouldn't it be cleaner to follow the existing metadata
>> API since HDR is a type of metadata?

There is very little need to expose InfoFrames to userspace: most of the
current InfoFrame data is consumed by the video capture driver. That's
how it is done in all existing HDMI video receivers. Any relevant data
for userspace such as colorspace information is exposed via V4L2 data
structures.

There are only a few InfoFrames whose data needs to be exposed to userspace:
the SPD InfoFrame (not implemented today in a driver, but perfectly suited
for controls since this rarely changes), and the HDR metadata that you need.

Also, all InfoFrame hardware implementations that I have seen expose the
InfoFrame data through registers (often over an i2c bus), and that's
really not suitable for the metadata API.

Now, the Extended InfoFrame information is different beast, and there
the metadata API is a reasonable approach. Not the only approach, perhaps,
but certainly something to consider.

So supposed we support this as a metadata device node, then it is
an option to just dump all InfoFrames into the buffer. I.e., the pixel
format used to define the format of the stream would say that it consists
of all InfoFrames preceding the video frame concatenated together, and
you have to parse it in userspace to find the InfoFrame types that you
need. It is up to the driver to decide if all InfoFrames or only a subset
are copied into the buffer.

But be aware that the video driver always has to parse the data as well
in order to extract the AVI InfoFrame and a few others in order to
program the video receiver.

The only reason IMHO to dump all InfoFrames into the buffer is if that's
how the DMA engine operates. If you need to manually do this, then only
copy the Extended InfoFrames.

>>
>> This is why we were originally thinking that a 2 node approach (1 for
>> video 1 for metadata) would have been cleaner. 
> 
> I would also like to add that the implementation of the request API is
> currently not able to support inline devices (with a live source). This

For traditional non-MC drivers this is supported in the request API. Only
for MC drivers is this a problem. I don't know if this upcoming driver is
going the MC route.

> could be changed, but would require a very significant amount of work if
> we want to implement it properly. As we already have a metadata capture
> API, I really don't see why this should go through the control
> framework.
> 
> Regarding HDR10 static metadata, while I agree that controls should work
> for this purpose, I think we also need to take into account API
> consistency. If we use the metadata API for dynamic metadata, we will
> end up with two different APIs to cary two different types of metadata.
> Is this desirable, or, given that we already plan for HDR10+ support,
> would implementing HDR10 support through the metadata API be a better
> solution for userspace ?

The Dynamic Range and Mastering InfoFrame also needs to be processed by
the driver, so it makes much more sense to have the driver expose the
relevant data through one or more controls.

That does not prevent you from dumping this in the metadata buffer as
well (as described above), but you really only want to do that if that's
the most efficient way for the DMA engine. But if the buffer is non
consistent memory, then you need sync the buffer to the cpu before the
driver can read it.

So, in conclusion, I think it is OK to use a metadata device for the
Extended InfoFrames. I don't think it is a good idea to dump *all*
InfoFrames in the metadata buffer, especially if the driver has to
sync the buffer to the cpu in order to obtain the non-Extended-InfoFrame
information.

> 
>>>>>> That said, some work in the control framework is probably needed to
>>>>>> streamline things a bit: 
>>>>>>
>>>>>> 1) it should be possible to increase the size of compound controls
>>>>>> later if new fields are added. This is on the TODO list already since
>>>>>> it is desired functionality for codecs.
>>>>>>
>>>>>> 2) tentative, needs research first: add some sort of mechanism to
>>>>>> mmap the control payload to avoid mem copies. That would make controls
>>>>>> much more useful for large metadata.
>>>>>
>>>>> Let's not forget that we would then also need to mmap the control per
>>>>> request, which will become challenging if we want to be able to
>>>>> pre-map everything like we do for buffers instead of mapping and
>>>>> unmapping for every request.
>>
>> Same concern here. If we want to pre-map everything like buffers,
>> wouldn't we essentially be replicating the behavior of buffers. Then
>> the only difference would be that we are doing g_ctrl instead of
>> qbuf/dqbuf right?

Right. But qbuf/dqbuf has quite a bit of overhead as well (dealing with
multiple streaming modes, cache management), and while that is perfectly
acceptable when dealing with large buffers and DMA engines, I wonder if
a mmap-ed control for mid-sized data (say up to 32/64 kBytes) isn't more
efficient.

That said, this functionality does not exist, and it would need some
performance testing as well comparing the two. It's just a gut feeling
without any hard proof.

Regards,

	Hans

> 
> I couldn't have said that better :-)
> 
>>>>>> I'm not sure when I will have time to work on that, though.
>>>>>>
>>>>>>>> Does it accept any InfoFrame or only specific InfoFrame types? Or
>>>>>>>> is this programmable?
>>>>>>>
>>>>>>> HDMI Rx IP accepts all types of InfoFrames.
>>>>>>>
>>>>>>>>> We have 2 different proposals that we believe will work:
>>>>>>>>>
>>>>>>>>> A. 2 video node approach (1 for video, 1 for metadata) - This will
>>>>>>>>>    align with current v4l metadata structure (i.e. uvc) but
>>>>>>>>>    will require our HDMI RX driver to register a subdev and
>>>>>>>>>    device node
>>>>>>>>> 	a. Our HDMI RX driver will register a v4l subdev (for
>>>>>>>>> 	   video data) and a metadata node
>>>>>>>>> 		i. Is this acceptable?
>>>>>>>>> 	b. Applications will qbuf/dqbuf to both video and metadata nodes
>>>>>>>>> 	   for each frame
>>>>>>>>>
>>>>>>>>> B. 1 video node approach - This will avoid mixing v4l subdev and
>>>>>>>>>    v4l device node functionality inside HDMI RX driver but it
>>>>>>>>>    strays from current v4l metadata architecture and also
>>>>>>>>>    changes v4l subdev functionality
>>>>>>>>> 	a. We would add a "read" function to v4l subdev's
>>>>>>>>> 		i. This will also require us to add some "capabilities" field
>>>>>>>>> 		   to subdev or be able to query for the "read" function
>>>>>>>>> 	b. HDMI Rx driver will register a v4l subdev with "read"
>>>>>>>>> 	   function/capability
>>>>>>>>> 	c. Application can directly pass a buffer in the "read" function
>>>>>>>>> 	   to HDMI RX subdev to obtain HDR metadata
>>>>>>>>> 		i. We will need to pass subdev name from application or be able
>>>>>>>>> 		   to query all subdevs for this "read"
>>>>>>>>> 		   capability, is this acceptable?
>>>>>>>>>
>>>>>>>>> Please let me know your opinions on which approach is best or
>>>>>>>>> propose another approach if these 2 are unfit. Thanks
> 

Re: [RFC] V4L HDR Architecture Proposal

[VDRU VDRU]

Hi,

I was wondering what the current status is on this? Is there any plan
to also add Dolby Vision as well?

Thanks

On Wed, Feb 19, 2020 at 2:43 AM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 2/12/20 5:48 PM, Laurent Pinchart wrote:
> > Hi Hans,
> >
> > On Thu, Jan 30, 2020 at 07:00:12AM +0000, Dylan Yip wrote:
> >> On Tuesday, January 28, 2020 11:31 PM, Hans Verkuil wrote:
> >>> On 1/29/20 7:14 AM, Dylan Yip wrote:
> >>>> On Friday, January 24, 2020 4:09 AM, Laurent Pinchart wrote:
> >>>>> On Fri, Jan 24, 2020 at 11:10:06AM +0100, Hans Verkuil wrote:
> >>>>>> On 1/24/20 10:04 AM, Vishal Sagar wrote:
> >>>>>>> On Thursday, January 23, 2020 6:36 PM, Hans Verkuil wrote:
> >>>>>>>> On 1/22/20 9:13 PM, Dylan Yip wrote:
> >>>>>>>>> Hi All,
> >>>>>>>>>
> >>>>>>>>> We are planning to add HDR10 and HDR10+ metadata support into the
> >>>>>>>>> V4L framework and were hoping for some feedback before we started
> >>>>>>>>> implementation.
> >>>>>>>>
> >>>>>>>> Nice!
> >>>>>>>>
> >>>>>>>>> For context, Xilinx HDMI RX IP currently uses a AXI LITE
> >>>>>>>>> interface where HDR metadata is obtained from a hardware FIFO. To
> >>>>>>>>> access these packets a CPU copy is required.
> >>>>>>>>> We are in the process of migrating towards a AXI MM interface
> >>>>>>>>> where the hardware will directly write HDR metadata into memory.
> >>>>>>>>> Currently the HDMI RX driver (https://github.com/Xilinx/hdmi-
> >>>>>>>>> modules/blob/master/hdmi/xilinx-hdmirx.c) is modeled as a v4l
> >>>>>>>>> subdev. This is linked to a DMA IP which utilizes the DMA engine
> >>>>>>>>> APIs and registers itself as a video node for video data.
> >>>>>>>>>
> >>>>>>>>> HDR10 will only consist of static metadata which will come once
> >>>>>>>>> per stream.
> >>>>>>>>> However, HDR10+ will have dynamic metadata which can potentially
> >>>>>>>>> come once per frame and be up to ~4000 bytes. We would like V4L
> >>>>>>>>> architecture to be flexible to support both.
> >>>>>>>>
> >>>>>>>> The key here is the difference between Extended InfoFrames that
> >>>>>>>> can be long and the others, that have a maximum size. The latter
> >>>>>>>> should be handled by controls, the first is more difficult.
> >>>>>>>
> >>>>>>> Are you suggesting to handle static HDR via read only v4l controls
> >>>>>>> in a meta video node?
> >>>>>>
> >>>>>> Yes. It's very suitable for that purpose.
> >>>>
> >>>> So are you saying we should create a separate metadata node and add the
> >>>> v4l control there or would we add the v4l control to the existing video data
> >>>> node? If it is the former, what's the point of creating the metadata node
> >>>> since we won't qbuf/dqbuf to it?
> >>>
> >>> I'm sorry, I misread your original question. Static HDR should be handled via
> >>> read only v4l controls in the existing video node, not in a meta video node.
> >>
> >> Ah thanks for the clarification.
> >>
> >>>>>>>> Can you tell a bit more about how the hardware operates? Are all
> >>>>>>>> InfoFrames obtained through the hw fifo, or are some stored in
> >>>>>>>> registers and some go through the fifo?
> >>>>>>>
> >>>>>>> In the current implementation of the HDMI Rx IP, all InfoFrames are
> >>>>>>> read from a register byte by byte which has FIFO at the back.
> >>>>>>> The register is accessible by an AXI Lite interface.
> >>>>>>> The FIFO can store maximum 8 packets. Each packet is 36 bytes in
> >>>>>>> size (31 bytes data and 5 bytes ECC calculated by IP).
> >>>>>>> InfoFrames are one type of packets.
> >>>>>>
> >>>>>> Does one packet correspond to one InfoFrame? Or are they all
> >>>>>> concatenated and hacked up into packets for the FIFO?
> >>>>>>
> >>>>>> This probably won't work well for large Extended InfoFrames of 4kB
> >>>>>> or more: the driver would have to be able to read from the FIFO very
> >>>>>> quickly in order to prevent data from being lost, right? Hence the
> >>>>>> development of the AXIMM interface referred to below.
> >>>>>>
> >>>>>>> There are other types like General Control Packet, Audio Clock
> >>>>>>> Regeneration Packet, etc. referred in Table 5-8 packet types in
> >>>>>>> HDMI specification v1.4b)
> >>>>>>>
> >>>>>>> In future we plan on adding an AXIMM interface in the IP to handle
> >>>>>>> Dynamic HDR. The tentative behavior will be as below - The driver
> >>>>>>> will provide a buffer pointer to the IP via a register. The IP will dump
> >>>>>>> the infoframes's extracted data into this buffer.
> >>>>>>> With Frame sync, IP will return the length of the buffer in the
> >>>>>>> provided buffer.
> >>>>>>>
> >>>>>>>> Does the hardware set maximum sizes for specific InfoFrames or the
> >>>>>>>> total size of all InfoFrames combined? Or can it be any size?
> >>>>>>>>
> >>>>>>> Hope the above info about FIFO depth for current HDMI Rx IP answers
> >>>>>>> this.
> >>>>>>
> >>>>>> Right, so the driver will provide the maximum size for all
> >>>>>> InfoFrames that can occur between two video frames.
> >>>>>>
> >>>>>> And the driver will parse the received InfoFrames.
> >>>>>>
> >>>>>> I am strongly leaning towards using a control for the HDR10+
> >>>>>> InfoFrame as well: it fits well with the Request API where controls
> >>>>>> can be cleanly associated with a specific video frame, and the
> >>>>>> amount of data isn't that large.
> >>>>>
> >>>>> This however leads me to a simple question: why do we have a metadata
> >>>>> API in the first place if everything should go through controls ?
> >>
> >> I have the same concern as Laurent here. Why are we supporting HDR
> >> metadata through controls but using the metadata API for other types
> >> of metadata? Wouldn't it be cleaner to follow the existing metadata
> >> API since HDR is a type of metadata?
>
> There is very little need to expose InfoFrames to userspace: most of the
> current InfoFrame data is consumed by the video capture driver. That's
> how it is done in all existing HDMI video receivers. Any relevant data
> for userspace such as colorspace information is exposed via V4L2 data
> structures.
>
> There are only a few InfoFrames whose data needs to be exposed to userspace:
> the SPD InfoFrame (not implemented today in a driver, but perfectly suited
> for controls since this rarely changes), and the HDR metadata that you need.
>
> Also, all InfoFrame hardware implementations that I have seen expose the
> InfoFrame data through registers (often over an i2c bus), and that's
> really not suitable for the metadata API.
>
> Now, the Extended InfoFrame information is different beast, and there
> the metadata API is a reasonable approach. Not the only approach, perhaps,
> but certainly something to consider.
>
> So supposed we support this as a metadata device node, then it is
> an option to just dump all InfoFrames into the buffer. I.e., the pixel
> format used to define the format of the stream would say that it consists
> of all InfoFrames preceding the video frame concatenated together, and
> you have to parse it in userspace to find the InfoFrame types that you
> need. It is up to the driver to decide if all InfoFrames or only a subset
> are copied into the buffer.
>
> But be aware that the video driver always has to parse the data as well
> in order to extract the AVI InfoFrame and a few others in order to
> program the video receiver.
>
> The only reason IMHO to dump all InfoFrames into the buffer is if that's
> how the DMA engine operates. If you need to manually do this, then only
> copy the Extended InfoFrames.
>
> >>
> >> This is why we were originally thinking that a 2 node approach (1 for
> >> video 1 for metadata) would have been cleaner.
> >
> > I would also like to add that the implementation of the request API is
> > currently not able to support inline devices (with a live source). This
>
> For traditional non-MC drivers this is supported in the request API. Only
> for MC drivers is this a problem. I don't know if this upcoming driver is
> going the MC route.
>
> > could be changed, but would require a very significant amount of work if
> > we want to implement it properly. As we already have a metadata capture
> > API, I really don't see why this should go through the control
> > framework.
> >
> > Regarding HDR10 static metadata, while I agree that controls should work
> > for this purpose, I think we also need to take into account API
> > consistency. If we use the metadata API for dynamic metadata, we will
> > end up with two different APIs to cary two different types of metadata.
> > Is this desirable, or, given that we already plan for HDR10+ support,
> > would implementing HDR10 support through the metadata API be a better
> > solution for userspace ?
>
> The Dynamic Range and Mastering InfoFrame also needs to be processed by
> the driver, so it makes much more sense to have the driver expose the
> relevant data through one or more controls.
>
> That does not prevent you from dumping this in the metadata buffer as
> well (as described above), but you really only want to do that if that's
> the most efficient way for the DMA engine. But if the buffer is non
> consistent memory, then you need sync the buffer to the cpu before the
> driver can read it.
>
> So, in conclusion, I think it is OK to use a metadata device for the
> Extended InfoFrames. I don't think it is a good idea to dump *all*
> InfoFrames in the metadata buffer, especially if the driver has to
> sync the buffer to the cpu in order to obtain the non-Extended-InfoFrame
> information.
>
> >
> >>>>>> That said, some work in the control framework is probably needed to
> >>>>>> streamline things a bit:
> >>>>>>
> >>>>>> 1) it should be possible to increase the size of compound controls
> >>>>>> later if new fields are added. This is on the TODO list already since
> >>>>>> it is desired functionality for codecs.
> >>>>>>
> >>>>>> 2) tentative, needs research first: add some sort of mechanism to
> >>>>>> mmap the control payload to avoid mem copies. That would make controls
> >>>>>> much more useful for large metadata.
> >>>>>
> >>>>> Let's not forget that we would then also need to mmap the control per
> >>>>> request, which will become challenging if we want to be able to
> >>>>> pre-map everything like we do for buffers instead of mapping and
> >>>>> unmapping for every request.
> >>
> >> Same concern here. If we want to pre-map everything like buffers,
> >> wouldn't we essentially be replicating the behavior of buffers. Then
> >> the only difference would be that we are doing g_ctrl instead of
> >> qbuf/dqbuf right?
>
> Right. But qbuf/dqbuf has quite a bit of overhead as well (dealing with
> multiple streaming modes, cache management), and while that is perfectly
> acceptable when dealing with large buffers and DMA engines, I wonder if
> a mmap-ed control for mid-sized data (say up to 32/64 kBytes) isn't more
> efficient.
>
> That said, this functionality does not exist, and it would need some
> performance testing as well comparing the two. It's just a gut feeling
> without any hard proof.
>
> Regards,
>
>         Hans
>
> >
> > I couldn't have said that better :-)
> >
> >>>>>> I'm not sure when I will have time to work on that, though.
> >>>>>>
> >>>>>>>> Does it accept any InfoFrame or only specific InfoFrame types? Or
> >>>>>>>> is this programmable?
> >>>>>>>
> >>>>>>> HDMI Rx IP accepts all types of InfoFrames.
> >>>>>>>
> >>>>>>>>> We have 2 different proposals that we believe will work:
> >>>>>>>>>
> >>>>>>>>> A. 2 video node approach (1 for video, 1 for metadata) - This will
> >>>>>>>>>    align with current v4l metadata structure (i.e. uvc) but
> >>>>>>>>>    will require our HDMI RX driver to register a subdev and
> >>>>>>>>>    device node
> >>>>>>>>>       a. Our HDMI RX driver will register a v4l subdev (for
> >>>>>>>>>          video data) and a metadata node
> >>>>>>>>>               i. Is this acceptable?
> >>>>>>>>>       b. Applications will qbuf/dqbuf to both video and metadata nodes
> >>>>>>>>>          for each frame
> >>>>>>>>>
> >>>>>>>>> B. 1 video node approach - This will avoid mixing v4l subdev and
> >>>>>>>>>    v4l device node functionality inside HDMI RX driver but it
> >>>>>>>>>    strays from current v4l metadata architecture and also
> >>>>>>>>>    changes v4l subdev functionality
> >>>>>>>>>       a. We would add a "read" function to v4l subdev's
> >>>>>>>>>               i. This will also require us to add some "capabilities" field
> >>>>>>>>>                  to subdev or be able to query for the "read" function
> >>>>>>>>>       b. HDMI Rx driver will register a v4l subdev with "read"
> >>>>>>>>>          function/capability
> >>>>>>>>>       c. Application can directly pass a buffer in the "read" function
> >>>>>>>>>          to HDMI RX subdev to obtain HDR metadata
> >>>>>>>>>               i. We will need to pass subdev name from application or be able
> >>>>>>>>>                  to query all subdevs for this "read"
> >>>>>>>>>                  capability, is this acceptable?
> >>>>>>>>>
> >>>>>>>>> Please let me know your opinions on which approach is best or
> >>>>>>>>> propose another approach if these 2 are unfit. Thanks
> >
>