Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: Proposal for a low-level Linux display framework
From: Keith Packard @ 2011-09-16  4:53 UTC (permalink / raw)
  To: Florian Tobias Schandinat, Alex Deucher
  Cc: linux-fbdev, linaro-dev, linux-kernel, dri-devel, Archit Taneja,
	Clark, Rob
In-Reply-To: <4E724659.5080709@gmx.de>

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

On Thu, 15 Sep 2011 18:39:21 +0000, Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:

> Well, I'm not against sharing the code and not against taking DRM's current
> implementation as a base but the steps required to make it generally acceptable
> would be to split it of, probably as a standalone module and strip all DRM
> specific things off. Than all things that require EDID can use it, DRM can add
> DRM-specific things on top and fb can add fb-specific things.

The rendering portions of the DRM drivers are all device-specific. The
core DRM ioctls are largely about providing some sharing control over
the device, mapping memory around and mode setting.

> One of my biggest problems with KMS is that it has (naturally) a lot more
> complexity than the fb API which leads to instability.

The mode setting portions are of necessity the same. The KMS API exposes
more functionality for mode setting, but doesn't actually require any
additional hardware-specific knowledge. You still have to be able to
bring the hardware up from power on and light up every connected
monitor.

However, if you want acceleration, you're going to run into bugs that
crash the machine. It's a sad reality that graphics hardware just isn't
able to recover cleanly in all cases from programmer errors, and that
includes errors that come from user mode.

Hardware is improving in this area, and reset is getting more reliable
than it used to be. But, until we can context switch the graphics
hardware at arbitrary points during execution, we're kinda stuck with
using the really big reset hammer when programs go awry.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Keith Packard @ 2011-09-16  0:55 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel, dri-devel, linaro-dev, Clark, Rob,
	Archit Taneja
In-Reply-To: <1316107275.23214.99.camel@deskari>

[-- Attachment #1: Type: text/plain, Size: 2162 bytes --]

On Thu, 15 Sep 2011 20:21:15 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> 2) panel drivers, handles panel specific things. Each panel may support
> custom commands and features, for which we need a dedicated driver. And
> this driver is not platform specific, but should work with any platform
> which has the output used with the panel.

Right, we've got DDC ports (which are just i2c) and DisplayPort aux
channel stuff.

The DDC stuff is abstracted out and shared across the drivers, but the
DisplayPort aux channel code is not -- it's duplicated in every output
driver. 

> DSI bus is a half-duplex serial bus, and while it's designed for
> displays you could use it easily for any communication between the SoC
> and the peripheral.

Yeah, HDMI uses DDC for all kinds of crazy stuff in the CE world.

> The point is that we cannot have standard "MIPI DSI command mode panel
> driver" which would work for all DSI cmd mode panels, but we need (in
> the worst case) separate driver for each panel.

It sounds like we do want to share code for those bits, much like we
have DDC split out now. And, we should do something about the
DisplayPort aux channel stuff to avoid duplicating it everywhere.

I'm not sure a common interface to all of these different
channels makes sense, but surely a DSI library and an aux channel
library would fit nicely alongside the existing DDC library.

I suspect helper functions would be a good model to follow, rather than
trying to create a whole new device infrastructure; some of the
communication paths aren't easily separable from the underlying output
devices.

Oh, I think you're also trying to get at how we expose some of these
controls outside of the display driver -- right now, they're mostly
exposed as properties on the output device. Things like backlight
brightness, a million analog TV output values, dithering control and
other more esoteric controls.

DRM properties include booleans, enumerations, integer ranges and chunks
of binary data. Some are read-only (like EDID data), some are writable
(like overscan values).

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 13/13] drivers/video: fsl-diu-fb: remove unusued MEM_ALLOC_THRESHOLD
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

If there was ever any code that used MEM_ALLOC_THRESHOLD, it was removed a
long time ago.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 include/linux/fsl-diu-fb.h |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/include/linux/fsl-diu-fb.h b/include/linux/fsl-diu-fb.h
index 35ac0c5..df23f59 100644
--- a/include/linux/fsl-diu-fb.h
+++ b/include/linux/fsl-diu-fb.h
@@ -20,11 +20,6 @@
 #ifndef __FSL_DIU_FB_H__
 #define __FSL_DIU_FB_H__
 
-/* Arbitrary threshold to determine the allocation method
- * See mpc8610fb_set_par(), map_video_memory(), and unmap_video_memory()
- */
-#define MEM_ALLOC_THRESHOLD (1024*768*4+32)
-
 #include <linux/types.h>
 
 struct mfb_chroma_key {
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 12/13] drivers/video: fsl-diu-fb: the video buffer is not I/O memory
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

The video buffer is not uncached memory-mapped I/O, so don't tag the virtual
address as __iomem.  It's also not a u8*.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 include/linux/fsl-diu-fb.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/fsl-diu-fb.h b/include/linux/fsl-diu-fb.h
index 5ebffa6..35ac0c5 100644
--- a/include/linux/fsl-diu-fb.h
+++ b/include/linux/fsl-diu-fb.h
@@ -162,7 +162,7 @@ struct diu_hw {
 };
 
 struct diu_addr {
-	__u8 __iomem *vaddr;	/* Virtual address */
+	void *vaddr;		/* Virtual address */
 	dma_addr_t paddr;	/* Physical address */
 	__u32 	   offset;
 };
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 11/13] drivers/video: fsl-diu-fb: use a normal for-loop to uninstall framebuffers
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Uninstalling the framebuffers in reverse order is unnecessary and makes
the for-loop awkward.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index c66f6ed..5137fbb 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1543,9 +1543,9 @@ static int __devinit fsl_diu_probe(struct platform_device *pdev)
 	return 0;
 
 error:
-	for (i = ARRAY_SIZE(machine_data->fsl_diu_info);
-		i > 0; i--)
-		uninstall_fb(machine_data->fsl_diu_info[i - 1]);
+	for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
+		uninstall_fb(machine_data->fsl_diu_info[i]);
+
 	if (pool.ad.vaddr)
 		free_buf(&pdev->dev, &pool.ad,
 			 sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
@@ -1575,8 +1575,8 @@ static int fsl_diu_remove(struct platform_device *pdev)
 	machine_data = dev_get_drvdata(&pdev->dev);
 	disable_lcdc(machine_data->fsl_diu_info[0]);
 	free_irq_local(machine_data->irq);
-	for (i = ARRAY_SIZE(machine_data->fsl_diu_info); i > 0; i--)
-		uninstall_fb(machine_data->fsl_diu_info[i - 1]);
+	for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
+		uninstall_fb(machine_data->fsl_diu_info[i]);
 	if (pool.ad.vaddr)
 		free_buf(&pdev->dev, &pool.ad,
 			 sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 10/13] drivers/video: fsl-diu-fb: fix memory leak on error
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

We were forgetting to unmap the video memory if fsl_diu_check_var() fails.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 3776949..c66f6ed 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1200,6 +1200,7 @@ static int __devinit install_fb(struct fb_info *info)
 
 	if (fsl_diu_check_var(&info->var, info)) {
 		dev_err(info->dev, "fsl_diu_check_var failed\n");
+		unmap_video_memory(info);
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 09/13] drivers/video: fsl-diu-fb: fix potential memcpy buffer overflow bug
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

It makes no sense to limit the size of a strncpy() to the length of
the source string.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 19bfbf5..3776949 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -655,7 +655,7 @@ static void set_fix(struct fb_info *info)
 	struct fb_var_screeninfo *var = &info->var;
 	struct mfb_info *mfbi = info->par;
 
-	strncpy(fix->id, mfbi->id, strlen(mfbi->id));
+	strncpy(fix->id, mfbi->id, sizeof(fix->id));
 	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
 	fix->type = FB_TYPE_PACKED_PIXELS;
 	fix->accel = FB_ACCEL_NONE;
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 08/13] drivers/video: fsl-diu-fb: set the driver name to "fsl-diu-fb"
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Use the name "fsl-diu-fb" in the Freescale DIU framebuffer driver
during registrations.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 4d22399..19bfbf5 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1274,7 +1274,7 @@ static int request_irq_local(int irq)
 	/* Read to clear the status */
 	in_be32(&hw->int_status);
 
