Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-16 Thread Sascha Hauer
On Tue, Oct 15, 2013 at 11:35:00AM +0100, Russell King - ARM Linux wrote:
> On Tue, Oct 15, 2013 at 10:18:30AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Oct 15, 2013 at 09:46:18AM +0200, Sascha Hauer wrote:
> > > This sounds like the wrong clock polarity. Could you try inverting
> > > sig_cfg.clk_pol in imx-drm/ipuv3-crtc.c?
> > 
> > I tweaked it a different way - I used devmem2 to directly poke at the
> > register.  Inverting bit 17 (iow, clearing it) seems to fix the
> > problem.
> 
> As a follow-up, the driver says this:
> 
> unsigned clk_pol:1; /* true = rising edge */
> 
> This is interpreted by the ipu-di.c driver as:
> 
> if (!sig->clk_pol)
> di_gen |= DI_GEN_POLARITY_DISP_CLK;
> 
> note the inversion.  U-boot does something slightly different here
> (apparantly it describes clk_pol in the same way though):
> 
> if (sig.clk_pol)
> di_gen |= DI_GEN_POL_CLK;
> 
> It's also reported that u-boot sets sig.clk_pol when
> FB_SYNC_CLK_LAT_FALL bit is not set (which confirms that clk_pol
> indicates rising edge active.)
> 
> Now, the confusing bit.  The docs for the imx6s/dl say that bit 17
> when set is "active high" vs clear "active low".  This appears to
> define a level active state.  The code seems to define an edge
> instead.  What's more is that u-boot and the kernel seem to describe
> 'clk_pol' in the same way yet interpret it differently.
> 
> Should that inversion in the kernel be there?

I could measure the pin with an oscilloscope on some board using
the parallel display output. That should how this bit really behaves
and we can cleanup the comments and/or code.

Won't have time for this before Edinburgh though.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-15 Thread Russell King - ARM Linux
On Tue, Oct 15, 2013 at 10:18:30AM +0100, Russell King - ARM Linux wrote:
> On Tue, Oct 15, 2013 at 09:46:18AM +0200, Sascha Hauer wrote:
> > This sounds like the wrong clock polarity. Could you try inverting
> > sig_cfg.clk_pol in imx-drm/ipuv3-crtc.c?
> 
> I tweaked it a different way - I used devmem2 to directly poke at the
> register.  Inverting bit 17 (iow, clearing it) seems to fix the
> problem.

As a follow-up, the driver says this:

unsigned clk_pol:1; /* true = rising edge */

This is interpreted by the ipu-di.c driver as:

if (!sig->clk_pol)
di_gen |= DI_GEN_POLARITY_DISP_CLK;

note the inversion.  U-boot does something slightly different here
(apparantly it describes clk_pol in the same way though):

if (sig.clk_pol)
di_gen |= DI_GEN_POL_CLK;

It's also reported that u-boot sets sig.clk_pol when
FB_SYNC_CLK_LAT_FALL bit is not set (which confirms that clk_pol
indicates rising edge active.)

Now, the confusing bit.  The docs for the imx6s/dl say that bit 17
when set is "active high" vs clear "active low".  This appears to
define a level active state.  The code seems to define an edge
instead.  What's more is that u-boot and the kernel seem to describe
'clk_pol' in the same way yet interpret it differently.

Should that inversion in the kernel be there?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-15 Thread Sascha Hauer
On Tue, Oct 15, 2013 at 11:00:26AM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 14, 2013 at 11:47:17PM -0300, Fabio Estevam wrote:
> > On Mon, Oct 14, 2013 at 2:38 PM, Russell King - ARM Linux
> >  wrote:
> > 
> > > Shouldn't the above be in patch 1 (or 1.5) rather than patch 2?  Patch 2
> > > advertises itself as adding support for the wandboard, but in actual
> > > fact it's adding the generic bits too.
> > 
> > Thanks for your review. Will address your comments in v3.
> > 
> > Philipp Zabel mentioned that he will move imx-drm out of staging, so
> > will send v3 after Philipp's patch gets into linux-next.
> 
> That's unfortunate, especially given the bug with the clock polarity
> (which, although I can tweak the register directly, it's not obvious
> that changing the clk_pol initialisation is the correct solution.)
> 
> There's also another issue here: the lack of DRM prime support.  As
> you have a separate GPU and separate VPU, you really need dmabuf
> support implemented so that you can hand your scanout buffers/overlays
> to other subsystems.

