Skip to contentSkip to conversation

i.MX23/28 framebuffer driver

PATCH New Sascha Hauer 16 participants 36 messages Feb 9, 2011 – Feb 21, 2011 2 messages not in the archive

Subsystems (4)
lore
Share & tools
lore

Sascha Hauer

author maintainer
#0 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

Hi all,

The following patches add framebuffer support for Freescale i.MX23/28 processors.

Sascha Hauer (2):
video: Add i.MX23/28 framebuffer driver ARM i.MX23/28: Add framebuffer device support

 arch/arm/mach-mxs/clock-mx23.c             |    2 +-
 arch/arm/mach-mxs/clock-mx28.c             |    1 +
 arch/arm/mach-mxs/devices-mx23.h           |    4 +
 arch/arm/mach-mxs/devices-mx28.h           |    4 +
 arch/arm/mach-mxs/devices/Kconfig          |    4 +
 arch/arm/mach-mxs/devices/Makefile         |    1 +
 arch/arm/mach-mxs/devices/platform-mxsfb.c |   46 ++
 arch/arm/mach-mxs/include/mach/fb.h        |   48 ++
 drivers/video/Kconfig                      |    9 +
 drivers/video/Makefile                     |    1 +
 drivers/video/mxsfb.c                      |  914 ++++++++++++++++++++++++++++
 11 files changed, 1033 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-mxs/devices/platform-mxsfb.c
 create mode 100644 arch/arm/mach-mxs/include/mach/fb.h
 create mode 100644 drivers/video/mxsfb.c
author maintainer
#1 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Cc Paul Mundt <[email protected]>
Cc linux-fbdev at vger.kernel.org
Cc Shawn Guo <[email protected]>
---
 arch/arm/mach-mxs/include/mach/fb.h |   48 ++
 drivers/video/Kconfig               |    9 +
 drivers/video/Makefile              |    1 +
 drivers/video/mxsfb.c               |  914 +++++++++++++++++++++++++++++++++++
 4 files changed, 972 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mxs/include/mach/fb.h
 create mode 100644 drivers/video/mxsfb.c
arch/arm/mach-mxs/include/mach/fb.h new file +48 -0
new file mode 100644
index 0000000..923f397
@@ -0,0 +1,48 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#ifndef __MACH_FB_H
+#define __MACH_FB_H
+
+#include <linux/fb.h>
+
+#define STMLCDIF_8BIT 1	/** pixel data bus to the display is of 8 bit width */
+#define STMLCDIF_16BIT 0 /** pixel data bus to the display is of 16 bit width */
+#define STMLCDIF_18BIT 2 /** pixel data bus to the display is of 18 bit width */
+#define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
+
+#define FB_SYNC_DATA_ENABLE_HIGH_ACT	64
+
+struct mxsfb_platform_data {
+	struct fb_videomode *mode_list;
+	unsigned mode_count;
+
+	unsigned default_bpp;
+
+	unsigned dotclk_delay;	/* refer manual HW_LCDIF_VDCTRL4 register */
+	unsigned ld_intf_width;	/* refer STMLCDIF_* macros */
+
+	unsigned fb_size;	/* Size of the video memory. If zero a
+				 * default will be used
+				 */
+	unsigned long fb_phys;	/* physical address for the video memory. If
+				 * zero the framebuffer memory will be dynamically
+				 * allocated. If specified,fb_size must also be specified.
+				 * fb_phys must be unused by Linux.
+				 */
+};
+
+#endif /* __MACH_FB_H */
drivers/video/Kconfig +9 -0
index 6bafb51b..e0ea23f 100644
@@ -2365,6 +2365,15 @@ config FB_JZ4740
 	help
 	  Framebuffer support for the JZ4740 SoC.
 
+config FB_MXS
+	tristate "MXS LCD framebuffer support"
+	depends on FB && ARCH_MXS
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	help
+	  Framebuffer support for the MXS SoC.
+
 source "drivers/video/omap/Kconfig"
 source "drivers/video/omap2/Kconfig"
 
drivers/video/Makefile +1 -0
index 8c8fabd..9a096ae 100644
@@ -153,6 +153,7 @@ obj-$(CONFIG_FB_BFIN_T350MCQB)	  += bfin-t350mcqb-fb.o
 obj-$(CONFIG_FB_BFIN_7393)        += bfin_adv7393fb.o
 obj-$(CONFIG_FB_MX3)		  += mx3fb.o
 obj-$(CONFIG_FB_DA8XX)		  += da8xx-fb.o
+obj-$(CONFIG_FB_MXS)		  += mxsfb.o
 
 # the test framebuffer is last
 obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
drivers/video/mxsfb.c new file +914 -0

914 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#2 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Hi Sascha,

Quoted 13 lines · Sascha Hauer
Cc Paul Mundt <[email protected]>
Cc linux-fbdev at vger.kernel.org
Cc Shawn Guo <[email protected]>
---
 arch/arm/mach-mxs/include/mach/fb.h |   48 ++
 drivers/video/Kconfig               |    9 +
 drivers/video/Makefile              |    1 +
 drivers/video/mxsfb.c               |  914 +++++++++++++++++++++++++++++++++++
 4 files changed, 972 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mxs/include/mach/fb.h
 create mode 100644 drivers/video/mxsfb.c

[...]

Quoted 5 lines
drivers/video/mxsfb.c new file +0 -0
new file mode 100644
index 0000000..92a5804

[...]

drivers/video/mxsfb.c line 862

Quoted 8 lines
drivers/video/mxsfb.c new file +7 -0
+static int __devexit mxsfb_remove(struct platform_device *pdev)
+{
+	struct fb_info *fb_info = platform_get_drvdata(pdev);
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	mxsfb_disable_controller(fb_info);

IMO this should also be guarded by an:
| if (host->enabled)
otherwise it may lead to an unbalanced clk_disable() call.

Lothar Wa?mann

Signature · 9 lines
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
#3 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

In reply to Sascha Hauer
Quoted 5 lines
drivers/video/mxsfb.c new file +5 -0
+#define CTRL				0x00
+#define CTRL1				0x10
+#define MX28_CTRL2			0x20
+#define MX23_TRANSFER_COUNT		0x20
+#define MX28_TRANSFER_COUNT		0x30

drivers/video/mxsfb.c line 65

Quoted 10 lines
drivers/video/mxsfb.c new file +10 -0
+#define MX28_CUR_BUF			0x40
+#define MX28_NEXT_BUF			0x50
+#define MX23_CUR_BUF			0x30
+#define MX23_NEXT_BUF			0x40
+#define TIMING				0x60
+#define VDCTRL0				0x70
+#define VDCTRL1				0x80
+#define VDCTRL2				0x90
+#define VDCTRL3				0xa0
+#define VDCTRL4				0xb0

Why you give up mx23/mx28 register define role, which generate from SOC xml. There is a set header files for each mx23/mx28 module, which generate from xml. I know original header files affect run time one Image.
But I think we can copy common part of such register definition because That keep consistent with mx23/mx28 data sheet. Data sheet and header file generate from one source xml.

HW_<Module name>_<Register name>.
BM_<Module name>_<Register name>_Bit name.

#4 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 10 lines · Li Frank-B20596 wrote:
Quoted 10 lines
drivers/video/mxsfb.c new file +10 -0
+#define CTRL				0x00
+#define CTRL1				0x10
+#define MX28_CTRL2			0x20
+#define MX23_TRANSFER_COUNT		0x20
+#define MX28_TRANSFER_COUNT		0x30
+#define MX28_CUR_BUF			0x40
+#define MX28_NEXT_BUF			0x50
+#define MX23_CUR_BUF			0x30
+#define MX23_NEXT_BUF			0x40
+#define TIMING				0x60
Quoted 8 lines

drivers/video/mxsfb.c line 65

Quoted 5 lines
drivers/video/mxsfb.c new file +5 -0
+#define VDCTRL0				0x70
+#define VDCTRL1				0x80
+#define VDCTRL2				0x90
+#define VDCTRL3				0xa0
+#define VDCTRL4				0xb0

Why you give up mx23/mx28 register define role, which generate from SOC xml.

Your macros prevent me from writing short and compact code. If you need more than one of these macros you always have to split each line to follow the 80 columns rule. Unreadable.

Quoted 8 lines

There is a set header files for each mx23/mx28 module, which generate from xml. I know original header files affect run time one Image. But I think we can copy common part of such register definition because That keep consistent with mx23/mx28 data sheet. Data sheet and header file generate from one source xml.

HW_<Module name>_<Register name>.
BM_<Module name>_<Register name>_Bit name.

IMHO when I define the macros where they belong to, there is not need for this redundant HW_<Module name> or BW__<Module name> prefixes. They are just needless.

Regards,
Juergen

Signature · 4 lines
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |
#5 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 21 lines
Quoted 7 lines

drivers/video/mxsfb.c line 65

Quoted 4 lines
drivers/video/mxsfb.c new file +4 -0
+#define VDCTRL1				0x80
+#define VDCTRL2				0x90
+#define VDCTRL3				0xa0
+#define VDCTRL4				0xb0

Why you give up mx23/mx28 register define role, which generate from SOC xml.

Your macros prevent me from writing short and compact code. If you need more
than one of these macros you always have to split each line to follow the 80
columns rule. Unreadable.

Quoted 4 lines

There is a set header files for each mx23/mx28 module, which generate from xml. I know original header files affect run time one Image. But I think we can copy common part of such register definition because That keep consistent with mx23/mx28 data sheet. Data sheet and header

file

Quoted 2 lines

generate from one source xml.

Quoted 7 lines
Quoted 2 lines

HW_<Module name>_<Register name>.
BM_<Module name>_<Register name>_Bit name.

IMHO when I define the macros where they belong to, there is not need for this
redundant HW_<Module name> or BW__<Module name> prefixes. They are just needless.

At first, someone complain name is longer. But during mx23/mx28 developing, Everyone start enjoy such definition because there are not error happen about register and bit position definition and identical map to silicon spec. Developer needn't look up register header file when coding, just write down Register name or bit name according to mx23/mx28 spec.

If you still think it is too long, I suggest keep HW_ and BM_ prefix to distinguish Which one is register name, which one is bit mask.

Imx23/imx28 register have consistent convention.
HW_ is register name
BM_ is bit mask
BP_ is bit position.
BF_(x) is ((x<<BP)&BM_)

Quoted 4 lines

Regards,
Juergen

#6 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 23 lines · Li Frank-B20596 wrote:
Quoted 23 lines
Quoted 7 lines

drivers/video/mxsfb.c line 65

Quoted 4 lines
drivers/video/mxsfb.c new file +4 -0
+#define VDCTRL1				0x80
+#define VDCTRL2				0x90
+#define VDCTRL3				0xa0
+#define VDCTRL4				0xb0

Why you give up mx23/mx28 register define role, which generate from SOC xml.

Your macros prevent me from writing short and compact code. If you need more than one of these macros you always have to split each line to follow the 80 columns rule. Unreadable.

Quoted 10 lines

There is a set header files for each mx23/mx28 module, which generate from xml. I know original header files affect run time one Image. But I think we can copy common part of such register definition because That keep consistent with mx23/mx28 data sheet. Data sheet and header file

generate from one source xml.

HW_<Module name>_<Register name>.
BM_<Module name>_<Register name>_Bit name.

Quoted 8 lines
Quoted 3 lines

IMHO when I define the macros where they belong to, there is not need for this redundant HW_<Module name> or BW__<Module name> prefixes. They are just needless.

At first, someone complain name is longer. But during mx23/mx28 developing, Everyone start enjoy such definition because there are not error happen about register and bit position definition and identical map to silicon spec.

This kind of macro encryption _may_ help when you are coding the driver the first time. But after reading and reading it again (while testing and debugging) all these prefixes and suffixes do not add any valuable information. They only fill up your lines, enlarges your source code and make you blind for the real bug...

Everyone who wants to see how source code looks that uses these longs macros I can recommend reading the so called 'bootlets' source code. :-))

Quoted 1 line

Developer needn't look up register header file when coding, just

That's why I add these macros into the source file instead into a header file.

Quoted 1 line

write down Register name or bit name according to mx23/mx28 spec.

And sometimes the spec is incomplete or just wrong and so on. Independent of any macro naming policy.

Quoted 2 lines

If you still think it is too long, I suggest keep HW_ and BM_ prefix to distinguish Which one is register name, which one is bit mask.

The used macro in the address part of the writel() must be an offset, while the macro used in the value part must be a bit definition. Anything else is redundant.

Regards,
Juergen

Signature · 4 lines
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |
#7 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 11 lines

This kind of macro encryption _may_ help when you are coding the driver the
first time. But after reading and reading it again (while testing and debugging) all these prefixes and suffixes do not add any valuable information. They only fill up your lines, enlarges your source code and make
you blind for the real bug...

Everyone who wants to see how source code looks that uses these longs macros I
can recommend reading the so called 'bootlets' source code. :-))

The complex of bootlets is not long macro, but the work around to make sure Chip PMU work safely. If there are not such macro, power_prep will become more difficult to understand.

Quoted 11 lines
Quoted 2 lines

Developer needn't look up register header file when coding, just

That's why I add these macros into the source file instead into a header file.

Quoted 1 line

write down Register name or bit name according to mx23/mx28 spec.

And sometimes the spec is incomplete or just wrong and so on. Independent of
any macro naming policy.

Not at all. Mx23/mx28 can keep spec, header file and RTL consistent just because using one source and multiple output. During Freescale mx28 BSP develop, never happen bit and register definition error.

Quoted 9 lines
Quoted 3 lines

If you still think it is too long, I suggest keep HW_ and BM_ prefix to distinguish Which one is register name, which one is bit mask.

The used macro in the address part of the writel() must be an offset, while
the macro used in the value part must be a bit definition. Anything else is
redundant.

If name is too short, there are possible to cause name pollution. BM_, BP_, BF_ have their implication.

VDCTRL4_SET_DOTCLK_DLY(x), according to our macro policy, it should be BF_VDCTRL4_SET_DOTCLK_DLY(x).

Generally, you need
Reg = readl(offset+VDCTRL4);
Reg &=~ ((0x7)<<29);
Reg |= VDCTRL4_SET_DOTCLK_DLY(value)
Writel(reg, offset+VDCTRL4)

Compared with our macro policy.

Reg = readl(offset+HW_VDCTRL4);
Reg &=~ BM_VDCTRL4_SET_DOTCLK_DLY;
Reg |= BF_VDCTRL4_SET_DOTCLK_DLY(value)
Writel(reg, offset+HW_VDCTRL4)

Quoted 4 lines

Regards,
Juergen

#8 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 8 lines · Li Frank-B20596 wrote:
Quoted 8 lines

This kind of macro encryption _may_ help when you are coding the driver the
first time. But after reading and reading it again (while testing and debugging) all these prefixes and suffixes do not add any valuable information. They only fill up your lines, enlarges your source code and make
you blind for the real bug...

Quoted 7 lines
Quoted 3 lines

Everyone who wants to see how source code looks that uses these longs macros I
can recommend reading the so called 'bootlets' source code. :-))

The complex of bootlets is not long macro, but the work around to make sure Chip PMU work safely. If there are not such macro, power_prep will become more difficult to understand.

Its your opinion...

Quoted 17 lines
Quoted 9 lines
Quoted 1 line

Developer needn't look up register header file when coding, just

That's why I add these macros into the source file instead into a header file.

Quoted 1 line

write down Register name or bit name according to mx23/mx28 spec.

And sometimes the spec is incomplete or just wrong and so on. Independent of any macro naming policy.

Not at all. Mx23/mx28 can keep spec, header file and RTL consistent just because using one source and multiple output. During Freescale mx28 BSP develop, never happen bit and register definition error.

Quoted 3 lines
Quoted 2 lines

If you still think it is too long, I suggest keep HW_ and BM_ prefix to distinguish Which one is register name, which one is bit mask.

Quoted 8 lines
Quoted 5 lines

The used macro in the address part of the writel() must be an offset, while
the macro used in the value part must be a bit definition. Anything else is
redundant.

If name is too short, there are possible to cause name pollution. BM_, BP_, BF_ have their implication.

If you collect everything in header files: Yes. If you keep the macros where they are used, IMHO: No.

Quoted 6 lines

VDCTRL4_SET_DOTCLK_DLY(x), according to our macro policy, it should be BF_VDCTRL4_SET_DOTCLK_DLY(x).

Generally, you need
Reg = readl(offset+VDCTRL4);
Reg &=~ ((0x7)<<29);

What about?

Quoted 1 line

Reg &= ~VDCTRL4_SET_DOTCLK_DLY(0x7);

Or?

Quoted 1 line

Reg &= ~VDCTRL4_SET_DOTCLK_DLY_MASK;

Okay, the even get longer and longer.

Quoted 9 lines

Reg |= VDCTRL4_SET_DOTCLK_DLY(value)
Writel(reg, offset+VDCTRL4)

Compared with our macro policy.

Reg = readl(offset+HW_VDCTRL4);
Reg &=~ BM_VDCTRL4_SET_DOTCLK_DLY;
Reg |= BF_VDCTRL4_SET_DOTCLK_DLY(value)
Writel(reg, offset+HW_VDCTRL4)

Anywhere else such naming policy in the kernel? I have to deal with all kind of CPUs, not only i.MX23/28.

Regards,
Juergen

Signature · 4 lines
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |
#9 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 7 lines

What about?

Quoted 1 line

Reg &= ~VDCTRL4_SET_DOTCLK_DLY(0x7);

Or?

Quoted 1 line

Reg &= ~VDCTRL4_SET_DOTCLK_DLY_MASK;

Okay, the even get longer and longer.

If use 0x7, you have to take care bit width.
If use VDCTRL4_SET_DOTCLK_DLY_MASK, I think BF_ and BM_ is better.

Quoted 5 lines
Quoted 5 lines

Reg |= VDCTRL4_SET_DOTCLK_DLY(value)
Writel(reg, offset+VDCTRL4)

Compared with our macro policy.

Quoted 8 lines
Quoted 4 lines

Reg = readl(offset+HW_VDCTRL4);
Reg &=~ BM_VDCTRL4_SET_DOTCLK_DLY;
Reg |= BF_VDCTRL4_SET_DOTCLK_DLY(value)
Writel(reg, offset+HW_VDCTRL4)

Anywhere else such naming policy in the kernel? I have to deal with all kind
of CPUs, not only i.MX23/28.

This driver is for mx23/mx28.
I have not found such policy in the kernel, otherwise I have generated that at beginning. It is better if all of mx23/mx28 driver follow the same role.

#10 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

In reply to Sascha Hauer

drivers/video/mxsfb.c line 329

Quoted 5 lines
drivers/video/mxsfb.c new file +5 -0
+
+	/* if it was disabled, re-enable the mode again */
+	reg = readl(host->base + CTRL);
+	reg |= CTRL_DOTCLK_MODE;
+	writel(reg, host->base + CTRL);

writel(CTRL_DOTCLK_MODE, host->base + CTRL_SET) CTRL_SET is CTRL+0x4
SET and CLR register is easier than Read and write back.

#11 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 9 lines · Li Frank-B20596 wrote:

drivers/video/mxsfb.c line 329

Quoted 5 lines
drivers/video/mxsfb.c new file +5 -0
+
+	/* if it was disabled, re-enable the mode again */
+	reg = readl(host->base + CTRL);
+	reg |= CTRL_DOTCLK_MODE;
+	writel(reg, host->base + CTRL);

writel(CTRL_DOTCLK_MODE, host->base + CTRL_SET) CTRL_SET is CTRL+0x4
SET and CLR register is easier than Read and write back.

But you must always check, if the register really has such a SET and CLEAR feature. Not all registers have this feature...

But in this case you are right.

But I would prefer:
#define SET 4
#define CLEAR 8
[...]
writel(CTRL_DOTCLK_MODE, host->base + CTRL + SET)
:-)

Regards,
Juergen

Signature · 4 lines
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |
#12 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 11 lines
Quoted 9 lines · Li Frank-B20596 wrote:

drivers/video/mxsfb.c line 329

Quoted 5 lines
drivers/video/mxsfb.c new file +5 -0
+
+	/* if it was disabled, re-enable the mode again */
+	reg = readl(host->base + CTRL);
+	reg |= CTRL_DOTCLK_MODE;
+	writel(reg, host->base + CTRL);

writel(CTRL_DOTCLK_MODE, host->base + CTRL_SET) CTRL_SET is CTRL+0x4
SET and CLR register is easier than Read and write back.

Quoted 3 lines

But you must always check, if the register really has such a SET and CLEAR
feature. Not all registers have this feature...

That's a reason why I suggest use register header that generate from xml. HW_XX_CTRL_SET will exist if there are such register in hardware. If hardware have not such register, there are not _SET and _CLR.

Compiler will report error if user use _SET but hardware not such register.

Quoted 12 lines

But in this case you are right.

But I would prefer:
#define SET 4
#define CLEAR 8
[...]
writel(CTRL_DOTCLK_MODE, host->base + CTRL + SET)
:-)

Regards,
Juergen

#13 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

In reply to Juergen Beisert
Quoted 5 lines

But I would prefer:
#define SET 4
#define CLEAR 8
[...]
writel(CTRL_DOTCLK_MODE, host->base + CTRL + SET)

NACK.

This will indeed not be caught by the compiler.

And while I like the shorter macros better as well, an MXSFB_ prefix should be added IMO. Macros like CTRL and TIMING are not too well protected.

Regards,

Wolfram

Signature · 9 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110210/d36ddd33/attachment.sig>
author maintainer
#14 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 5 lines · Wolfram Sang
Quoted 5 lines

But I would prefer:
#define SET 4
#define CLEAR 8
[...]
writel(CTRL_DOTCLK_MODE, host->base + CTRL + SET)

For this we already have __mxs_setl.

Quoted 8 lines

NACK.

This will indeed not be caught by the compiler.

And while I like the shorter macros better as well, an MXSFB_ prefix should be added IMO. Macros like CTRL and TIMING are not too well protected.

+1 for a prefix, be it MXSFB_ or LCDIF_, will change this.

Otherwise can we please stop about arguing about register defines? It's too much a matter of taste and there is no perfect way to solve this.

Sascha

Signature · 4 lines
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-5555 |
#15 lore raw

PATCH 1/2 video: Add i.MX23/28 framebuffer driver

Quoted 18 lines · Wolfram Sang
Quoted 5 lines
Quoted 5 lines

But I would prefer:
#define SET 4
#define CLEAR 8
[...]
writel(CTRL_DOTCLK_MODE, host->base + CTRL + SET)

For this we already have __mxs_setl.

Quoted 8 lines

NACK.

This will indeed not be caught by the compiler.

And while I like the shorter macros better as well, an MXSFB_ prefix should be added IMO. Macros like CTRL and TIMING are not too well protected.

Quoted 7 lines

+1 for a prefix, be it MXSFB_ or LCDIF_, will change this.

Otherwise can we please stop about arguing about register defines? It's too much a matter of taste and there is no perfect way to solve this.

Sascha

Guo Shawn mmc driver's patch is good example to use mx23/mx28 register macro.

Best regards
Frank Li

author maintainer
#16 lore raw

PATCH 2/2 ARM i.MX23/28: Add framebuffer device support

In reply to Sascha Hauer
---
 arch/arm/mach-mxs/clock-mx23.c             |    2 +-
 arch/arm/mach-mxs/clock-mx28.c             |    1 +
 arch/arm/mach-mxs/devices-mx23.h           |    4 ++
 arch/arm/mach-mxs/devices-mx28.h           |    4 ++
 arch/arm/mach-mxs/devices/Kconfig          |    4 ++
 arch/arm/mach-mxs/devices/Makefile         |    1 +
 arch/arm/mach-mxs/devices/platform-mxsfb.c |   46 ++++++++++++++++++++++++++++
 7 files changed, 61 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-mxs/devices/platform-mxsfb.c
arch/arm/mach-mxs/clock-mx23.c +1 -1
index ca72a05..bfc7f27 100644
@@ -446,7 +446,7 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK(NULL, "hclk", hbus_clk)
 	_REGISTER_CLOCK(NULL, "usb", usb_clk)
 	_REGISTER_CLOCK(NULL, "audio", audio_clk)
-	_REGISTER_CLOCK(NULL, "pwm", pwm_clk)
+	_REGISTER_CLOCK("imx23-fb", NULL, lcdif_clk)
 };
 
 static int clk_misc_init(void)
arch/arm/mach-mxs/clock-mx28.c +1 -0
index fd1c4c5..6a7ebcb 100644
@@ -620,6 +620,7 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK(NULL, "pwm", pwm_clk)
 	_REGISTER_CLOCK(NULL, "lradc", lradc_clk)
 	_REGISTER_CLOCK(NULL, "spdif", spdif_clk)
+	_REGISTER_CLOCK("imx28-fb", NULL, lcdif_clk)
 };
 
 static int clk_misc_init(void)
arch/arm/mach-mxs/devices-mx23.h +4 -0
index 1256788..b9745a2 100644
@@ -10,7 +10,11 @@
  */
 #include <mach/mx23.h>
 #include <mach/devices-common.h>
+#include <mach/fb.h>
 
 extern const struct amba_device mx23_duart_device __initconst;
 #define mx23_add_duart() \
 	mxs_add_duart(&mx23_duart_device)
+
+struct platform_device *__init mx23_add_mxsfb(
+		const struct mxsfb_platform_data *pdata);
arch/arm/mach-mxs/devices-mx28.h +4 -0
index 33773a6..c902dc7 100644
@@ -10,6 +10,7 @@
  */
 #include <mach/mx28.h>
 #include <mach/devices-common.h>
+#include <mach/fb.h>
 
 extern const struct amba_device mx28_duart_device __initconst;
 #define mx28_add_duart() \
@@ -18,3 +19,6 @@ extern const struct amba_device mx28_duart_device __initconst;
 extern const struct mxs_fec_data mx28_fec_data[] __initconst;
 #define mx28_add_fec(id, pdata) \
 	mxs_add_fec(&mx28_fec_data[id], pdata)
+
+struct platform_device *__init mx28_add_mxsfb(
+		const struct mxsfb_platform_data *pdata);
arch/arm/mach-mxs/devices/Kconfig +4 -0
index cf7dc1a..1538cb9 100644
@@ -4,3 +4,7 @@ config MXS_HAVE_AMBA_DUART
 
 config MXS_HAVE_PLATFORM_FEC
 	bool
+
+config MXS_HAVE_PLATFORM_MXSFB
+	bool
+
arch/arm/mach-mxs/devices/Makefile +1 -0
index d0a09f6..0cf8b48 100644
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MXS_HAVE_AMBA_DUART) += amba-duart.o
 obj-$(CONFIG_MXS_HAVE_PLATFORM_FEC) += platform-fec.o
+obj-$(CONFIG_MXS_HAVE_PLATFORM_MXSFB) += platform-mxsfb.o
arch/arm/mach-mxs/devices/platform-mxsfb.c new file +46 -0

46 lines not on this page · raw message

Signature · 1 line
1.7.2.3
reviewer
#17 lore raw

PATCH 2/2 ARM i.MX23/28: Add framebuffer device support

Hi Sascha,

Quoted 18 lines · Sascha Hauer

---

?arch/arm/mach-mxs/clock-mx23.c ? ? ? ? ? ? | ? ?2 +-
?arch/arm/mach-mxs/clock-mx28.c ? ? ? ? ? ? | ? ?1 +
?arch/arm/mach-mxs/devices-mx23.h ? ? ? ? ? | ? ?4 ++
?arch/arm/mach-mxs/devices-mx28.h ? ? ? ? ? | ? ?4 ++
?arch/arm/mach-mxs/devices/Kconfig ? ? ? ? ?| ? ?4 ++
?arch/arm/mach-mxs/devices/Makefile ? ? ? ? | ? ?1 +
?arch/arm/mach-mxs/devices/platform-mxsfb.c | ? 46 ++++++++++++++++++++++++++++

?7 files changed, 61 insertions(+), 1 deletions(-) ?create mode 100644 arch/arm/mach-mxs/devices/platform-mxsfb.c

arch/arm/mach-mxs/clock-mx23.c +0 -0
index ca72a05..bfc7f27 100644

arch/arm/mach-mxs/clock-mx23.c static struct clk_lookup lookups[] = { · line 449

Quoted 5 lines
arch/arm/mach-mxs/clock-mx23.c +1 -1
@@ -446,7 +446,7 @@ static struct clk_lookup lookups[] = {
 ? ? ? ?_REGISTER_CLOCK(NULL, "hclk", hbus_clk)
 ? ? ? ?_REGISTER_CLOCK(NULL, "usb", usb_clk)
 ? ? ? ?_REGISTER_CLOCK(NULL, "audio", audio_clk)
- ? ? ? _REGISTER_CLOCK(NULL, "pwm", pwm_clk)
+ ? ? ? _REGISTER_CLOCK("imx23-fb", NULL, lcdif_clk)

Is the removal of the pwm clock intended here?

Regards,

Fabio Estevam

#18 lore raw

PATCH 2/2 ARM i.MX23/28: Add framebuffer device support

In reply to Sascha Hauer

arch/arm/mach-mxs/clock-mx23.c static struct clk_lookup lookups[] = { · line 450

Quoted 12 lines
arch/arm/mach-mxs/clock- renamed from arch/arm/mach-mxs/clock-mx23.c +0 -0

mx23.c
index ca72a05..bfc7f27 100644

arch/arm/mach-mxs/clock-mx23.c +1 -1
@@ -446,7 +446,7 @@ static struct clk_lookup lookups[] = {
 	_REGISTER_CLOCK(NULL, "hclk", hbus_clk)
 	_REGISTER_CLOCK(NULL, "usb", usb_clk)
 	_REGISTER_CLOCK(NULL, "audio", audio_clk)
-	_REGISTER_CLOCK(NULL, "pwm", pwm_clk)
+	_REGISTER_CLOCK("imx23-fb", NULL, lcdif_clk)
 };

Why remove mx23 pwm clk here?

#19 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

In reply to Sascha Hauer
Quoted 4 lines · Sascha Hauer

Hi all,

The following patches add framebuffer support for Freescale i.MX23/28 processors.

Hi Sascha,

The driver looks good as far as I can tell, but I'd recommend submitting it to linux-fbdev at vger.kernel.org for review in addition to linux-arm-kernel. The frame buffer layer is largely unmaintained these days, but there are still people on that list that care.

Did you consider making the driver a KMS driver instead of a frame buffer? I think the recommendation these days is to start out with KMS for new drivers, which will be somewhat simpler and give you a frame buffer device as well. I don't think that there is a need to change over any drivers from fb to kms though, since you've already done the work.

Arnd

author maintainer
#20 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

Hi Arnd,

Quoted 6 lines · Arnd Bergmann
Quoted 4 lines · Sascha Hauer

Hi all,

The following patches add framebuffer support for Freescale i.MX23/28 processors.

Quoted 7 lines

Hi Sascha,

The driver looks good as far as I can tell, but I'd recommend submitting it to linux-fbdev at vger.kernel.org for review in addition to linux-arm-kernel. The frame buffer layer is largely unmaintained these days, but there are still people on that list that care.

The driver patch itself is Cced to linux-fbdev, only the introductory mail is not.

Quoted 7 lines

Did you consider making the driver a KMS driver instead of a frame buffer? I think the recommendation these days is to start out with KMS for new drivers, which will be somewhat simpler and give you a frame buffer device as well. I don't think that there is a need to change over any drivers from fb to kms though, since you've already done the work.

I tried doing so for the i.MX51 which supports multiple displays on dvi and vga outputs and thus could make good use of KMS and friends. Anyway, I got stuck quite fast. The KMS stuff is tightly coupled with DRM/DRI and needs many many callbacks to implement. Additionally the userspace tools expect a nvidia/amd/intel driver and do not have a generic fallback. I think this stuff is good for implementing a full blown graphics driver, but is lacking support for simple framebuffer grapics. I'd love to go this way but it still requires a lot of work.

Sascha

Signature · 4 lines
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-5555 |
#21 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

Quoted 2 lines · Sascha Hauer

The driver patch itself is Cced to linux-fbdev, only the introductory mail is not.

Ok, I see.

Quoted 7 lines
Quoted 6 lines

Did you consider making the driver a KMS driver instead of a frame buffer? I think the recommendation these days is to start out with KMS for new drivers, which will be somewhat simpler and give you a frame buffer device as well. I don't think that there is a need to change over any drivers from fb to kms though, since you've already done the work.

Quoted 8 lines

I tried doing so for the i.MX51 which supports multiple displays on dvi and vga outputs and thus could make good use of KMS and friends. Anyway, I got stuck quite fast. The KMS stuff is tightly coupled with DRM/DRI and needs many many callbacks to implement. Additionally the userspace tools expect a nvidia/amd/intel driver and do not have a generic fallback. I think this stuff is good for implementing a full blown graphics driver, but is lacking support for simple framebuffer grapics. I'd love to go this way but it still requires a lot of work.

Ok. This sounds like a lot of upfront work indeed, to make KMS more generic, though I think a number of driver would benefit from it eventually. It could be something for the Linaro graphics working group to look at in the following 11.11 release, depending on how many other people are interested in getting there.

Arnd

#22 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

Quoted 5 lines · Arnd Bergmann

Ok. This sounds like a lot of upfront work indeed, to make KMS more generic, though I think a number of driver would benefit from it eventually. It could be something for the Linaro graphics working group to look at in the following 11.11 release, depending on how many other people are interested in getting there.

There is already a blueprint at linaro:
https://blueprints.launchpad.net/linaro-graphics-wg/+spec/multimedia-linaro-dri-1105

We have recently tested the state of Qt acceleration on the MX51, in order to find out which is the best way to make use of the possibilities. However, at least at the moment this requires highly experimental Qt sceenegraph git branches, and even then it results in lower perfomance than with software rendering ...

But for the long term, the linaro blueprint looks like the way to go.

rsc

Signature · 4 lines
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-5555 |
#23 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

In reply to Arnd Bergmann
Quoted 23 lines · Arnd Bergmann
Quoted 2 lines · Sascha Hauer

The driver patch itself is Cced to linux-fbdev, only the introductory mail is not.

Ok, I see.

Quoted 7 lines
Quoted 6 lines

Did you consider making the driver a KMS driver instead of a frame buffer? I think the recommendation these days is to start out with KMS for new drivers, which will be somewhat simpler and give you a frame buffer device as well. I don't think that there is a need to change over any drivers from fb to kms though, since you've already done the work.

Quoted 8 lines

I tried doing so for the i.MX51 which supports multiple displays on dvi and vga outputs and thus could make good use of KMS and friends. Anyway, I got stuck quite fast. The KMS stuff is tightly coupled with DRM/DRI and needs many many callbacks to implement. Additionally the userspace tools expect a nvidia/amd/intel driver and do not have a generic fallback. I think this stuff is good for implementing a full blown graphics driver, but is lacking support for simple framebuffer grapics. I'd love to go this way but it still requires a lot of work.

Quoted 6 lines

Ok. This sounds like a lot of upfront work indeed, to make KMS more generic, though I think a number of driver would benefit from it eventually. It could be something for the Linaro graphics working group to look at in the following 11.11 release, depending on how many other people are interested in getting there.

fwiw, it seems to me like xorg could have some more common code to deal with the KMS part of DRM.. the various userspace xorg drivers end up having a lot of very similar code to deal with enumerating CRTCs/outputs and modes, handle hotplug, etc.

I'd been experimenting a bit on the side w/ the DRM driver framework (
http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu
), but had to add a good chunk of mostly boilerplate code to our xorg driver in order just to test it. Maybe some generic support for KMS in xf86-video-fbdev would have made this easier to develop the kernel part without in parallel having to implement the userspace part. I'm not sure if this is the sort of thing the linaro-wg has in mind?

BR,
-R

#24 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

Quoted 7 lines · Rob

I'd been experimenting a bit on the side w/ the DRM driver framework (
http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu
), but had to add a good chunk of mostly boilerplate code to our xorg driver in order just to test it. Maybe some generic support for KMS in xf86-video-fbdev would have made this easier to develop the kernel part without in parallel having to implement the userspace part. I'm not sure if this is the sort of thing the linaro-wg has in mind?

I'm not sure what the the linaro multimedia wg thinks of this, but the kernel code you linked looks like it's doing exactly the right thing.

Arnd

#25 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

Speaking for the Linaro graphics working group, I think it's great. And, I think you're right, that if enough of the KMS support in xf86-video-* is similar enough (I was only aware of intel and nouveau supporting it properly at current), pulling it out into a common layer would make it easier to support in new drivers (including fbdev).

cheers,
Jesse

Quoted 13 lines · Arnd Bergmann
Quoted 2 lines · Rob

I'd been experimenting a bit on the side w/ the DRM driver framework (

http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu

Quoted 5 lines

), but had to add a good chunk of mostly boilerplate code to our xorg driver in order just to test it. Maybe some generic support for KMS in xf86-video-fbdev would have made this easier to develop the kernel part without in parallel having to implement the userspace part. I'm not sure if this is the sort of thing the linaro-wg has in mind?

I'm not sure what the the linaro multimedia wg thinks of this, but the kernel code you linked looks like it's doing exactly the right thing.

Quoted 7 lines

Arnd

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110216/78490466/attachment-0001.html>

#26 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff.. I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

BR,
-R

Quoted 32 lines · Jesse Barker

Speaking for the Linaro graphics working group, I think it's great.? And, I think you're right, that if enough of the KMS support in xf86-video-* is similar enough (I was only aware of intel and nouveau supporting it properly at current), pulling it out into a common layer would make it easier to support in new drivers (including fbdev).

cheers,
Jesse

Quoted 20 lines · Arnd Bergmann
Quoted 8 lines · Rob

I'd been experimenting a bit on the side w/ the DRM driver framework (

http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu
), but had to add a good chunk of mostly boilerplate code to our xorg driver in order just to test it. ?Maybe some generic support for KMS in xf86-video-fbdev would have made this easier to develop the kernel part without in parallel having to implement the userspace part. ?I'm not sure if this is the sort of thing the linaro-wg has in mind?

I'm not sure what the the linaro multimedia wg thinks of this, but the kernel code you linked looks like it's doing exactly the right thing.

? ? ? ?Arnd

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

#27 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

There is already one KMS abstraction layer (libkms.so) in libdrm, maybe it can serve as what we needed.

Regards,
Jammy

Quoted 48 lines · Rob

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff.. I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

BR,
-R

Quoted 1 line · Jesse Barker

Speaking for the Linaro graphics working group, I think it's great. And,

I

Quoted 2 lines

think you're right, that if enough of the KMS support in xf86-video-* is similar enough (I was only aware of intel and nouveau supporting it

properly

Quoted 12 lines

at current), pulling it out into a common layer would make it easier to support in new drivers (including fbdev).

cheers,
Jesse

Quoted 5 lines · Arnd Bergmann
Quoted 3 lines · Rob

I'd been experimenting a bit on the side w/ the DRM driver framework (

http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu

Quoted 9 lines
Quoted 9 lines
Quoted 5 lines

), but had to add a good chunk of mostly boilerplate code to our xorg driver in order just to test it. Maybe some generic support for KMS in xf86-video-fbdev would have made this easier to develop the kernel part without in parallel having to implement the userspace part. I'm not sure if this is the sort of thing the linaro-wg has in mind?

I'm not sure what the the linaro multimedia wg thinks of this, but the kernel code you linked looks like it's doing exactly the right thing.

Quoted 8 lines
Quoted 6 lines

Arnd

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Quoted 5 lines

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110217/47c57980/attachment-0001.html>

#28 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

Hmm, I was thinking more on the xf86 side of things.. ie. to provide default implementations of xf86CrtcFuncsRec and xf86OutputFuncsRec functions..

BR,
-R

Quoted 57 lines · Jammy Zhou

There is already one KMS abstraction layer (libkms.so) in libdrm, maybe it can serve as what we needed.

Regards,
Jammy

Quoted 49 lines · Rob

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff.. ?I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

BR,
-R

Quoted 6 lines · Jesse Barker

Speaking for the Linaro graphics working group, I think it's great.? And, I think you're right, that if enough of the KMS support in xf86-video-* is similar enough (I was only aware of intel and nouveau supporting it properly at current), pulling it out into a common layer would make it easier to support in new drivers (including fbdev).

Quoted 26 lines

cheers,
Jesse

Quoted 20 lines · Arnd Bergmann
Quoted 8 lines · Rob

I'd been experimenting a bit on the side w/ the DRM driver framework (

http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu
), but had to add a good chunk of mostly boilerplate code to our xorg driver in order just to test it. ?Maybe some generic support for KMS in xf86-video-fbdev would have made this easier to develop the kernel part without in parallel having to implement the userspace part. ?I'm not sure if this is the sort of thing the linaro-wg has in mind?

I'm not sure what the the linaro multimedia wg thinks of this, but the kernel code you linked looks like it's doing exactly the right thing.

? ? ? ?Arnd

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

#29 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

I also noticed that default XRandR1.2+ implementation is missing in X side. If we can implement one, it would be beneficial for all ARM platforms. By the way, does X driver of TI support XRandR1.2+?

Regards,
Jammy

Quoted 54 lines · Rob

Hmm, I was thinking more on the xf86 side of things.. ie. to provide default implementations of xf86CrtcFuncsRec and xf86OutputFuncsRec functions..

BR,
-R

Quoted 2 lines · Jammy Zhou

There is already one KMS abstraction layer (libkms.so) in libdrm, maybe

it can serve as what we needed.

Quoted 17 lines

Regards,
Jammy

Quoted 12 lines · Rob

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff.. I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

BR,
-R

On Wed, Feb 16, 2011 at 8:42 AM, Jesse Barker <[email protected]>

Quoted 1 line · wrote:
Quoted 1 line
Quoted 1 line

Speaking for the Linaro graphics working group, I think it's great.

And, I

Quoted 1 line
Quoted 1 line
Quoted 1 line

think you're right, that if enough of the KMS support in xf86-video-*

is

Quoted 1 line
Quoted 1 line
Quoted 1 line

similar enough (I was only aware of intel and nouveau supporting it

properly

Quoted 1 line
Quoted 1 line
Quoted 1 line

at current), pulling it out into a common layer would make it easier

to

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines

support in new drivers (including fbdev).

cheers,
Jesse

Quoted 3 lines · Arnd Bergmann
Quoted 1 line · Rob

I'd been experimenting a bit on the side w/ the DRM driver

framework (

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

), but had to add a good chunk of mostly boilerplate code to our

xorg

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

driver in order just to test it. Maybe some generic support for

KMS

Quoted 25 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

in xf86-video-fbdev would have made this easier to develop the

kernel

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

part without in parallel having to implement the userspace part.

I'm

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line

not sure if this is the sort of thing the linaro-wg has in mind?

I'm not sure what the the linaro multimedia wg thinks of this, but

the

Quoted 16 lines
Quoted 15 lines
Quoted 10 lines
Quoted 8 lines

kernel code you linked looks like it's doing exactly the right thing.

Arnd

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110218/dae57eaa/attachment-0001.html>

#30 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

I'm in the process of adding xrandr support to our xorg driver.. definitely more built-in support on the X side would make this easier.

BR,
-R

Quoted 90 lines · Jammy Zhou

I also noticed that default XRandR1.2+ implementation is missing in X side. If we can implement one, it would be beneficial for all ARM platforms. By the way, does X driver of TI support XRandR1.2+?

Regards,
Jammy

Quoted 80 lines · Rob

Hmm, I was thinking more on the xf86 side of things.. ie. to provide default implementations of xf86CrtcFuncsRec and xf86OutputFuncsRec functions..

BR,
-R

Quoted 71 lines · Jammy Zhou

There is already one KMS abstraction layer (libkms.so) in libdrm, maybe it can serve as what we needed.

Regards,
Jammy

Quoted 62 lines · Rob

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff.. ?I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

BR,
-R

Quoted 10 lines · Jesse Barker

Speaking for the Linaro graphics working group, I think it's great. And, I
think you're right, that if enough of the KMS support in xf86-video-* is
similar enough (I was only aware of intel and nouveau supporting it properly
at current), pulling it out into a common layer would make it easier to
support in new drivers (including fbdev).

Quoted 34 lines

cheers,
Jesse

Quoted 28 lines · Arnd Bergmann
Quoted 4 lines · Rob

I'd been experimenting a bit on the side w/ the DRM driver framework (

Quoted 10 lines

http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu
), but had to add a good chunk of mostly boilerplate code to our xorg
driver in order just to test it. ?Maybe some generic support for KMS
in xf86-video-fbdev would have made this easier to develop the kernel
part without in parallel having to implement the userspace part. ?I'm
not sure if this is the sort of thing the linaro-wg has in mind?

I'm not sure what the the linaro multimedia wg thinks of this, but the
kernel code you linked looks like it's doing exactly the right thing.

? ? ? ?Arnd

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

_______________________________________________
linaro-dev mailing list
linaro-dev at lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

#31 lore raw
In reply to Clark, Rob
Quoted 7 lines

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff.. I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

I have been thinking about this as well. One of the short coming for DRM on embedded platforms is the lack of a small well defined library that one could use. Right now libkms only handles buffer allocation. The mode setting is currently tied to the libdrm library. It would be nice to seperate the two out more. Move the mode setting code to libkms and then have libdrm be a wrapper around this. This way libdrm can both support the legacy drm drivers and the new kms drivers at the same time. It also makes a clear seperation. Jakob if you are willing to take this work I will gladly seen you patches.

A message that is not in the archive (sent off-list or not yet received): alpine.LFD.2.00.1102171545470.21278-FZi0V3Vbi30CUdFEqe4BF2D2FQJk+8+b@public.gmane.org

#33 lore raw

To accommodate the fact of independent display controller and GPU components in ARM SOC, it will be better if we can separate KMS from DRM both in kernel space and user space (i.e, create a new drivers/video/kms directory for kernel side, move KMS related code in libdrm to libkms in user space). Then we can build xrandr1.2+ support based on KMS for ARM platforms, even if DRM is still not supported. Besides, for buffer management part (GEM) in DRM, if possible, we can also make it an independent module leaving DRM just a wrapper, so that the GEM stuff can be used more flexibly.

Regards,
Jammy

Quoted 18 lines · James Simmons
Quoted 7 lines

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff.. I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

I have been thinking about this as well. One of the short coming for DRM on embedded platforms is the lack of a small well defined library that one could use. Right now libkms only handles buffer allocation. The mode setting is currently tied to the libdrm library. It would be nice to seperate the two out more. Move the mode setting code to libkms and then have libdrm be a wrapper around this. This way libdrm can both support the legacy drm drivers and the new kms drivers at the same time. It also makes a clear seperation. Jakob if you are willing to take this work I will gladly seen you patches.

Quoted 5 lines

_______________________________________________
linaro-dev mailing list [email protected]
http://lists.linaro.org/mailman/listinfo/linaro-dev

_______________________________________________
linaro-dev mailing list [email protected]
http://lists.linaro.org/mailman/listinfo/linaro-dev

A message that is not in the archive (sent off-list or not yet received): AANLkTimcwr4vpycSQCH9vMaB+umh+yHD_+WD2MJNqMOB-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#35 lore raw

I'm all for a more modular drm, although I think the framework of CRTCs, encoders, and connectors is a nice fit, at least for our hw. It would be nice to have a better way to expose overlays. And I'm still thinking about how/if GEM fits in with our various video and 2/3d accelerators.

BR,
-R

Quoted 38 lines · Jammy Zhou

To accommodate the fact of independent display controller and GPU components in ARM SOC, it will be better if we can separate KMS from DRM both in kernel space and user space (i.e, create a new drivers/video/kms directory for kernel side, move KMS related code in libdrm to libkms in user space). Then we can build xrandr1.2+ support based on KMS for ARM platforms, even if DRM is still not supported. Besides, for buffer management part (GEM) in DRM, if possible, we can also make it an independent module leaving DRM just a wrapper, so that the GEM stuff can be used more flexibly.

Regards,
Jammy

Quoted 22 lines · James Simmons
Quoted 7 lines

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff..  I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

I have been thinking about this as well. One of the short coming for DRM on embedded platforms is the lack of a small well defined library that one could use. Right now libkms only handles buffer allocation. The mode setting is currently tied to the libdrm library. It would be nice to seperate the two out more. Move the mode setting code to libkms and then have libdrm be a wrapper around this. This way libdrm can both support the legacy drm drivers and the new kms drivers at the same time. It also makes a clear seperation. Jakob if you are willing to take this work I will gladly seen you patches.

_______________________________________________
linaro-dev mailing list [email protected]
http://lists.linaro.org/mailman/listinfo/linaro-dev

#36 lore raw
In reply to James Simmons
Quoted 8 lines · James Simmons
Quoted 7 lines

I'm still in the learning-as-I-go phase here, so definitely not ready to propose a solution, but it does seem to me like there is room for some sort of kms-helper library here to handle more of the boilerplate xf86-video-* stuff.. ?I guess I'll have a better picture once I have a chance to add support for the various multi-monitor configurations. But certainly would be interested if anyone already has some ideas.

Quoted 9 lines

I have been thinking about this as well. One of the short coming for DRM on embedded platforms is the lack of a small well defined library that one could use. Right now libkms only handles buffer allocation. The mode setting is currently tied to the libdrm library. It would be nice to seperate the two out more. Move the mode setting code to libkms and then have libdrm be a wrapper around this. This way libdrm can both support the legacy drm drivers and the new kms drivers at the same time. It also makes a clear seperation. Jakob if you are willing to take this work I will gladly seen you patches.

the problem with that is libkms relies on libdrm, so you'd have a circular dependency.

With the addition of the dumb ioctl to the kernel we can certainly avoid some of that dependency,
but you guys are missing one important point about libkms, its meant to be a fallback.

Why?

well because most GPUs in reality have non-generic memory layouts, they prefer having tiled buffers for different things and different types of tiling. There is simply no nice way to abstract that out, since it depends on the userspace libraries that use this. Starting off with an untiled allocation for the frontbuffer can pretty much leave you messed up when you plug anything like acceleration in afterwards.

I have been looking at this from the USB driver pov as well, and we can probably resurrect the old xf86-video-modesetting driver along with some code in the X server maybe, mesa also has a libkms fallback X driver that works quite well if just a bit messy to build (since its in mesa).

Dave.

#37 lore raw

PATCH 1/1 i.MX23/28 framebuffer driver

In reply to Sascha Hauer

Hi Sascha,

Quoted 11 lines · ext Sascha Hauer

Hi Arnd,

Quoted 7 lines · Arnd Bergmann

Did you consider making the driver a KMS driver instead of a frame buffer? I think the recommendation these days is to start out with KMS for new drivers, which will be somewhat simpler and give you a frame buffer device as well. I don't think that there is a need to change over any drivers from fb to kms though, since you've already done the work.

Quoted 8 lines

I tried doing so for the i.MX51 which supports multiple displays on dvi and vga outputs and thus could make good use of KMS and friends. Anyway, I got stuck quite fast. The KMS stuff is tightly coupled with DRM/DRI and needs many many callbacks to implement. Additionally the userspace tools expect a nvidia/amd/intel driver and do not have a generic fallback. I think this stuff is good for implementing a full blown graphics driver, but is lacking support for simple framebuffer grapics. I'd love to go this way but it still requires a lot of work.

I will get the i.MX28 EVK within a couple of weeks. I'm curious of KMS, which I don't know at all, and I would like to work on it. We could join the efforts. What do you think?

Best regards,
Domenico