From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752535AbcGOM73 (ORCPT ); Fri, 15 Jul 2016 08:59:29 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:41281 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752060AbcGOM65 (ORCPT ); Fri, 15 Jul 2016 08:58:57 -0400 Message-ID: <1468587534.7213.41.camel@pengutronix.de> Subject: Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format From: Philipp Zabel To: Steve Longerbeam Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Suresh Dhandapani Date: Fri, 15 Jul 2016 14:58:54 +0200 In-Reply-To: <5786C876.50300@mentor.com> References: <1467932621-358-1-git-send-email-steve_longerbeam@mentor.com> <1467932621-358-13-git-send-email-steve_longerbeam@mentor.com> <1467999258.2365.71.camel@pengutronix.de> <578278C7.8020503@mentor.com> <5786C876.50300@mentor.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2001:67c:670:100:96de:80ff:fec2:9969 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am Mittwoch, den 13.07.2016, 16:02 -0700 schrieb Steve Longerbeam: > On 07/10/2016 09:33 AM, Steve Longerbeam wrote: > > > > > > On 07/08/2016 10:34 AM, Philipp Zabel wrote: > >> Am Donnerstag, den 07.07.2016, 16:03 -0700 schrieb Steve Longerbeam: > >>> From: Suresh Dhandapani > >>> > >>> This patch will change the register IPU_CSI0_CCIR_CODE_2 value from > >>> 0x40596 to 0x405A6. The change is related to the Start of field 1 > >>> first blanking line command bit[5-3] for NTSC format only. This > >>> change is dependent with ADV chip where the NEWAVMODE is set to 0 > >>> in register 0x31. Setting NEWAVMODE to "0" in ADV means "EAV/SAV > >>> codes generated to suit analog devices encoders". > >>> > >>> Signed-off-by: Suresh Dhandapani > >>> --- > >>> drivers/gpu/ipu-v3/ipu-csi.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c > >>> index 0eac28c..ec81958 100644 > >>> --- a/drivers/gpu/ipu-v3/ipu-csi.c > >>> +++ b/drivers/gpu/ipu-v3/ipu-csi.c > >>> @@ -422,7 +422,7 @@ int ipu_csi_init_interface(struct ipu_csi *csi, > >>> ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN, > >>> CSI_CCIR_CODE_1); > >>> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2); > >>> + ipu_csi_write(csi, 0x405A6, CSI_CCIR_CODE_2); > >>> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3); > >>> } else { > >>> dev_err(csi->ipu->dev, > >> This looks like a very hardware specific hack? I'll at least have to > >> test if that also works with other analog decoders. > > > > Hi Philipp, > > > > Yes it's a hack, but it has always been a hack (hardcoded values). And the > > reason is simple, nobody AFAIK (including me) understands how to program > > these CSI_CCIR_CODE registers, the description in the reference manual is > > complete gibberish. > > Hi Philipp, Ian over at linux-media helped me to understand these registers a > little better, although there are still mysteries given the poor documentation. > You should have been copied on that linux-media thread. > > > > > The reason we made this change is that, in discussions with Analog Devices, > > they recommended setting NEWAVMODE, which changes the positions of > > the AV codes sent by the ADV7180 on the bt.656 bus. It took Suresh at least > > a full day of reverse engineering (Suresh correct me if I am wrong) to hit > > on the correct values in these registers to regain stable video after switching > > the ADV7180 to NEWAVMODE. > > So this NEWAVMODE is somehow breaking from the BT.656 standard, which > necessitated the change to CSI_CCIR_CODE_2. So NEWAVMODE if enabled in > the ADV7180 will break other capture backends that are expecting standard > BT.656 SAV/EAV codes. So NEWAVMODE should not be used and I will remove > this patch in the next version. Ok. To use that mode, first a new v4l2 mbus type and corresponding DT bindings would have to be added. regards Philipp