lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 1 Sep 2017 11:20:31 +0800 From: "Yang, Wenyou" <Wenyou.Yang@...rochip.com> To: Hans Verkuil <hverkuil@...all.nl>, <mchehab@...pensource.com> CC: <Nicolas.Ferre@...rochip.com>, <linux-kernel@...r.kernel.org>, <sakari.ailus@....fi>, <corbet@....net>, <linux-arm-kernel@...ts.infradead.org>, <linux-media@...r.kernel.org> Subject: Re: [PATCH 1/3] media: atmel-isc: Not support RBG format from sensor. Hi Hans, On 2017/8/24 14:41, Hans Verkuil wrote: > On 08/24/2017 08:25 AM, Yang, Wenyou wrote: >> >> On 2017/8/23 18:37, Hans Verkuil wrote: >>> On 08/22/17 09:30, Wenyou.Yang@...rochip.com wrote: >>>> Hi Hans, >>>> >>>>> -----Original Message----- >>>>> From: Hans Verkuil [mailto:hverkuil@...all.nl] >>>>> Sent: 2017年8月22日 15:00 >>>>> To: Wenyou Yang - A41535 <Wenyou.Yang@...rochip.com>; Mauro Carvalho >>>>> Chehab <mchehab@...pensource.com> >>>>> Cc: Nicolas Ferre - M43238 <Nicolas.Ferre@...rochip.com>; linux- >>>>> kernel@...r.kernel.org; Sakari Ailus <sakari.ailus@....fi>; Jonathan Corbet >>>>> <corbet@....net>; linux-arm-kernel@...ts.infradead.org; Linux Media Mailing List >>>>> <linux-media@...r.kernel.org> >>>>> Subject: Re: [PATCH 1/3] media: atmel-isc: Not support RBG format from sensor. >>>>> >>>>> On 08/22/2017 03:18 AM, Yang, Wenyou wrote: >>>>>> Hi Hans, >>>>>> >>>>>> On 2017/8/21 22:07, Hans Verkuil wrote: >>>>>>> On 08/17/2017 09:16 AM, Wenyou Yang wrote: >>>>>>>> The 12-bit parallel interface supports the Raw Bayer, YCbCr, >>>>>>>> Monochrome and JPEG Compressed pixel formats from the external >>>>>>>> sensor, not support RBG pixel format. >>>>>>>> >>>>>>>> Signed-off-by: Wenyou Yang <wenyou.yang@...rochip.com> >>>>>>>> --- >>>>>>>> >>>>>>>> drivers/media/platform/atmel/atmel-isc.c | 5 +++++ >>>>>>>> 1 file changed, 5 insertions(+) >>>>>>>> >>>>>>>> diff --git a/drivers/media/platform/atmel/atmel-isc.c >>>>>>>> b/drivers/media/platform/atmel/atmel-isc.c >>>>>>>> index d4df3d4ccd85..535bb03783fe 100644 >>>>>>>> --- a/drivers/media/platform/atmel/atmel-isc.c >>>>>>>> +++ b/drivers/media/platform/atmel/atmel-isc.c >>>>>>>> @@ -1478,6 +1478,11 @@ static int isc_formats_init(struct isc_device *isc) >>>>>>>> while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, >>>>>>>> NULL, &mbus_code)) { >>>>>>>> mbus_code.index++; >>>>>>>> + >>>>>>>> + /* Not support the RGB pixel formats from sensor */ >>>>>>>> + if ((mbus_code.code & 0xf000) == 0x1000) >>>>>>>> + continue; >>>>>>> Am I missing something? Here you skip any RGB mediabus formats, but >>>>>>> in patch 3/3 you add RGB mediabus formats. But this patch prevents >>>>>>> those new formats from being selected, right? >>>>>> This patch prevents getting the RGB format from the sensor directly. >>>>>> The RGB format can be produced by ISC controller by itself. >>>>> OK, I think I see what is going on here. The isc_formats array really is two arrays >>>>> in one: up to RAW_FMT_IND_END it describes what it can receive from the >>>>> source, and after that it describes what it can convert it to. >>>> Not exactly. >>>> >>>> Yes, up to RAW_FMT_IND_END, these formats must be got from the senor, they are RAW formats. >>>> From ISC_FMT_IND_START to ISC_FMT_IND_END, they can be generated by the ISC controller. >>>> It is possible they can be got from the sensor too, the driver will check it. >>>> If it can be got from both the sensor and the ISC controller, the user can use the "sensor_preferred" parameter to decide from which one to get. >>>> The RBG formats are the exception. >>>> >>>>> But if you can't handle RGB formats from the sensor, then why not make sure >>>>> none of the mbus codes in isc_formats uses RGB? That makes much more sense. >>>>> >>>>> E.g.: >>>>> >>>>> { V4L2_PIX_FMT_RGB565, MEDIA_BUS_FMT_RGB565_2X8_LE, 16, >>>>> ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, >>>>> ISC_RLP_CFG_MODE_RGB565, >>>>> ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x7b, >>>>> false, false }, >>>>> >>>>> Why use MEDIA_BUS_FMT_RGB565_2X8_LE if this apparently is not supported? >>>> This array is also the lists of all formats supported by the ISC(including got from the sensor). >>>> The RGB formats are only generated by the ISC controller, not from the sensor. >>> You're adding code that skips any entries of the table where mbus_code is an >>> RGB code. But this can also be done by not having RGB mbus codes in the table >>> in the first place since they make no sense if the HW cannot handle that! >>> Set the mbus_code to e.g. 0 for such entries, that makes more sense. >>> >>> I also strongly suggest changing how the table is organized since those >>> _FMT_IND_ indices are all to easy to get wrong (and frankly hard to understand). >> Yes, you are right, I will change it. Do you have some advice? > Two options spring to mind: split into two tables or add a bool that tells whether > the format can be created by the isc or not. I reworked the format table, http://lists.infradead.org/pipermail/linux-arm-kernel/2017-August/529683.html Please give your comments. > > Regards, > > Hans Best Regards, Wenyou Yang
Powered by blists - more mailing lists