[PATCH] omap4-fb: add driver
Sascha Hauer
s.hauer at pengutronix.de
Fri Apr 5 04:59:17 EDT 2013
Hi Christoph,
mostly ok, some comments inline.
On Thu, Apr 04, 2013 at 08:42:42AM +0200, Christoph Fritz wrote:
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 6d6b08f..15eaa2f 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -45,6 +45,14 @@ config DRIVER_VIDEO_S3C24XX
> help
> Add support for the S3C244x LCD controller.
>
> +config DRIVER_VIDEO_OMAP4
> + bool "OMAP4 framebuffer driver"
\t please
> + depends on ARCH_OMAP4
> + help
> + Add support for the OMAP4 Display Controller.
> + DSI is unsupported, only DISPC parallel mode on LCD2
> + is supported.
> +
> if DRIVER_VIDEO_S3C24XX
> +
> +struct omap4fb_colors {
> + struct fb_bitfield red;
> + struct fb_bitfield green;
> + struct fb_bitfield blue;
> + struct fb_bitfield transp;
> +};
> +
> +static struct omap4fb_colors const omap4FB_COLORS[] = {
lower case variable names please.
> + [0] = {
> + .red = { .length = 0, .offset = 0 },
> + },
> + [1] = {
> + .blue = { .length = 8, .offset = 0 },
> + .green = { .length = 8, .offset = 8 },
> + .red = { .length = 8, .offset = 16 },
> + },
> + [2] = {
> + .blue = { .length = 8, .offset = 0 },
> + .green = { .length = 8, .offset = 8 },
> + .red = { .length = 8, .offset = 16 },
> + .transp = { .length = 8, .offset = 24 },
> + },
> +};
> +
> +static void omap4fb_fill_bootenv(struct omap4fb_device *fbi,
> + size_t screen_size)
> +{
> + char buf[sizeof("bootsplash=0x12345678+12345678910")];
> + int rc;
> +
> + snprintf(buf, sizeof buf, "bootsplash=0x%08lx+%zu",
> + (unsigned long)fbi->info.screen_base,
> + screen_size);
> +
> + rc = dev_set_param(fbi->dev, "bootargs", buf);
> + if (rc < 0) {
> + dev_err(fbi->dev, "failed to set bootargs '%s': %d\n",
> + buf, rc);
> + }
> +}
Where is this used in the kernel? You should probably rather use the
simple fb driver Steven Warren suggested.
> +
> + rc = register_framebuffer(info);
> + if (rc < 0) {
> + dev_err(dev, "failed to register framebuffer: %d\n", rc);
> + goto out;
> + }
> +
> + rc = 0;
> + dev_info(dev, "registered omap4 framebuffer\n");
The context is already in dev_info, so you can just do a
dev_info(dev, "registered\n");
> +/* TRM: 10.2.7.3 Display Controller Registers */
> +struct omap4_regs_dispc {
> + uint32_t const revision;
> + OMAP4_RESERVED(0x04, 0x10);
> + uint32_t sysconfig;
> + uint32_t const sysstatus;
> + uint32_t irqstatus;
> + uint32_t irqenable;
> + OMAP4_RESERVED(0x20, 0x40);
> + uint32_t control1;
> + uint32_t config1;
> + OMAP4_RESERVED(0x48, 0x4C);
> +
> + uint32_t default_color[2];
> + uint32_t trans_color[2];
> +
> + uint32_t const line_status;
> + uint32_t line_number;
> + uint32_t timing_h1;
> + uint32_t timing_v1;
> + uint32_t pol_freq1;
> + uint32_t divisor1;
> + uint32_t global_alpha;
> + uint32_t size_tv;
> + uint32_t size_lcd1;
> +
> + struct {
> + uint32_t ba[2];
> + uint32_t position;
> + uint32_t size;
> + OMAP4_RESERVED(0x90, 0xA0);
> + uint32_t attributes;
> + uint32_t buf_threshold;
> + uint32_t const buf_size_status;
> + uint32_t row_inc;
> + uint32_t pixel_inc;
> + OMAP4_RESERVED(0xB4, 0xB8);
> + uint32_t table_ba;
> + } gfx;
> +
> + struct omap4_regs_dispc_vid vid1;
> + OMAP4_RESERVED(0x144, 0x14C);
> + struct omap4_regs_dispc_vid vid2;
> +
> + uint32_t data1_cycle[3];
> +
> + uint32_t vid1_fir_coef_v[8];
> + uint32_t vid2_fir_coef_v[8];
> + uint32_t cpr1_coef_r;
> + uint32_t cpr1_coef_g;
> + uint32_t cpr1_coef_b;
> + uint32_t gfx_preload;
> + uint32_t vid_preload[2];
> + uint32_t control2;
> + OMAP4_RESERVED(0x23C, 0x300);
> +
> + struct {
> + uint32_t accu[2];
> + uint32_t ba[2];
> + struct {
> + uint32_t h;
> + uint32_t hv;
> + } fir_coef[8];
> + uint32_t fir_coef_v[8];
> + uint32_t attributes;
> + uint32_t conv_coef0;
> + uint32_t conv_coef1;
> + uint32_t conv_coef2;
> + uint32_t conv_coef3;
> + uint32_t conv_coef4;
> + uint32_t const buf_size_status;
> + uint32_t buf_threshold;
> + uint32_t fir;
> + uint32_t picture_size;
> + uint32_t pixel_inc;
> + uint32_t position;
> + uint32_t preload;
> + uint32_t row_inc;
> + uint32_t size;
> + } vid3;
> +
> + uint32_t default_color2;
> + uint32_t trans_color2;
> + uint32_t cpr2_coef_b;
> + uint32_t cpr2_coef_g;
> + uint32_t cpr2_coef_r;
> + uint32_t data2_cycle[3];
> + uint32_t size_lcd2;
> + OMAP4_RESERVED(0x3D0, 0x400);
> + uint32_t timing_h2;
> + uint32_t timing_v2;
> + uint32_t pol_freq2;
> + uint32_t divisor2;
> + OMAP4_RESERVED(0x410, 0x500);
> +
> + struct {
> + uint32_t accu[2];
> + uint32_t ba[2];
> + struct {
> + uint32_t h;
> + uint32_t hv;
> + } fir_coef[8];
> + uint32_t fir_coef_v[8];
> + uint32_t attributes;
> + uint32_t conv_coef0;
> + uint32_t conv_coef1;
> + uint32_t conv_coef2;
> + uint32_t conv_coef3;
> + uint32_t conv_coef4;
> + uint32_t const buf_size_status;
> + uint32_t buf_threshold;
> + uint32_t fir;
> + uint32_t picture_size;
> + uint32_t pixel_inc;
> + OMAP4_RESERVED(0x59C, 0x5A4);
> + uint32_t row_inc;
> + uint32_t size;
> + } wb;
> +
> + OMAP4_RESERVED(0x5AC, 0x600);
> + uint32_t vid1_ba_uv[2];
> + uint32_t vid2_ba_uv[2];
> + uint32_t vid3_ba_uv[2];
> + uint32_t wb_ba_uv[2];
> + uint32_t config2;
> + uint32_t vid1_attributes2;
> + uint32_t vid2_attributes2;
> + uint32_t vid3_attributes2;
> + uint32_t gamma_table0;
> + uint32_t gamma_table1;
> + uint32_t gamma_table2;
> + uint32_t vid1_fir2;
> + uint32_t vid1_accu2[2];
> + struct {
> + uint32_t h2;
> + uint32_t hv2;
> + } vid1_fir_coef[8];
> + uint32_t vid1_fir_coef_v2[8];
> + uint32_t vid2_fir2;
> + uint32_t vid2_accu2[2];
> + struct {
> + uint32_t h2;
> + uint32_t hv2;
> + } vid2_fir_coef[8];
> + uint32_t vid2_fir_coef_v2[8];
> + OMAP4_RESERVED(0x714, 0x724);
> + uint32_t vid3_fir2;
> + uint32_t vid3_accu2[2];
> + struct {
> + uint32_t h2;
> + uint32_t hv2;
> + } vid3_fir_coef[8];
> + uint32_t vid3_fir_coef_v2[8];
> + uint32_t wb_fir2;
> + uint32_t wb_accu2[2];
> + OMAP4_RESERVED(0x79C, 0x7A0);
> + struct {
> + uint32_t h2;
> + uint32_t hv2;
> + } wb_fir_coef[8];
> + uint32_t wb_fir_coef_v2[8];
> + uint32_t global_buffer;
> + uint32_t divisor;
> + OMAP4_RESERVED(0x808, 0x810);
> + uint32_t wb_attributes2;
> +};
This looks awful. I wonder who ever had the idea that placing structs
over registers is a good idea.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
More information about the barebox
mailing list