Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Mon, Oct 28, 2013 at 06:26:49PM -0200, Fabio Estevam wrote: + /*Wait for PHY PLL lock */ + msec = 4; + val = hdmi_readb(hdmi, HDMI_PHY_STAT0) HDMI_PHY_TX_PHY_LOCK; + while (!val) { + udelay(1000); + if (msec-- == 0) { + dev_dbg(hdmi-dev, PHY PLL not locked\n); + return -EINVAL; + } + val = hdmi_readb(hdmi, HDMI_PHY_STAT0) HDMI_PHY_TX_PHY_LOCK; + } This loop is not always sufficient for the PLL to always lock. It's also buggy because it behaves like this: msec = 4 read register check bit - still one? yes - wait 1ms msec == 0? no - msec = 3 read register ... msec eventually msec = 0 read register check bit - still one yes - wait 1ms msec == 0? yes - print debug and exit So the last wait for 1ms performs no real function other than to delay the inevitable exit. If we really want to wait 5ms for the condition we're testing to become true, then do it like this: msec = 5; do { val = hdmi_readb(hdmi, HDMI_PHY_STAT0) HDMI_PHY_TX_PHY_LOCK; if (!val) break; if (msec == 0) { dev_err(hdmi-dev, PHY PLL not locked\n); return -EINVAL; } udelay(1000); msec--; } while (1); This way, we check for the success condition immediately before checking for the failure condition, and only waiting after that. I'd also suggest making it a dev_err() so that it's obvious when it doesn't lock - the current dev_dbg() remains hidden and causes failures which don't seem to be visible to the user via the logs. Hence why this issue has been overlooked... +static void hdmi_av_composer(struct imx_hdmi *hdmi, + const struct drm_display_mode *mode) +{ + u8 inv_val; + struct hdmi_vmode *vmode = hdmi-hdmi_data.video_mode; + int hblank, vblank, h_de_hs, v_de_vs, hsync_len, vsync_len; + + vmode-mhsyncpolarity = !!(mode-flags DRM_MODE_FLAG_PHSYNC); + vmode-mvsyncpolarity = !!(mode-flags DRM_MODE_FLAG_PVSYNC); + vmode-minterlaced = !!(mode-flags DRM_MODE_FLAG_INTERLACE); + vmode-mpixelclock = mode-htotal * mode-vtotal * + drm_mode_vrefresh(mode); We can do better here: vmode-mpixelclock = mode-clock * 1000; as the above is not quite correct when considering all the possibilities. Since we're given the pixel clock in the mode structure, we might as well make use it. + /* Set up VSYNC active edge delay (in pixel clks) */ + vsync_len = mode-vsync_end - mode-vsync_start; + hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH); Commentry - vsync len is in lines not pixel clocks. Other than that, it seems to work here on the Carrier-1. Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Mon, Oct 28, 2013 at 7:19 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Oct 28, 2013 at 06:26:49PM -0200, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com This is based on the initial work done by Sascha Hauer and Tony Prisk. It looks like you've also taken some of the suggestions I've made too - like the voltage drive and symbol transmit control settings. It would be nice to identify the changes a bit better so that those of us who are testing your drivers (and have patches on top of them to make things work) know what's changed. Yes, I have been out the two past weeks and tried to capture all of your inputs concerning the HDMI driver. Today I was able to get access to a wandboard solo and the HDMI image does look different compared to a wandboard quad board: - The Linux logo penguins are steady on mx6q, but on mx6solo the contour lines look like they flicker. - The kernel log text appears in good white colour with mx6q, but not so white on mx6solo (tending to green). Even the U-boot HDMI splash screen is not very steady on mx6solo. I will try to look at these mx6 solo HDMI issues. Regards, Fabio Estevam ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Tue, Oct 29, 2013 at 01:45:50PM -0200, Fabio Estevam wrote: On Tue, Oct 29, 2013 at 1:00 PM, Fabio Estevam feste...@gmail.com wrote: Today I was able to get access to a wandboard solo and the HDMI image does look different compared to a wandboard quad board: - The Linux logo penguins are steady on mx6q, but on mx6solo the contour lines look like they flicker. - The kernel log text appears in good white colour with mx6q, but not so white on mx6solo (tending to green). Even the U-boot HDMI splash screen is not very steady on mx6solo. I will try to look at these mx6 solo HDMI issues. With the change below I am able to get a good HDMI quality image on mx6solo (and mx6quad as well): Yes, I need that too on the Carrier-1 board, as suggested by Sascha. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Tue, Oct 29, 2013 at 08:01:46PM -0200, Fabio Estevam wrote: Hi Greg, On Mon, Oct 28, 2013 at 7:03 PM, Greg KH gre...@linuxfoundation.org wrote: And why is new support being added to this driver instead of working to get it out of staging? Philipp Zabel mentioned that he will submit the imx-drm driver out of staging: http://www.spinics.net/lists/dri-devel/msg46784.html The issues surrounding DT and DRM were given an airing last week at kernel summit in a session chaired by David Airlie. The Kernel Summit session was to discuss issues surrounding DT and DRM, or more specifically DT and componentised subsystems like DRM, ASoC and V4L2. Grant Likely has said that he will attempt to summarise some of the discussion there; unfortunately I'm not aware of anyone making any real minutes of what was discussed - it wasn't a formal session. I also had discussions with David Airlie and, separately, with the Pengutronix folk about imx-drm. It is very clear to me that David is against imx-drm moving into drivers/gpu/drm - the driver requires an amount of rework in various places. So I don't think it will move out of drivers/staging any time soon. I have a few other bits to sort out first, but I've volunteered to try to resolve some of these blocking issues so that imx-drm can move forward and hopefully out of staging. To put it another way, moving this driver out of staging is not as simple as moving the files; it requires real work to address the issues the subsystem maintainer has with this code. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] imx-drm: Add mx6 hdmi transmitter support
On Mon, Oct 28, 2013 at 06:26:49PM -0200, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com This is based on the initial work done by Sascha Hauer and Tony Prisk. It looks like you've also taken some of the suggestions I've made too - like the voltage drive and symbol transmit control settings. It would be nice to identify the changes a bit better so that those of us who are testing your drivers (and have patches on top of them to make things work) know what's changed. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel