| 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
| ||
|
Message-ID: <de946538-c958-1694-9e28-e2ab5d6fd997@gmail.com>
Date: Tue, 16 Apr 2019 16:59:40 -0700
From: Steve Longerbeam <slongerbeam@...il.com>
To: Niklas Söderlund <niklas.soderlund@...natech.se>
Cc: linux-media@...r.kernel.org, Eugeniu Rosca <erosca@...adit-jv.com>,
Michael Rodin <mrodin@...adit-jv.com>,
Eugen Friedrich <efriedrich@...adit-jv.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
"open list:MEDIA DRIVERS FOR RENESAS - VIN"
<linux-renesas-soc@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] media: rcar-csi2: Fix field detection
Hi Niklas,
On 4/16/19 4:50 PM, Niklas Söderlund wrote:
> Hi Steve,
>
> Thanks for your work.
>
> I upported most rcar-csi2 patches but a few of them are still pending,
> an alternating version based on this BSP patch is already on its way.
>
> https://patchwork.linuxtv.org/patch/55623/
Great, that patch confirms that FLD_NUM should be set to 0 or 1 when
setting FLD_DET_SEL = 0x01.
But your patch has it inverted. NTSC should be FLD_NUM=1, PAL should be
FLD_NUM=0. Can you please confirm that after reading the commit
description of my RFC patch.
> As a general feedback calling v4l2_subdev_call() for g_std is not the
> best way to handle this problem in a MC centric driver. There is a
> discussion about in in earlier versions of the patch mentioned above.
Ok, yes that's true. I borrowed that code from rcar-3.9.0 BSP, but the
important thing is to get the above right and confirmed.
Steve
>
> On 2019-04-16 16:38:07 -0700, Steve Longerbeam wrote:
>> The RCAR Gen3 hardware manual doesn't mention that the "Word Count" field
>> in the MIPI CSI-2 Frame Start packet is really the frame number. FS packets
>> are short packets, and short packets do not have a WC but rather a 16-bit
>> data field, and for Frame synchronization packets (FS/FE) the 16-bit data
>> field contains a frame number.
>>
>> Here is a reprinting from the MIPI CSI-2 specification, version 2
>> (section 9.1.2 Low Level Protocol Short Packet Format):
>>
>> "Figure 42 and Figure 43 show the Low Level Protocol Short Packet
>> structures for the D-PHY and C-PHY physical layer options, respectively.
>> For each option, the Short Packet structure matches the Packet Header of
>> the corresponding Low Level Protocol Long Packet structure with the
>> exception that the Packet Header Word Count (WC) field shall be replaced
>> by the Short Packet Data Field...For Frame Synchronization Data Types
>> the Short Packet Data Field shall be the frame number..."
>>
>> Also in section 9.8.1 Frame Synchronization Packets, the CSI-2 spec reads:
>>
>> "The behavior of the 16-bit frame number shall be as one of the following
>> - Frame number is always zero – frame number is inoperative.
>> - Frame number increments by 1 for every FS packet with the same Virtual
>> Channel and is periodically reset to one e.g. 1, 2, 1, 2, 1, 2, 1, 2 or
>> 1, 2, 3, 4, 1, 2, 3, 4"
>>
>> So from the above, FLD_NUM in the FLD register matches against the
>> frame number transmitted by the CSI-2 source in the Frame Start Packet,
>> and goes as "1,2,1,2,1,2" or "1,2,3,4,...,1,2,3,4". If there is a match,
>> the RCAR CSI-2 receiver declares the field as the even, e.g. the BOTTOM,
>> field.
>>
>> NTSC transmits bottom/even field first, and PAL and HD transmit top/odd
>> field first. So fields with a frame number 1 in the CSI-2 Frame Start
>> Packet are bottom/even fields for NTSC, and frame number 2 are bottom/even
>> fields for PAL/HD.
>>
>> But the above assumes the transmitting CSI-2 sensor is sending frame
>> numbers in the sequence "1,2,1,2,...". But according to the CSI-2 spec,
>> sensors are also allowed to send frame numbers that simply increment as
>> in "1,2,3,4,...". To generalize to catch those cases, set FLD_DET_SEL
>> to 0x01, which according to the RCAR Gen3 hardware manual means "the
>> field is detected as the EVEN field when FLD_NUM[0] matches WC[0]".
>> Then, the even/bottom fields have odd frame numbers for NTSC, and even
>> frame numbers for PAL (this patch assumes HDMI sources will not
>> implement .g_std(), so std will be 0 in that case).
>>
>> Finally, set the FLD register to non-zero value only if trnsmitter is
>> sending ALTERNATE fields.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@...il.com>
>> ---
>> drivers/media/platform/rcar-vin/rcar-csi2.c | 37 ++++++++++++++++++---
>> 1 file changed, 32 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
>> index a438ec2c218f..c5bee20d7907 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
>> @@ -67,6 +67,7 @@ struct rcar_csi2;
>> /* Field Detection Control */
>> #define FLD_REG 0x1c
>> #define FLD_FLD_NUM(n) (((n) & 0xff) << 16)
>> +#define FLD_FLD_DET_SEL(n) (((n) & 0x3) << 4)
>> #define FLD_FLD_EN4 BIT(3)
>> #define FLD_FLD_EN3 BIT(2)
>> #define FLD_FLD_EN2 BIT(1)
>> @@ -462,10 +463,11 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp)
>> return mbps;
>> }
>>
>> -static int rcsi2_start(struct rcar_csi2 *priv)
>> +static int rcsi2_start(struct rcar_csi2 *priv, struct v4l2_subdev *nextsd)
>> {
>> const struct rcar_csi2_format *format;
>> - u32 phycnt, vcdt = 0, vcdt2 = 0;
>> + u32 phycnt, vcdt = 0, vcdt2 = 0, fld = 0;
>> + v4l2_std_id std = 0;
>> unsigned int i;
>> int mbps, ret;
>>
>> @@ -476,6 +478,10 @@ static int rcsi2_start(struct rcar_csi2 *priv)
>> /* Code is validated in set_fmt. */
>> format = rcsi2_code_to_fmt(priv->mf.code);
>>
>> + ret = v4l2_subdev_call(nextsd, video, g_std, &std);
>> + if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
>> + return ret;
>> +
>> /*
>> * Enable all supported CSI-2 channels with virtual channel and
>> * data type matching.
>> @@ -509,9 +515,30 @@ static int rcsi2_start(struct rcar_csi2 *priv)
>> rcsi2_reset(priv);
>> rcsi2_write(priv, PHTC_REG, 0);
>>
>> + /*
>> + * NTSC standard transmits even/bottom field first, then odd/top.
>> + * PAL standard and interlaced HD transmit odd/top field first,
>> + * then even/bottom.
>> + *
>> + * For CSI-2 sensors that transmit frame numbers (in the CSI-2
>> + * Frame Start packet) as "1,2,1,2,..." or "1,2,3,4,5,...", then:
>> + *
>> + * - for NTSC, the even/bottom fields have odd frame numbers.
>> + * - for PAL and HD, the even/bottom fields have even frame numbers.
>> + */
>> + if (priv->mf.field == V4L2_FIELD_ALTERNATE) {
>> + fld = FLD_FLD_DET_SEL(1) | FLD_FLD_EN4 | FLD_FLD_EN3 |
>> + FLD_FLD_EN2 | FLD_FLD_EN;
>> +
>> + if (std & V4L2_STD_525_60)
>> + fld |= FLD_FLD_NUM(1); /* NTSC */
>> + else
>> + fld |= FLD_FLD_NUM(0); /* PAL or HD */
>> + }
>> +
>> /* Configure */
>> - rcsi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
>> - FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
>> + rcsi2_write(priv, FLD_REG, fld);
>> +
>> rcsi2_write(priv, VCDT_REG, vcdt);
>> if (vcdt2)
>> rcsi2_write(priv, VCDT2_REG, vcdt2);
>> @@ -591,7 +618,7 @@ static int rcsi2_s_stream(struct v4l2_subdev *sd, int enable)
>> if (enable && priv->stream_count == 0) {
>> pm_runtime_get_sync(priv->dev);
>>
>> - ret = rcsi2_start(priv);
>> + ret = rcsi2_start(priv, nextsd);
>> if (ret) {
>> pm_runtime_put(priv->dev);
>> goto out;
>> --
>> 2.17.1
>>
Powered by blists - more mailing lists