Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752603AbbH0Ijs (ORCPT ); Thu, 27 Aug 2015 04:39:48 -0400 Received: from metis.ext.pengutronix.de ([92.198.50.35]:36682 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751449AbbH0Ijo (ORCPT ); Thu, 27 Aug 2015 04:39:44 -0400 Message-ID: <1440664752.3233.36.camel@pengutronix.de> Subject: Re: [PATCH 04/12] gpu: imx: fix support for interlaced modes From: Philipp Zabel To: Russell King 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 10:39:12 +0200 In-Reply-To: References: <20150808160251.GM7557@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 Content-Length: 5930 Lines: 180 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: > 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. > 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. > + } 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. > }, { > - .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. > ipu_di_write(di, v_total / 2 - 1, DI_SCR_CONF); > } > > @@ -605,10 +602,8 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig) > > /* set y_sel = 1 */ > di_gen |= 0x10000000; > - di_gen |= DI_GEN_POLARITY_5; > - di_gen |= DI_GEN_POLARITY_8; > > - vsync_cnt = 7; > + vsync_cnt = 3; > } else { > ipu_di_sync_config_noninterlaced(di, sig, div); regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/