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

From: Ramesh Shanmugasundaram <hidden>
Date: 2016-11-14 15:53:36
Also in: linux-media, linux-renesas-soc

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:

quoted

Hi Hans,

On Friday 11 Nov 2016 14:53:58 Hans Verkuil wrote:

quoted

On 11/10/2016 09:08 AM, Laurent Pinchart wrote:

quoted

Antti, Hans, ping ? Please see below.

On Friday 04 Nov 2016 09:23:29 Ramesh Shanmugasundaram wrote:

quoted

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

quoted

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

quoted

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

quoted

This patch adds documentation for the three new SDR formats

V4L2_SDR_FMT_SCU16BE
V4L2_SDR_FMT_SCU18BE
V4L2_SDR_FMT_SCU20BE

[snip]

quoted

+
+       -  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.

quoted

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.

quoted

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

quoted

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)?

quoted

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.

quoted

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.

quoted

For a buffer of size 200

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

quoted

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

IQ.

quoted

There are some modes where this h/w combo can also do IQ IQ IQ

pattern.

quoted

Those modes are not added in the RFC patchset.

quoted

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.

quoted

- 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.

quoted

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.

quoted

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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help