From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751553AbdBUJ4d (ORCPT ); Tue, 21 Feb 2017 04:56:33 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:52837 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750992AbdBUJ4W (ORCPT ); Tue, 21 Feb 2017 04:56:22 -0500 X-AuditID: cbfec7ef-f79d26d00000420c-22-58ac0ec4685b Subject: Re: [PATCH v4 1/4] [media] exynos-gsc: Use 576p instead 720p as a threshold for colorspaces To: Thibault Saunier , linux-kernel@vger.kernel.org Cc: Mauro Carvalho Chehab , Marek Szyprowski , Kukjin Kim , Mauro Carvalho Chehab , Nicolas Dufresne , Andi Shyti , linux-media@vger.kernel.org, Shuah Khan , Javier Martinez Canillas , linux-samsung-soc@vger.kernel.org, Krzysztof Kozlowski , Inki Dae , Sylwester Nawrocki , linux-arm-kernel@lists.infradead.org, Ulf Hansson , Hans Verkuil From: Andrzej Hajda Message-id: Date: Tue, 21 Feb 2017 10:56:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-version: 1.0 In-reply-to: <20170213190836.26972-2-thibault.saunier@osg.samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02Sa0iTURzGOXsve7Vmb9P031UahhVpCn04UJRFxBt9KCTaKijf8tVZzsuW kn0oIUpzkeGUbBUEieCcqJuWTQvbllNLZzdt1bQw122itbKLZTW3wm+//znPw/95DochpM+o BUxG1hFBncVnyuhQ8nrH9944e5hRkXDFtwrfsHso3K3ziHDZ0HkSe0eNIlw6/IHATmeDGJuG +yn8yHKZxmcbmilc6bwtwnV2txhXm36K8FBVLrZ8HqSxzVtE4YrP4yT2tZcR2FEnT5JyLe4q xJkMZ2juRX8bzZmrTnBNpW4xZ/rkprhzTQbE+UxLuO4vPvGOkD2h61KFzIx8Qb16fUqosqrf TuT0RR8trflNFaKx+SUohAF2DdxvrBUHOBL6BuvpEhTKSNlqBB+LRoKDD8FYuZ7455i4Yib+ q7SWenFg8CCwvO4UlSCGCWfToOLMPr8hgpXDxMAp0q8hWB0F7qY70/todgX8MrtoP0vY9eDr fTp9TrLLoKvaLPLzPFYBLU/PBjVz4ZtukPRzCLsFxh6/mk5EsAlQWW4TBTgazMbR6XTAVjDg vTZA+AMBuxhM7cEGm+GHtiPI4fDe0RTsvwge6bRkwKtF8Km0UxwYyhFMjVcGHWvB5nhABbaF Qdn1C8EFEig+LQ0gB7/fLg2oN4LHOxl8xicIXK5W+jyK1s/oo5/RQT+jw1VEGFCEkKdRpQua xHgNr9LkZaXHH8xWmdDfH3dvyjHWgkZO7rQilkGy2ZKUCYNCSvH5mgKVFQFDyCIke2cZFVJJ Kl9wTFBn71fnZQoaK1rIkLIoya2rj+VSNp0/IhwWhBxB/e9WxIQsKEQ7jXN2SQuVUT2n7O/G 0+bkk3fjv3ePlF3WVdeEh21iDKkXTxSQXGSFzJZ/eIqP+3ItaivKndtePN6lVB4aGjbffJ7k ihwNP3A82eF8GNP3csNCPnnSqLqkiG0eOKbVx7RmPOnJbSRz2+KowYHt22q/xoZ5jRbn8oe7 k23yhjdWGalR8okrCbWG/wPyUIyXbQMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrBIsWRmVeSWpSXmKPExsVy+t/xy7rH+dZEGOw7LWCx/cgzVotTk58x WUy6P4HF4s3bNUwW/Y9fM1ucP7+B3WLT42usFpd3zWGz6NmwldVixvl9TBZrj9xlt1i26Q+T xf0lhRa7vtxjszj8pp3VYuqXDywWnw9MYrY4vjbcQchjx90ljB6bVnWyedy5tofNY/OSeo8t /XfZPTZ9usvq0bdlFaPH501yHqe+fmYP4Ixys8lITUxJLVJIzUvOT8nMS7dVCg1x07VQUshL zE21VYrQ9Q0JUlIoS8wpBfKMDNCAg3OAe7CSvl2CW8aSa0eYCy7IV/Sv/M/awPhesouRk0NC wETi29zNzBC2mMSFe+vZuhi5OIQEljBKrJ71mhXCecYoceD7OSCHg0NYIE1iamccSIOIQLjE sXuNUA1XGSUWfvnDBOIwC0xllVj47ykbSBWbgKbE3803wWxeATuJz+dusIPYLAKqEieXbWYC sUUFIiTmP13FBFEjKPFj8j0WEJtTwE3i/ZWHzCCLmQX0JO5f1AIJMwvIS2xe85Z5AqPALCQd sxCqZiGpWsDIvIpRJLW0ODc9t9hIrzgxt7g0L10vOT93EyMw1rcd+7llB2PXu+BDjAIcjEo8 vDt+rIoQYk0sK67MPcQowcGsJML7hXdNhBBvSmJlVWpRfnxRaU5q8SFGU6AXJjJLiSbnA9NQ Xkm8oYmhuaWhkbGFhbmRkZI479QPV8KFBNITS1KzU1MLUotg+pg4OKUaGLkeuYU+EDzyeOEp W934gJfzLFoNDogn+Wy0qZniPSVldnfZxu+Ty2RqPkt4vJJIFziu8Gr2PCnN0+kB7NMiTNmv X/Fe6XX0xydlQVOmkpRlrhvLtJt40i//uGCQbzVpyrK3Szkf3XpePUUw5esTtwW35tVeftmU Kcav8Yr/I4PTrLqQn+rnc5VYijMSDbWYi4oTAXkc1OULAwAA X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170221095617eucas1p205178624f37d1d3e9f781e2af48801d6 X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?QW5kcnplaiBIYWpkYRtTUlBPTC1LZXJuZWwgKFRQKRvsgrw=?= =?UTF-8?B?7ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?QW5kcnplaiBIYWpkYRtTUlBPTC1LZXJuZWwgKFRQKRtTYW1z?= =?UTF-8?B?dW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20170213191052epcas3p3af2c3165d94bc82b77f7a7fab6550b15 X-RootMTR: 20170213191052epcas3p3af2c3165d94bc82b77f7a7fab6550b15 References: <20170213190836.26972-1-thibault.saunier@osg.samsung.com> <20170213190836.26972-2-thibault.saunier@osg.samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13.02.2017 20:08, Thibault Saunier wrote: > From: Javier Martinez Canillas > > The media documentation says that the V4L2_COLORSPACE_SMPTE170M colorspace > should be used for SDTV and V4L2_COLORSPACE_REC709 for HDTV. But drivers > don't agree on the display resolution that should be used as a threshold. > > >From EIA CEA 861B about colorimetry for various resolutions: > > - 5.1 480p, 480i, 576p, 576i, 240p, and 288p > The color space used by the 480-line, 576-line, 240-line, and 288-line > formats will likely be based on SMPTE 170M [1]. > - 5.2 1080i, 1080p, and 720p > The color space used by the high definition formats will likely be > based on ITU-R BT.709-4 > > This indicates that in the case that userspace does not specify what > colorspace should be used, we should use 576p as a threshold to set > V4L2_COLORSPACE_REC709 instead of V4L2_COLORSPACE_REC709. Even if it is > only 'likely' and not a requirement it is the best guess we can make. > > The stream should have been encoded with the information and userspace > has to pass it to the driver if it is not the case, otherwise we won't be > able to handle it properly anyhow. > > Also, check for the resolution in G_FMT instead unconditionally setting > the V4L2_COLORSPACE_REC709 colorspace. > > Signed-off-by: Javier Martinez Canillas > Signed-off-by: Thibault Saunier > Reviewed-by: Andrzej Hajda Hi Thibault, Have you analyzed Hans response for the previous patchset [1] ? I am not an expert in the subject but it seems he is right. GSCALER datasheet uses term 'color space conversion' to describe conversions between RGB and YCbCr, not for describe colorspaces as in V4L2. GSC_(IN|OUT)_RGB_(HD|SD)_(WIDE|NARROW) macros used to set IN_CON and OUT_CON registers of GSCALER are probably used incorrectly, they should not be set according to pix_mp->colorspace but rather according to pix_mp->ycbcr_enc and pix_mp->quantization. pix_mp->colorspace should be just copied from output to capture side. Please fix the patch accordingly, and if you are interested you can prepare another patch to fix register setting. [1]: https://lkml.org/lkml/2017/2/10/473 Regards Andrzej > > --- > > Changes in v4: > - Reword commit message to better back our assumptions on specifications > > Changes in v3: > - Do not check values in the g_fmt functions as Andrzej explained in previous review > - Added 'Reviewed-by: Andrzej Hajda ' > > Changes in v2: None > > drivers/media/platform/exynos-gsc/gsc-core.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c > index 59a634201830..db7d9883861b 100644 > --- a/drivers/media/platform/exynos-gsc/gsc-core.c > +++ b/drivers/media/platform/exynos-gsc/gsc-core.c > @@ -472,7 +472,7 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f) > > pix_mp->num_planes = fmt->num_planes; > > - if (pix_mp->width >= 1280) /* HD */ > + if (pix_mp->width > 720 && pix_mp->height > 576) /* HD */ > pix_mp->colorspace = V4L2_COLORSPACE_REC709; > else /* SD */ > pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M; > @@ -519,9 +519,13 @@ int gsc_g_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f) > pix_mp->height = frame->f_height; > pix_mp->field = V4L2_FIELD_NONE; > pix_mp->pixelformat = frame->fmt->pixelformat; > - pix_mp->colorspace = V4L2_COLORSPACE_REC709; > pix_mp->num_planes = frame->fmt->num_planes; > > + if (pix_mp->width > 720 && pix_mp->height > 576) /* HD */ > + pix_mp->colorspace = V4L2_COLORSPACE_REC709; > + else /* SD */ > + pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M; > + > for (i = 0; i < pix_mp->num_planes; ++i) { > pix_mp->plane_fmt[i].bytesperline = (frame->f_width * > frame->fmt->depth[i]) / 8;