From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752192AbbH0Jkm (ORCPT ); Thu, 27 Aug 2015 05:40:42 -0400 Received: from metis.ext.pengutronix.de ([92.198.50.35]:58551 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751300AbbH0Jkk (ORCPT ); Thu, 27 Aug 2015 05:40:40 -0400 Message-ID: <1440668430.3233.68.camel@pengutronix.de> Subject: Re: [PATCH 04/12] gpu: imx: fix support for interlaced modes From: Philipp Zabel To: Russell King - ARM Linux Cc: linux-rockchip@lists.infradead.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Andy Yan , Yakir Yang , Fabio Estevam , Sascha Hauer , Jon Nettleton Date: Thu, 27 Aug 2015 11:40:30 +0200 In-Reply-To: <20150827085411.GE21084@n2100.arm.linux.org.uk> References: <20150808160251.GM7557@n2100.arm.linux.org.uk> <1440664752.3233.36.camel@pengutronix.de> <20150827085411.GE21084@n2100.arm.linux.org.uk> 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 Donnerstag, den 27.08.2015, 09:54 +0100 schrieb Russell King - ARM Linux: > On Thu, Aug 27, 2015 at 10:39:12AM +0200, Philipp Zabel wrote: > > Hi Russell, > > > > Am Samstag, den 08.08.2015, 17:03 +0100 schrieb Russell King: > > > The support for interlaced video modes seems to be broken; we don't use > > > anything other than the vtotal/htotal from the timing information to > > > define the various sync counters. > > > > I finally made time to test this series: > > > > Tested-by: Philipp Zabel > > on i.MX6 GK802 via HDMI connected to a TV (1080p60, 1080i60). > > > > Unfortunately these timings are completely different from what Freescale > > came up with for the TV Encoder on i.MX5, but the code we have currently > > in mainline doesn't work for that either. I suppose I'll follow up with > > a patch that adds yet another sync counter setup for the i.MX5/TVE case. > > > > I'd like to take the two ipu-v3 patches, making a few small changes on > > this one: > > Please don't split the series up. The reason it's a series is because > there's interdependencies between the patches. In that case, can I take the whole series? Or would you like to respin and have my Acked-by: Philipp Zabel with the changes below: > > > Freescale patches for interlaced video support contain an alternative > > > sync counter setup, which we include here. This setup produces the > > > hsync and vsync via the normal counter 2 and 3, but moves the display > > > enable signal from counter 5 to counter 6. Therefore, we need to > > > change the display controller setup as well. > > > > > > The corresponding Freescale patches for this change are: > > > iMX6-HDMI-support-interlaced-display-mode.patch > > > IPU-fine-tuning-the-interlace-display-timing-for-CEA.patch > > > > > > This produces a working interlace format output from the IPU. > > > > ... on i.MX6 via HDMI. > > It should also be correct for any other source which wants interlaced > output. ... on i.MX6, then. Right now I don't know what is the effect of the undocumented settings on the i.MX5's IPUv3M. > > > Signed-off-by: Russell King > > > --- > > > drivers/gpu/ipu-v3/ipu-dc.c | 18 ++++++++--- > > > drivers/gpu/ipu-v3/ipu-di.c | 79 +++++++++++++++++++++------------------------ > > > 2 files changed, 51 insertions(+), 46 deletions(-) > > > > > > diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c > > > index 9ef2e1f54ca4..aa560855c1dc 100644 > > > --- a/drivers/gpu/ipu-v3/ipu-dc.c > > > +++ b/drivers/gpu/ipu-v3/ipu-dc.c > > > @@ -183,12 +183,22 @@ int ipu_dc_init_sync(struct ipu_dc *dc, struct ipu_di *di, bool interlaced, > > > } > > > > > > if (interlaced) { > > > - dc_link_event(dc, DC_EVT_NL, 0, 3); > > > - dc_link_event(dc, DC_EVT_EOL, 0, 2); > > > - dc_link_event(dc, DC_EVT_NEW_DATA, 0, 1); > > > + int word, addr; > > > + > > > + if (dc->di) { > > > + addr = 1; > > > + word = 1; > > > > These two are really one and the same. The address written to the link > > register for the given event has to point to the address of the > > microcode instruction written to the template memory that should be > > executed on this event. > > > > I'd like to drop the word variable and use addr for both. > > Ok. As I said in the commit message, this code comes from Freescale's > patches which I pointed to above. [...] > > > @@ -211,66 +215,59 @@ static void ipu_di_sync_config_interlaced(struct ipu_di *di, > > > sig->mode.hback_porch + sig->mode.hfront_porch; > > > u32 v_total = sig->mode.vactive + sig->mode.vsync_len + > > > sig->mode.vback_porch + sig->mode.vfront_porch; > > > - u32 reg; > > > struct di_sync_config cfg[] = { > > > { > > > - .run_count = h_total / 2 - 1, > > > - .run_src = DI_SYNC_CLK, > > > + /* 1: internal VSYNC for each frame */ > > > + .run_count = v_total * 2 - 1, > > > + .run_src = 3, /* == counter 7 */ > > > > Do you know why this works? The Reference Manual v2 lists that value as > > "NA" in the DI counter #1 Run Resolution field. > > Yes, I noticed that Freescale were using values for the source fields > which were marked as NA in the manual. As it works, I can only assume > that the engineer who came up with this setup has more knowledge than > is public. I'd like small a comment here that yes, we know this is marked as NA/Reserved in the manuals. [...] > I went through the counter setup to understand how it works, and it > seems correct provided you accept that the various source values do > work as the code claims, which includes creating the vsync at the > appropriate half-scanline position without needing this hack. > > It's not easy to work back from the counter setup to get a mental > picture of what's going on, but it is possible to do so. Yes, being able to actually reference counters with higher numbers makes things a lot easier to follow. regards Philipp From mboxrd@z Thu Jan 1 00:00:00 1970 From: Philipp Zabel Subject: Re: [PATCH 04/12] gpu: imx: fix support for interlaced modes Date: Thu, 27 Aug 2015 11:40:30 +0200 Message-ID: <1440668430.3233.68.camel@pengutronix.de> References: <20150808160251.GM7557@n2100.arm.linux.org.uk> <1440664752.3233.36.camel@pengutronix.de> <20150827085411.GE21084@n2100.arm.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20150827085411.GE21084@n2100.arm.linux.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Russell King - ARM Linux Cc: Fabio Estevam , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, Yakir Yang , Andy Yan , linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org QW0gRG9ubmVyc3RhZywgZGVuIDI3LjA4LjIwMTUsIDA5OjU0ICswMTAwIHNjaHJpZWIgUnVzc2Vs bCBLaW5nIC0gQVJNCkxpbnV4Ogo+IE9uIFRodSwgQXVnIDI3LCAyMDE1IGF0IDEwOjM5OjEyQU0g KzAyMDAsIFBoaWxpcHAgWmFiZWwgd3JvdGU6Cj4gPiBIaSBSdXNzZWxsLAo+ID4gCj4gPiBBbSBT YW1zdGFnLCBkZW4gMDguMDguMjAxNSwgMTc6MDMgKzAxMDAgc2NocmllYiBSdXNzZWxsIEtpbmc6 Cj4gPiA+IFRoZSBzdXBwb3J0IGZvciBpbnRlcmxhY2VkIHZpZGVvIG1vZGVzIHNlZW1zIHRvIGJl IGJyb2tlbjsgd2UgZG9uJ3QgdXNlCj4gPiA+IGFueXRoaW5nIG90aGVyIHRoYW4gdGhlIHZ0b3Rh bC9odG90YWwgZnJvbSB0aGUgdGltaW5nIGluZm9ybWF0aW9uIHRvCj4gPiA+IGRlZmluZSB0aGUg dmFyaW91cyBzeW5jIGNvdW50ZXJzLgo+ID4gCj4gPiBJIGZpbmFsbHkgbWFkZSB0aW1lIHRvIHRl c3QgdGhpcyBzZXJpZXM6Cj4gPiAKPiA+IFRlc3RlZC1ieTogUGhpbGlwcCBaYWJlbCA8cC56YWJl bEBwZW5ndXRyb25peC5kZT4KPiA+IG9uIGkuTVg2IEdLODAyIHZpYSBIRE1JIGNvbm5lY3RlZCB0 byBhIFRWICgxMDgwcDYwLCAxMDgwaTYwKS4KPiA+IAo+ID4gVW5mb3J0dW5hdGVseSB0aGVzZSB0 aW1pbmdzIGFyZSBjb21wbGV0ZWx5IGRpZmZlcmVudCBmcm9tIHdoYXQgRnJlZXNjYWxlCj4gPiBj YW1lIHVwIHdpdGggZm9yIHRoZSBUViBFbmNvZGVyIG9uIGkuTVg1LCBidXQgdGhlIGNvZGUgd2Ug aGF2ZSBjdXJyZW50bHkKPiA+IGluIG1haW5saW5lIGRvZXNuJ3Qgd29yayBmb3IgdGhhdCBlaXRo ZXIuIEkgc3VwcG9zZSBJJ2xsIGZvbGxvdyB1cCB3aXRoCj4gPiBhIHBhdGNoIHRoYXQgYWRkcyB5 ZXQgYW5vdGhlciBzeW5jIGNvdW50ZXIgc2V0dXAgZm9yIHRoZSBpLk1YNS9UVkUgY2FzZS4KPiA+ IAo+ID4gSSdkIGxpa2UgdG8gdGFrZSB0aGUgdHdvIGlwdS12MyBwYXRjaGVzLCBtYWtpbmcgYSBm ZXcgc21hbGwgY2hhbmdlcyBvbgo+ID4gdGhpcyBvbmU6Cj4gCj4gUGxlYXNlIGRvbid0IHNwbGl0 IHRoZSBzZXJpZXMgdXAuICBUaGUgcmVhc29uIGl0J3MgYSBzZXJpZXMgaXMgYmVjYXVzZQo+IHRo ZXJlJ3MgaW50ZXJkZXBlbmRlbmNpZXMgYmV0d2VlbiB0aGUgcGF0Y2hlcy4KCkluIHRoYXQgY2Fz ZSwgY2FuIEkgdGFrZSB0aGUgd2hvbGUgc2VyaWVzPyBPciB3b3VsZCB5b3UgbGlrZSB0byByZXNw aW4KYW5kIGhhdmUgbXkgQWNrZWQtYnk6IFBoaWxpcHAgWmFiZWwgPHAuemFiZWxAcGVuZ3V0cm9u aXguZGU+IHdpdGggdGhlCmNoYW5nZXMgYmVsb3c6Cgo+ID4gPiBGcmVlc2NhbGUgcGF0Y2hlcyBm b3IgaW50ZXJsYWNlZCB2aWRlbyBzdXBwb3J0IGNvbnRhaW4gYW4gYWx0ZXJuYXRpdmUKPiA+ID4g c3luYyBjb3VudGVyIHNldHVwLCB3aGljaCB3ZSBpbmNsdWRlIGhlcmUuICBUaGlzIHNldHVwIHBy b2R1Y2VzIHRoZQo+ID4gPiBoc3luYyBhbmQgdnN5bmMgdmlhIHRoZSBub3JtYWwgY291bnRlciAy IGFuZCAzLCBidXQgbW92ZXMgdGhlIGRpc3BsYXkKPiA+ID4gZW5hYmxlIHNpZ25hbCBmcm9tIGNv dW50ZXIgNSB0byBjb3VudGVyIDYuICBUaGVyZWZvcmUsIHdlIG5lZWQgdG8KPiA+ID4gY2hhbmdl IHRoZSBkaXNwbGF5IGNvbnRyb2xsZXIgc2V0dXAgYXMgd2VsbC4KPiA+ID4gCj4gPiA+IFRoZSBj b3JyZXNwb25kaW5nIEZyZWVzY2FsZSBwYXRjaGVzIGZvciB0aGlzIGNoYW5nZSBhcmU6Cj4gPiA+ ICAgaU1YNi1IRE1JLXN1cHBvcnQtaW50ZXJsYWNlZC1kaXNwbGF5LW1vZGUucGF0Y2gKPiA+ID4g ICBJUFUtZmluZS10dW5pbmctdGhlLWludGVybGFjZS1kaXNwbGF5LXRpbWluZy1mb3ItQ0VBLnBh dGNoCj4gPiA+IAo+ID4gPiBUaGlzIHByb2R1Y2VzIGEgd29ya2luZyBpbnRlcmxhY2UgZm9ybWF0 IG91dHB1dCBmcm9tIHRoZSBJUFUuCj4gPiAKPiA+IC4uLiBvbiBpLk1YNiB2aWEgSERNSS4KPiAK PiBJdCBzaG91bGQgYWxzbyBiZSBjb3JyZWN0IGZvciBhbnkgb3RoZXIgc291cmNlIHdoaWNoIHdh bnRzIGludGVybGFjZWQKPiBvdXRwdXQuCgouLi4gb24gaS5NWDYsIHRoZW4uIFJpZ2h0IG5vdyBJ IGRvbid0IGtub3cgd2hhdCBpcyB0aGUgZWZmZWN0IG9mIHRoZQp1bmRvY3VtZW50ZWQgc2V0dGlu Z3Mgb24gdGhlIGkuTVg1J3MgSVBVdjNNLgoKPiA+ID4gU2lnbmVkLW9mZi1ieTogUnVzc2VsbCBL aW5nIDxybWsra2VybmVsQGFybS5saW51eC5vcmcudWs+Cj4gPiA+IC0tLQo+ID4gPiAgZHJpdmVy cy9ncHUvaXB1LXYzL2lwdS1kYy5jIHwgMTggKysrKysrKystLS0KPiA+ID4gIGRyaXZlcnMvZ3B1 L2lwdS12My9pcHUtZGkuYyB8IDc5ICsrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLQo+ID4gPiAgMiBmaWxlcyBjaGFuZ2VkLCA1MSBpbnNlcnRpb25zKCspLCA0NiBk ZWxldGlvbnMoLSkKPiA+ID4gCj4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9pcHUtdjMv aXB1LWRjLmMgYi9kcml2ZXJzL2dwdS9pcHUtdjMvaXB1LWRjLmMKPiA+ID4gaW5kZXggOWVmMmUx ZjU0Y2E0Li5hYTU2MDg1NWMxZGMgMTAwNjQ0Cj4gPiA+IC0tLSBhL2RyaXZlcnMvZ3B1L2lwdS12 My9pcHUtZGMuYwo+ID4gPiArKysgYi9kcml2ZXJzL2dwdS9pcHUtdjMvaXB1LWRjLmMKPiA+ID4g QEAgLTE4MywxMiArMTgzLDIyIEBAIGludCBpcHVfZGNfaW5pdF9zeW5jKHN0cnVjdCBpcHVfZGMg KmRjLCBzdHJ1Y3QgaXB1X2RpICpkaSwgYm9vbCBpbnRlcmxhY2VkLAo+ID4gPiAgCX0KPiA+ID4g IAo+ID4gPiAgCWlmIChpbnRlcmxhY2VkKSB7Cj4gPiA+IC0JCWRjX2xpbmtfZXZlbnQoZGMsIERD X0VWVF9OTCwgMCwgMyk7Cj4gPiA+IC0JCWRjX2xpbmtfZXZlbnQoZGMsIERDX0VWVF9FT0wsIDAs IDIpOwo+ID4gPiAtCQlkY19saW5rX2V2ZW50KGRjLCBEQ19FVlRfTkVXX0RBVEEsIDAsIDEpOwo+ ID4gPiArCQlpbnQgd29yZCwgYWRkcjsKPiA+ID4gKwo+ID4gPiArCQlpZiAoZGMtPmRpKSB7Cj4g PiA+ICsJCQlhZGRyID0gMTsKPiA+ID4gKwkJCXdvcmQgPSAxOwo+ID4gCj4gPiBUaGVzZSB0d28g YXJlIHJlYWxseSBvbmUgYW5kIHRoZSBzYW1lLiBUaGUgYWRkcmVzcyB3cml0dGVuIHRvIHRoZSBs aW5rCj4gPiByZWdpc3RlciBmb3IgdGhlIGdpdmVuIGV2ZW50IGhhcyB0byBwb2ludCB0byB0aGUg YWRkcmVzcyBvZiB0aGUKPiA+IG1pY3JvY29kZSBpbnN0cnVjdGlvbiB3cml0dGVuIHRvIHRoZSB0 ZW1wbGF0ZSBtZW1vcnkgdGhhdCBzaG91bGQgYmUKPiA+IGV4ZWN1dGVkIG9uIHRoaXMgZXZlbnQu Cj4gPiAKPiA+IEknZCBsaWtlIHRvIGRyb3AgdGhlIHdvcmQgdmFyaWFibGUgYW5kIHVzZSBhZGRy IGZvciBib3RoLgo+IAo+IE9rLiAgQXMgSSBzYWlkIGluIHRoZSBjb21taXQgbWVzc2FnZSwgdGhp cyBjb2RlIGNvbWVzIGZyb20gRnJlZXNjYWxlJ3MKPiBwYXRjaGVzIHdoaWNoIEkgcG9pbnRlZCB0 byBhYm92ZS4KWy4uLl0KPiA+ID4gQEAgLTIxMSw2NiArMjE1LDU5IEBAIHN0YXRpYyB2b2lkIGlw dV9kaV9zeW5jX2NvbmZpZ19pbnRlcmxhY2VkKHN0cnVjdCBpcHVfZGkgKmRpLAo+ID4gPiAgCQlz aWctPm1vZGUuaGJhY2tfcG9yY2ggKyBzaWctPm1vZGUuaGZyb250X3BvcmNoOwo+ID4gPiAgCXUz MiB2X3RvdGFsID0gc2lnLT5tb2RlLnZhY3RpdmUgKyBzaWctPm1vZGUudnN5bmNfbGVuICsKPiA+ ID4gIAkJc2lnLT5tb2RlLnZiYWNrX3BvcmNoICsgc2lnLT5tb2RlLnZmcm9udF9wb3JjaDsKPiA+ ID4gLQl1MzIgcmVnOwo+ID4gPiAgCXN0cnVjdCBkaV9zeW5jX2NvbmZpZyBjZmdbXSA9IHsKPiA+ ID4gIAkJewo+ID4gPiAtCQkJLnJ1bl9jb3VudCA9IGhfdG90YWwgLyAyIC0gMSwKPiA+ID4gLQkJ CS5ydW5fc3JjID0gRElfU1lOQ19DTEssCj4gPiA+ICsJCQkvKiAxOiBpbnRlcm5hbCBWU1lOQyBm b3IgZWFjaCBmcmFtZSAqLwo+ID4gPiArCQkJLnJ1bl9jb3VudCA9IHZfdG90YWwgKiAyIC0gMSwK PiA+ID4gKwkJCS5ydW5fc3JjID0gMywJCQkvKiA9PSBjb3VudGVyIDcgKi8KPiA+IAo+ID4gRG8g eW91IGtub3cgd2h5IHRoaXMgd29ya3M/IFRoZSBSZWZlcmVuY2UgTWFudWFsIHYyIGxpc3RzIHRo YXQgdmFsdWUgYXMKPiA+ICJOQSIgaW4gdGhlIERJIGNvdW50ZXIgIzEgUnVuIFJlc29sdXRpb24g ZmllbGQuCj4gCj4gWWVzLCBJIG5vdGljZWQgdGhhdCBGcmVlc2NhbGUgd2VyZSB1c2luZyB2YWx1 ZXMgZm9yIHRoZSBzb3VyY2UgZmllbGRzCj4gd2hpY2ggd2VyZSBtYXJrZWQgYXMgTkEgaW4gdGhl IG1hbnVhbC4gIEFzIGl0IHdvcmtzLCBJIGNhbiBvbmx5IGFzc3VtZQo+IHRoYXQgdGhlIGVuZ2lu ZWVyIHdobyBjYW1lIHVwIHdpdGggdGhpcyBzZXR1cCBoYXMgbW9yZSBrbm93bGVkZ2UgdGhhbgo+ IGlzIHB1YmxpYy4KCkknZCBsaWtlIHNtYWxsIGEgY29tbWVudCBoZXJlIHRoYXQgeWVzLCB3ZSBr bm93IHRoaXMgaXMgbWFya2VkIGFzCk5BL1Jlc2VydmVkIGluIHRoZSBtYW51YWxzLgoKWy4uLl0K PiBJIHdlbnQgdGhyb3VnaCB0aGUgY291bnRlciBzZXR1cCB0byB1bmRlcnN0YW5kIGhvdyBpdCB3 b3JrcywgYW5kIGl0Cj4gc2VlbXMgY29ycmVjdCBwcm92aWRlZCB5b3UgYWNjZXB0IHRoYXQgdGhl IHZhcmlvdXMgc291cmNlIHZhbHVlcyBkbwo+IHdvcmsgYXMgdGhlIGNvZGUgY2xhaW1zLCB3aGlj aCBpbmNsdWRlcyBjcmVhdGluZyB0aGUgdnN5bmMgYXQgdGhlCj4gYXBwcm9wcmlhdGUgaGFsZi1z Y2FubGluZSBwb3NpdGlvbiB3aXRob3V0IG5lZWRpbmcgdGhpcyBoYWNrLgo+IAo+IEl0J3Mgbm90 IGVhc3kgdG8gd29yayBiYWNrIGZyb20gdGhlIGNvdW50ZXIgc2V0dXAgdG8gZ2V0IGEgbWVudGFs Cj4gcGljdHVyZSBvZiB3aGF0J3MgZ29pbmcgb24sIGJ1dCBpdCBpcyBwb3NzaWJsZSB0byBkbyBz by4KClllcywgYmVpbmcgYWJsZSB0byBhY3R1YWxseSByZWZlcmVuY2UgY291bnRlcnMgd2l0aCBo aWdoZXIgbnVtYmVycyBtYWtlcwp0aGluZ3MgYSBsb3QgZWFzaWVyIHRvIGZvbGxvdy4KCnJlZ2Fy ZHMKUGhpbGlwcAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3Jn Cmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: p.zabel@pengutronix.de (Philipp Zabel) Date: Thu, 27 Aug 2015 11:40:30 +0200 Subject: [PATCH 04/12] gpu: imx: fix support for interlaced modes In-Reply-To: <20150827085411.GE21084@n2100.arm.linux.org.uk> References: <20150808160251.GM7557@n2100.arm.linux.org.uk> <1440664752.3233.36.camel@pengutronix.de> <20150827085411.GE21084@n2100.arm.linux.org.uk> Message-ID: <1440668430.3233.68.camel@pengutronix.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Am Donnerstag, den 27.08.2015, 09:54 +0100 schrieb Russell King - ARM Linux: > On Thu, Aug 27, 2015 at 10:39:12AM +0200, Philipp Zabel wrote: > > Hi Russell, > > > > Am Samstag, den 08.08.2015, 17:03 +0100 schrieb Russell King: > > > The support for interlaced video modes seems to be broken; we don't use > > > anything other than the vtotal/htotal from the timing information to > > > define the various sync counters. > > > > I finally made time to test this series: > > > > Tested-by: Philipp Zabel > > on i.MX6 GK802 via HDMI connected to a TV (1080p60, 1080i60). > > > > Unfortunately these timings are completely different from what Freescale > > came up with for the TV Encoder on i.MX5, but the code we have currently > > in mainline doesn't work for that either. I suppose I'll follow up with > > a patch that adds yet another sync counter setup for the i.MX5/TVE case. > > > > I'd like to take the two ipu-v3 patches, making a few small changes on > > this one: > > Please don't split the series up. The reason it's a series is because > there's interdependencies between the patches. In that case, can I take the whole series? Or would you like to respin and have my Acked-by: Philipp Zabel with the changes below: > > > Freescale patches for interlaced video support contain an alternative > > > sync counter setup, which we include here. This setup produces the > > > hsync and vsync via the normal counter 2 and 3, but moves the display > > > enable signal from counter 5 to counter 6. Therefore, we need to > > > change the display controller setup as well. > > > > > > The corresponding Freescale patches for this change are: > > > iMX6-HDMI-support-interlaced-display-mode.patch > > > IPU-fine-tuning-the-interlace-display-timing-for-CEA.patch > > > > > > This produces a working interlace format output from the IPU. > > > > ... on i.MX6 via HDMI. > > It should also be correct for any other source which wants interlaced > output. ... on i.MX6, then. Right now I don't know what is the effect of the undocumented settings on the i.MX5's IPUv3M. > > > Signed-off-by: Russell King > > > --- > > > drivers/gpu/ipu-v3/ipu-dc.c | 18 ++++++++--- > > > drivers/gpu/ipu-v3/ipu-di.c | 79 +++++++++++++++++++++------------------------ > > > 2 files changed, 51 insertions(+), 46 deletions(-) > > > > > > diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c > > > index 9ef2e1f54ca4..aa560855c1dc 100644 > > > --- a/drivers/gpu/ipu-v3/ipu-dc.c > > > +++ b/drivers/gpu/ipu-v3/ipu-dc.c > > > @@ -183,12 +183,22 @@ int ipu_dc_init_sync(struct ipu_dc *dc, struct ipu_di *di, bool interlaced, > > > } > > > > > > if (interlaced) { > > > - dc_link_event(dc, DC_EVT_NL, 0, 3); > > > - dc_link_event(dc, DC_EVT_EOL, 0, 2); > > > - dc_link_event(dc, DC_EVT_NEW_DATA, 0, 1); > > > + int word, addr; > > > + > > > + if (dc->di) { > > > + addr = 1; > > > + word = 1; > > > > These two are really one and the same. The address written to the link > > register for the given event has to point to the address of the > > microcode instruction written to the template memory that should be > > executed on this event. > > > > I'd like to drop the word variable and use addr for both. > > Ok. As I said in the commit message, this code comes from Freescale's > patches which I pointed to above. [...] > > > @@ -211,66 +215,59 @@ static void ipu_di_sync_config_interlaced(struct ipu_di *di, > > > sig->mode.hback_porch + sig->mode.hfront_porch; > > > u32 v_total = sig->mode.vactive + sig->mode.vsync_len + > > > sig->mode.vback_porch + sig->mode.vfront_porch; > > > - u32 reg; > > > struct di_sync_config cfg[] = { > > > { > > > - .run_count = h_total / 2 - 1, > > > - .run_src = DI_SYNC_CLK, > > > + /* 1: internal VSYNC for each frame */ > > > + .run_count = v_total * 2 - 1, > > > + .run_src = 3, /* == counter 7 */ > > > > Do you know why this works? The Reference Manual v2 lists that value as > > "NA" in the DI counter #1 Run Resolution field. > > Yes, I noticed that Freescale were using values for the source fields > which were marked as NA in the manual. As it works, I can only assume > that the engineer who came up with this setup has more knowledge than > is public. I'd like small a comment here that yes, we know this is marked as NA/Reserved in the manuals. [...] > I went through the counter setup to understand how it works, and it > seems correct provided you accept that the various source values do > work as the code claims, which includes creating the vsync at the > appropriate half-scanline position without needing this hack. > > It's not easy to work back from the counter setup to get a mental > picture of what's going on, but it is possible to do so. Yes, being able to actually reference counters with higher numbers makes things a lot easier to follow. regards Philipp