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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux