From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Longerbeam Subject: Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format Date: Sun, 10 Jul 2016 09:33:11 -0700 Message-ID: <578278C7.8020503@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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1467999258.2365.71.camel@pengutronix.de> Sender: linux-kernel-owner@vger.kernel.org To: Philipp Zabel , Steve Longerbeam Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Suresh Dhandapani List-Id: dri-devel@lists.freedesktop.org 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. 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. Steve From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933442AbcGJQdR (ORCPT ); Sun, 10 Jul 2016 12:33:17 -0400 Received: from relay1.mentorg.com ([192.94.38.131]:50478 "EHLO relay1.mentorg.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933144AbcGJQdQ (ORCPT ); Sun, 10 Jul 2016 12:33:16 -0400 Subject: Re: [PATCH 12/16] gpu: ipu-v3: Fix CSI0 blur in NTSC format To: Philipp Zabel , Steve Longerbeam 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> CC: , , Suresh Dhandapani From: Steve Longerbeam Message-ID: <578278C7.8020503@mentor.com> Date: Sun, 10 Jul 2016 09:33:11 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <1467999258.2365.71.camel@pengutronix.de> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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. Steve