-	ret = request_irq(irq, fsl_diu_isr, 0, "diu", NULL);
+	ret = request_irq(irq, fsl_diu_isr, 0, "fsl-diu-fb", NULL);
 	if (!ret) {
 		ints = INT_PARERR | INT_LS_BF_VS;
 #if !defined(CONFIG_NOT_COHERENT_CACHE)
@@ -1634,7 +1634,7 @@ MODULE_DEVICE_TABLE(of, fsl_diu_match);
 
 static struct platform_driver fsl_diu_driver = {
 	.driver = {
-		.name = "fsl_diu",
+		.name = "fsl-diu-fb",
 		.owner = THIS_MODULE,
 		.of_match_table = fsl_diu_match,
 	},
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 07/13] drivers/video: fsl-diu-fb: improve local variable usage in some functions
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Clean up the local variable usage in request_irq_local() and allocate_buf().
This streamlines the code without affecting functionality.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |   23 ++++++++++-------------
 1 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index c893954..4d22399 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1265,14 +1265,14 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
 
 static int request_irq_local(int irq)
 {
-	unsigned long status, ints;
+	u32 ints;
 	struct diu *hw;
 	int ret;
 
 	hw = dr.diu_reg;
 
 	/* Read to clear the status */
-	status = in_be32(&hw->int_status);
+	in_be32(&hw->int_status);
 
 	ret = request_irq(irq, fsl_diu_isr, 0, "diu", NULL);
 	if (!ret) {
@@ -1285,7 +1285,7 @@ static int request_irq_local(int irq)
 			ints |= INT_VSYNC_WB;
 
 		/* Read to clear the status */
-		status = in_be32(&hw->int_status);
+		in_be32(&hw->int_status);
 		out_be32(&hw->int_mask, ints);
 	}
 
@@ -1336,23 +1336,20 @@ static int fsl_diu_resume(struct platform_device *ofdev)
 static int allocate_buf(struct device *dev, struct diu_addr *buf, u32 size,
 			u32 bytes_align)
 {
-	u32 offset, ssize;
-	u32 mask;
-	dma_addr_t paddr = 0;
+	u32 offset;
+	dma_addr_t mask;
 
-	ssize = size + bytes_align;
-	buf->vaddr = dma_alloc_coherent(dev, ssize, &paddr, GFP_DMA |
-							     __GFP_ZERO);
+	buf->vaddr +		dma_alloc_coherent(dev, size + bytes_align, &buf->paddr,
+				   GFP_DMA | __GFP_ZERO);
 	if (!buf->vaddr)
 		return -ENOMEM;
 
-	buf->paddr = (__u32) paddr;
-
 	mask = bytes_align - 1;
-	offset = (u32)buf->paddr & mask;
+	offset = buf->paddr & mask;
 	if (offset) {
 		buf->offset = bytes_align - offset;
-		buf->paddr = (u32)buf->paddr + offset;
+		buf->paddr = buf->paddr + offset;
 	} else
 		buf->offset = 0;
 
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 06/13] drivers/video: fsl-diu-fb: remove redundant default video mode
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

The framebuffer layer already uses the first video mode defined in the
fb_videomode array as a default, so there's no need to duplicate the
first entry into a stand-alone structure.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |   32 +++++++-------------------------
 1 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 96417ab..c893954 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -36,26 +36,10 @@
 #include "edid.h"
 
 /*
- * These parameters give default parameters
- * for video output 1024x768,
- * FIXME - change timing to proper amounts
- * hsync 31.5kHz, vsync 60Hz
+ * List of supported video modes
+ *
+ * The first entry is the default video mode
  */
-static struct fb_videomode __devinitdata fsl_diu_default_mode = {
-	.refresh	= 60,
-	.xres		= 1024,
-	.yres		= 768,
-	.pixclock	= 15385,
-	.left_margin	= 160,
-	.right_margin	= 24,
-	.upper_margin	= 29,
-	.lower_margin	= 3,
-	.hsync_len	= 136,
-	.vsync_len	= 6,
-	.sync		= FB_SYNC_COMP_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
-	.vmode		= FB_VMODE_NONINTERLACED
-};
-
 static struct fb_videomode __devinitdata fsl_diu_mode_db[] = {
 	{
 		.name		= "1024x768-60",
@@ -1165,8 +1149,8 @@ static int __devinit install_fb(struct fb_info *info)
 	} else {
 		aoi_mode = init_aoi_mode;
 	}
-	rc = fb_find_mode(&info->var, info, aoi_mode, db, dbsize,
-			  &fsl_diu_default_mode, default_bpp);
+	rc = fb_find_mode(&info->var, info, aoi_mode, db, dbsize, NULL,
+			  default_bpp);
 	if (!rc) {
 		/*
 		 * For plane 0 we continue and look into
@@ -1180,10 +1164,8 @@ static int __devinit install_fb(struct fb_info *info)
 
 	if (!has_default_mode) {
 		rc = fb_find_mode(&info->var, info, aoi_mode, fsl_diu_mode_db,
-				  ARRAY_SIZE(fsl_diu_mode_db),
-				  &fsl_diu_default_mode,
-				  default_bpp);
-		if (rc > 0 && rc < 5)
+			ARRAY_SIZE(fsl_diu_mode_db), NULL, default_bpp);
+		if (rc)
 			has_default_mode = 1;
 	}
 
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 05/13] drivers/video: fsl-diu-fb: improve device tree usage
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Implement various improvements to the way the Freescale DIU framebuffer
driver access the device tree.

1) Use of_iomap() instead of of_address_to_resource() and ioremap()

2) Use be32_to_cpup() instead of directly dereferencing the device_node
   pointer.

3) Rename variable 'ofdev' to 'pdev' to avoid any confusion that it's
   a platform_device pointer, not an of_device pointer (of_device no
   longer exists).

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |   65 ++++++++++++++++---------------------------
 1 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 6c544cf..96417ab 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1425,13 +1425,12 @@ static ssize_t show_monitor(struct device *device,
 	return 0;
 }
 
-static int __devinit fsl_diu_probe(struct platform_device *ofdev)
+static int __devinit fsl_diu_probe(struct platform_device *pdev)
 {
-	struct device_node *np = ofdev->dev.of_node;
+	struct device_node *np = pdev->dev.of_node;
 	struct mfb_info *mfbi;
 	phys_addr_t dummy_ad_addr = 0;
 	int ret, i, error = 0;
-	struct resource res;
 	struct fsl_diu_data *machine_data;
 	int diu_mode;
 
@@ -1441,9 +1440,9 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 
 	for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++) {
 		machine_data->fsl_diu_info[i] -			framebuffer_alloc(sizeof(struct mfb_info), &ofdev->dev);
+			framebuffer_alloc(sizeof(struct mfb_info), &pdev->dev);
 		if (!machine_data->fsl_diu_info[i]) {
-			dev_err(&ofdev->dev, "cannot allocate memory\n");
+			dev_err(&pdev->dev, "cannot allocate memory\n");
 			ret = -ENOMEM;
 			goto error2;
 		}
@@ -1463,19 +1462,9 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 		}
 	}
 
-	ret = of_address_to_resource(np, 0, &res);
-	if (ret) {
-		dev_err(&ofdev->dev, "could not obtain DIU address\n");
-		goto error;
-	}
-	if (!res.start) {
-		dev_err(&ofdev->dev, "invalid DIU address\n");
-		goto error;
-	}
-
-	dr.diu_reg = ioremap(res.start, sizeof(struct diu));
+	dr.diu_reg = of_iomap(np, 0);
 	if (!dr.diu_reg) {
-		dev_err(&ofdev->dev, "Err: can't map DIU registers!\n");
+		dev_err(&pdev->dev, "cannot map DIU registers\n");
 		ret = -EFAULT;
 		goto error2;
 	}
@@ -1488,25 +1477,25 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 	machine_data->irq = irq_of_parse_and_map(np, 0);
 
 	if (!machine_data->irq) {
-		dev_err(&ofdev->dev, "could not get DIU IRQ\n");
+		dev_err(&pdev->dev, "could not get DIU IRQ\n");
 		ret = -EINVAL;
 		goto error;
 	}
 	machine_data->monitor_port = monitor_port;
 
 	/* Area descriptor memory pool aligns to 64-bit boundary */
-	if (allocate_buf(&ofdev->dev, &pool.ad,
+	if (allocate_buf(&pdev->dev, &pool.ad,
 			 sizeof(struct diu_ad) * FSL_AOI_NUM, 8))
 		return -ENOMEM;
 
 	/* Get memory for Gamma Table  - 32-byte aligned memory */
-	if (allocate_buf(&ofdev->dev, &pool.gamma, 768, 32)) {
+	if (allocate_buf(&pdev->dev, &pool.gamma, 768, 32)) {
 		ret = -ENOMEM;
 		goto error;
 	}
 
 	/* For performance, cursor bitmap buffer aligns to 32-byte boundary */
-	if (allocate_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
+	if (allocate_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
 			 32)) {
 		ret = -ENOMEM;
 		goto error;
@@ -1548,16 +1537,13 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 		mfbi->ad->paddr = pool.ad.paddr + i * sizeof(struct diu_ad);
 		ret = install_fb(machine_data->fsl_diu_info[i]);
 		if (ret) {
-			dev_err(&ofdev->dev,
-				"Failed to register framebuffer %d\n",
-				i);
+			dev_err(&pdev->dev, "could not register fb %d\n", i);
 			goto error;
 		}
 	}
 
 	if (request_irq_local(machine_data->irq)) {
-		dev_err(machine_data->fsl_diu_info[0]->dev,
-			"could not request irq for diu.");
+		dev_err(&pdev->dev, "could not claim irq\n");
 		goto error;
 	}
 
@@ -1569,12 +1555,11 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 	error = device_create_file(machine_data->fsl_diu_info[0]->dev,
 				  &machine_data->dev_attr);
 	if (error) {
-		dev_err(machine_data->fsl_diu_info[0]->dev,
-			"could not create sysfs %s file\n",
+		dev_err(&pdev->dev, "could not create sysfs file %s\n",
 			machine_data->dev_attr.attr.name);
 	}
 
-	dev_set_drvdata(&ofdev->dev, machine_data);
+	dev_set_drvdata(&pdev->dev, machine_data);
 	return 0;
 
 error:
@@ -1582,12 +1567,12 @@ error:
 		i > 0; i--)
 		uninstall_fb(machine_data->fsl_diu_info[i - 1]);
 	if (pool.ad.vaddr)
-		free_buf(&ofdev->dev, &pool.ad,
+		free_buf(&pdev->dev, &pool.ad,
 			 sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
 	if (pool.gamma.vaddr)
-		free_buf(&ofdev->dev, &pool.gamma, 768, 32);
+		free_buf(&pdev->dev, &pool.gamma, 768, 32);
 	if (pool.cursor.vaddr)
-		free_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
+		free_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
 			 32);
 	if (machine_data->dummy_aoi_virt)
 		fsl_diu_free(machine_data->dummy_aoi_virt, 64);
@@ -1602,25 +1587,23 @@ error2:
 	return ret;
 }
 
-
-static int fsl_diu_remove(struct platform_device *ofdev)
+static int fsl_diu_remove(struct platform_device *pdev)
 {
 	struct fsl_diu_data *machine_data;
 	int i;
 
-	machine_data = dev_get_drvdata(&ofdev->dev);
+	machine_data = dev_get_drvdata(&pdev->dev);
 	disable_lcdc(machine_data->fsl_diu_info[0]);
 	free_irq_local(machine_data->irq);
 	for (i = ARRAY_SIZE(machine_data->fsl_diu_info); i > 0; i--)
 		uninstall_fb(machine_data->fsl_diu_info[i - 1]);
 	if (pool.ad.vaddr)
-		free_buf(&ofdev->dev, &pool.ad,
+		free_buf(&pdev->dev, &pool.ad,
 			 sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
 	if (pool.gamma.vaddr)
-		free_buf(&ofdev->dev, &pool.gamma, 768, 32);
+		free_buf(&pdev->dev, &pool.gamma, 768, 32);
 	if (pool.cursor.vaddr)
-		free_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
-			 32);
+		free_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2, 32);
 	if (machine_data->dummy_aoi_virt)
 		fsl_diu_free(machine_data->dummy_aoi_virt, 64);
 	iounmap(dr.diu_reg);
@@ -1722,7 +1705,7 @@ static int __init fsl_diu_init(void)
 	 * Freescale PLRU requires 13/8 times the cache size to do a proper
 	 * displacement flush
 	 */
-	coherence_data_size = *prop * 13;
+	coherence_data_size = be32_to_cpup(prop) * 13;
 	coherence_data_size /= 8;
 
 	prop = of_get_property(np, "d-cache-line-size", NULL);
@@ -1732,7 +1715,7 @@ static int __init fsl_diu_init(void)
 		of_node_put(np);
 		return -ENODEV;
 	}
-	d_cache_line_size = *prop;
+	d_cache_line_size = be32_to_cpup(prop);
 
 	of_node_put(np);
 	coherence_data = vmalloc(coherence_data_size);
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 04/13] drivers/video: fsl-diu-fb: fix compilation warning
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Fix this compilation warning in the Freescale DIU framebuffer driver:

warning: 'dummy_ad_addr' may be used uninitialized in this function

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 226f4bc..6c544cf 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1429,7 +1429,7 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
 	struct mfb_info *mfbi;
-	phys_addr_t dummy_ad_addr;
+	phys_addr_t dummy_ad_addr = 0;
 	int ret, i, error = 0;
 	struct resource res;
 	struct fsl_diu_data *machine_data;
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 03/13] drivers/video: fsl-diu-fb: remove unused ioctls
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Remove some unused ioctl commands, and treat those commands as unsupported
instead of ignored.

Also remove struct mfb_alpha, which isn't used by any ioctl.  It may have
been once intended for MFB_SET_ALPHA, but that ioctl uses a different
data structure.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |   12 ------------
 include/linux/fsl-diu-fb.h |    5 -----
 2 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 10ee411..226f4bc 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -902,9 +902,6 @@ static int fsl_diu_setcolreg(unsigned int regno, unsigned int red,
 			ret = 0;
 		}
 		break;
-	case FB_VISUAL_STATIC_PSEUDOCOLOR:
-	case FB_VISUAL_PSEUDOCOLOR:
-		break;
 	}
 
 	return ret;
@@ -1056,15 +1053,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		if (copy_to_user(buf, ad, sizeof(*ad)))
 			return -EFAULT;
 		break;
-	case FBIOGET_HWCINFO:
-		pr_debug("FBIOGET_HWCINFO:0x%08x\n", FBIOGET_HWCINFO);
-		break;
-	case FBIOPUT_MODEINFO:
-		pr_debug("FBIOPUT_MODEINFO:0x%08x\n", FBIOPUT_MODEINFO);
-		break;
-	case FBIOGET_DISPINFO:
-		pr_debug("FBIOGET_DISPINFO:0x%08x\n", FBIOGET_DISPINFO);
-		break;
 
 	default:
 		dev_err(info->dev, "unknown ioctl command (0x%08X)\n", cmd);
diff --git a/include/linux/fsl-diu-fb.h b/include/linux/fsl-diu-fb.h
index daa9952..5ebffa6 100644
--- a/include/linux/fsl-diu-fb.h
+++ b/include/linux/fsl-diu-fb.h
@@ -27,11 +27,6 @@
 
 #include <linux/types.h>
 
-struct mfb_alpha {
-	int enable;
-	int alpha;
-};
-
 struct mfb_chroma_key {
 	int enable;
 	__u8  red_max;
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 02/13] drivers/video: fsl-diu-fb: clean up printk usage
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Remove debug printk messages (they don't help in debugging), replace
printk(KERN_xxx with its pr_xxx or dev_xxx equivalent, and add a couple
missing error messages.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |  105 +++++++------------------------------------
 1 files changed, 17 insertions(+), 88 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index dbb4cb7..10ee411 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -322,14 +322,9 @@ static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
 {
 	void *virt;

-	pr_debug("size=%zu\n", size);
-
 	virt = alloc_pages_exact(size, GFP_DMA | __GFP_ZERO);
-	if (virt) {
+	if (virt)
 		*phys = virt_to_phys(virt);
-		pr_debug("virt=%p phys=%llx\n", virt,
-			(unsigned long long)*phys);
-	}

 	return virt;
 }
@@ -343,8 +338,6 @@ static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
  */
 static void fsl_diu_free(void *virt, size_t size)
 {
-	pr_debug("virt=%p size=%zu\n", virt, size);
-
 	if (virt && size)
 		free_pages_exact(virt, size);
 }
@@ -368,7 +361,6 @@ static int fsl_diu_enable_panel(struct fb_info *info)
 	struct fsl_diu_data *machine_data = mfbi->parent;
 	int res = 0;

-	pr_debug("enable_panel index %d\n", mfbi->index);
 	if (mfbi->type != MFB_TYPE_OFF) {
 		switch (mfbi->index) {
 		case 0:				/* plane 0 */
@@ -585,9 +577,6 @@ static void adjust_aoi_size_position(struct fb_var_screeninfo *var,
 static int fsl_diu_check_var(struct fb_var_screeninfo *var,
 				struct fb_info *info)
 {
-	pr_debug("check_var xres: %d\n", var->xres);
-	pr_debug("check_var yres: %d\n", var->yres);
-
 	if (var->xres_virtual < var->xres)
 		var->xres_virtual = var->xres;
 	if (var->yres_virtual < var->yres)
@@ -720,7 +709,6 @@ static void update_lcdc(struct fb_info *info)

 	diu_ops.set_gamma_table(machine_data->monitor_port, pool.gamma.vaddr);

-	pr_debug("update-lcdc: HW - %p\n Disabling DIU\n", hw);
 	disable_lcdc(info);

 	/* Program DIU registers */
@@ -732,9 +720,6 @@ static void update_lcdc(struct fb_info *info)
 	out_be32(&hw->bgnd_wb, 0); 		/* BGND_WB */
 	out_be32(&hw->disp_size, (var->yres << 16 | var->xres));
 						/* DISP SIZE */
-	pr_debug("DIU xres: %d\n", var->xres);
-	pr_debug("DIU yres: %d\n", var->yres);
-
 	out_be32(&hw->wb_size, 0); /* WB SIZE */
 	out_be32(&hw->wb_mem_addr, 0); /* WB MEM ADDR */

@@ -751,15 +736,6 @@ static void update_lcdc(struct fb_info *info)

 	out_be32(&hw->vsyn_para, temp);

-	pr_debug("DIU right_margin - %d\n", var->right_margin);
-	pr_debug("DIU left_margin - %d\n", var->left_margin);
-	pr_debug("DIU hsync_len - %d\n", var->hsync_len);
-	pr_debug("DIU upper_margin - %d\n", var->upper_margin);
-	pr_debug("DIU lower_margin - %d\n", var->lower_margin);
-	pr_debug("DIU vsync_len - %d\n", var->vsync_len);
-	pr_debug("DIU HSYNC - 0x%08x\n", hw->hsyn_para);
-	pr_debug("DIU VSYNC - 0x%08x\n", hw->vsyn_para);
-
 	diu_ops.set_pixel_clock(var->pixclock);

 	out_be32(&hw->syn_pol, 0);	/* SYNC SIGNALS POLARITY */
@@ -776,14 +752,9 @@ static int map_video_memory(struct fb_info *info)
 	phys_addr_t phys;
 	u32 smem_len = info->fix.line_length * info->var.yres_virtual;

-	pr_debug("info->var.xres_virtual = %d\n", info->var.xres_virtual);
-	pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
-	pr_debug("info->fix.line_length  = %d\n", info->fix.line_length);
-	pr_debug("MAP_VIDEO_MEMORY: smem_len = %u\n", smem_len);
-
 	info->screen_base = fsl_diu_alloc(smem_len, &phys);
 	if (info->screen_base = NULL) {
-		printk(KERN_ERR "Unable to allocate fb memory\n");
+		dev_err(info->dev, "unable to allocate fb memory\n");
 		return -ENOMEM;
 	}
 	mutex_lock(&info->mm_lock);
@@ -792,10 +763,6 @@ static int map_video_memory(struct fb_info *info)
 	mutex_unlock(&info->mm_lock);
 	info->screen_size = info->fix.smem_len;

-	pr_debug("Allocated fb @ paddr=0x%08lx, size=%d.\n",
-		 info->fix.smem_start, info->fix.smem_len);
-	pr_debug("screen base %p\n", info->screen_base);
-
 	return 0;
 }

@@ -852,11 +819,10 @@ static int fsl_diu_set_par(struct fb_info *info)
 	if (len != info->fix.smem_len) {
 		if (info->fix.smem_start)
 			unmap_video_memory(info);
-		pr_debug("SET PAR: smem_len = %d\n", info->fix.smem_len);

 		/* Memory allocation for framebuffer */
 		if (map_video_memory(info)) {
-			printk(KERN_ERR "Unable to allocate fb memory 1\n");
+			dev_err(info->dev, "unable to allocate fb memory 1\n");
 			return -ENOMEM;
 		}
 	}
@@ -1023,21 +989,17 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		if (copy_from_user(&pix_fmt, buf, sizeof(pix_fmt)))
 			return -EFAULT;
 		ad->pix_fmt = pix_fmt;
-		pr_debug("Set pixel format to 0x%08x\n", ad->pix_fmt);
 		break;
 	case MFB_GET_PIXFMT:
 		pix_fmt = ad->pix_fmt;
 		if (copy_to_user(buf, &pix_fmt, sizeof(pix_fmt)))
 			return -EFAULT;
-		pr_debug("get pixel format 0x%08x\n", ad->pix_fmt);
 		break;
 	case MFB_SET_AOID:
 		if (copy_from_user(&aoi_d, buf, sizeof(aoi_d)))
 			return -EFAULT;
 		mfbi->x_aoi_d = aoi_d.x_aoi_d;
 		mfbi->y_aoi_d = aoi_d.y_aoi_d;
-		pr_debug("set AOI display offset of index %d to (%d,%d)\n",
-				 mfbi->index, aoi_d.x_aoi_d, aoi_d.y_aoi_d);
 		fsl_diu_check_var(&info->var, info);
 		fsl_diu_set_aoi(info);
 		break;
@@ -1046,14 +1008,11 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		aoi_d.y_aoi_d = mfbi->y_aoi_d;
 		if (copy_to_user(buf, &aoi_d, sizeof(aoi_d)))
 			return -EFAULT;
-		pr_debug("get AOI display offset of index %d (%d,%d)\n",
-				mfbi->index, aoi_d.x_aoi_d, aoi_d.y_aoi_d);
 		break;
 	case MFB_GET_ALPHA:
 		global_alpha = mfbi->g_alpha;
 		if (copy_to_user(buf, &global_alpha, sizeof(global_alpha)))
 			return -EFAULT;
-		pr_debug("get global alpha of index %d\n", mfbi->index);
 		break;
 	case MFB_SET_ALPHA:
 		/* set panel information */
@@ -1062,7 +1021,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		ad->src_size_g_alpha = (ad->src_size_g_alpha & (~0xff)) |
 							(global_alpha & 0xff);
 		mfbi->g_alpha = global_alpha;
-		pr_debug("set global alpha for index %d\n", mfbi->index);
 		break;
 	case MFB_SET_CHROMA_KEY:
 		/* set panel winformation */
@@ -1090,7 +1048,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 			ad->ckmin_g = ck.green_min;
 			ad->ckmin_b = ck.blue_min;
 		}
-		pr_debug("set chroma key\n");
 		break;
 	case FBIOGET_GWINFO:
 		if (mfbi->type = MFB_TYPE_OFF)
@@ -1110,7 +1067,7 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		break;

 	default:
-		printk(KERN_ERR "Unknown ioctl command (0x%08X)\n", cmd);
+		dev_err(info->dev, "unknown ioctl command (0x%08X)\n", cmd);
 		return -ENOIOCTLCMD;
 	}

@@ -1131,7 +1088,6 @@ static int fsl_diu_open(struct fb_info *info, int user)
 	spin_lock(&diu_lock);
 	mfbi->count++;
 	if (mfbi->count = 1) {
-		pr_debug("open plane index %d\n", mfbi->index);
 		fsl_diu_check_var(&info->var, info);
 		res = fsl_diu_set_par(info);
 		if (res < 0)
@@ -1157,7 +1113,6 @@ static int fsl_diu_release(struct fb_info *info, int user)
 	spin_lock(&diu_lock);
 	mfbi->count--;
 	if (mfbi->count = 0) {
-		pr_debug("release plane index %d\n", mfbi->index);
 		res = fsl_diu_disable_panel(info);
 		if (res < 0)
 			mfbi->count++;
@@ -1222,26 +1177,9 @@ static int __devinit install_fb(struct fb_info *info)
 	} else {
 		aoi_mode = init_aoi_mode;
 	}
-	pr_debug("mode used = %s\n", aoi_mode);
 	rc = fb_find_mode(&info->var, info, aoi_mode, db, dbsize,
 			  &fsl_diu_default_mode, default_bpp);
-	switch (rc) {
-	case 1:
-		pr_debug("using mode specified in @mode\n");
-		break;
-	case 2:
-		pr_debug("using mode specified in @mode "
-			"with ignored refresh rate\n");
-		break;
-	case 3:
-		pr_debug("using mode default mode\n");
-		break;
-	case 4:
-		pr_debug("using mode from list\n");
-		break;
-	default:
-		pr_debug("rc = %d\n", rc);
-		pr_debug("failed to find mode\n");
+	if (!rc) {
 		/*
 		 * For plane 0 we continue and look into
 		 * driver's internal modedb.
@@ -1250,7 +1188,6 @@ static int __devinit install_fb(struct fb_info *info)
 			has_default_mode = 0;
 		else
 			return -EINVAL;
-		break;
 	}

 	if (!has_default_mode) {
@@ -1286,33 +1223,26 @@ static int __devinit install_fb(struct fb_info *info)
 		fb_videomode_to_var(&info->var, modedb);
 	}

-	pr_debug("xres_virtual %d\n", info->var.xres_virtual);
-	pr_debug("bits_per_pixel %d\n", info->var.bits_per_pixel);
-
-	pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
-	pr_debug("info->fix.line_length = %d\n", info->fix.line_length);
-
 	if (mfbi->type = MFB_TYPE_OFF)
 		mfbi->blank = FB_BLANK_NORMAL;
 	else
 		mfbi->blank = FB_BLANK_UNBLANK;

 	if (fsl_diu_check_var(&info->var, info)) {
-		printk(KERN_ERR "fb_check_var failed");
+		dev_err(info->dev, "fsl_diu_check_var failed\n");
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}

 	if (register_framebuffer(info) < 0) {
-		printk(KERN_ERR "register_framebuffer failed");
+		dev_err(info->dev, "register_framebuffer failed\n");
 		unmap_video_memory(info);
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}

 	mfbi->registered = 1;
-	printk(KERN_INFO "fb%d: %s fb device registered successfully.\n",
-		 info->node, info->fix.id);
+	dev_info(info->dev, "%s registered successfully\n", mfbi->id);

 	return 0;
 }
@@ -1344,7 +1274,6 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
 		/* This is the workaround for underrun */
 		if (status & INT_UNDRUN) {
 			out_be32(&hw->diu_mode, 0);
-			pr_debug("Err: DIU occurs underrun!\n");
 			udelay(1);
 			out_be32(&hw->diu_mode, 1);
 		}
@@ -1376,9 +1305,7 @@ static int request_irq_local(int irq)
 	status = in_be32(&hw->int_status);

 	ret = request_irq(irq, fsl_diu_isr, 0, "diu", NULL);
-	if (ret)
-		pr_info("Request diu IRQ failed.\n");
-	else {
+	if (!ret) {
 		ints = INT_PARERR | INT_LS_BF_VS;
 #if !defined(CONFIG_NOT_COHERENT_CACHE)
 		ints |=	INT_VSYNC;
@@ -1557,7 +1484,6 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 		dev_err(&ofdev->dev, "invalid DIU address\n");
 		goto error;
 	}
-	dev_dbg(&ofdev->dev, "%s, res.start: 0x%08x\n", __func__, res.start);

 	dr.diu_reg = ioremap(res.start, sizeof(struct diu));
 	if (!dr.diu_reg) {
@@ -1787,17 +1713,19 @@ static int __init fsl_diu_init(void)
 #else
 	monitor_port = fsl_diu_name_to_port(monitor_string);
 #endif
-	printk(KERN_INFO "Freescale DIU driver\n");
+	pr_info("Freescale Display Interface Unit (DIU) framebuffer driver\n");

 #ifdef CONFIG_NOT_COHERENT_CACHE
 	np = of_find_node_by_type(NULL, "cpu");
 	if (!np) {
-		printk(KERN_ERR "Err: can't find device node 'cpu'\n");
+		pr_err("fsl-diu-fb: can't find 'cpu' device node\n");
 		return -ENODEV;
 	}

 	prop = of_get_property(np, "d-cache-size", NULL);
 	if (prop = NULL) {
+		pr_err("fsl-diu-fb: missing 'd-cache-size' property' "
+		       "in 'cpu' node\n");
 		of_node_put(np);
 		return -ENODEV;
 	}
@@ -1811,6 +1739,8 @@ static int __init fsl_diu_init(void)

 	prop = of_get_property(np, "d-cache-line-size", NULL);
 	if (prop = NULL) {
+		pr_err("fsl-diu-fb: missing 'd-cache-line-size' property' "
+		       "in 'cpu' node\n");
 		of_node_put(np);
 		return -ENODEV;
 	}
@@ -1824,8 +1754,7 @@ static int __init fsl_diu_init(void)

 	ret = platform_driver_register(&fsl_diu_driver);
 	if (ret) {
-		printk(KERN_ERR
-			"fsl-diu: failed to register platform driver\n");
+		pr_err("fsl-diu-fb: failed to register platform driver\n");
 #if defined(CONFIG_NOT_COHERENT_CACHE)
 		vfree(coherence_data);
 #endif
@@ -1853,7 +1782,7 @@ module_param_named(mode, fb_mode, charp, 0);
 MODULE_PARM_DESC(mode,
 	"Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
 module_param_named(bpp, default_bpp, ulong, 0);
-MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified mode");
+MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified in 'mode'");
 module_param_named(monitor, monitor_string, charp, 0);
 MODULE_PARM_DESC(monitor, "Specify the monitor port "
 	"(\"dvi\", \"lvds\", or \"dlvds\") if supported by the platform");
--
1.7.3.4



^ permalink raw reply related

* [PATCH 01/13] drivers/video: fsl-diu-fb: clean up whitespace and formatting
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Fix various indentation and line length problems in the Freescale
DIU framebuffer driver.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |  104 +++++++++++++++++++++++---------------------
 1 files changed, 55 insertions(+), 49 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 0f1933b..dbb4cb7 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -223,53 +223,53 @@ struct mfb_info {
 
 static struct mfb_info mfb_template[] = {
 	{		/* AOI 0 for plane 0 */
-	.index = 0,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel0",
-	.registered = 0,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 0,
+		.index = 0,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel0",
+		.registered = 0,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 0 for plane 1 */
-	.index = 1,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel1 AOI0",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 0,
+		.index = 1,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel1 AOI0",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 1 for plane 1 */
-	.index = 2,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel1 AOI1",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 480,
+		.index = 2,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel1 AOI1",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 480,
 	},
 	{		/* AOI 0 for plane 2 */
-	.index = 3,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel2 AOI0",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 640,
-	.y_aoi_d = 0,
+		.index = 3,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel2 AOI0",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 640,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 1 for plane 2 */
-	.index = 4,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel2 AOI1",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 640,
-	.y_aoi_d = 480,
+		.index = 4,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel2 AOI1",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 640,
+		.y_aoi_d = 480,
 	},
 };
 
@@ -715,8 +715,8 @@ static void update_lcdc(struct fb_info *info)
 	/* Prep for DIU init  - gamma table, cursor table */
 
 	for (i = 0; i <= 2; i++)
-	   for (j = 0; j <= 255; j++)
-	      *gamma_table_base++ = j;
+		for (j = 0; j <= 255; j++)
+			*gamma_table_base++ = j;
 
 	diu_ops.set_gamma_table(machine_data->monitor_port, pool.gamma.vaddr);
 
@@ -887,7 +887,7 @@ static int fsl_diu_set_par(struct fb_info *info)
 
 static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
 {
-	return ((val<<width) + 0x7FFF - val)>>16;
+	return ((val << width) + 0x7FFF - val) >> 16;
 }
 
 /*
@@ -899,8 +899,9 @@ static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
  * pseudo_palette in struct fb_info. For pseudocolor mode we have a limited
  * color palette.
  */
-static int fsl_diu_setcolreg(unsigned regno, unsigned red, unsigned green,
-			   unsigned blue, unsigned transp, struct fb_info *info)
+static int fsl_diu_setcolreg(unsigned int regno, unsigned int red,
+			     unsigned int green, unsigned int blue,
+			     unsigned int transp, struct fb_info *info)
 {
 	int ret = 1;
 
@@ -1350,6 +1351,7 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
 #if defined(CONFIG_NOT_COHERENT_CACHE)
 		else if (status & INT_VSYNC) {
 			unsigned int i;
+
 			for (i = 0; i < coherence_data_size;
 				i += d_cache_line_size)
 				__asm__ __volatile__ (
@@ -1381,6 +1383,7 @@ static int request_irq_local(int irq)
 #if !defined(CONFIG_NOT_COHERENT_CACHE)
 		ints |=	INT_VSYNC;
 #endif
+
 		if (dr.mode = MFB_MODE2 || dr.mode = MFB_MODE3)
 			ints |= INT_VSYNC_WB;
 
@@ -1388,6 +1391,7 @@ static int request_irq_local(int irq)
 		status = in_be32(&hw->int_status);
 		out_be32(&hw->int_mask, ints);
 	}
+
 	return ret;
 }
 
@@ -1454,15 +1458,15 @@ static int allocate_buf(struct device *dev, struct diu_addr *buf, u32 size,
 		buf->paddr = (u32)buf->paddr + offset;
 	} else
 		buf->offset = 0;
+
 	return 0;
 }
 
 static void free_buf(struct device *dev, struct diu_addr *buf, u32 size,
 		     u32 bytes_align)
 {
-	dma_free_coherent(dev, size + bytes_align,
-				buf->vaddr, (buf->paddr - buf->offset));
-	return;
+	dma_free_coherent(dev, size + bytes_align, buf->vaddr,
+			  buf->paddr - buf->offset);
 }
 
 static ssize_t store_monitor(struct device *device,
@@ -1798,8 +1802,9 @@ static int __init fsl_diu_init(void)
 		return -ENODEV;
 	}
 
-	/* Freescale PLRU requires 13/8 times the cache size to do a proper
-	   displacement flush
+	/*
+	 * Freescale PLRU requires 13/8 times the cache size to do a proper
+	 * displacement flush
 	 */
 	coherence_data_size = *prop * 13;
 	coherence_data_size /= 8;
@@ -1816,6 +1821,7 @@ static int __init fsl_diu_init(void)
 	if (!coherence_data)
 		return -ENOMEM;
 #endif
+
 	ret = platform_driver_register(&fsl_diu_driver);
 	if (ret) {
 		printk(KERN_ERR
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 0/13] drivers/video: fsl-diu-fb: several minor improvements
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

This patchset includes several minor improvements to the Freescale DIU
framebuffer driver.


^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 21:31 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja,
	Clark, Rob
In-Reply-To: <4E72561C.7060603@gmx.de>

> Okay, I see your problem. It's a bit strange you don't have acceleration. I

The hardware has 3D acceleration but not open so we can't support it.
There is no 2D acceleration - which seems to be increasingly common.

At some point I'll add hardware scrolling however by using the GTT to
implemnent scroll wrapping.

> sound like a big problem to me, but pretty inefficient, so probably copying the
> existing ones and adjusting it to your needs would be preferred (just like the
> sys* implementations exist).

I did have a look at the current ones but fixing them up given scan lines
can span page boundaries looked pretty horrible so I deferred it until I
feel inspired.

Alan

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Florian Tobias Schandinat @ 2011-09-15 19:46 UTC (permalink / raw)
  To: Alan Cox
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja,
	Clark, Rob
In-Reply-To: <20110915200514.74bdcd90@lxorguk.ukuu.org.uk>

On 09/15/2011 07:05 PM, Alan Cox wrote:
>> What is your problem with discontigous framebuffers? (I assume discontigous
>> refers to the pages the framebuffer is composed of)
>> Sounds to me like you should implement your own fb_mmap and either map it
>> contigous to screen_base or implement your own fb_read/write.
>> In theory you could even have each pixel at a completely different memory
>> location although some userspace wouldn't be happy when it could no longer mmap
>> the framebuffer.
> 
> The mmap side is trivial, the problem is that the fb layer default
> implementations of blits, fills etc only work on a kernel linear frame
> buffer. And (for example) there is not enough linear stolen memory on
> some Intel video for a 1080p console on HDMI even though the hardware is
> perfectly capable of using an HDTV as its monitor. Nor - on a 32bit box-
> is there enough space to vremap it.

Okay, I see your problem. It's a bit strange you don't have acceleration. I
guess you need either your own implementation of those or adding function
pointers like fb_read/write just without the __user and use those instead of
direct memory access in the cfb* implementation if screen_base is NULL. Does not
sound like a big problem to me, but pretty inefficient, so probably copying the
existing ones and adjusting it to your needs would be preferred (just like the
sys* implementations exist).


Best regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alex Deucher @ 2011-09-15 19:45 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Alan Cox, Keith Packard, linux-fbdev, linaro-dev, linux-kernel,
	dri-devel, Archit Taneja, Clark, Rob
In-Reply-To: <4E724F93.1050203@gmx.de>

On Thu, Sep 15, 2011 at 3:18 PM, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 09/15/2011 06:58 PM, Alan Cox wrote:
>>> Well, I rather think that the fb API is more user centric to allow every program
>>> to use it directly in contrast to the KMS/DRM API which aims to support every
>>> feature the hardware has. For this the fb API should not change much, but I
>>> understand some additions were needed for some special users, probably limited
>>> to X and wayland.
>>
>> Wayland needs vblank frame buffer switching and the like. Likewise given
>> you want to composite buffers really any serious accelerated device ends
>> up needing a full memory manager and that ends up needing a buffer
>> manager. Wayland needs clients to be doing their own rendering into
>> objects which means authorisation and management of the render engine
>> which ends up looking much like DRM.
>
> As you have DRM now and as I'm not interested in wayland I won't discuss this,
> but I guess it might be a good start for Geert's question what would be needed
> to use it on dumb framebuffers.
>
>>> One of my biggest problems with KMS is that it has (naturally) a lot more
>>> complexity than the fb API which leads to instability. Basically it's very
>>
>> It shouldn't do - and a sample of one (your machine) is not a
>> statistically valid set. Fb is pretty much ununsable in contrast on my
>> main box, but that's not a statistically valid sample either.
>>
>> I'm not that convinced by the complexity either. For a simple video card
>> setup such as those that the fb layer can kind of cope with (ie linear
>> buffer, simple mode changes, no client rendering, no vblank flipping,
>> limited mode management, no serious multi-head) a DRM driver is also
>> pretty tiny and simple.
>
> Yes, if you limit DRM to the functionality of the fb API I guess you could reach
> the same stability level. But where can I do this? Where is a option to forbid
> all acceleration or at least limit to the acceleration that can be done without
> any risk?

Right now most of the KMS DRM drivers do not support accelerated fb,
so as long as you don't run accelerated X or a 3D app, it should be
just as stable as an fb driver.  You may run into modesetting fail in
certain cases due to wonky hardware or driver bugs, but you will hit
that with an fb driver as well.

Alex

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 19:28 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Keith Packard, linaro-dev-cunTk1MwBs8s++Sfvej+rw,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Archit Taneja, Alex Deucher
In-Reply-To: <4E724F93.1050203-Mmb7MZpHnFY@public.gmane.org>

> As you have DRM now and as I'm not interested in wayland I won't discuss this,
> but I guess it might be a good start for Geert's question what would be needed
> to use it on dumb framebuffers.

GMA500 is basically a 2D or dumb frame buffer setup but with a lot of
rather complicated output and memory management required due to the
hardware. With the latest changes to GEM (private objects) it's basically
trivial to use the frame buffer management interfaces.

> Yes, if you limit DRM to the functionality of the fb API I guess you could reach
> the same stability level. But where can I do this? Where is a option to forbid
> all acceleration or at least limit to the acceleration that can be done without
> any risk?

A driver can provide such module options as it wants.

> That's a really difficult question. Determining the users is difficult and there
> are people that use their hardware very long, for example we are about to get a
> new driver for i740. For the framebuffer infrastructure I guess you have to at
> least wait for my death.

I doubt it'll be that long - but you are right it will take time and
there isn't really any need to push or force it. These things take care
of themselves and in time nobody will care about the old fb stuff, either
because DRM covers it all or equally possibly because it doesn't support
3D holographic projection 8)

Alan

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Florian Tobias Schandinat @ 2011-09-15 19:18 UTC (permalink / raw)
  To: Alan Cox
  Cc: Alex Deucher, Keith Packard, linux-fbdev, linaro-dev,
	linux-kernel, dri-devel, Archit Taneja, Clark, Rob
In-Reply-To: <20110915195804.4e6d965b@lxorguk.ukuu.org.uk>

On 09/15/2011 06:58 PM, Alan Cox wrote:
>> Well, I rather think that the fb API is more user centric to allow every program
>> to use it directly in contrast to the KMS/DRM API which aims to support every
>> feature the hardware has. For this the fb API should not change much, but I
>> understand some additions were needed for some special users, probably limited
>> to X and wayland.
> 
> Wayland needs vblank frame buffer switching and the like. Likewise given
> you want to composite buffers really any serious accelerated device ends
> up needing a full memory manager and that ends up needing a buffer
> manager. Wayland needs clients to be doing their own rendering into
> objects which means authorisation and management of the render engine
> which ends up looking much like DRM.

As you have DRM now and as I'm not interested in wayland I won't discuss this,
but I guess it might be a good start for Geert's question what would be needed
to use it on dumb framebuffers.

>> One of my biggest problems with KMS is that it has (naturally) a lot more
>> complexity than the fb API which leads to instability. Basically it's very
> 
> It shouldn't do - and a sample of one (your machine) is not a
> statistically valid set. Fb is pretty much ununsable in contrast on my
> main box, but that's not a statistically valid sample either.
> 
> I'm not that convinced by the complexity either. For a simple video card
> setup such as those that the fb layer can kind of cope with (ie linear
> buffer, simple mode changes, no client rendering, no vblank flipping,
> limited mode management, no serious multi-head) a DRM driver is also
> pretty tiny and simple.

Yes, if you limit DRM to the functionality of the fb API I guess you could reach
the same stability level. But where can I do this? Where is a option to forbid
all acceleration or at least limit to the acceleration that can be done without
any risk?

>> Well, I think it's too late to really fix this thing. We now have 3 APIs in the
>> kernel that have to be kept. Probably the best we can do now is figure out how
>> we can reduce code duplication and do extensions to those APIs in a way that
>> they are compatible with each other or completely independent and can be used
>> across the APIs.
> 
> I think it comes down to 'when nobody is using the old fb drivers they can
> drop into staging and oblivion'. Right now the fb layer is essentially
> compatibility glue on most modern x86 platforms.

That's a really difficult question. Determining the users is difficult and there
are people that use their hardware very long, for example we are about to get a
new driver for i740. For the framebuffer infrastructure I guess you have to at
least wait for my death.


Regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 19:05 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja,
	Clark, Rob
In-Reply-To: <4E723A20.2040002@gmx.de>

> What is your problem with discontigous framebuffers? (I assume discontigous
> refers to the pages the framebuffer is composed of)
> Sounds to me like you should implement your own fb_mmap and either map it
> contigous to screen_base or implement your own fb_read/write.
> In theory you could even have each pixel at a completely different memory
> location although some userspace wouldn't be happy when it could no longer mmap
> the framebuffer.

The mmap side is trivial, the problem is that the fb layer default
implementations of blits, fills etc only work on a kernel linear frame
buffer. And (for example) there is not enough linear stolen memory on
some Intel video for a 1080p console on HDMI even though the hardware is
perfectly capable of using an HDTV as its monitor. Nor - on a 32bit box-
is there enough space to vremap it.

Alan

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 18:58 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit, Taneja,
	Clark, Rob
In-Reply-To: <4E724659.5080709@gmx.de>

> Well, I rather think that the fb API is more user centric to allow every program
> to use it directly in contrast to the KMS/DRM API which aims to support every
> feature the hardware has. For this the fb API should not change much, but I
> understand some additions were needed for some special users, probably limited
> to X and wayland.

Wayland needs vblank frame buffer switching and the like. Likewise given
you want to composite buffers really any serious accelerated device ends
up needing a full memory manager and that ends up needing a buffer
manager. Wayland needs clients to be doing their own rendering into
objects which means authorisation and management of the render engine
which ends up looking much like DRM.

> One of my biggest problems with KMS is that it has (naturally) a lot more
> complexity than the fb API which leads to instability. Basically it's very

It shouldn't do - and a sample of one (your machine) is not a
statistically valid set. Fb is pretty much ununsable in contrast on my
main box, but that's not a statistically valid sample either.

I'm not that convinced by the complexity either. For a simple video card
setup such as those that the fb layer can kind of cope with (ie linear
buffer, simple mode changes, no client rendering, no vblank flipping,
limited mode management, no serious multi-head) a DRM driver is also
pretty tiny and simple.

> Well, I think it's too late to really fix this thing. We now have 3 APIs in the
> kernel that have to be kept. Probably the best we can do now is figure out how
> we can reduce code duplication and do extensions to those APIs in a way that
> they are compatible with each other or completely independent and can be used
> across the APIs.

I think it comes down to 'when nobody is using the old fb drivers they can
drop into staging and oblivion'. Right now the fb layer is essentially
compatibility glue on most modern x86 platforms.

Alan

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Florian Tobias Schandinat @ 2011-09-15 18:39 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Keith Packard, linux-fbdev, linaro-dev, linux-kernel, dri-devel,
	Archit Taneja, Clark, Rob
In-Reply-To: <CADnq5_NnzLtJmSzOgyppnBtv96DRTJ71u6ZSrs56KJ-F5CC-mw@mail.gmail.com>

On 09/15/2011 05:52 PM, Alex Deucher wrote:
> On Thu, Sep 15, 2011 at 1:12 PM, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
>> On 09/15/2011 03:50 PM, Keith Packard wrote:
>>> On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>
>>>> 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
>>>> the plan is to make DRM the core Linux display framework, upon which
>>>> everything else is built, and fb and v4l2 are changed to use DRM.
>>>
>>> I'd like to think we could make DRM the underlying display framework;
>>> it already exposes an fb interface, and with overlays, a bit more of the
>>> v4l2 stuff is done as well. Certainly eliminating three copies of mode
>>> setting infrastructure would be nice...
>>
>> Interesting that this comes from the people that pushed the latest mode setting
>> code into the kernel. But I don't think that this will happen, the exposed user
>> interfaces will be around for decades and the infrastructure code could be
>> shared, in theory.
>> For fb and V4L2 I think we'll develop some level of interoperability, share
>> concepts and maybe even some code. The FOURCC pixel formats and overlays are
>> such examples. As Laurent is really interested in it I think we can get some
>> nice progress here.
>> For fb and DRM the situation is entirely different. The last proposal I remember
>> ended in the DRM people stating that only their implementation is acceptable as
>> is and we could use it. Such attitude is not helpful and as I don't see any
>> serious intention of the DRM guys to cooperate I think those subsystems are more
>> likely to diverge. At least I'll never accept any change to the fb
>> infrastructure that requires DRM.
> 
> Not exactly.  This point was that the drm modesetting and EDID
> handling was derived from X which has had 20+ years of of quirks and
> things added to it to deal with tons of wonky monitors and such.  That
> information should be preserved.  As mode structs and EDID handling
> are pretty self contained, why not use the DRM variants of that code
> rather than writing a new version?

Well, I'm not against sharing the code and not against taking DRM's current
implementation as a base but the steps required to make it generally acceptable
would be to split it of, probably as a standalone module and strip all DRM
specific things off. Than all things that require EDID can use it, DRM can add
DRM-specific things on top and fb can add fb-specific things.

> While the DRM has historically targeted 3D acceleration, that is not a
> requirement to use the DRM KMS modesetting API.  The current fb API
> has no concept of display controllers or connectors or overlays, etc.
> To match it to modern hardware, it needs a major overhaul.  Why create
> a new modern fb interface that's largely the same as DRM KMS?  What if
> we just consider the KMS API as the new fb API?  If there are any
> inadequacies in the DRM KMS API we would be happy to work out any
> changes.

Well, I rather think that the fb API is more user centric to allow every program
to use it directly in contrast to the KMS/DRM API which aims to support every
feature the hardware has. For this the fb API should not change much, but I
understand some additions were needed for some special users, probably limited
to X and wayland.
One of my biggest problems with KMS is that it has (naturally) a lot more
complexity than the fb API which leads to instability. Basically it's very
difficult to implement a framebuffer in a way that it crashes your machine
during operation which is quite a contrast to my KMS/DRM experience on my toy
(on my work machines I use framebuffer only). And I really hate it when I have
to type my passwords again just because the KMS/DRM thing allowed a program to
crash my machine. Yes, those are driver bugs but the API encourages them and I
did not yet find the feature/config option DOES_NOT_CRASH or SLOW_BUT_STABLE.
And as I said already, I think the fb API is a lot better for direct interaction
with userspace programs and certainly has more direct users at the moment.

> Please don't claim that the DRM developers do not want to cooperate.
> I realize that people have strong opinions about existing APIs, put
> there has been just as much, if not more obstinacy from the v4l and fb
> people.

Well, I think it's too late to really fix this thing. We now have 3 APIs in the
kernel that have to be kept. Probably the best we can do now is figure out how
we can reduce code duplication and do extensions to those APIs in a way that
they are compatible with each other or completely independent and can be used
across the APIs.


Best regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Rob Clark @ 2011-09-15 18:32 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja
In-Reply-To: <1316107275.23214.99.camel@deskari>

On Thu, Sep 15, 2011 at 12:21 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2011-09-15 at 10:50 -0500, Keith Packard wrote:
>> On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>> > 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
>> > the plan is to make DRM the core Linux display framework, upon which
>> > everything else is built, and fb and v4l2 are changed to use DRM.
>>
>> I'd like to think we could make DRM the underlying display framework;
>> it already exposes an fb interface, and with overlays, a bit more of the
>> v4l2 stuff is done as well. Certainly eliminating three copies of mode
>> setting infrastructure would be nice...
>
> Ok, sounds good to me. We (as in OMAP display people) are already
> planning to take DRM into use, so no problem there.
>
>> > But even if it was done like that, I see that it's combining two
>> > separate things: 1) the lower level HW control, and 2) the upper level
>> > buffer management, policies and userspace interfaces.
>>
>> Those are split between the DRM layer and the underlying device driver,
>> which provides both kernel (via fb) and user space interfaces.
>
> I'm not so familiar with DRM, but with device driver you mean a driver
> for the the hardware which handles display output (gfx cards or whatever
> it is on that platform)?

I think he is more referring to the DRM core and the individual device drivers..

We are (AFAIK) unique in having a two layer driver, where the DRM part
is more of a wrapper (for the KMS parts)... but I see that as more of
a transition thing.. eventually we should be able to merge it all into
the DRM layer.

> If so, it sounds good. That quite well matches what omapdss driver does
> currently for us. But we still have semi-complex omapdrm between omapdss
> and the standard drm layer.
>
> Rob, would you say omapdrm is more of a DRM wrapper for omapdss than a
> real separate entity? If so, then we could possibly in the future (when
> nobody else uses omapdss) change omapdss to support DRM natively. (or
> make omapdrm support omap HW natively, which ever way =).

Yeah, I think eventually it would make sense to merge all into one.
Although I'm not sure about how best to handle various different
custom DSI panels..

BR,
-R


>> > 2) It's missing the panel driver part. This is rather important on
>> > embedded systems, as the panels often are not "dummy" panels, but they
>> > need things like custom initialization, sending commands to adjust
>> > backlight, etc.
>>
>> We integrate the panel (and other video output) drivers into the device
>> drivers. With desktop chips, they're not easily separable. None of the
>> desktop output drivers are simple; things like DisplayPort require link
>> training, and everyone needs EDID. We share some of that code in the DRM
>> layer today, and it would be nice to share even more.
>
> I don't think we speak of similar panel drivers. I think there are two
> different drivers here:
>
> 1) output drivers, handles the output from the SoC / gfx card. For
> example DVI, DisplayPort, MIPI DPI/DBI/DSI.
>
> 2) panel drivers, handles panel specific things. Each panel may support
> custom commands and features, for which we need a dedicated driver. And
> this driver is not platform specific, but should work with any platform
> which has the output used with the panel.
>
> As an example, DSI command mode displays can be quite complex:
>
> DSI bus is a half-duplex serial bus, and while it's designed for
> displays you could use it easily for any communication between the SoC
> and the peripheral.
>
> The panel could have a feature like content adaptive backlight control,
> and this would be configured via the DSI bus, sending a particular
> command to the panel (possibly by first reading something from the
> panel). The panel driver would accomplish this more or less the same way
> one uses, say, i2c, so it would use the platform's DSI support to send
> and receive packets.
>
> Or a more complex scenario (but still a realistic scenario, been there,
> done that) is sending the image to the panel in multiple parts, and
> between each part sending configuration commands to the panel. (and
> still getting it done in time so we avoid tearing).
>
> And to complicate things more, there are DSI bus features like LP mode
> (low power, basically low speed mode) and HS mode (high speed), virtual
> channel IDs, and whatnot, which each panel may need to be used in
> particular manner. Some panels may require initial configuration done in
> LP, or configuration commands sent to a certain virtual channel ID.
>
> The point is that we cannot have standard "MIPI DSI command mode panel
> driver" which would work for all DSI cmd mode panels, but we need (in
> the worst case) separate driver for each panel.
>
> The same goes to lesser extent for other panels also. Some are
> configured via i2c or spi.
>
>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox