Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround
On 3/9/2017 8:52 PM, Steve Longerbeam wrote:
> There is a pin conflict with GPIO_6. This pin functions as a power
> input pin to the OV5642 camera sensor, but ENET uses it as the h/w
> workaround for erratum ERR006687, to wake-up the ARM cores on normal
> RX and TX packet done events. So we need to remove the h/w workaround
> to support the OV5642. The result is that the CPUidle driver will no
> longer allow entering the deep idle states on the sabrelite.
>
> This is a partial revert of
>
> commit 6261c4c8f13e ("ARM: dts: imx6qdl-sabrelite: use GPIO_6 for FEC
> interrupt.")
> commit a28eeb43ee57 ("ARM: dts: imx6: tag boards that have the HW workaround
> for ERR006687")
>
> Signed-off-by: Steve Longerbeam
> ---
> arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> index 8413179..89dce27 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> @@ -270,9 +270,6 @@
> txd1-skew-ps = <0>;
> txd2-skew-ps = <0>;
> txd3-skew-ps = <0>;
How about
+#if !IS_ENABLED(CONFIG_VIDEO_OV5642)
> - interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
> - <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
> - fsl,err006687-workaround-present;
+#endif
Is that allowed ?
> status = "okay";
> };
>
> @@ -373,7 +370,6 @@
> MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL 0x1b030
> /* Phy reset */
> MX6QDL_PAD_EIM_D23__GPIO3_IO23 0x000b0
> - MX6QDL_PAD_GPIO_6__ENET_IRQ 0x000b1
> >;
> };
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display.
On 11/13/2013 2:23 AM, Denis Carikli wrote: + /* rgb666 */ + ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666); + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */ + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */ + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */ + return 0; } Since, rgb24 and bgr24 reverse the byte numbers /* rgb24 */ ipu_dc_map_clear(priv, IPU_DC_MAP_RGB24); ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 0, 7, 0xff); /* blue */ ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 1, 15, 0xff); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 2, 23, 0xff); /* red */ /* bgr24 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24); ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 2, 7, 0xff); /* red */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */ Shouldn't rgb666 and bgr666 do the same? Currently we have, /* bgr666 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666); ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */ Where I'd expect to see /* bgr666 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666); ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */ So, it looks like you are adding a duplicate of bgr666 because bgr666 is wrong. Also, I'd prefer to keep the entries in 0,1,2 byte number order(blue, green, red, assuming byte 0 is always blue) so that duplicates are easier to spot. Not related to this patch, but the comments on gbr24 appear wrong as well. /* gbr24 */ ipu_dc_map_clear(priv, IPU_DC_MAP_GBR24); ipu_dc_map_config(priv, IPU_DC_MAP_GBR24, 2, 15, 0xff); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_GBR24, 1, 7, 0xff); /* blue */ ipu_dc_map_config(priv, IPU_DC_MAP_GBR24, 0, 23, 0xff); /* red */ Should be /* brg24 */ ipu_dc_map_clear(priv, IPU_DC_MAP_BRG24); ipu_dc_map_config(priv, IPU_DC_MAP_BRG24, 0, 23, 0xff); /* blue*/ ipu_dc_map_config(priv, IPU_DC_MAP_BRG24, 1, 7, 0xff); /* green */ ipu_dc_map_config(priv, IPU_DC_MAP_BRG24, 2, 15, 0xff); /* red */ Of course, my understanding may be totally wrong. If so, please show me the light! Troy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] imx-drm: Add mx6 hdmi transmitter support
On 10/31/2013 1:23 PM, Troy Kisky wrote:
+/* Workaround to clear the overflow condition */
+static void imx_hdmi_clear_overflow(struct imx_hdmi *hdmi)
+{
+int count;
+u8 val;
+
+/* TMDS software reset */
+hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ,
HDMI_MC_SWRSTZ);
+
+val = hdmi_readb(hdmi, HDMI_FC_INVIDCONF);
+if (hdmi->dev_type == IMX6DL_HDMI) {
+hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
+return;
+}
+
+for (count = 0; count < 5; count++)
+hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
+}
+
Hi Fabio,
I get a magenta line down the left side of the screen unless I replace
the 5 with a 6.
ie.
+for (count = 0; count < 6; count++)
+hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
This is with a imx6q processor. With the 6, the picture looks great!
Troy
It also works if I change the 5 to a 4! A 3 fails though.
To summarize:
0 works
1 fails
2 works
3 fails
4 works
5 fails
6 works
Very strange.
Troy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] imx-drm: Add mx6 hdmi transmitter support
+/* Workaround to clear the overflow condition */
+static void imx_hdmi_clear_overflow(struct imx_hdmi *hdmi)
+{
+ int count;
+ u8 val;
+
+ /* TMDS software reset */
+ hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ, HDMI_MC_SWRSTZ);
+
+ val = hdmi_readb(hdmi, HDMI_FC_INVIDCONF);
+ if (hdmi->dev_type == IMX6DL_HDMI) {
+ hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
+ return;
+ }
+
+ for (count = 0; count < 5; count++)
+ hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
+}
+
Hi Fabio,
I get a magenta line down the left side of the screen unless I replace
the 5 with a 6.
ie.
+ for (count = 0; count < 6; count++)
+ hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
This is with a imx6q processor. With the 6, the picture looks great!
Troy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
On 10/16/2013 1:27 PM, Russell King - ARM Linux wrote:
On Wed, Oct 16, 2013 at 12:37:42PM -0700, Troy Kisky wrote:
On 10/16/2013 10:03 AM, Russell King - ARM Linux wrote:
On Tue, Oct 15, 2013 at 10:17:07AM -0300, Fabio Estevam wrote:
On Tue, Oct 15, 2013 at 10:10 AM, Russell King - ARM Linux
wrote:
Another point on patch 1. Sorry, I don't have patch 1 to reply to, it
seems it was deleted from linux-arm-kernel's moderation queue.
drm_mode_connector_attach_encoder() is called too early, before the
base.id field in the encoder has been initialised. This causes the
connectors encoder array to be empty, and userspace KMS to fail.
There's also bugs in the CSC setting too - it runs off the end of the
array and gcc warns about this. The code was clearly wrong.
You may wish to combine this patch with patch 1 to sort all that out.
For the patch below:
Signed-off-by: Russell King
Tested-by: Russell King
Thanks, Russell.
Will submit v3 when I am back to the office.
Okay, I still have a problem with HDMI: I have a magenta vertical line
down the left hand side of the frame, the displayed frame is shifted
right by the width of that line and the right hand side is missing some
pixels.
First off, the hsync position programmed into the HDMI registers appears
to be wrong.
I'm at a loss why imx-hdmi is obfuscated with a conversion from a
drm_display_mode structure to a fb_videomode. This adds additional
confusion and additional opportunities for bugs; this is probably
exactly why the hsync position is wrong.
In CEA-861-B for 720p @60Hz:
DE: __^^^
HS: ___^^^___
^ ^ ^
| | 220 clocks
| 40 clocks
110 clocks
The IMX6 manual says HSYINCINDELAY0 is "Hsync active edge delay. Integer
number of pixel clock cycles from de non-active edge". So, this should be
110. Yet it ends up being programmed as 220, leading to a magenta vertical
bar down the left hand side of the display.
Now, if you look at Documentation/fb/framebuffer.txt, in this case, the
right margin should be 110 clocks, hsync len should be 40 and the left
margin should be 220 clocks.
However, this is not what your conversion to a fb_videomode does. It
reverses the left and right margin. A similar confusion also exists
in the conversion of the upper/lower margins too.
The DRM model is this (for 720p @60Hz):
0 1280 1390 1430 1650
|===||---|--|
^ ^^ ^ ^
start hdisplay hsync_start hsync_end htotal
The fb model is the same as the above but rotated:
left margindisplayedright margin hsync_len
|--|===||---|
So, the left margin is the bit between hsync_end and htotal, and the
right margin is the bit between hdisplay and hsync_start. Exactly
the same analysis applies to the upper/lower margins.
What I suggest is that the use of fb_videomode is entirely killed off
in this driver to remove this layer of confusion and instead the DRM
model is stuck to within this DRM driver.
Now on to the next problem. HSYNC/VSYNC polarity.
So, this is the mode which is set:
1280x720 (0x41) 74.2MHz +HSync +VSync *current +preferred
h: width 1280 start 1390 end 1430 total 1650 skew0 clock
45.0KHz
v: height 720 start 725 end 730 total 750 clock 60.0Hz
Note the positive HSync and VSync polarity - this is correct, backed
up by CEA-861-B.
The IPU DI0 is configured thusly in its general control register:
0x264 = 0x26, which is what is set when the requested mode
has DRM_MODE_FLAG_PHSYNC and DRM_MODE_FLAG_PVSYNC flags set.
However, if we look at the HDMI config: 0x121000 = 0x18. Active low
hsync/vsync. This is quite obvious when you look at the code -
convert_to_video_timing() does *nothing* at all when converting the
sync polarities, which means hdmi_av_composer() doesn't program them
appropriately. And yes, poking 0x78 into this register finally fixes
the problem.
Yet another reason why this silly conversion from one structure form
to another is a Very Bad Idea. Until I found this, I was merely going
to send a patch to sort out convert_to_video_timing(), but quite frankly
I'm going to kill this thing off right now.
Another thing:
static int imx_hdmi_setup(struct imx_hdmi *hdmi, struct drm_display_mode *mode)
{
int ret;
convert_to_video_timing(&hdmi->fb_mode, mode);
hdmi_disable_overflow_interrupts(hdmi);
hdmi->vic = 6;
It's quite wrong to force every video mode set to be CEA mode 6. IIRC,
There is a function in DRM which will tell you the CEA mode number.
Again, I'll fix this in my patch rewriting this bit of the driver to
be correct.
I'm also suspicious of the "HD
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
On 10/16/2013 10:03 AM, Russell King - ARM Linux wrote:
On Tue, Oct 15, 2013 at 10:17:07AM -0300, Fabio Estevam wrote:
On Tue, Oct 15, 2013 at 10:10 AM, Russell King - ARM Linux
wrote:
Another point on patch 1. Sorry, I don't have patch 1 to reply to, it
seems it was deleted from linux-arm-kernel's moderation queue.
drm_mode_connector_attach_encoder() is called too early, before the
base.id field in the encoder has been initialised. This causes the
connectors encoder array to be empty, and userspace KMS to fail.
There's also bugs in the CSC setting too - it runs off the end of the
array and gcc warns about this. The code was clearly wrong.
You may wish to combine this patch with patch 1 to sort all that out.
For the patch below:
Signed-off-by: Russell King
Tested-by: Russell King
Thanks, Russell.
Will submit v3 when I am back to the office.
Okay, I still have a problem with HDMI: I have a magenta vertical line
down the left hand side of the frame, the displayed frame is shifted
right by the width of that line and the right hand side is missing some
pixels.
First off, the hsync position programmed into the HDMI registers appears
to be wrong.
I'm at a loss why imx-hdmi is obfuscated with a conversion from a
drm_display_mode structure to a fb_videomode. This adds additional
confusion and additional opportunities for bugs; this is probably
exactly why the hsync position is wrong.
In CEA-861-B for 720p @60Hz:
DE: __^^^
HS: ___^^^___
^ ^ ^
| | 220 clocks
| 40 clocks
110 clocks
The IMX6 manual says HSYINCINDELAY0 is "Hsync active edge delay. Integer
number of pixel clock cycles from de non-active edge". So, this should be
110. Yet it ends up being programmed as 220, leading to a magenta vertical
bar down the left hand side of the display.
Now, if you look at Documentation/fb/framebuffer.txt, in this case, the
right margin should be 110 clocks, hsync len should be 40 and the left
margin should be 220 clocks.
However, this is not what your conversion to a fb_videomode does. It
reverses the left and right margin. A similar confusion also exists
in the conversion of the upper/lower margins too.
The DRM model is this (for 720p @60Hz):
0 1280 1390 1430 1650
|===||---|--|
^ ^^ ^ ^
start hdisplay hsync_start hsync_end htotal
The fb model is the same as the above but rotated:
left margindisplayedright margin hsync_len
|--|===||---|
So, the left margin is the bit between hsync_end and htotal, and the
right margin is the bit between hdisplay and hsync_start. Exactly
the same analysis applies to the upper/lower margins.
What I suggest is that the use of fb_videomode is entirely killed off
in this driver to remove this layer of confusion and instead the DRM
model is stuck to within this DRM driver.
Now on to the next problem. HSYNC/VSYNC polarity.
So, this is the mode which is set:
1280x720 (0x41) 74.2MHz +HSync +VSync *current +preferred
h: width 1280 start 1390 end 1430 total 1650 skew0 clock 45.0KHz
v: height 720 start 725 end 730 total 750 clock 60.0Hz
Note the positive HSync and VSync polarity - this is correct, backed
up by CEA-861-B.
The IPU DI0 is configured thusly in its general control register:
0x264 = 0x26, which is what is set when the requested mode
has DRM_MODE_FLAG_PHSYNC and DRM_MODE_FLAG_PVSYNC flags set.
However, if we look at the HDMI config: 0x121000 = 0x18. Active low
hsync/vsync. This is quite obvious when you look at the code -
convert_to_video_timing() does *nothing* at all when converting the
sync polarities, which means hdmi_av_composer() doesn't program them
appropriately. And yes, poking 0x78 into this register finally fixes
the problem.
Yet another reason why this silly conversion from one structure form
to another is a Very Bad Idea. Until I found this, I was merely going
to send a patch to sort out convert_to_video_timing(), but quite frankly
I'm going to kill this thing off right now.
Another thing:
static int imx_hdmi_setup(struct imx_hdmi *hdmi, struct drm_display_mode *mode)
{
int ret;
convert_to_video_timing(&hdmi->fb_mode, mode);
hdmi_disable_overflow_interrupts(hdmi);
hdmi->vic = 6;
It's quite wrong to force every video mode set to be CEA mode 6. IIRC,
There is a function in DRM which will tell you the CEA mode number.
Again, I'll fix this in my patch rewriting this bit of the driver to
be correct.
I'm also suspicious of the "HDMI Initialization Step" comments, because
they make no sense:
/* HDMI Initialization Step B.4 */
static void imx_hdmi_enable_video_path(struct imx_hdmi *hdmi)
{
yet:
/* HDMI Initialization Step B