See http://www.spinics.net/lists/linux-driver-devel/msg39324.html

Besides other things this adds prime support.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-15 Thread Russell King - ARM Linux
On Mon, Oct 14, 2013 at 11:47:17PM -0300, Fabio Estevam wrote:
> On Mon, Oct 14, 2013 at 2:38 PM, Russell King - ARM Linux
>  wrote:
> 
> > Shouldn't the above be in patch 1 (or 1.5) rather than patch 2?  Patch 2
> > advertises itself as adding support for the wandboard, but in actual
> > fact it's adding the generic bits too.
> 
> Thanks for your review. Will address your comments in v3.
> 
> Philipp Zabel mentioned that he will move imx-drm out of staging, so
> will send v3 after Philipp's patch gets into linux-next.

That's unfortunate, especially given the bug with the clock polarity
(which, although I can tweak the register directly, it's not obvious
that changing the clk_pol initialisation is the correct solution.)

There's also another issue here: the lack of DRM prime support.  As
you have a separate GPU and separate VPU, you really need dmabuf
support implemented so that you can hand your scanout buffers/overlays
to other subsystems.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-15 Thread Russell King - ARM Linux
On Tue, Oct 15, 2013 at 09:46:18AM +0200, Sascha Hauer wrote:
> On Mon, Oct 14, 2013 at 11:50:29PM +0100, Russell King - ARM Linux wrote:
> > Answers to these two questions may help stop me wasting a lot of time
> > chasing what is a really weird bug.
> > 
> > So, I have X up and running on the Cubox-i carrier-1, using the imx-drm
> > stuff (I've slightly hacked my xf86-armada X driver to get this working.)
> > This works fine - it detects the connectors, selects an appropriate
> > video mode and produces a picture of the correct shape and size.
> > 
> > However... I see really weird effects colouring effects - almost like
> > water over the image.  It's certain colour transitions in the image
> > which seem to trigger this.  There are also certain pixels which
> > "twinkle".
> > 
> > Text looks very strange too.  Rather than the font being crisp and clear,
> > it looks like there's red and green shift to it - but its not that it's
> > all shifted in that way.
> > 
> > Now, if I use the modetest utility from libdrm-2.4.43 to display a SMPTE
> > test pattern, this again looks right, but there are several vertical
> > single pixel lines of noise.  The most striking one is below the upper
> > red bar in the lower dark area.  I have a single pixel noisy vertical
> > line.
> > 
> > I've tried the kernel which Rabeeh supplied, which is based on BSP 4.1.0,
> > and this works fine as far as I can tell with the same image (though,
> > it's a little difficult converting the XWD dump to something that can
> > be directly poked into /dev/fb0..., although this results in R/B swap.)
> > 
> > Any ideas where to start looking?
> 
> This sounds like the wrong clock polarity. Could you try inverting
> sig_cfg.clk_pol in imx-drm/ipuv3-crtc.c?

I tweaked it a different way - I used devmem2 to directly poke at the
register.  Inverting bit 17 (iow, clearing it) seems to fix the
problem.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-15 Thread Sascha Hauer
On Mon, Oct 14, 2013 at 11:50:29PM +0100, Russell King - ARM Linux wrote:
> Answers to these two questions may help stop me wasting a lot of time
> chasing what is a really weird bug.
> 
> So, I have X up and running on the Cubox-i carrier-1, using the imx-drm
> stuff (I've slightly hacked my xf86-armada X driver to get this working.)
> This works fine - it detects the connectors, selects an appropriate
> video mode and produces a picture of the correct shape and size.
> 
> However... I see really weird effects colouring effects - almost like
> water over the image.  It's certain colour transitions in the image
> which seem to trigger this.  There are also certain pixels which
> "twinkle".
> 
> Text looks very strange too.  Rather than the font being crisp and clear,
> it looks like there's red and green shift to it - but its not that it's
> all shifted in that way.
> 
> Now, if I use the modetest utility from libdrm-2.4.43 to display a SMPTE
> test pattern, this again looks right, but there are several vertical
> single pixel lines of noise.  The most striking one is below the upper
> red bar in the lower dark area.  I have a single pixel noisy vertical
> line.
> 
> I've tried the kernel which Rabeeh supplied, which is based on BSP 4.1.0,
> and this works fine as far as I can tell with the same image (though,
> it's a little difficult converting the XWD dump to something that can
> be directly poked into /dev/fb0..., although this results in R/B swap.)
> 
> Any ideas where to start looking?

This sounds like the wrong clock polarity. Could you try inverting
sig_cfg.clk_pol in imx-drm/ipuv3-crtc.c?

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-14 Thread Fabio Estevam
On Mon, Oct 14, 2013 at 7:50 PM, Russell King - ARM Linux
 wrote:

> Okay, next question(s).
>
> 1. How well tested is any of this imx-drm stuff?
>
> 2. What sort of testing has it been subject to?

Most of my tests with the imx-drm driver have been through the LVDS
panel so far.

For HDMI, I haven't gone through too much of testing yet, only basic
raw framebuffer at full HD resolution.

I know Tony/Robert (and Sascha) and some folks at the Wandboard
mailing list have also tested HDMI with the previous version of the
HDMI driver that was posted by Tony.

> Answers to these two questions may help stop me wasting a lot of time
> chasing what is a really weird bug.
>
> So, I have X up and running on the Cubox-i carrier-1, using the imx-drm
> stuff (I've slightly hacked my xf86-armada X driver to get this working.)
> This works fine - it detects the connectors, selects an appropriate
> video mode and produces a picture of the correct shape and size.
>
> However... I see really weird effects colouring effects - almost like
> water over the image.  It's certain colour transitions in the image
> which seem to trigger this.  There are also certain pixels which
> "twinkle".
>
> Text looks very strange too.  Rather than the font being crisp and clear,
> it looks like there's red and green shift to it - but its not that it's
> all shifted in that way.
>
> Now, if I use the modetest utility from libdrm-2.4.43 to display a SMPTE
> test pattern, this again looks right, but there are several vertical
> single pixel lines of noise.  The most striking one is below the upper
> red bar in the lower dark area.  I have a single pixel noisy vertical
> line.

Ok, interesting. Will run a video test pattern via gstreamer
videotestsrc plugin to see how it behaves.

I am travelling tomorrow, so it will be at last in 2 weeks that I will
be able to access a mx6 hardware.

In the meantime, maybe one of the folks in Cc could share some ideas.

Regards,

Fabio Estevam
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-14 Thread Fabio Estevam
On Mon, Oct 14, 2013 at 2:38 PM, Russell King - ARM Linux
 wrote:

> Shouldn't the above be in patch 1 (or 1.5) rather than patch 2?  Patch 2
> advertises itself as adding support for the wandboard, but in actual
> fact it's adding the generic bits too.

Thanks for your review. Will address your comments in v3.

Philipp Zabel mentioned that he will move imx-drm out of staging, so
will send v3 after Philipp's patch gets into linux-next.

Regards,

Fabio Estevam
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-14 Thread Russell King - ARM Linux
On Mon, Oct 14, 2013 at 06:40:30PM +0100, Russell King - ARM Linux wrote:
> Another thing that my build testing (and use on cubox-i) picked up:
> 
> On Thu, Oct 03, 2013 at 03:51:26PM -0300, Fabio Estevam wrote:
> > diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
> > index 9e8ae11..65e54b4 100644
> > --- a/arch/arm/boot/dts/imx6dl.dtsi
> > +++ b/arch/arm/boot/dts/imx6dl.dtsi
> > @@ -88,3 +88,7 @@
> > crtcs = <&ipu1 0>, <&ipu1 1>;
> > };
> >  };
> > +
> > +&hdmi {
> > +   crtcs = <&ipu1 0>, <&ipu1 1>;
> > +}
> 
> Missing semi-colon.

Okay, next question(s).

1. How well tested is any of this imx-drm stuff?

2. What sort of testing has it been subject to?

Answers to these two questions may help stop me wasting a lot of time
chasing what is a really weird bug.

So, I have X up and running on the Cubox-i carrier-1, using the imx-drm
stuff (I've slightly hacked my xf86-armada X driver to get this working.)
This works fine - it detects the connectors, selects an appropriate
video mode and produces a picture of the correct shape and size.

However... I see really weird effects colouring effects - almost like
water over the image.  It's certain colour transitions in the image
which seem to trigger this.  There are also certain pixels which
"twinkle".

Text looks very strange too.  Rather than the font being crisp and clear,
it looks like there's red and green shift to it - but its not that it's
all shifted in that way.

Now, if I use the modetest utility from libdrm-2.4.43 to display a SMPTE
test pattern, this again looks right, but there are several vertical
single pixel lines of noise.  The most striking one is below the upper
red bar in the lower dark area.  I have a single pixel noisy vertical
line.

I've tried the kernel which Rabeeh supplied, which is based on BSP 4.1.0,
and this works fine as far as I can tell with the same image (though,
it's a little difficult converting the XWD dump to something that can
be directly poked into /dev/fb0..., although this results in R/B swap.)

Any ideas where to start looking?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-14 Thread Russell King - ARM Linux
On Thu, Oct 03, 2013 at 03:51:26PM -0300, Fabio Estevam wrote:
> diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
> index 9e8ae11..65e54b4 100644
> --- a/arch/arm/boot/dts/imx6dl.dtsi
> +++ b/arch/arm/boot/dts/imx6dl.dtsi
> @@ -88,3 +88,7 @@
>   crtcs = <&ipu1 0>, <&ipu1 1>;
>   };
>  };
> +
> +&hdmi {
> + crtcs = <&ipu1 0>, <&ipu1 1>;
> +}
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index f024ef2..d2467f5 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -159,3 +159,7 @@
>   crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
>   };
>  };
> +
> +&hdmi {
> + crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
> +};
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index ccd55c2..b148cb3 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -1301,6 +1301,16 @@
>   };
>   };
>  
> + hdmi: hdmi@012 {
> + compatible = "fsl,imx6q-hdmi";
> + reg = <0x0012 0x9000>;
> + interrupts = <0 115 0x04>;
> + gpr = <&gpr>;
> + clocks = <&clks 123>, <&clks 124>;
> + clock-names = "iahb", "isfr";
> + status = "disabled";
> + };
> +
>   dcic1: dcic@020e4000 {
>   reg = <0x020e4000 0x4000>;
>   interrupts = <0 124 0x04>;

Shouldn't the above be in patch 1 (or 1.5) rather than patch 2?  Patch 2
advertises itself as adding support for the wandboard, but in actual
fact it's adding the generic bits too.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/3] ARM: dts: imx6qdl-wandboard: Add HDMI support

2013-10-14 Thread Russell King - ARM Linux
Another thing that my build testing (and use on cubox-i) picked up:

On Thu, Oct 03, 2013 at 03:51:26PM -0300, Fabio Estevam wrote:
> diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
> index 9e8ae11..65e54b4 100644
> --- a/arch/arm/boot/dts/imx6dl.dtsi
> +++ b/arch/arm/boot/dts/imx6dl.dtsi
> @@ -88,3 +88,7 @@
>   crtcs = <&ipu1 0>, <&ipu1 1>;
>   };
>  };
> +
> +&hdmi {
> + crtcs = <&ipu1 0>, <&ipu1 1>;
> +}

Missing semi-colon.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel