Date:   Sat, 4 Nov 2017 10:53:31 +0100
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Tim Harvey <tharvey@...eworks.com>
Cc:     linux-media <linux-media@...r.kernel.org>,
        alsa-devel@...a-project.org,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Steve Longerbeam <slongerbeam@...il.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Hans Verkuil <hansverk@...co.com>,
        Mauro Carvalho Chehab <mchehab@...pensource.com>
Subject: Re: [PATCH v2 3/4] media: i2c: Add TDA1997x HDMI receiver driver

On 11/04/2017 01:17 AM, Tim Harvey wrote:
> On Mon, Oct 23, 2017 at 10:05 AM, Tim Harvey <tharvey@...eworks.com> wrote:
>>
>> On Fri, Oct 20, 2017 at 9:23 AM, Hans Verkuil <hverkuil@...all.nl> wrote:
>>
>>>>
>>>> I see the AVI infoframe has hdmi_quantization_range and
>>>> hdmi_ycc_quantization_range along with vid_code.
>>>>
>>>> I'm not at all clear what to do with this information. Is there
>>>> anything you see in the datasheet [1] that points to something I need
>>>> to be doing?
>>>
>>> You can ignore hdmi_ycc_quantization_range, it is the hdmi_quantization_range
>>> that you need to read out.
>>>
>>> The TDA can receive the following formats:
>>>
>>> RGB Full Range
>>> RGB Limited Range
>>> YUV Bt.601 (aka SMPTE 170M)
>>> YUV Rec.709
>>>
>>> The YUV formats are always limited range.
>>>
>>> The TDA can transmit RGB and YUV to the SoC. You want RGB to be full range and
>>> YUV to be limited range. YUV can be either 601 or 709.
>>>
>>> So if the TDA transmits RGB then you need to support the following conversions:
>>>
>>> RGB Full -> RGB Full
>>> RGB Limited -> RGB Full
>>> YUV 601 -> RGB Full
>>> YUV 709 -> RGB Full
>>>
>>> And if the TDA transmits YUV then you need these conversions:
>>>
>>> RGB Full -> YUV601 or YUV709
>>> RGB Limited -> YUV601 or YUV709
>>> YUV601 -> YUV601
>>> YUV709 -> YUV709
>>>
>>> For the RGB to YUV conversion you have a choice of converting to YUV601 or 709.
>>> I recommend to either always convert to YUV601 or to let it depend on the resolution
>>> (SDTV YUV601, HDTV YUV709).
>>>
>>
>> Ok - this is a good explanation that I should be able to follow. I
>> will make sure to take into account hdmi_quantization_range when I
>> setup the colorspace conversion matrix for v3.
> 
> Hans,
> 
> I'm having trouble figuring out the conversion matrix to use between
> limited and full.
> 
> Currently I have the following conversion matrices, the values which
> came from some old vendor code:
> 
>         /* Colorspace conversion matrix coefficients and offsets */
>         struct color_matrix_coefs {
>                 /* Input offsets */
>                 s16 offint1;
>                 s16 offint2;
>                 s16 offint3;
>                 /* Coeficients */
>                 s16 p11coef;
>                 s16 p12coef;
>                 s16 p13coef;
>                 s16 p21coef;
>                 s16 p22coef;
>                 s16 p23coef;
>                 s16 p31coef;
>                 s16 p32coef;
>                 s16 p33coef;
>                 /* Output offsets */
>                 s16 offout1;
>                 s16 offout2;
>                 s16 offout3;
>         };
>         /* Conversion matrixes */
>         enum {
>                 ITU709_RGBLIMITED,
>                 ITU601_RGBLIMITED,
>                 RGBLIMITED_ITU601,
>        };
>        static const struct color_matrix_coefs conv_matrix[] = {
>                 /* ITU709 -> RGBLimited */
>                 {
>                         -256, -2048,  -2048,
>                         4096, -1875,   -750,
>                         4096,  6307,      0,
>                         4096,     0,   7431,
>                          256,   256,    256,
>                 },
>                 /* YUV601 limited -> RGB limited */
>                 {
>                         -256, -2048,  -2048,
>                         4096, -2860,  -1378,
>                         4096,  5615,      0,
>                         4096,     0,   7097,
>                         256,    256,    256,
>                 },
>                 /* RGB limited -> ITU601 */
>                 {
>                         -256,  -256,   -256,
>                         2404,  1225,    467,
>                         -1754, 2095,   -341,
>                         -1388, -707,   2095,
>                         256,   2048,   2048,
>                 },
>         };
> 
> Assuming the above are correct this leaves me missing RGB limitted ->
> RGB full, YUV601 -> RGB full, YUV709 -> RGB full, and RGB Full ->
> YUV601.
> 
> I don't have documentation for the registers but I'm assuming the
> input offset is applied first, then the multiplication by the coef,
> then the output offset is applied. I'm looking over
> https://en.wikipedia.org/wiki/YUV for colorspace conversion matrices
> but I'm unable to figure out how to apply those to the above. Any
> ideas?

For the YUV to RGB full conversions all you need to do is to change
the last row to 0, 0, 0 (since you no longer apply an offset) and
multiply all matrix coefficients by (255 / 219) to ensure the matrix
result is in the range [0-255] instead of [0-219].

For the RGB lim to RGB full conversion you need this matrix:

-256, -256, -256,
S, 0, 0,
0, S, 0,
0, 0, S,
0, 0, 0

Where S = 4096 * 255 / 219. I'm assuming 4096 equals 1.0 using fixed
point format.

Actually, looking at the order of the matrix values I suspect it might
be:

-256, -256, -256,
0, S, 0,
0, 0, S,
S, 0, 0,
0, 0, 0

You'll have to test this to verify which of the two is the right one.

For RGB Full to YUV you use the RGB lim to YUV values but replace the
first line by 0, 0, 0 and multiply the 3x3 matrix values with (219 / 255).

That should do it.

Regards,

	Hans

Powered by blists - more mailing lists