From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756597AbdKDJxl (ORCPT ); Sat, 4 Nov 2017 05:53:41 -0400 Received: from lb1-smtp-cloud7.xs4all.net ([194.109.24.24]:58452 "EHLO lb1-smtp-cloud7.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756499AbdKDJxi (ORCPT ); Sat, 4 Nov 2017 05:53:38 -0400 Subject: Re: [PATCH v2 3/4] media: i2c: Add TDA1997x HDMI receiver driver To: Tim Harvey Cc: linux-media , alsa-devel@alsa-project.org, "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Shawn Guo , Steve Longerbeam , Philipp Zabel , Hans Verkuil , Mauro Carvalho Chehab References: <1507783506-3884-1-git-send-email-tharvey@gateworks.com> <1507783506-3884-4-git-send-email-tharvey@gateworks.com> <230ceb18-1d69-7fa8-acb0-c810094f8e50@xs4all.nl> <5c68003a-380d-d339-718f-47bce64cdae8@xs4all.nl> From: Hans Verkuil Message-ID: Date: Sat, 4 Nov 2017 10:53:31 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-CMAE-Envelope: MS4wfPJJ3+su8XqjVJ2B2hx+fEpYb1BngZ963lKa0YU4P8JLZSUm8rLxxUq+6lOk0/ZsIFj0jttYytdfjTG6p6zm4pYkxoRf8qQEVy+ngK7StrkiTQ7BE5Zw ftR6zZGgHg2NzcpdMaYczPFJ/+hEzabnG1st2NzrhQMZ1N7OHgA5jczczyaCLdQkcWqV7b2JSXsk0aGM7lwLPDgib3VBTozUWLRDInYBk/k5xPAXPNbtWwI1 q8cBJJCFWZP0A3KU7ltqClwCR1RLZ5a3wNLZ5N3RVzWQjjL/e3L10J5ksengnhziEDxY3RL2TZiQavrAMSYk06YzqVUmIjlh0si3o1mAzGpgMiRYMPi9a3AD lhXHvB2xQp/Gy4lxRA3k7aJoGjk136nz5pnG5Jx8VPCgpdnTKKsGCVec0clGDXgRh+rHDvdEl6V+Kdv4VT5F0QbV4Fkupb5Ah0J5riwbNryXfal8QSwEergt Th+O50Z56yjc6tw/ Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/04/2017 01:17 AM, Tim Harvey wrote: > On Mon, Oct 23, 2017 at 10:05 AM, Tim Harvey wrote: >> >> On Fri, Oct 20, 2017 at 9:23 AM, Hans Verkuil 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