From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752870AbbH0Iyl (ORCPT ); Thu, 27 Aug 2015 04:54:41 -0400 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:60079 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751988AbbH0Iyg (ORCPT ); Thu, 27 Aug 2015 04:54:36 -0400 Date: Thu, 27 Aug 2015 09:54:13 +0100 From: Russell King - ARM Linux To: Philipp Zabel 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 Subject: Re: [PATCH 04/12] gpu: imx: fix support for interlaced modes Message-ID: <20150827085411.GE21084@n2100.arm.linux.org.uk> References: <20150808160251.GM7557@n2100.arm.linux.org.uk> <1440664752.3233.36.camel@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1440664752.3233.36.camel@pengutronix.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > 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. > > 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. > > + } else { > > + addr = 0; > > + word = 0; > > + } > > + > > + dc_link_event(dc, DC_EVT_NL, addr, 3); > > + dc_link_event(dc, DC_EVT_EOL, addr, 2); > > + dc_link_event(dc, DC_EVT_NEW_DATA, addr, 1); > > > > /* Init template microcode */ > > - dc_write_tmpl(dc, 0, WROD(0), 0, map, SYNC_WAVE, 0, 8, 1); > > + dc_write_tmpl(dc, word, WROD(0), 0, map, SYNC_WAVE, 0, 6, 1); > > } else { > > if (dc->di) { > > dc_link_event(dc, DC_EVT_NL, 2, 3); > > diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c > > index a96991c5c15f..359268e3a166 100644 > > --- a/drivers/gpu/ipu-v3/ipu-di.c > > +++ b/drivers/gpu/ipu-v3/ipu-di.c > > @@ -71,6 +71,10 @@ enum di_sync_wave { > > DI_SYNC_HSYNC = 3, > > DI_SYNC_VSYNC = 4, > > DI_SYNC_DE = 6, > > + > > + DI_SYNC_CNT1 = 2, /* counter >= 2 only */ > > + DI_SYNC_CNT4 = 5, /* counter >= 5 only */ > > + DI_SYNC_CNT5 = 6, /* counter >= 6 only */ > > }; > > > > #define SYNC_WAVE 0 > > @@ -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. > > }, { > > - .run_count = h_total - 11, > > + /* PIN2: HSYNC waveform */ > > + .run_count = h_total - 1, > > .run_src = DI_SYNC_CLK, > > - .cnt_down = 4, > > + .cnt_polarity_gen_en = 1, > > + .cnt_polarity_trigger_src = DI_SYNC_CLK, > > + .cnt_down = sig->mode.hsync_len * 2, > > }, { > > - .run_count = v_total * 2 - 1, > > - .run_src = DI_SYNC_INT_HSYNC, > > - .offset_count = 1, > > - .offset_src = DI_SYNC_INT_HSYNC, > > - .cnt_down = 4, > > + /* PIN3: VSYNC waveform */ > > + .run_count = v_total - 1, > > + .run_src = 4, /* == counter 7 */ > > Same here, ... > > > + .cnt_polarity_gen_en = 1, > > + .cnt_polarity_trigger_src = 4, /* == counter 7 */ > > ... and same here, the DI counter #3 polarity Clear select field lists > the value 4 as "Reserved". > > > + .cnt_down = sig->mode.vsync_len * 2, > > + .cnt_clr_src = DI_SYNC_CNT1, > > }, { > [...] > > } > > }; > > > > ipu_di_sync_config(di, cfg, 0, ARRAY_SIZE(cfg)); > > > > - /* set gentime select and tag sel */ > > - reg = ipu_di_read(di, DI_SW_GEN1(9)); > > - reg &= 0x1FFFFFFF; > > - reg |= (3 - 1) << 29 | 0x00008000; > > - ipu_di_write(di, reg, DI_SW_GEN1(9)); > > As far as I understood, attaching counter #9 to counter #3 is needed to > generate the second vsync on i.MX5. Since this doesn't currently work, > I'm fine with removing it for now. 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. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [PATCH 04/12] gpu: imx: fix support for interlaced modes Date: Thu, 27 Aug 2015 09:54:13 +0100 Message-ID: <20150827085411.GE21084@n2100.arm.linux.org.uk> References: <20150808160251.GM7557@n2100.arm.linux.org.uk> <1440664752.3233.36.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <1440664752.3233.36.camel@pengutronix.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Philipp Zabel 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 T24gVGh1LCBBdWcgMjcsIDIwMTUgYXQgMTA6Mzk6MTJBTSArMDIwMCwgUGhpbGlwcCBaYWJlbCB3 cm90ZToKPiBIaSBSdXNzZWxsLAo+IAo+IEFtIFNhbXN0YWcsIGRlbiAwOC4wOC4yMDE1LCAxNzow MyArMDEwMCBzY2hyaWViIFJ1c3NlbGwgS2luZzoKPiA+IFRoZSBzdXBwb3J0IGZvciBpbnRlcmxh Y2VkIHZpZGVvIG1vZGVzIHNlZW1zIHRvIGJlIGJyb2tlbjsgd2UgZG9uJ3QgdXNlCj4gPiBhbnl0 aGluZyBvdGhlciB0aGFuIHRoZSB2dG90YWwvaHRvdGFsIGZyb20gdGhlIHRpbWluZyBpbmZvcm1h dGlvbiB0bwo+ID4gZGVmaW5lIHRoZSB2YXJpb3VzIHN5bmMgY291bnRlcnMuCj4gCj4gSSBmaW5h bGx5IG1hZGUgdGltZSB0byB0ZXN0IHRoaXMgc2VyaWVzOgo+IAo+IFRlc3RlZC1ieTogUGhpbGlw cCBaYWJlbCA8cC56YWJlbEBwZW5ndXRyb25peC5kZT4KPiBvbiBpLk1YNiBHSzgwMiB2aWEgSERN SSBjb25uZWN0ZWQgdG8gYSBUViAoMTA4MHA2MCwgMTA4MGk2MCkuCj4gCj4gVW5mb3J0dW5hdGVs eSB0aGVzZSB0aW1pbmdzIGFyZSBjb21wbGV0ZWx5IGRpZmZlcmVudCBmcm9tIHdoYXQgRnJlZXNj YWxlCj4gY2FtZSB1cCB3aXRoIGZvciB0aGUgVFYgRW5jb2RlciBvbiBpLk1YNSwgYnV0IHRoZSBj b2RlIHdlIGhhdmUgY3VycmVudGx5Cj4gaW4gbWFpbmxpbmUgZG9lc24ndCB3b3JrIGZvciB0aGF0 IGVpdGhlci4gSSBzdXBwb3NlIEknbGwgZm9sbG93IHVwIHdpdGgKPiBhIHBhdGNoIHRoYXQgYWRk cyB5ZXQgYW5vdGhlciBzeW5jIGNvdW50ZXIgc2V0dXAgZm9yIHRoZSBpLk1YNS9UVkUgY2FzZS4K PiAKPiBJJ2QgbGlrZSB0byB0YWtlIHRoZSB0d28gaXB1LXYzIHBhdGNoZXMsIG1ha2luZyBhIGZl dyBzbWFsbCBjaGFuZ2VzIG9uCj4gdGhpcyBvbmU6CgpQbGVhc2UgZG9uJ3Qgc3BsaXQgdGhlIHNl cmllcyB1cC4gIFRoZSByZWFzb24gaXQncyBhIHNlcmllcyBpcyBiZWNhdXNlCnRoZXJlJ3MgaW50 ZXJkZXBlbmRlbmNpZXMgYmV0d2VlbiB0aGUgcGF0Y2hlcy4KCj4gPiBGcmVlc2NhbGUgcGF0Y2hl cyBmb3IgaW50ZXJsYWNlZCB2aWRlbyBzdXBwb3J0IGNvbnRhaW4gYW4gYWx0ZXJuYXRpdmUKPiA+ IHN5bmMgY291bnRlciBzZXR1cCwgd2hpY2ggd2UgaW5jbHVkZSBoZXJlLiAgVGhpcyBzZXR1cCBw cm9kdWNlcyB0aGUKPiA+IGhzeW5jIGFuZCB2c3luYyB2aWEgdGhlIG5vcm1hbCBjb3VudGVyIDIg YW5kIDMsIGJ1dCBtb3ZlcyB0aGUgZGlzcGxheQo+ID4gZW5hYmxlIHNpZ25hbCBmcm9tIGNvdW50 ZXIgNSB0byBjb3VudGVyIDYuICBUaGVyZWZvcmUsIHdlIG5lZWQgdG8KPiA+IGNoYW5nZSB0aGUg ZGlzcGxheSBjb250cm9sbGVyIHNldHVwIGFzIHdlbGwuCj4gPiAKPiA+IFRoZSBjb3JyZXNwb25k aW5nIEZyZWVzY2FsZSBwYXRjaGVzIGZvciB0aGlzIGNoYW5nZSBhcmU6Cj4gPiAgIGlNWDYtSERN SS1zdXBwb3J0LWludGVybGFjZWQtZGlzcGxheS1tb2RlLnBhdGNoCj4gPiAgIElQVS1maW5lLXR1 bmluZy10aGUtaW50ZXJsYWNlLWRpc3BsYXktdGltaW5nLWZvci1DRUEucGF0Y2gKPiA+IAo+ID4g VGhpcyBwcm9kdWNlcyBhIHdvcmtpbmcgaW50ZXJsYWNlIGZvcm1hdCBvdXRwdXQgZnJvbSB0aGUg SVBVLgo+IAo+IC4uLiBvbiBpLk1YNiB2aWEgSERNSS4KCkl0IHNob3VsZCBhbHNvIGJlIGNvcnJl Y3QgZm9yIGFueSBvdGhlciBzb3VyY2Ugd2hpY2ggd2FudHMgaW50ZXJsYWNlZApvdXRwdXQuCgo+ ID4gU2lnbmVkLW9mZi1ieTogUnVzc2VsbCBLaW5nIDxybWsra2VybmVsQGFybS5saW51eC5vcmcu dWs+Cj4gPiAtLS0KPiA+ICBkcml2ZXJzL2dwdS9pcHUtdjMvaXB1LWRjLmMgfCAxOCArKysrKysr Ky0tLQo+ID4gIGRyaXZlcnMvZ3B1L2lwdS12My9pcHUtZGkuYyB8IDc5ICsrKysrKysrKysrKysr KysrKysrKy0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQo+ID4gIDIgZmlsZXMgY2hhbmdlZCwgNTEg aW5zZXJ0aW9ucygrKSwgNDYgZGVsZXRpb25zKC0pCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9kcml2 ZXJzL2dwdS9pcHUtdjMvaXB1LWRjLmMgYi9kcml2ZXJzL2dwdS9pcHUtdjMvaXB1LWRjLmMKPiA+ IGluZGV4IDllZjJlMWY1NGNhNC4uYWE1NjA4NTVjMWRjIDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVy cy9ncHUvaXB1LXYzL2lwdS1kYy5jCj4gPiArKysgYi9kcml2ZXJzL2dwdS9pcHUtdjMvaXB1LWRj LmMKPiA+IEBAIC0xODMsMTIgKzE4MywyMiBAQCBpbnQgaXB1X2RjX2luaXRfc3luYyhzdHJ1Y3Qg aXB1X2RjICpkYywgc3RydWN0IGlwdV9kaSAqZGksIGJvb2wgaW50ZXJsYWNlZCwKPiA+ICAJfQo+ ID4gIAo+ID4gIAlpZiAoaW50ZXJsYWNlZCkgewo+ID4gLQkJZGNfbGlua19ldmVudChkYywgRENf RVZUX05MLCAwLCAzKTsKPiA+IC0JCWRjX2xpbmtfZXZlbnQoZGMsIERDX0VWVF9FT0wsIDAsIDIp Owo+ID4gLQkJZGNfbGlua19ldmVudChkYywgRENfRVZUX05FV19EQVRBLCAwLCAxKTsKPiA+ICsJ CWludCB3b3JkLCBhZGRyOwo+ID4gKwo+ID4gKwkJaWYgKGRjLT5kaSkgewo+ID4gKwkJCWFkZHIg PSAxOwo+ID4gKwkJCXdvcmQgPSAxOwo+IAo+IFRoZXNlIHR3byBhcmUgcmVhbGx5IG9uZSBhbmQg dGhlIHNhbWUuIFRoZSBhZGRyZXNzIHdyaXR0ZW4gdG8gdGhlIGxpbmsKPiByZWdpc3RlciBmb3Ig dGhlIGdpdmVuIGV2ZW50IGhhcyB0byBwb2ludCB0byB0aGUgYWRkcmVzcyBvZiB0aGUKPiBtaWNy b2NvZGUgaW5zdHJ1Y3Rpb24gd3JpdHRlbiB0byB0aGUgdGVtcGxhdGUgbWVtb3J5IHRoYXQgc2hv dWxkIGJlCj4gZXhlY3V0ZWQgb24gdGhpcyBldmVudC4KPiAKPiBJJ2QgbGlrZSB0byBkcm9wIHRo ZSB3b3JkIHZhcmlhYmxlIGFuZCB1c2UgYWRkciBmb3IgYm90aC4KCk9rLiAgQXMgSSBzYWlkIGlu IHRoZSBjb21taXQgbWVzc2FnZSwgdGhpcyBjb2RlIGNvbWVzIGZyb20gRnJlZXNjYWxlJ3MKcGF0 Y2hlcyB3aGljaCBJIHBvaW50ZWQgdG8gYWJvdmUuCgo+ID4gKwkJfSBlbHNlIHsKPiA+ICsJCQlh ZGRyID0gMDsKPiA+ICsJCQl3b3JkID0gMDsKPiA+ICsJCX0KPiA+ICsKPiA+ICsJCWRjX2xpbmtf ZXZlbnQoZGMsIERDX0VWVF9OTCwgYWRkciwgMyk7Cj4gPiArCQlkY19saW5rX2V2ZW50KGRjLCBE Q19FVlRfRU9MLCBhZGRyLCAyKTsKPiA+ICsJCWRjX2xpbmtfZXZlbnQoZGMsIERDX0VWVF9ORVdf REFUQSwgYWRkciwgMSk7Cj4gPiAgCj4gPiAgCQkvKiBJbml0IHRlbXBsYXRlIG1pY3JvY29kZSAq Lwo+ID4gLQkJZGNfd3JpdGVfdG1wbChkYywgMCwgV1JPRCgwKSwgMCwgbWFwLCBTWU5DX1dBVkUs IDAsIDgsIDEpOwo+ID4gKwkJZGNfd3JpdGVfdG1wbChkYywgd29yZCwgV1JPRCgwKSwgMCwgbWFw LCBTWU5DX1dBVkUsIDAsIDYsIDEpOwo+ID4gIAl9IGVsc2Ugewo+ID4gIAkJaWYgKGRjLT5kaSkg ewo+ID4gIAkJCWRjX2xpbmtfZXZlbnQoZGMsIERDX0VWVF9OTCwgMiwgMyk7Cj4gPiBkaWZmIC0t Z2l0IGEvZHJpdmVycy9ncHUvaXB1LXYzL2lwdS1kaS5jIGIvZHJpdmVycy9ncHUvaXB1LXYzL2lw dS1kaS5jCj4gPiBpbmRleCBhOTY5OTFjNWMxNWYuLjM1OTI2OGUzYTE2NiAxMDA2NDQKPiA+IC0t LSBhL2RyaXZlcnMvZ3B1L2lwdS12My9pcHUtZGkuYwo+ID4gKysrIGIvZHJpdmVycy9ncHUvaXB1 LXYzL2lwdS1kaS5jCj4gPiBAQCAtNzEsNiArNzEsMTAgQEAgZW51bSBkaV9zeW5jX3dhdmUgewo+ ID4gIAlESV9TWU5DX0hTWU5DID0gMywKPiA+ICAJRElfU1lOQ19WU1lOQyA9IDQsCj4gPiAgCURJ X1NZTkNfREUgPSA2LAo+ID4gKwo+ID4gKwlESV9TWU5DX0NOVDEgPSAyLAkvKiBjb3VudGVyID49 IDIgb25seSAqLwo+ID4gKwlESV9TWU5DX0NOVDQgPSA1LAkvKiBjb3VudGVyID49IDUgb25seSAq Lwo+ID4gKwlESV9TWU5DX0NOVDUgPSA2LAkvKiBjb3VudGVyID49IDYgb25seSAqLwo+ID4gIH07 Cj4gPiAgCj4gPiAgI2RlZmluZSBTWU5DX1dBVkUgMAo+ID4gQEAgLTIxMSw2NiArMjE1LDU5IEBA IHN0YXRpYyB2b2lkIGlwdV9kaV9zeW5jX2NvbmZpZ19pbnRlcmxhY2VkKHN0cnVjdCBpcHVfZGkg KmRpLAo+ID4gIAkJc2lnLT5tb2RlLmhiYWNrX3BvcmNoICsgc2lnLT5tb2RlLmhmcm9udF9wb3Jj aDsKPiA+ICAJdTMyIHZfdG90YWwgPSBzaWctPm1vZGUudmFjdGl2ZSArIHNpZy0+bW9kZS52c3lu Y19sZW4gKwo+ID4gIAkJc2lnLT5tb2RlLnZiYWNrX3BvcmNoICsgc2lnLT5tb2RlLnZmcm9udF9w b3JjaDsKPiA+IC0JdTMyIHJlZzsKPiA+ICAJc3RydWN0IGRpX3N5bmNfY29uZmlnIGNmZ1tdID0g ewo+ID4gIAkJewo+ID4gLQkJCS5ydW5fY291bnQgPSBoX3RvdGFsIC8gMiAtIDEsCj4gPiAtCQkJ LnJ1bl9zcmMgPSBESV9TWU5DX0NMSywKPiA+ICsJCQkvKiAxOiBpbnRlcm5hbCBWU1lOQyBmb3Ig ZWFjaCBmcmFtZSAqLwo+ID4gKwkJCS5ydW5fY291bnQgPSB2X3RvdGFsICogMiAtIDEsCj4gPiAr CQkJLnJ1bl9zcmMgPSAzLAkJCS8qID09IGNvdW50ZXIgNyAqLwo+IAo+IERvIHlvdSBrbm93IHdo eSB0aGlzIHdvcmtzPyBUaGUgUmVmZXJlbmNlIE1hbnVhbCB2MiBsaXN0cyB0aGF0IHZhbHVlIGFz Cj4gIk5BIiBpbiB0aGUgREkgY291bnRlciAjMSBSdW4gUmVzb2x1dGlvbiBmaWVsZC4KClllcywg SSBub3RpY2VkIHRoYXQgRnJlZXNjYWxlIHdlcmUgdXNpbmcgdmFsdWVzIGZvciB0aGUgc291cmNl IGZpZWxkcwp3aGljaCB3ZXJlIG1hcmtlZCBhcyBOQSBpbiB0aGUgbWFudWFsLiAgQXMgaXQgd29y a3MsIEkgY2FuIG9ubHkgYXNzdW1lCnRoYXQgdGhlIGVuZ2luZWVyIHdobyBjYW1lIHVwIHdpdGgg dGhpcyBzZXR1cCBoYXMgbW9yZSBrbm93bGVkZ2UgdGhhbgppcyBwdWJsaWMuCgo+ID4gIAkJfSwg ewo+ID4gLQkJCS5ydW5fY291bnQgPSBoX3RvdGFsIC0gMTEsCj4gPiArCQkJLyogUElOMjogSFNZ TkMgd2F2ZWZvcm0gKi8KPiA+ICsJCQkucnVuX2NvdW50ID0gaF90b3RhbCAtIDEsCj4gPiAgCQkJ LnJ1bl9zcmMgPSBESV9TWU5DX0NMSywKPiA+IC0JCQkuY250X2Rvd24gPSA0LAo+ID4gKwkJCS5j bnRfcG9sYXJpdHlfZ2VuX2VuID0gMSwKPiA+ICsJCQkuY250X3BvbGFyaXR5X3RyaWdnZXJfc3Jj ID0gRElfU1lOQ19DTEssCj4gPiArCQkJLmNudF9kb3duID0gc2lnLT5tb2RlLmhzeW5jX2xlbiAq IDIsCj4gPiAgCQl9LCB7Cj4gPiAtCQkJLnJ1bl9jb3VudCA9IHZfdG90YWwgKiAyIC0gMSwKPiA+ IC0JCQkucnVuX3NyYyA9IERJX1NZTkNfSU5UX0hTWU5DLAo+ID4gLQkJCS5vZmZzZXRfY291bnQg PSAxLAo+ID4gLQkJCS5vZmZzZXRfc3JjID0gRElfU1lOQ19JTlRfSFNZTkMsCj4gPiAtCQkJLmNu dF9kb3duID0gNCwKPiA+ICsJCQkvKiBQSU4zOiBWU1lOQyB3YXZlZm9ybSAqLwo+ID4gKwkJCS5y dW5fY291bnQgPSB2X3RvdGFsIC0gMSwKPiA+ICsJCQkucnVuX3NyYyA9IDQsCQkJLyogPT0gY291 bnRlciA3ICovCj4gCj4gU2FtZSBoZXJlLCAuLi4KPiAKPiA+ICsJCQkuY250X3BvbGFyaXR5X2dl bl9lbiA9IDEsCj4gPiArCQkJLmNudF9wb2xhcml0eV90cmlnZ2VyX3NyYyA9IDQsCS8qID09IGNv dW50ZXIgNyAqLwo+IAo+IC4uLiBhbmQgc2FtZSBoZXJlLCB0aGUgREkgY291bnRlciAjMyBwb2xh cml0eSBDbGVhciBzZWxlY3QgZmllbGQgbGlzdHMKPiB0aGUgdmFsdWUgNCBhcyAiUmVzZXJ2ZWQi Lgo+IAo+ID4gKwkJCS5jbnRfZG93biA9IHNpZy0+bW9kZS52c3luY19sZW4gKiAyLAo+ID4gKwkJ CS5jbnRfY2xyX3NyYyA9IERJX1NZTkNfQ05UMSwKPiA+ICAJCX0sIHsKPiBbLi4uXQo+ID4gIAkJ fQo+ID4gIAl9Owo+ID4gIAo+ID4gIAlpcHVfZGlfc3luY19jb25maWcoZGksIGNmZywgMCwgQVJS QVlfU0laRShjZmcpKTsKPiA+ICAKPiA+IC0JLyogc2V0IGdlbnRpbWUgc2VsZWN0IGFuZCB0YWcg c2VsICovCj4gPiAtCXJlZyA9IGlwdV9kaV9yZWFkKGRpLCBESV9TV19HRU4xKDkpKTsKPiA+IC0J cmVnICY9IDB4MUZGRkZGRkY7Cj4gPiAtCXJlZyB8PSAoMyAtIDEpIDw8IDI5IHwgMHgwMDAwODAw MDsKPiA+IC0JaXB1X2RpX3dyaXRlKGRpLCByZWcsIERJX1NXX0dFTjEoOSkpOwo+IAo+IEFzIGZh ciBhcyBJIHVuZGVyc3Rvb2QsIGF0dGFjaGluZyBjb3VudGVyICM5IHRvIGNvdW50ZXIgIzMgaXMg bmVlZGVkIHRvCj4gZ2VuZXJhdGUgdGhlIHNlY29uZCB2c3luYyBvbiBpLk1YNS4gU2luY2UgdGhp cyBkb2Vzbid0IGN1cnJlbnRseSB3b3JrLAo+IEknbSBmaW5lIHdpdGggcmVtb3ZpbmcgaXQgZm9y IG5vdy4KCkkgd2VudCB0aHJvdWdoIHRoZSBjb3VudGVyIHNldHVwIHRvIHVuZGVyc3RhbmQgaG93 IGl0IHdvcmtzLCBhbmQgaXQKc2VlbXMgY29ycmVjdCBwcm92aWRlZCB5b3UgYWNjZXB0IHRoYXQg dGhlIHZhcmlvdXMgc291cmNlIHZhbHVlcyBkbwp3b3JrIGFzIHRoZSBjb2RlIGNsYWltcywgd2hp Y2ggaW5jbHVkZXMgY3JlYXRpbmcgdGhlIHZzeW5jIGF0IHRoZQphcHByb3ByaWF0ZSBoYWxmLXNj YW5saW5lIHBvc2l0aW9uIHdpdGhvdXQgbmVlZGluZyB0aGlzIGhhY2suCgpJdCdzIG5vdCBlYXN5 IHRvIHdvcmsgYmFjayBmcm9tIHRoZSBjb3VudGVyIHNldHVwIHRvIGdldCBhIG1lbnRhbApwaWN0 dXJlIG9mIHdoYXQncyBnb2luZyBvbiwgYnV0IGl0IGlzIHBvc3NpYmxlIHRvIGRvIHNvLgoKLS0g CkZUVEMgYnJvYWRiYW5kIGZvciAwLjhtaWxlIGxpbmU6IGN1cnJlbnRseSBhdCAxMC41TWJwcyBk b3duIDQwMGticHMgdXAKYWNjb3JkaW5nIHRvIHNwZWVkdGVzdC5uZXQuCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QK ZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Au b3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@arm.linux.org.uk (Russell King - ARM Linux) Date: Thu, 27 Aug 2015 09:54:13 +0100 Subject: [PATCH 04/12] gpu: imx: fix support for interlaced modes In-Reply-To: <1440664752.3233.36.camel@pengutronix.de> References: <20150808160251.GM7557@n2100.arm.linux.org.uk> <1440664752.3233.36.camel@pengutronix.de> Message-ID: <20150827085411.GE21084@n2100.arm.linux.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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. > > 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. > > 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. > > + } else { > > + addr = 0; > > + word = 0; > > + } > > + > > + dc_link_event(dc, DC_EVT_NL, addr, 3); > > + dc_link_event(dc, DC_EVT_EOL, addr, 2); > > + dc_link_event(dc, DC_EVT_NEW_DATA, addr, 1); > > > > /* Init template microcode */ > > - dc_write_tmpl(dc, 0, WROD(0), 0, map, SYNC_WAVE, 0, 8, 1); > > + dc_write_tmpl(dc, word, WROD(0), 0, map, SYNC_WAVE, 0, 6, 1); > > } else { > > if (dc->di) { > > dc_link_event(dc, DC_EVT_NL, 2, 3); > > diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c > > index a96991c5c15f..359268e3a166 100644 > > --- a/drivers/gpu/ipu-v3/ipu-di.c > > +++ b/drivers/gpu/ipu-v3/ipu-di.c > > @@ -71,6 +71,10 @@ enum di_sync_wave { > > DI_SYNC_HSYNC = 3, > > DI_SYNC_VSYNC = 4, > > DI_SYNC_DE = 6, > > + > > + DI_SYNC_CNT1 = 2, /* counter >= 2 only */ > > + DI_SYNC_CNT4 = 5, /* counter >= 5 only */ > > + DI_SYNC_CNT5 = 6, /* counter >= 6 only */ > > }; > > > > #define SYNC_WAVE 0 > > @@ -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. > > }, { > > - .run_count = h_total - 11, > > + /* PIN2: HSYNC waveform */ > > + .run_count = h_total - 1, > > .run_src = DI_SYNC_CLK, > > - .cnt_down = 4, > > + .cnt_polarity_gen_en = 1, > > + .cnt_polarity_trigger_src = DI_SYNC_CLK, > > + .cnt_down = sig->mode.hsync_len * 2, > > }, { > > - .run_count = v_total * 2 - 1, > > - .run_src = DI_SYNC_INT_HSYNC, > > - .offset_count = 1, > > - .offset_src = DI_SYNC_INT_HSYNC, > > - .cnt_down = 4, > > + /* PIN3: VSYNC waveform */ > > + .run_count = v_total - 1, > > + .run_src = 4, /* == counter 7 */ > > Same here, ... > > > + .cnt_polarity_gen_en = 1, > > + .cnt_polarity_trigger_src = 4, /* == counter 7 */ > > ... and same here, the DI counter #3 polarity Clear select field lists > the value 4 as "Reserved". > > > + .cnt_down = sig->mode.vsync_len * 2, > > + .cnt_clr_src = DI_SYNC_CNT1, > > }, { > [...] > > } > > }; > > > > ipu_di_sync_config(di, cfg, 0, ARRAY_SIZE(cfg)); > > > > - /* set gentime select and tag sel */ > > - reg = ipu_di_read(di, DI_SW_GEN1(9)); > > - reg &= 0x1FFFFFFF; > > - reg |= (3 - 1) << 29 | 0x00008000; > > - ipu_di_write(di, reg, DI_SW_GEN1(9)); > > As far as I understood, attaching counter #9 to counter #3 is needed to > generate the second vsync on i.MX5. Since this doesn't currently work, > I'm fine with removing it for now. 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. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.