[RFC,5/5] doc_rst: media: New SDR formats SC16, SC18 & SC20

  This patch adds documentation for the three new SDR formats

V4L2_SDR_FMT_SCU16BE
V4L2_SDR_FMT_SCU18BE
V4L2_SDR_FMT_SCU20BE

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
 .../media/uapi/v4l/pixfmt-sdr-scu16be.rst          | 44 ++++++++++++++++++++
 .../media/uapi/v4l/pixfmt-sdr-scu18be.rst          | 48 ++++++++++++++++++++++
 .../media/uapi/v4l/pixfmt-sdr-scu20be.rst          | 48 ++++++++++++++++++++++
 Documentation/media/uapi/v4l/sdr-formats.rst       |  3 ++
 4 files changed, 143 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst
  

Hi Ramesh,

Thank you for the patch.

On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
> This patch adds documentation for the three new SDR formats
> 
> V4L2_SDR_FMT_SCU16BE
> V4L2_SDR_FMT_SCU18BE
> V4L2_SDR_FMT_SCU20BE
> 
> Signed-off-by: Ramesh Shanmugasundaram
> <ramesh.shanmugasundaram@bp.renesas.com> ---
>  .../media/uapi/v4l/pixfmt-sdr-scu16be.rst          | 44 ++++++++++++++++++
>  .../media/uapi/v4l/pixfmt-sdr-scu18be.rst          | 48 +++++++++++++++++++
>  .../media/uapi/v4l/pixfmt-sdr-scu20be.rst          | 48 +++++++++++++++++++
>  Documentation/media/uapi/v4l/sdr-formats.rst       |  3 ++
>  4 files changed, 143 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst
> 
> diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
> b/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst new file mode 100644
> index 0000000..d6c2123
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
> @@ -0,0 +1,44 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-SDR-FMT-SCU16BE:
> +
> +******************************
> +V4L2_SDR_FMT_SCU16BE ('SCU16')

The value between parentheses is the ASCII representation of the 4CC, it 
should be SC16. Same comment for the other formats.

> +******************************
> +
> +Sliced complex unsigned 16-bit big endian IQ sample
> +
> +
> +Description
> +===========
> +
> +This format contains a sequence of complex number samples. Each complex
> +number consist of two parts called In-phase and Quadrature (IQ). Both I
> +and Q are represented as a 16 bit unsigned big endian number. I value
> +starts first and Q value starts at an offset equalling half of the buffer
> +size. 14 bit data is stored in 16 bit space with unused stuffed bits
> +padded with 0.

Please specify here how the 14-bit numbers are aligned (i.e. padding in bits 
15:14 or bits 1:0 or any other strange option). Same comment for the other 
formats.

> +
> +**Byte Order.**
> +Each cell is one byte.
> +
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    -  .. row 1

Please use the more compact table stable

	* - start + 0:
	  - I'\ :sub:`0[D13:D6]`
	  ...

Same comment for the other formats.

> +
> +       -  start + 0:
> +
> +       -  I'\ :sub:`0[D13:D6]`
> +
> +       -  I'\ :sub:`0[D5:D0]`
> +
> +    -  .. row 2
> +
> +       -  start + buffer_size/2:
> +
> +       -  Q'\ :sub:`0[D13:D6]`
> +
> +       -  Q'\ :sub:`0[D5:D0]`

The format looks planar, does it use one V4L2 plane (as does NV12) or two V4L2 
planes (as does NV12M) ? Same question for the other formats.

> diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
> b/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst new file mode 100644
> index 0000000..e6e0aff
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
> @@ -0,0 +1,48 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-SDR-FMT-SCU18BE:
> +
> +******************************
> +V4L2_SDR_FMT_SCU18BE ('SCU18')
> +******************************
> +
> +Sliced complex unsigned 18-bit big endian IQ sample
> +
> +
> +Description
> +===========
> +
> +This format contains a sequence of complex number samples. Each complex
> +number consist of two parts called In-phase and Quadrature (IQ). Both I
> +and Q are represented as a 18 bit unsigned big endian number. I value
> +starts first and Q value starts at an offset equalling half of the buffer
> +size. 16 bit data is stored in 18 bit space with unused stuffed bits
> +padded with 0.

Your example below suggests that 18 bit data is stored in 24 bits. Similar 
comment for SCU20.

> +
> +**Byte Order.**
> +Each cell is one byte.
> +
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    -  .. row 1
> +
> +       -  start + 0:
> +
> +       -  I'\ :sub:`0[D17:D10]`
> +
> +       -  I'\ :sub:`0[D9:D2]`
> +
> +       -  I'\ :sub:`0[D1:D0]`
> +
> +    -  .. row 2
> +
> +       -  start + buffer_size/2:
> +
> +       -  Q'\ :sub:`0[D17:D10]`
> +
> +       -  Q'\ :sub:`0[D9:D2]`
> +
> +       -  Q'\ :sub:`0[D1:D0]`
> diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst
> b/Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst new file mode 100644
> index 0000000..374e0a3
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst
> @@ -0,0 +1,48 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-SDR-FMT-SCU20BE:
> +
> +******************************
> +V4L2_SDR_FMT_SCU20BE ('SCU20')
> +******************************
> +
> +Sliced complex unsigned 20-bit big endian IQ sample
> +
> +
> +Description
> +===========
> +
> +This format contains a sequence of complex number samples. Each complex
> +number consist of two parts called In-phase and Quadrature (IQ). Both I
> +and Q are represented as a 20 bit unsigned big endian number. I value
> +starts first and Q value starts at an offset equalling half of the buffer
> +size. 18 bit data is stored in 20 bit space with unused stuffed bits
> +padded with 0.
> +
> +**Byte Order.**
> +Each cell is one byte.
> +
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    -  .. row 1
> +
> +       -  start + 0:
> +
> +       -  I'\ :sub:`0[D19:D12]`
> +
> +       -  I'\ :sub:`0[D11:D4]`
> +
> +       -  I'\ :sub:`0[D3:D0]`
> +
> +    -  .. row 2
> +
> +       -  start + buffer_size/2:
> +
> +       -  Q'\ :sub:`0[D19:D12]`
> +
> +       -  Q'\ :sub:`0[D11:D4]`
> +
> +       -  Q'\ :sub:`0[D3:D0]`
> diff --git a/Documentation/media/uapi/v4l/sdr-formats.rst
> b/Documentation/media/uapi/v4l/sdr-formats.rst index f863c08..4c01cf9
> 100644
> --- a/Documentation/media/uapi/v4l/sdr-formats.rst
> +++ b/Documentation/media/uapi/v4l/sdr-formats.rst
> @@ -17,3 +17,6 @@ These formats are used for :ref:`SDR <sdr>` interface
> only. pixfmt-sdr-cs08
>      pixfmt-sdr-cs14le
>      pixfmt-sdr-ru12le
> +    pixfmt-sdr-scu16be
> +    pixfmt-sdr-scu18be
> +    pixfmt-sdr-scu20be
  

Hi Laurent,

Thank you for the review comments.

> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
> > This patch adds documentation for the three new SDR formats
> >
> > V4L2_SDR_FMT_SCU16BE
> > V4L2_SDR_FMT_SCU18BE
> > V4L2_SDR_FMT_SCU20BE
> >
> > Signed-off-by: Ramesh Shanmugasundaram
> > <ramesh.shanmugasundaram@bp.renesas.com> ---
> >  .../media/uapi/v4l/pixfmt-sdr-scu16be.rst          | 44
> ++++++++++++++++++
> >  .../media/uapi/v4l/pixfmt-sdr-scu18be.rst          | 48
> +++++++++++++++++++
> >  .../media/uapi/v4l/pixfmt-sdr-scu20be.rst          | 48
> +++++++++++++++++++
> >  Documentation/media/uapi/v4l/sdr-formats.rst       |  3 ++
> >  4 files changed, 143 insertions(+)
> >  create mode 100644
> > Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
> >  create mode 100644
> > Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
> >  create mode 100644
> > Documentation/media/uapi/v4l/pixfmt-sdr-scu20be.rst
> >
> > diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
> > b/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst new file mode
> > 100644 index 0000000..d6c2123
> > --- /dev/null
> > +++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu16be.rst
> > @@ -0,0 +1,44 @@
> > +.. -*- coding: utf-8; mode: rst -*-
> > +
> > +.. _V4L2-SDR-FMT-SCU16BE:
> > +
> > +******************************
> > +V4L2_SDR_FMT_SCU16BE ('SCU16')
> 
> The value between parentheses is the ASCII representation of the 4CC, it
> should be SC16. Same comment for the other formats.

