linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rodin, Michael (Ferchau; ADITG/ESM1)" <mrodin@de.adit-jv.com>
To: "niklas.soderlund@ragnatech.se" <niklas.soderlund@ragnatech.se>,
	"kieran.bingham@ideasonboard.com"
	<kieran.bingham@ideasonboard.com>
Cc: "mchehab@kernel.org" <mchehab@kernel.org>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"michael@rodin.online" <michael@rodin.online>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH] media: i2c: adv748x: initialize bit 7 of csi_tx_top_reg_1f
Date: Thu, 16 May 2019 14:24:13 +0000	[thread overview]
Message-ID: <AC35D0CFBC66A84AAA9DF4334B52828D136181C3@HI2EXCH01.adit-jv.com> (raw)
In-Reply-To: <20190510190124.GE28561@bigcity.dyn.berto.se>

Hi Kieran and Niklas,

thank you for your responses!

> Hi Kieran and Michael,
>
> On 2019-05-10 17:25:24 +0100, Kieran Bingham wrote:
> <snip>
>
> >
> > Niklas, how does RCar-CSI determine the top/bottom sequence?
>
> That patch just got merged in the media tree a few days ago,
>
>     9f7983bdc4925ae2 ("media: rcar-csi2: Propagate the FLD signal for NTSC
> and PAL")
>
> >
> > Do we have field inversion currently? (or one which is perhaps swapped
> > somewhere along the pipeline in rcar-vin?)
> >
>
> I'm not sure which tree this patch is developed on but Steve Longerbeam
> posted a RFC which IMHO had the fields inverted, there was a discussion in
his
> thread [1] where I tried to get to the bottom of the issue. My conclusions
> there might be wrong due to the issues addressed in this patch.
>
> Michael: Did you have Steve's patch in your tree when testing this?
>
> 1. https://patchwork.kernel.org/patch/10904263/
>
> --
> Regards,
> Niklas Söderlund

I had another version of Steve's patch when testing, but the FLD_NUM
setting was still the opposite compared to 9f7983bdc4925ae2
("media: rcar-csi2: Propagate the FLD signal for NTSC and PAL").
I could send all patches from the private pull request which Steve Longerbeam
has created for ADIT if you want to better understand my test results,
but probably they can not be applied to the current mainline development tree.
The patch for adv748x I used for testing looks a bit different as well,
so it can be applied to the tree used by ADIT. But the functionality
is the same (I can provide the patch as well if it is required.).
There are also concerns regarding VnMC.FOC bit (I tested V4L2_FIELD_INTERLACED mode 
and in my tests I figured out, that this bit does not exactly do
what the Renesas Hardware Manual describes and should be always set to 0
regardless whether NTSC or PAL are used. But I had only Raspberry Pi as
NTSC test source and no additional NTSC camera for verification,
so the results may be not reliable.).


Niklas, in [1] you mentioned that you could read the WC counter (which is in
fact the frame number in case of frame start packets) in the interrupt
INT_FSFE and it would start at 0. Could you please share the patch you used
for reading? As Steve Longerbeam mentioned in [2], this would be a CSI spec 
violation, which he has cited in the commit message of his RFC patch[3]. It's important 
to mention that the violation would be on the side of the adv748x chip (adv7482 on my 
Salvator-X board), because it creates the Frame Start packets. And according to the description 
of the FRAMENUMBER_INTERLACED bit in [4] (page 193), adv7481 should always send the
"1,2,1,2..." CSI frame number sequence (I could  not find a generic document
valid for all adv748x but I doubt that it would be  different there.).
So starting with CSI frame number 0 would even violate specification in it's 
own data sheet. Another possibility could also be a silicon bug in rcar CSI interface,
which would decrement the WC value by one. 


[1] https://patchwork.kernel.org/patch/10904263/#22594157
[2] https://patchwork.kernel.org/patch/10904263/#22594563
[3] https://patchwork.kernel.org/patch/10904263
[4] https://www.analog.com/media/en/technical-documentation/user-guides/ADV7481_UG-747.pdf

Regards,
Michael

  reply	other threads:[~2019-05-16 14:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1557502240-16274-1-git-send-email-mrodin@de.adit-jv.com>
2019-05-10 16:25 ` [PATCH] media: i2c: adv748x: initialize bit 7 of csi_tx_top_reg_1f Kieran Bingham
2019-05-10 19:01   ` Niklas Söderlund
2019-05-16 14:24     ` Rodin, Michael (Ferchau; ADITG/ESM1) [this message]
2019-05-16 20:44       ` niklas.soderlund

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AC35D0CFBC66A84AAA9DF4334B52828D136181C3@HI2EXCH01.adit-jv.com \
    --to=mrodin@de.adit-jv.com \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=michael@rodin.online \
    --cc=niklas.soderlund@ragnatech.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).