| Message ID | 1380826287-30253-2-git-send-email-fabio.estevam@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@0120000 { > + compatible = "fsl,imx6q-hdmi"; > + reg = <0x00120000 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.
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.
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?
On Mon, Oct 14, 2013 at 2:38 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> 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
On Mon, Oct 14, 2013 at 7:50 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> 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
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
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.
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 > <linux@arm.linux.org.uk> 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.
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 > > <linux@arm.linux.org.uk> 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
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?
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
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-wandboard.dtsi b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi index a55113e..758c17f 100644 --- a/arch/arm/boot/dts/imx6qdl-wandboard.dtsi +++ b/arch/arm/boot/dts/imx6qdl-wandboard.dtsi @@ -51,6 +51,19 @@ status = "okay"; }; + +&hdmi { + ddc = <&i2c1>; + status = "okay"; +}; + +&i2c1 { + clock-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c1_1>; + status = "okay"; +}; + &i2c2 { clock-frequency = <100000>; pinctrl-names = "default"; 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@0120000 { + compatible = "fsl,imx6q-hdmi"; + reg = <0x00120000 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>;
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> --- Changes since v1: - None arch/arm/boot/dts/imx6dl.dtsi | 4 ++++ arch/arm/boot/dts/imx6q.dtsi | 4 ++++ arch/arm/boot/dts/imx6qdl-wandboard.dtsi | 13 +++++++++++++ arch/arm/boot/dts/imx6qdl.dtsi | 10 ++++++++++ 4 files changed, 31 insertions(+)