Agreed. I corrected it after I sent the patch :-(.

> 
> > +******************************
> > +
> > +Sliced complex unsigned 16-bit big endian IQ sample
> > +
> > +
> > +Description
> > +===========
> > +
> > +This format contains a sequence of complex number samples. Each
> > +complex number consist of two parts called In-phase and Quadrature
> > +(IQ). Both I and Q are represented as a 16 bit unsigned big endian
> > +number. I value starts first and Q value starts at an offset
> > +equalling half of the buffer size. 14 bit data is stored in 16 bit
> > +space with unused stuffed bits padded with 0.
> 
> Please specify here how the 14-bit numbers are aligned (i.e. padding in
> bits
> 15:14 or bits 1:0 or any other strange option). Same comment for the other
> formats.

You are right. Actually the representation would be something like below. I will correct this for all the 3 formats. Thanks.

<------------------------32bits---------------------->
<--14 bit data + 2bit status---- 16bit padded zeros-->
<--14 bit data + 2bit status---- 16bit padded zeros-->

> 
> > +
> > +**Byte Order.**
> > +Each cell is one byte.
> > +
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    -  .. row 1
> 
> Please use the more compact table stable

Agreed.

> 
> 	* - start + 0:
> 	  - I'\ :sub:`0[D13:D6]`
> 	  ...
> 
> Same comment for the other formats.

Agreed.

> 
> > +
> > +       -  start + 0:
> > +
> > +       -  I'\ :sub:`0[D13:D6]`
> > +
> > +       -  I'\ :sub:`0[D5:D0]`
> > +
> > +    -  .. row 2
> > +
> > +       -  start + buffer_size/2:
> > +
> > +       -  Q'\ :sub:`0[D13:D6]`
> > +
> > +       -  Q'\ :sub:`0[D5:D0]`
> 
> The format looks planar, does it use one V4L2 plane (as does NV12) or two
> V4L2 planes (as does NV12M) ? Same question for the other formats.

Thank you for bringing up this topic. This is one of the key design dilemma.

The I & Q data for these three SDR formats comes from two different DMA channels and hence two separate pointers -> we could say it is v4l2 multi-planar. Right now, I am making it look like a single plane by presenting the data in one single buffer ptr. 

For e.g. multi-planar SC16 format would look something like this

<------------------------32bits---------------------->
<--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0 
<--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4 
...
<--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0 
<--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4 

My concerns are

1) These formats are not a standard as the video "Image Formats". These formats are possible when we use DRIF + MAX2175 combination. If we interface with a different tuner vendor, the above format(s) MAY/MAY NOT be re-usable. We do not know at this point. This is the main open item for discussion in the cover letter.

2) MPLANE support within V4L2 seems specific to video. Please correct me if this is wrong interpretation.
	- struct v4l2_format contains v4l2_sdr_format and v4l2_pix_format_mplane as members of union. Should I create a new v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most of the video specific members would be unused (it would be similar to using v4l2_pix_format itself instead of v4l2_sdr_format)?
	
	- The above decision (accomodate SDR & MPLANE) needs to be propagated across the framework. Is this the preferred approach?
	
It goes back to point (1). As of today, the change set for this combo (DRIF+MAX2175) introduces new SDR formats only. Should it add further SDR+MPLANE support to the framework as well?

I would appreciate your suggestions on this regard.

> 
> > diff --git a/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
> > b/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst new file mode
> > 100644 index 0000000..e6e0aff
> > --- /dev/null
> > +++ b/Documentation/media/uapi/v4l/pixfmt-sdr-scu18be.rst
> > @@ -0,0 +1,48 @@
> > +.. -*- coding: utf-8; mode: rst -*-
> > +
> > +.. _V4L2-SDR-FMT-SCU18BE:
> > +
> > +******************************
> > +V4L2_SDR_FMT_SCU18BE ('SCU18')
> > +******************************
> > +
> > +Sliced complex unsigned 18-bit big endian IQ sample
> > +
> > +
> > +Description
> > +===========
> > +
> > +This format contains a sequence of complex number samples. Each
> > +complex number consist of two parts called In-phase and Quadrature
> > +(IQ). Both I and Q are represented as a 18 bit unsigned big endian
> > +number. I value starts first and Q value starts at an offset
> > +equalling half of the buffer size. 16 bit data is stored in 18 bit
> > +space with unused stuffed bits padded with 0.
> 
> Your example below suggests that 18 bit data is stored in 24 bits. Similar
> comment for SCU20.

Agreed. The corrected representation is as I mentioned in the earlier comment.

Thanks,
Ramesh
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Laurent,

Any further thoughts on the SDR format please (especially the comment below). I would appreciate your feedback.

> > On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
> > > This patch adds documentation for the three new SDR formats
> > >
> > > V4L2_SDR_FMT_SCU16BE
> > > V4L2_SDR_FMT_SCU18BE
> > > V4L2_SDR_FMT_SCU20BE
[snip]
> > > +
> > > +       -  start + 0:
> > > +
> > > +       -  I'\ :sub:`0[D13:D6]`
> > > +
> > > +       -  I'\ :sub:`0[D5:D0]`
> > > +
> > > +    -  .. row 2
> > > +
> > > +       -  start + buffer_size/2:
> > > +
> > > +       -  Q'\ :sub:`0[D13:D6]`
> > > +
> > > +       -  Q'\ :sub:`0[D5:D0]`
> >
> > The format looks planar, does it use one V4L2 plane (as does NV12) or
> > two
> > V4L2 planes (as does NV12M) ? Same question for the other formats.
> 
> Thank you for bringing up this topic. This is one of the key design
> dilemma.
> 
> The I & Q data for these three SDR formats comes from two different DMA
> channels and hence two separate pointers -> we could say it is v4l2 multi-
> planar. Right now, I am making it look like a single plane by presenting
> the data in one single buffer ptr.
> 
> For e.g. multi-planar SC16 format would look something like this
> 
> <------------------------32bits---------------------->
> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4 ...
> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
> 
> My concerns are
> 
> 1) These formats are not a standard as the video "Image Formats". These
> formats are possible when we use DRIF + MAX2175 combination. If we
> interface with a different tuner vendor, the above format(s) MAY/MAY NOT
> be re-usable. We do not know at this point. This is the main open item for
> discussion in the cover letter.
> 
> 2) MPLANE support within V4L2 seems specific to video. Please correct me
> if this is wrong interpretation.
> 	- struct v4l2_format contains v4l2_sdr_format and
> v4l2_pix_format_mplane as members of union. Should I create a new
> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most of
> the video specific members would be unused (it would be similar to using
> v4l2_pix_format itself instead of v4l2_sdr_format)?
> 
> 	- The above decision (accomodate SDR & MPLANE) needs to be
> propagated across the framework. Is this the preferred approach?
> 
> It goes back to point (1). As of today, the change set for this combo
> (DRIF+MAX2175) introduces new SDR formats only. Should it add further
> SDR+MPLANE support to the framework as well?
> 
> I would appreciate your suggestions on this regard.
> 

Thanks,
Ramesh
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Ramesh,

On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
> Hi Laurent,
> 
> Any further thoughts on the SDR format please (especially the comment
> below). I would appreciate your feedback.
>
> >> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
> >>> This patch adds documentation for the three new SDR formats
> >>> 
> >>> V4L2_SDR_FMT_SCU16BE
> >>> V4L2_SDR_FMT_SCU18BE
> >>> V4L2_SDR_FMT_SCU20BE
> 
> [snip]
> 
> >>> +
> >>> +       -  start + 0:
> >>> +
> >>> +       -  I'\ :sub:`0[D13:D6]`
> >>> +
> >>> +       -  I'\ :sub:`0[D5:D0]`
> >>> +
> >>> +    -  .. row 2
> >>> +
> >>> +       -  start + buffer_size/2:
> >>> +
> >>> +       -  Q'\ :sub:`0[D13:D6]`
> >>> +
> >>> +       -  Q'\ :sub:`0[D5:D0]`
> >> 
> >> The format looks planar, does it use one V4L2 plane (as does NV12) or
> >> two V4L2 planes (as does NV12M) ? Same question for the other formats.
> > 
> > Thank you for bringing up this topic. This is one of the key design
> > dilemma.
> > 
> > The I & Q data for these three SDR formats comes from two different DMA
> > channels and hence two separate pointers -> we could say it is v4l2 multi-
> > planar. Right now, I am making it look like a single plane by presenting
> > the data in one single buffer ptr.
> > 
> > For e.g. multi-planar SC16 format would look something like this
> > 
> > <------------------------32bits---------------------->
> > <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
> > <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4 ...
> > <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
> > <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
> > 
> > My concerns are
> > 
> > 1) These formats are not a standard as the video "Image Formats". These
> > formats are possible when we use DRIF + MAX2175 combination. If we
> > interface with a different tuner vendor, the above format(s) MAY/MAY NOT
> > be re-usable. We do not know at this point. This is the main open item for
> > discussion in the cover letter.

If the formats are really device-specific then they should be documented 
accordingly and not made generic.

> > 2) MPLANE support within V4L2 seems specific to video. Please correct me
> > if this is wrong interpretation.
> > 
> > - struct v4l2_format contains v4l2_sdr_format and
> > v4l2_pix_format_mplane as members of union. Should I create a new
> > v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most of
> > the video specific members would be unused (it would be similar to using
> > v4l2_pix_format itself instead of v4l2_sdr_format)?

I have no answer to that question as I'm not familiar with SDR. Antti, you've 
added v4l2_sdr_format to the API, what's your opinion ? Hans, as you've acked 
the patch, your input would be appreciated as well.

> > - The above decision (accomodate SDR & MPLANE) needs to be
> > propagated across the framework. Is this the preferred approach?
> > 
> > It goes back to point (1). As of today, the change set for this combo
> > (DRIF+MAX2175) introduces new SDR formats only. Should it add further
> > SDR+MPLANE support to the framework as well?
> > 
> > I would appreciate your suggestions on this regard.
  

Hello

On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
> Hi Ramesh,
>
> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
>> Hi Laurent,
>>
>> Any further thoughts on the SDR format please (especially the comment
>> below). I would appreciate your feedback.
>>
>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
>>>>> This patch adds documentation for the three new SDR formats
>>>>>
>>>>> V4L2_SDR_FMT_SCU16BE
>>>>> V4L2_SDR_FMT_SCU18BE
>>>>> V4L2_SDR_FMT_SCU20BE
>>
>> [snip]
>>
>>>>> +
>>>>> +       -  start + 0:
>>>>> +
>>>>> +       -  I'\ :sub:`0[D13:D6]`
>>>>> +
>>>>> +       -  I'\ :sub:`0[D5:D0]`
>>>>> +
>>>>> +    -  .. row 2
>>>>> +
>>>>> +       -  start + buffer_size/2:
>>>>> +
>>>>> +       -  Q'\ :sub:`0[D13:D6]`
>>>>> +
>>>>> +       -  Q'\ :sub:`0[D5:D0]`
>>>>
>>>> The format looks planar, does it use one V4L2 plane (as does NV12) or
>>>> two V4L2 planes (as does NV12M) ? Same question for the other formats.
>>>
>>> Thank you for bringing up this topic. This is one of the key design
>>> dilemma.
>>>
>>> The I & Q data for these three SDR formats comes from two different DMA
>>> channels and hence two separate pointers -> we could say it is v4l2 multi-
>>> planar. Right now, I am making it look like a single plane by presenting
>>> the data in one single buffer ptr.
>>>
>>> For e.g. multi-planar SC16 format would look something like this
>>>
>>> <------------------------32bits---------------------->
>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4 ...
>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
>>>
>>> My concerns are
>>>
>>> 1) These formats are not a standard as the video "Image Formats". These
>>> formats are possible when we use DRIF + MAX2175 combination. If we
>>> interface with a different tuner vendor, the above format(s) MAY/MAY NOT
>>> be re-usable. We do not know at this point. This is the main open item for
>>> discussion in the cover letter.
>
> If the formats are really device-specific then they should be documented
> accordingly and not made generic.
>
>>> 2) MPLANE support within V4L2 seems specific to video. Please correct me
>>> if this is wrong interpretation.
>>>
>>> - struct v4l2_format contains v4l2_sdr_format and
>>> v4l2_pix_format_mplane as members of union. Should I create a new
>>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most of
>>> the video specific members would be unused (it would be similar to using
>>> v4l2_pix_format itself instead of v4l2_sdr_format)?
>
> I have no answer to that question as I'm not familiar with SDR. Antti, you've
> added v4l2_sdr_format to the API, what's your opinion ? Hans, as you've acked
> the patch, your input would be appreciated as well.

If I understood correctly this hardware provides I and Q samples via 
different channels and driver now combines those channels as a 
sequential IQ sample pairs. I have never seen any other than hw which 
provides IQ IQ IQ IQ ... IQ.
This is
I I I I ... I
Q Q Q Q ... Q
I am not very familiar with planars, but it sounds like it is correct 
approach. So I think should be added rather than emulate packet 
sequential format.

>
>>> - The above decision (accomodate SDR & MPLANE) needs to be
>>> propagated across the framework. Is this the preferred approach?
>>>
>>> It goes back to point (1). As of today, the change set for this combo
>>> (DRIF+MAX2175) introduces new SDR formats only. Should it add further
>>> SDR+MPLANE support to the framework as well?
>>>
>>> I would appreciate your suggestions on this regard.
>

regards
Antti
  

Hi Antti,

Thanks for the response.

> Subject: Re: [RFC 5/5] doc_rst: media: New SDR formats SC16, SC18 & SC20

> 

> Hello

> 

> On 11/02/2016 10:58 PM, Laurent Pinchart wrote:

> > Hi Ramesh,

> >

> > On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:

> >> Hi Laurent,

> >>

> >> Any further thoughts on the SDR format please (especially the comment

> >> below). I would appreciate your feedback.

> >>

> >>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:

> >>>>> This patch adds documentation for the three new SDR formats

> >>>>>

> >>>>> V4L2_SDR_FMT_SCU16BE

> >>>>> V4L2_SDR_FMT_SCU18BE

> >>>>> V4L2_SDR_FMT_SCU20BE

> >>

> >> [snip]

> >>

> >>>>> +

> >>>>> +       -  start + 0:

> >>>>> +

> >>>>> +       -  I'\ :sub:`0[D13:D6]`

> >>>>> +

> >>>>> +       -  I'\ :sub:`0[D5:D0]`

> >>>>> +

> >>>>> +    -  .. row 2

> >>>>> +

> >>>>> +       -  start + buffer_size/2:

> >>>>> +

> >>>>> +       -  Q'\ :sub:`0[D13:D6]`

> >>>>> +

> >>>>> +       -  Q'\ :sub:`0[D5:D0]`

> >>>>

> >>>> The format looks planar, does it use one V4L2 plane (as does NV12)

> >>>> or two V4L2 planes (as does NV12M) ? Same question for the other

> formats.

> >>>

> >>> Thank you for bringing up this topic. This is one of the key design

> >>> dilemma.

> >>>

> >>> The I & Q data for these three SDR formats comes from two different

> >>> DMA channels and hence two separate pointers -> we could say it is

> >>> v4l2 multi- planar. Right now, I am making it look like a single

> >>> plane by presenting the data in one single buffer ptr.

> >>>

> >>> For e.g. multi-planar SC16 format would look something like this

> >>>

> >>> <------------------------32bits---------------------->

> >>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0

> >>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4

> ...

> >>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0

> >>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4

> >>>

> >>> My concerns are

> >>>

> >>> 1) These formats are not a standard as the video "Image Formats".

> >>> These formats are possible when we use DRIF + MAX2175 combination.

> >>> If we interface with a different tuner vendor, the above format(s)

> >>> MAY/MAY NOT be re-usable. We do not know at this point. This is the

> >>> main open item for discussion in the cover letter.

> >

> > If the formats are really device-specific then they should be

> > documented accordingly and not made generic.

> >

> >>> 2) MPLANE support within V4L2 seems specific to video. Please

> >>> correct me if this is wrong interpretation.

> >>>

> >>> - struct v4l2_format contains v4l2_sdr_format and

> >>> v4l2_pix_format_mplane as members of union. Should I create a new

> >>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most

> >>> of the video specific members would be unused (it would be similar

> >>> to using v4l2_pix_format itself instead of v4l2_sdr_format)?

> >

> > I have no answer to that question as I'm not familiar with SDR. Antti,

> > you've added v4l2_sdr_format to the API, what's your opinion ? Hans,

> > as you've acked the patch, your input would be appreciated as well.

> 

> If I understood correctly this hardware provides I and Q samples via

> different channels and driver now combines those channels as a sequential

> IQ sample pairs. 


The driver combines the two buffer ptrs and present as one single buffer. For a buffer of size 200

ptr + 0   : I I I I ... I
ptr + 100 : Q Q Q Q ... Q

>I have never seen any other than hw which provides IQ IQ

> IQ IQ ... IQ.


There are some modes where this h/w combo can also do IQ IQ IQ pattern. Those modes are not added in the RFC patchset.

> This is

> I I I I ... I

> Q Q Q Q ... Q

> I am not very familiar with planars, but it sounds like it is correct

> approach. So I think should be added rather than emulate packet sequential

> format.


My understanding of V4L2 MPLANE constructs is limited to a quick code read only. At this point MPLANE support seems specific to video. SDR is defined as separate format like v4l2_pix_format. Questions would be - should we define new SDR_MPLANE? or merge SDR format with pix format & reuse existing MPLANE with some SDR extensions (if possible)? These seem big design decisions. Any suggestions please?

For my use case, MPLANE support does not seem to add significant benefit except it may be syntactically correct. I am doing cyclic DMA with a small set of h/w buffers and copying each stage to one mmapped vmalloc vb2_buffer at two offsets. If we add MPLANE support, it can be two non-contiguous buffer pointers. 

> 

> >

> >>> - The above decision (accomodate SDR & MPLANE) needs to be

> >>> propagated across the framework. Is this the preferred approach?

> >>>

> >>> It goes back to point (1). As of today, the change set for this

> >>> combo

> >>> (DRIF+MAX2175) introduces new SDR formats only. Should it add

> >>> further

> >>> SDR+MPLANE support to the framework as well?

> >>>

> >>> I would appreciate your suggestions on this regard.



Thanks,
Ramesh
  

Antti, Hans, ping ? Please see below.

On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:
> > On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
> >> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
> >>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
> >>>>> 
> >>>>>> This patch adds documentation for the three new SDR formats
> >>>>>>
> >>>>>> V4L2_SDR_FMT_SCU16BE
> >>>>>> V4L2_SDR_FMT_SCU18BE
> >>>>>> V4L2_SDR_FMT_SCU20BE
> >>>
> >>> [snip]
> >>>
> >>>>>> +
> >>>>>> +       -  start + 0:
> >>>>>> +
> >>>>>> +       -  I'\ :sub:`0[D13:D6]`
> >>>>>> +
> >>>>>> +       -  I'\ :sub:`0[D5:D0]`
> >>>>>> +
> >>>>>> +    -  .. row 2
> >>>>>> +
> >>>>>> +       -  start + buffer_size/2:
> >>>>>> +
> >>>>>> +       -  Q'\ :sub:`0[D13:D6]`
> >>>>>> +
> >>>>>> +       -  Q'\ :sub:`0[D5:D0]`
> >>>>>
> >>>>>
> >>>>>
> >>>>> The format looks planar, does it use one V4L2 plane (as does NV12)
> >>>>> or two V4L2 planes (as does NV12M) ? Same question for the other
> >>>>> formats.
> >>>>
> >>>> Thank you for bringing up this topic. This is one of the key design
> >>>> dilemma.
> >>>>
> >>>> The I & Q data for these three SDR formats comes from two different
> >>>> DMA channels and hence two separate pointers -> we could say it is
> >>>> v4l2 multi- planar. Right now, I am making it look like a single
> >>>> plane by presenting the data in one single buffer ptr.
> >>>>
> >>>> For e.g. multi-planar SC16 format would look something like this
> >>>>
> >>>> <------------------------32bits---------------------->
> >>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
> >>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4
> >>>> ...
> >>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
> >>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
> >>>>
> >>>> My concerns are
> >>>>
> >>>> 1) These formats are not a standard as the video "Image Formats".
> >>>> These formats are possible when we use DRIF + MAX2175 combination.
> >>>> If we interface with a different tuner vendor, the above format(s)
> >>>> MAY/MAY NOT be re-usable. We do not know at this point. This is the
> >>>> main open item for discussion in the cover letter.
> >>
> >> If the formats are really device-specific then they should be
> >> documented accordingly and not made generic.
> >>
> >>>> 2) MPLANE support within V4L2 seems specific to video. Please
> >>>> correct me if this is wrong interpretation.
> >>>>
> >>>> - struct v4l2_format contains v4l2_sdr_format and
> >>>> v4l2_pix_format_mplane as members of union. Should I create a new
> >>>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most
> >>>> of the video specific members would be unused (it would be similar
> >>>> to using v4l2_pix_format itself instead of v4l2_sdr_format)?
> >>
> >> I have no answer to that question as I'm not familiar with SDR. Antti,
> >> you've added v4l2_sdr_format to the API, what's your opinion ? Hans,
> >> as you've acked the patch, your input would be appreciated as well.
> > 
> > If I understood correctly this hardware provides I and Q samples via
> > different channels and driver now combines those channels as a sequential
> > IQ sample pairs. 
> 
> The driver combines the two buffer ptrs and present as one single buffer.
> For a buffer of size 200
>
> ptr + 0   : I I I I ... I
> ptr + 100 : Q Q Q Q ... Q
> 
> > I have never seen any other than hw which provides IQ IQ IQ IQ ... IQ.
> 
> There are some modes where this h/w combo can also do IQ IQ IQ pattern.
> Those modes are not added in the RFC patchset.
> 
> > This is
> > I I I I ... I
> > Q Q Q Q ... Q
> > I am not very familiar with planars, but it sounds like it is correct
> > approach. So I think should be added rather than emulate packet
> > sequential format.
> 
> My understanding of V4L2 MPLANE constructs is limited to a quick code read
> only. At this point MPLANE support seems specific to video. SDR is defined
> as separate format like v4l2_pix_format. Questions would be - should we
> define new SDR_MPLANE? or merge SDR format with pix format & reuse existing
> MPLANE with some SDR extensions (if possible)? These seem big design
> decisions. Any suggestions please?
>
> For my use case, MPLANE support does not seem to add significant benefit
> except it may be syntactically correct. I am doing cyclic DMA with a small
> set of h/w buffers and copying each stage to one mmapped vmalloc vb2_buffer
> at two offsets. If we add MPLANE support, it can be two non-contiguous
> buffer pointers. 
>
> >>>> - The above decision (accomodate SDR & MPLANE) needs to be
> >>>> propagated across the framework. Is this the preferred approach?
> >>>>
> >>>> It goes back to point (1). As of today, the change set for this
> >>>> combo (DRIF+MAX2175) introduces new SDR formats only. Should it add
> >>>> further SDR+MPLANE support to the framework as well?
> >>>>
> >>>> I would appreciate your suggestions on this regard.
  

Hello

On 11/10/2016 10:08 AM, Laurent Pinchart wrote:
> Antti, Hans, ping ? Please see below.
>
> On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:
>>> On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
>>>> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
>>>>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
>>>>>>>
>>>>>>>> This patch adds documentation for the three new SDR formats
>>>>>>>>
>>>>>>>> V4L2_SDR_FMT_SCU16BE
>>>>>>>> V4L2_SDR_FMT_SCU18BE
>>>>>>>> V4L2_SDR_FMT_SCU20BE
>>>>>
>>>>> [snip]
>>>>>
>>>>>>>> +
>>>>>>>> +       -  start + 0:
>>>>>>>> +
>>>>>>>> +       -  I'\ :sub:`0[D13:D6]`
>>>>>>>> +
>>>>>>>> +       -  I'\ :sub:`0[D5:D0]`
>>>>>>>> +
>>>>>>>> +    -  .. row 2
>>>>>>>> +
>>>>>>>> +       -  start + buffer_size/2:
>>>>>>>> +
>>>>>>>> +       -  Q'\ :sub:`0[D13:D6]`
>>>>>>>> +
>>>>>>>> +       -  Q'\ :sub:`0[D5:D0]`
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The format looks planar, does it use one V4L2 plane (as does NV12)
>>>>>>> or two V4L2 planes (as does NV12M) ? Same question for the other
>>>>>>> formats.
>>>>>>
>>>>>> Thank you for bringing up this topic. This is one of the key design
>>>>>> dilemma.
>>>>>>
>>>>>> The I & Q data for these three SDR formats comes from two different
>>>>>> DMA channels and hence two separate pointers -> we could say it is
>>>>>> v4l2 multi- planar. Right now, I am making it look like a single
>>>>>> plane by presenting the data in one single buffer ptr.
>>>>>>
>>>>>> For e.g. multi-planar SC16 format would look something like this
>>>>>>
>>>>>> <------------------------32bits---------------------->
>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4
>>>>>> ...
>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
>>>>>>
>>>>>> My concerns are
>>>>>>
>>>>>> 1) These formats are not a standard as the video "Image Formats".
>>>>>> These formats are possible when we use DRIF + MAX2175 combination.
>>>>>> If we interface with a different tuner vendor, the above format(s)
>>>>>> MAY/MAY NOT be re-usable. We do not know at this point. This is the
>>>>>> main open item for discussion in the cover letter.
>>>>
>>>> If the formats are really device-specific then they should be
>>>> documented accordingly and not made generic.
>>>>
>>>>>> 2) MPLANE support within V4L2 seems specific to video. Please
>>>>>> correct me if this is wrong interpretation.
>>>>>>
>>>>>> - struct v4l2_format contains v4l2_sdr_format and
>>>>>> v4l2_pix_format_mplane as members of union. Should I create a new
>>>>>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most
>>>>>> of the video specific members would be unused (it would be similar
>>>>>> to using v4l2_pix_format itself instead of v4l2_sdr_format)?
>>>>
>>>> I have no answer to that question as I'm not familiar with SDR. Antti,
>>>> you've added v4l2_sdr_format to the API, what's your opinion ? Hans,
>>>> as you've acked the patch, your input would be appreciated as well.
>>>
>>> If I understood correctly this hardware provides I and Q samples via
>>> different channels and driver now combines those channels as a sequential
>>> IQ sample pairs.
>>
>> The driver combines the two buffer ptrs and present as one single buffer.
>> For a buffer of size 200
>>
>> ptr + 0   : I I I I ... I
>> ptr + 100 : Q Q Q Q ... Q
>>
>>> I have never seen any other than hw which provides IQ IQ IQ IQ ... IQ.
>>
>> There are some modes where this h/w combo can also do IQ IQ IQ pattern.
>> Those modes are not added in the RFC patchset.
>>
>>> This is
>>> I I I I ... I
>>> Q Q Q Q ... Q
>>> I am not very familiar with planars, but it sounds like it is correct
>>> approach. So I think should be added rather than emulate packet
>>> sequential format.
>>
>> My understanding of V4L2 MPLANE constructs is limited to a quick code read
>> only. At this point MPLANE support seems specific to video. SDR is defined
>> as separate format like v4l2_pix_format. Questions would be - should we
>> define new SDR_MPLANE? or merge SDR format with pix format & reuse existing
>> MPLANE with some SDR extensions (if possible)? These seem big design
>> decisions. Any suggestions please?

struct v4l2_format contains union that has own format definition for 
video, video mplane and sdr (+many others). Basically on api there is 
own definitions for each type, so I think possible sdr mplane should be 
similarly own types and definitions.

>> For my use case, MPLANE support does not seem to add significant benefit
>> except it may be syntactically correct. I am doing cyclic DMA with a small
>> set of h/w buffers and copying each stage to one mmapped vmalloc vb2_buffer
>> at two offsets. If we add MPLANE support, it can be two non-contiguous
>> buffer pointers.

If there is no clear idea about need of mplane then that's also fine for me.

And whole mplane concept is new for me. I have never played with any 
v4l2 video formats nor mplane video formats.

I would still like to hear what Hans think about adding mplane.

>>
>>>>>> - The above decision (accomodate SDR & MPLANE) needs to be
>>>>>> propagated across the framework. Is this the preferred approach?
>>>>>>
>>>>>> It goes back to point (1). As of today, the change set for this
>>>>>> combo (DRIF+MAX2175) introduces new SDR formats only. Should it add
>>>>>> further SDR+MPLANE support to the framework as well?
>>>>>>
>>>>>> I would appreciate your suggestions on this regard.
>

regards
Antti
  

On 11/10/2016 09:08 AM, Laurent Pinchart wrote:
> Antti, Hans, ping ? Please see below.
> 
> On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:
>>> On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
>>>> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
>>>>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
>>>>>>>
>>>>>>>> This patch adds documentation for the three new SDR formats
>>>>>>>>
>>>>>>>> V4L2_SDR_FMT_SCU16BE
>>>>>>>> V4L2_SDR_FMT_SCU18BE
>>>>>>>> V4L2_SDR_FMT_SCU20BE
>>>>>
>>>>> [snip]
>>>>>
>>>>>>>> +
>>>>>>>> +       -  start + 0:
>>>>>>>> +
>>>>>>>> +       -  I'\ :sub:`0[D13:D6]`
>>>>>>>> +
>>>>>>>> +       -  I'\ :sub:`0[D5:D0]`
>>>>>>>> +
>>>>>>>> +    -  .. row 2
>>>>>>>> +
>>>>>>>> +       -  start + buffer_size/2:
>>>>>>>> +
>>>>>>>> +       -  Q'\ :sub:`0[D13:D6]`
>>>>>>>> +
>>>>>>>> +       -  Q'\ :sub:`0[D5:D0]`
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The format looks planar, does it use one V4L2 plane (as does NV12)
>>>>>>> or two V4L2 planes (as does NV12M) ? Same question for the other
>>>>>>> formats.
>>>>>>
>>>>>> Thank you for bringing up this topic. This is one of the key design
>>>>>> dilemma.
>>>>>>
>>>>>> The I & Q data for these three SDR formats comes from two different
>>>>>> DMA channels and hence two separate pointers -> we could say it is
>>>>>> v4l2 multi- planar. Right now, I am making it look like a single
>>>>>> plane by presenting the data in one single buffer ptr.
>>>>>>
>>>>>> For e.g. multi-planar SC16 format would look something like this
>>>>>>
>>>>>> <------------------------32bits---------------------->
>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4
>>>>>> ...
>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
>>>>>>
>>>>>> My concerns are
>>>>>>
>>>>>> 1) These formats are not a standard as the video "Image Formats".
>>>>>> These formats are possible when we use DRIF + MAX2175 combination.
>>>>>> If we interface with a different tuner vendor, the above format(s)
>>>>>> MAY/MAY NOT be re-usable. We do not know at this point. This is the
>>>>>> main open item for discussion in the cover letter.
>>>>
>>>> If the formats are really device-specific then they should be
>>>> documented accordingly and not made generic.
>>>>
>>>>>> 2) MPLANE support within V4L2 seems specific to video. Please
>>>>>> correct me if this is wrong interpretation.
>>>>>>
>>>>>> - struct v4l2_format contains v4l2_sdr_format and
>>>>>> v4l2_pix_format_mplane as members of union. Should I create a new
>>>>>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most
>>>>>> of the video specific members would be unused (it would be similar
>>>>>> to using v4l2_pix_format itself instead of v4l2_sdr_format)?
>>>>
>>>> I have no answer to that question as I'm not familiar with SDR. Antti,
>>>> you've added v4l2_sdr_format to the API, what's your opinion ? Hans,
>>>> as you've acked the patch, your input would be appreciated as well.
>>>
>>> If I understood correctly this hardware provides I and Q samples via
>>> different channels and driver now combines those channels as a sequential
>>> IQ sample pairs. 
>>
>> The driver combines the two buffer ptrs and present as one single buffer.
>> For a buffer of size 200
>>
>> ptr + 0   : I I I I ... I
>> ptr + 100 : Q Q Q Q ... Q
>>
>>> I have never seen any other than hw which provides IQ IQ IQ IQ ... IQ.
>>
>> There are some modes where this h/w combo can also do IQ IQ IQ pattern.
>> Those modes are not added in the RFC patchset.
>>
>>> This is
>>> I I I I ... I
>>> Q Q Q Q ... Q
>>> I am not very familiar with planars, but it sounds like it is correct
>>> approach. So I think should be added rather than emulate packet
>>> sequential format.
>>
>> My understanding of V4L2 MPLANE constructs is limited to a quick code read
>> only. At this point MPLANE support seems specific to video. SDR is defined
>> as separate format like v4l2_pix_format. Questions would be - should we
>> define new SDR_MPLANE? or merge SDR format with pix format & reuse existing
>> MPLANE with some SDR extensions (if possible)? These seem big design
>> decisions. Any suggestions please?
>>
>> For my use case, MPLANE support does not seem to add significant benefit
>> except it may be syntactically correct. I am doing cyclic DMA with a small
>> set of h/w buffers and copying each stage to one mmapped vmalloc vb2_buffer
>> at two offsets. If we add MPLANE support, it can be two non-contiguous
>> buffer pointers. 
>>
>>>>>> - The above decision (accomodate SDR & MPLANE) needs to be
>>>>>> propagated across the framework. Is this the preferred approach?
>>>>>>
>>>>>> It goes back to point (1). As of today, the change set for this
>>>>>> combo (DRIF+MAX2175) introduces new SDR formats only. Should it add
>>>>>> further SDR+MPLANE support to the framework as well?
>>>>>>
>>>>>> I would appreciate your suggestions on this regard.
> 

Some terminology first:

Planar formats separate the data into different memory areas: in this case
one part is all I and one part is all Q. This as opposed to interleaved
formats (IQIQIQIQ....).

As long as both planes fit in the same buffer all is fine. Since that is
the case here there is no need to introduce a new MPLANE API.

The MPLANE API was added for video to handle cases where the two planes
had to be in two different non-contiguous buffers.

So instead of passing one buffer pointer, you need to pass two or more
buffer pointers.

In hindsight we should have called it the MBUFFER API.

Oh well...

Anyway, since there is no problem here apparently to keep both planes
in one buffer there is also no need to introduce a SDR_MPLANE.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hi Hans,

On Friday 11 Nov 2016 14:53:58 Hans Verkuil wrote:
> On 11/10/2016 09:08 AM, Laurent Pinchart wrote:
> > Antti, Hans, ping ? Please see below.
> > 
> > On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:
> >>> On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
> >>>> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
> >>>>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
> >>>>>>>> This patch adds documentation for the three new SDR formats
> >>>>>>>> 
> >>>>>>>> V4L2_SDR_FMT_SCU16BE
> >>>>>>>> V4L2_SDR_FMT_SCU18BE
> >>>>>>>> V4L2_SDR_FMT_SCU20BE
> >>>>> 
> >>>>> [snip]
> >>>>> 
> >>>>>>>> +
> >>>>>>>> +       -  start + 0:
> >>>>>>>> +
> >>>>>>>> +       -  I'\ :sub:`0[D13:D6]`
> >>>>>>>> +
> >>>>>>>> +       -  I'\ :sub:`0[D5:D0]`
> >>>>>>>> +
> >>>>>>>> +    -  .. row 2
> >>>>>>>> +
> >>>>>>>> +       -  start + buffer_size/2:
> >>>>>>>> +
> >>>>>>>> +       -  Q'\ :sub:`0[D13:D6]`
> >>>>>>>> +
> >>>>>>>> +       -  Q'\ :sub:`0[D5:D0]`
> >>>>>>> 
> >>>>>>> The format looks planar, does it use one V4L2 plane (as does NV12)
> >>>>>>> or two V4L2 planes (as does NV12M) ? Same question for the other
> >>>>>>> formats.
> >>>>>> 
> >>>>>> Thank you for bringing up this topic. This is one of the key design
> >>>>>> dilemma.
> >>>>>> 
> >>>>>> The I & Q data for these three SDR formats comes from two different
> >>>>>> DMA channels and hence two separate pointers -> we could say it is
> >>>>>> v4l2 multi- planar. Right now, I am making it look like a single
> >>>>>> plane by presenting the data in one single buffer ptr.
> >>>>>> 
> >>>>>> For e.g. multi-planar SC16 format would look something like this
> >>>>>> 
> >>>>>> <------------------------32bits---------------------->
> >>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
> >>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4
> >>>>>> ...
> >>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
> >>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
> >>>>>> 
> >>>>>> My concerns are
> >>>>>> 
> >>>>>> 1) These formats are not a standard as the video "Image Formats".
> >>>>>> These formats are possible when we use DRIF + MAX2175 combination.
> >>>>>> If we interface with a different tuner vendor, the above format(s)
> >>>>>> MAY/MAY NOT be re-usable. We do not know at this point. This is the
> >>>>>> main open item for discussion in the cover letter.
> >>>> 
> >>>> If the formats are really device-specific then they should be
> >>>> documented accordingly and not made generic.
> >>>> 
> >>>>>> 2) MPLANE support within V4L2 seems specific to video. Please
> >>>>>> correct me if this is wrong interpretation.
> >>>>>> 
> >>>>>> - struct v4l2_format contains v4l2_sdr_format and
> >>>>>> v4l2_pix_format_mplane as members of union. Should I create a new
> >>>>>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most
> >>>>>> of the video specific members would be unused (it would be similar
> >>>>>> to using v4l2_pix_format itself instead of v4l2_sdr_format)?
> >>>> 
> >>>> I have no answer to that question as I'm not familiar with SDR. Antti,
> >>>> you've added v4l2_sdr_format to the API, what's your opinion ? Hans,
> >>>> as you've acked the patch, your input would be appreciated as well.
> >>> 
> >>> If I understood correctly this hardware provides I and Q samples via
> >>> different channels and driver now combines those channels as a
> >>> sequential
> >>> IQ sample pairs.
> >> 
> >> The driver combines the two buffer ptrs and present as one single buffer.
> >> For a buffer of size 200
> >> 
> >> ptr + 0   : I I I I ... I
> >> ptr + 100 : Q Q Q Q ... Q
> >> 
> >>> I have never seen any other than hw which provides IQ IQ IQ IQ ... IQ.
> >> 
> >> There are some modes where this h/w combo can also do IQ IQ IQ pattern.
> >> Those modes are not added in the RFC patchset.
> >> 
> >>> This is
> >>> I I I I ... I
> >>> Q Q Q Q ... Q
> >>> I am not very familiar with planars, but it sounds like it is correct
> >>> approach. So I think should be added rather than emulate packet
> >>> sequential format.
> >> 
> >> My understanding of V4L2 MPLANE constructs is limited to a quick code
> >> read
> >> only. At this point MPLANE support seems specific to video. SDR is
> >> defined
> >> as separate format like v4l2_pix_format. Questions would be - should we
> >> define new SDR_MPLANE? or merge SDR format with pix format & reuse
> >> existing
> >> MPLANE with some SDR extensions (if possible)? These seem big design
> >> decisions. Any suggestions please?
> >> 
> >> For my use case, MPLANE support does not seem to add significant benefit
> >> except it may be syntactically correct. I am doing cyclic DMA with a
> >> small
> >> set of h/w buffers and copying each stage to one mmapped vmalloc
> >> vb2_buffer
> >> at two offsets. If we add MPLANE support, it can be two non-contiguous
> >> buffer pointers.
> >> 
> >>>>>> - The above decision (accomodate SDR & MPLANE) needs to be
> >>>>>> propagated across the framework. Is this the preferred approach?
> >>>>>> 
> >>>>>> It goes back to point (1). As of today, the change set for this
> >>>>>> combo (DRIF+MAX2175) introduces new SDR formats only. Should it add
> >>>>>> further SDR+MPLANE support to the framework as well?
> >>>>>> 
> >>>>>> I would appreciate your suggestions on this regard.
> 
> Some terminology first:
> 
> Planar formats separate the data into different memory areas: in this case
> one part is all I and one part is all Q. This as opposed to interleaved
> formats (IQIQIQIQ....).
> 
> As long as both planes fit in the same buffer all is fine. Since that is
> the case here there is no need to introduce a new MPLANE API.
> 
> The MPLANE API was added for video to handle cases where the two planes
> had to be in two different non-contiguous buffers.

Not only that, it can also be used for cases where storing the two planes in 
separate buffers can be beneficial, even if a single contiguous buffer could 
work.

> So instead of passing one buffer pointer, you need to pass two or more
> buffer pointers.
> 
> In hindsight we should have called it the MBUFFER API.

The name was badly chosen, yes.

> Oh well...
> 
> Anyway, since there is no problem here apparently to keep both planes
> in one buffer there is also no need to introduce a SDR_MPLANE.

The question here is whether there could be a benefit in separating I and Q 
data in two buffers compared to storing them in the same buffer.
  

On 11/11/2016 02:57 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday 11 Nov 2016 14:53:58 Hans Verkuil wrote:
>> On 11/10/2016 09:08 AM, Laurent Pinchart wrote:
>>> Antti, Hans, ping ? Please see below.
>>>
>>> On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:
>>>>> On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
>>>>>> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
>>>>>>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
>>>>>>>>>> This patch adds documentation for the three new SDR formats
>>>>>>>>>>
>>>>>>>>>> V4L2_SDR_FMT_SCU16BE
>>>>>>>>>> V4L2_SDR_FMT_SCU18BE
>>>>>>>>>> V4L2_SDR_FMT_SCU20BE
>>>>>>>
>>>>>>> [snip]
>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +       -  start + 0:
>>>>>>>>>> +
>>>>>>>>>> +       -  I'\ :sub:`0[D13:D6]`
>>>>>>>>>> +
>>>>>>>>>> +       -  I'\ :sub:`0[D5:D0]`
>>>>>>>>>> +
>>>>>>>>>> +    -  .. row 2
>>>>>>>>>> +
>>>>>>>>>> +       -  start + buffer_size/2:
>>>>>>>>>> +
>>>>>>>>>> +       -  Q'\ :sub:`0[D13:D6]`
>>>>>>>>>> +
>>>>>>>>>> +       -  Q'\ :sub:`0[D5:D0]`
>>>>>>>>>
>>>>>>>>> The format looks planar, does it use one V4L2 plane (as does NV12)
>>>>>>>>> or two V4L2 planes (as does NV12M) ? Same question for the other
>>>>>>>>> formats.
>>>>>>>>
>>>>>>>> Thank you for bringing up this topic. This is one of the key design
>>>>>>>> dilemma.
>>>>>>>>
>>>>>>>> The I & Q data for these three SDR formats comes from two different
>>>>>>>> DMA channels and hence two separate pointers -> we could say it is
>>>>>>>> v4l2 multi- planar. Right now, I am making it look like a single
>>>>>>>> plane by presenting the data in one single buffer ptr.
>>>>>>>>
>>>>>>>> For e.g. multi-planar SC16 format would look something like this
>>>>>>>>
>>>>>>>> <------------------------32bits---------------------->
>>>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 0
>>>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0 + 4
>>>>>>>> ...
>>>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 0
>>>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1 + 4
>>>>>>>>
>>>>>>>> My concerns are
>>>>>>>>
>>>>>>>> 1) These formats are not a standard as the video "Image Formats".
>>>>>>>> These formats are possible when we use DRIF + MAX2175 combination.
>>>>>>>> If we interface with a different tuner vendor, the above format(s)
>>>>>>>> MAY/MAY NOT be re-usable. We do not know at this point. This is the
>>>>>>>> main open item for discussion in the cover letter.
>>>>>>
>>>>>> If the formats are really device-specific then they should be
>>>>>> documented accordingly and not made generic.
>>>>>>
>>>>>>>> 2) MPLANE support within V4L2 seems specific to video. Please
>>>>>>>> correct me if this is wrong interpretation.
>>>>>>>>
>>>>>>>> - struct v4l2_format contains v4l2_sdr_format and
>>>>>>>> v4l2_pix_format_mplane as members of union. Should I create a new
>>>>>>>> v4l2_sdr_format_mplane? If I have to use v4l2_pix_format_mplane most
>>>>>>>> of the video specific members would be unused (it would be similar
>>>>>>>> to using v4l2_pix_format itself instead of v4l2_sdr_format)?
>>>>>>
>>>>>> I have no answer to that question as I'm not familiar with SDR. Antti,
>>>>>> you've added v4l2_sdr_format to the API, what's your opinion ? Hans,
>>>>>> as you've acked the patch, your input would be appreciated as well.
>>>>>
>>>>> If I understood correctly this hardware provides I and Q samples via
>>>>> different channels and driver now combines those channels as a
>>>>> sequential
>>>>> IQ sample pairs.
>>>>
>>>> The driver combines the two buffer ptrs and present as one single buffer.
>>>> For a buffer of size 200
>>>>
>>>> ptr + 0   : I I I I ... I
>>>> ptr + 100 : Q Q Q Q ... Q
>>>>
>>>>> I have never seen any other than hw which provides IQ IQ IQ IQ ... IQ.
>>>>
>>>> There are some modes where this h/w combo can also do IQ IQ IQ pattern.
>>>> Those modes are not added in the RFC patchset.
>>>>
>>>>> This is
>>>>> I I I I ... I
>>>>> Q Q Q Q ... Q
>>>>> I am not very familiar with planars, but it sounds like it is correct
>>>>> approach. So I think should be added rather than emulate packet
>>>>> sequential format.
>>>>
>>>> My understanding of V4L2 MPLANE constructs is limited to a quick code
>>>> read
>>>> only. At this point MPLANE support seems specific to video. SDR is
>>>> defined
>>>> as separate format like v4l2_pix_format. Questions would be - should we
>>>> define new SDR_MPLANE? or merge SDR format with pix format & reuse
>>>> existing
>>>> MPLANE with some SDR extensions (if possible)? These seem big design
>>>> decisions. Any suggestions please?
>>>>
>>>> For my use case, MPLANE support does not seem to add significant benefit
>>>> except it may be syntactically correct. I am doing cyclic DMA with a
>>>> small
>>>> set of h/w buffers and copying each stage to one mmapped vmalloc
>>>> vb2_buffer
>>>> at two offsets. If we add MPLANE support, it can be two non-contiguous
>>>> buffer pointers.
>>>>
>>>>>>>> - The above decision (accomodate SDR & MPLANE) needs to be
>>>>>>>> propagated across the framework. Is this the preferred approach?
>>>>>>>>
>>>>>>>> It goes back to point (1). As of today, the change set for this
>>>>>>>> combo (DRIF+MAX2175) introduces new SDR formats only. Should it add
>>>>>>>> further SDR+MPLANE support to the framework as well?
>>>>>>>>
>>>>>>>> I would appreciate your suggestions on this regard.
>>
>> Some terminology first:
>>
>> Planar formats separate the data into different memory areas: in this case
>> one part is all I and one part is all Q. This as opposed to interleaved
>> formats (IQIQIQIQ....).
>>
>> As long as both planes fit in the same buffer all is fine. Since that is
>> the case here there is no need to introduce a new MPLANE API.
>>
>> The MPLANE API was added for video to handle cases where the two planes
>> had to be in two different non-contiguous buffers.
> 
> Not only that, it can also be used for cases where storing the two planes in 
> separate buffers can be beneficial, even if a single contiguous buffer could 
> work.
> 
>> So instead of passing one buffer pointer, you need to pass two or more
>> buffer pointers.
>>
>> In hindsight we should have called it the MBUFFER API.
> 
> The name was badly chosen, yes.
> 
>> Oh well...
>>
>> Anyway, since there is no problem here apparently to keep both planes
>> in one buffer there is also no need to introduce a SDR_MPLANE.
> 
> The question here is whether there could be a benefit in separating I and Q 
> data in two buffers compared to storing them in the same buffer.
> 

The MPLANE API is very messy and introducing something like SDR_MPLANE is not
something I would promote. If we want that, then we should first make a new
v4l2_buffer struct that simplifies MPLANE handling (we discussed that before).

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Hello Laurent, Antti, Hans,

> Subject: Re: [RFC 5/5] doc_rst: media: New SDR formats SC16, SC18 & SC20
> 
> On 11/11/2016 02:57 PM, Laurent Pinchart wrote:
> > Hi Hans,
> >
> > On Friday 11 Nov 2016 14:53:58 Hans Verkuil wrote:
> >> On 11/10/2016 09:08 AM, Laurent Pinchart wrote:
> >>> Antti, Hans, ping ? Please see below.
> >>>
> >>> On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:
> >>>>> On 11/02/2016 10:58 PM, Laurent Pinchart wrote:
> >>>>>> On Wednesday 02 Nov 2016 09:00:00 Ramesh Shanmugasundaram wrote:
> >>>>>>>>> On Wednesday 12 Oct 2016 15:10:29 Ramesh Shanmugasundaram wrote:
> >>>>>>>>>> This patch adds documentation for the three new SDR formats
> >>>>>>>>>>
> >>>>>>>>>> V4L2_SDR_FMT_SCU16BE
> >>>>>>>>>> V4L2_SDR_FMT_SCU18BE
> >>>>>>>>>> V4L2_SDR_FMT_SCU20BE
> >>>>>>>
> >>>>>>> [snip]
> >>>>>>>
> >>>>>>>>>> +
> >>>>>>>>>> +       -  start + 0:
> >>>>>>>>>> +
> >>>>>>>>>> +       -  I'\ :sub:`0[D13:D6]`
> >>>>>>>>>> +
> >>>>>>>>>> +       -  I'\ :sub:`0[D5:D0]`
> >>>>>>>>>> +
> >>>>>>>>>> +    -  .. row 2
> >>>>>>>>>> +
> >>>>>>>>>> +       -  start + buffer_size/2:
> >>>>>>>>>> +
> >>>>>>>>>> +       -  Q'\ :sub:`0[D13:D6]`
> >>>>>>>>>> +
> >>>>>>>>>> +       -  Q'\ :sub:`0[D5:D0]`
> >>>>>>>>>
> >>>>>>>>> The format looks planar, does it use one V4L2 plane (as does
> >>>>>>>>> NV12) or two V4L2 planes (as does NV12M) ? Same question for
> >>>>>>>>> the other formats.
> >>>>>>>>
> >>>>>>>> Thank you for bringing up this topic. This is one of the key
> >>>>>>>> design dilemma.
> >>>>>>>>
> >>>>>>>> The I & Q data for these three SDR formats comes from two
> >>>>>>>> different DMA channels and hence two separate pointers -> we
> >>>>>>>> could say it is
> >>>>>>>> v4l2 multi- planar. Right now, I am making it look like a
> >>>>>>>> single plane by presenting the data in one single buffer ptr.
> >>>>>>>>
> >>>>>>>> For e.g. multi-planar SC16 format would look something like
> >>>>>>>> this
> >>>>>>>>
> >>>>>>>> <------------------------32bits---------------------->
> >>>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0
> >>>>>>>> + 0
> >>>>>>>> <--I(14 bit data) + 2bit status--16bit padded zeros--> : start0
> >>>>>>>> + 4 ...
> >>>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1
> >>>>>>>> + 0
> >>>>>>>> <--Q(14 bit data) + 2bit status--16bit padded zeros--> : start1
> >>>>>>>> + 4
> >>>>>>>>
> >>>>>>>> My concerns are
> >>>>>>>>
> >>>>>>>> 1) These formats are not a standard as the video "Image Formats".
> >>>>>>>> These formats are possible when we use DRIF + MAX2175
> combination.
> >>>>>>>> If we interface with a different tuner vendor, the above
> >>>>>>>> format(s) MAY/MAY NOT be re-usable. We do not know at this
> >>>>>>>> point. This is the main open item for discussion in the cover
> letter.
> >>>>>>
> >>>>>> If the formats are really device-specific then they should be
> >>>>>> documented accordingly and not made generic.
> >>>>>>
> >>>>>>>> 2) MPLANE support within V4L2 seems specific to video. Please
> >>>>>>>> correct me if this is wrong interpretation.
> >>>>>>>>
> >>>>>>>> - struct v4l2_format contains v4l2_sdr_format and
> >>>>>>>> v4l2_pix_format_mplane as members of union. Should I create a
> >>>>>>>> new v4l2_sdr_format_mplane? If I have to use
> >>>>>>>> v4l2_pix_format_mplane most of the video specific members would
> >>>>>>>> be unused (it would be similar to using v4l2_pix_format itself
> instead of v4l2_sdr_format)?
> >>>>>>
> >>>>>> I have no answer to that question as I'm not familiar with SDR.
> >>>>>> Antti, you've added v4l2_sdr_format to the API, what's your
> >>>>>> opinion ? Hans, as you've acked the patch, your input would be
> appreciated as well.
> >>>>>
> >>>>> If I understood correctly this hardware provides I and Q samples
> >>>>> via different channels and driver now combines those channels as a
> >>>>> sequential IQ sample pairs.
> >>>>
> >>>> The driver combines the two buffer ptrs and present as one single
> buffer.
> >>>> For a buffer of size 200
> >>>>
> >>>> ptr + 0   : I I I I ... I
> >>>> ptr + 100 : Q Q Q Q ... Q
> >>>>
> >>>>> I have never seen any other than hw which provides IQ IQ IQ IQ ...
> IQ.
> >>>>
> >>>> There are some modes where this h/w combo can also do IQ IQ IQ
> pattern.
> >>>> Those modes are not added in the RFC patchset.
> >>>>
> >>>>> This is
> >>>>> I I I I ... I
> >>>>> Q Q Q Q ... Q
> >>>>> I am not very familiar with planars, but it sounds like it is
> >>>>> correct approach. So I think should be added rather than emulate
> >>>>> packet sequential format.
> >>>>
> >>>> My understanding of V4L2 MPLANE constructs is limited to a quick
> >>>> code read only. At this point MPLANE support seems specific to
> >>>> video. SDR is defined as separate format like v4l2_pix_format.
> >>>> Questions would be - should we define new SDR_MPLANE? or merge SDR
> >>>> format with pix format & reuse existing MPLANE with some SDR
> >>>> extensions (if possible)? These seem big design decisions. Any
> >>>> suggestions please?
> >>>>
> >>>> For my use case, MPLANE support does not seem to add significant
> >>>> benefit except it may be syntactically correct. I am doing cyclic
> >>>> DMA with a small set of h/w buffers and copying each stage to one
> >>>> mmapped vmalloc vb2_buffer at two offsets. If we add MPLANE
> >>>> support, it can be two non-contiguous buffer pointers.
> >>>>
> >>>>>>>> - The above decision (accomodate SDR & MPLANE) needs to be
> >>>>>>>> propagated across the framework. Is this the preferred approach?
> >>>>>>>>
> >>>>>>>> It goes back to point (1). As of today, the change set for this
> >>>>>>>> combo (DRIF+MAX2175) introduces new SDR formats only. Should it
> >>>>>>>> add further SDR+MPLANE support to the framework as well?
> >>>>>>>>
> >>>>>>>> I would appreciate your suggestions on this regard.
> >>
> >> Some terminology first:
> >>
> >> Planar formats separate the data into different memory areas: in this
> >> case one part is all I and one part is all Q. This as opposed to
> >> interleaved formats (IQIQIQIQ....).
> >>
> >> As long as both planes fit in the same buffer all is fine. Since that
> >> is the case here there is no need to introduce a new MPLANE API.
> >>
> >> The MPLANE API was added for video to handle cases where the two
> >> planes had to be in two different non-contiguous buffers.
> >
> > Not only that, it can also be used for cases where storing the two
> > planes in separate buffers can be beneficial, even if a single
> > contiguous buffer could work.
> >
> >> So instead of passing one buffer pointer, you need to pass two or
> >> more buffer pointers.
> >>
> >> In hindsight we should have called it the MBUFFER API.
> >
> > The name was badly chosen, yes.
> >
> >> Oh well...
> >>
> >> Anyway, since there is no problem here apparently to keep both planes
> >> in one buffer there is also no need to introduce a SDR_MPLANE.
> >
> > The question here is whether there could be a benefit in separating I
> > and Q data in two buffers compared to storing them in the same buffer.
> >
> 
> The MPLANE API is very messy and introducing something like SDR_MPLANE is
> not something I would promote. If we want that, then we should first make
> a new v4l2_buffer struct that simplifies MPLANE handling (we discussed
> that before).

Thank you for the comments and closure on this topic.

Thanks,
Ramesh
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html