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

[Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>]

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.

-- 
Regards,

Laurent Pinchart

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