Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH] da8xx-fb: Rounding FB size to satisfy SGX buffer requirements
From: Aditya Nellutla @ 2012-05-23  6:18 UTC (permalink / raw)
  To: linux-fbdev

In the real time use-case when SGX is used for rendering to FB buffers it has been
observed that, the available memory from framebuffer driver is not sufficient for
SGX under certain cases (like 16-bit WVGA resolution). SGX requires 2 swap buffers
with each of the buffers aligned to lcm(line_length, PAGE_SIZE).

Inorder to satisfy this requirement, we have two options,

	- Increase number of FB buffers (LCD_NUM_BUFFERS) to 3. This is not
	  recommended as we end up wasting huge memory in most of the cases.

	- Align FB buffers to lcm(line_length, PAGE_SIZE).This ensures framebuffer
	  size is increased to satisfy SGX requirements keeping alignment intact.

This patch makes sure that FB allocates buffers aligned to above formula.

Signed-off-by: Aditya Nellutla <aditya.n@ti.com>
---
 drivers/video/da8xx-fb.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 47118c7..2f24c19 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -31,6 +31,7 @@
 #include <linux/cpufreq.h>
 #include <linux/console.h>
 #include <linux/slab.h>
+#include <linux/lcm.h>
 #include <video/da8xx-fb.h>
 #include <asm/div64.h>
 
@@ -1114,6 +1115,7 @@ static int __devinit fb_probe(struct platform_device *device)
 	struct da8xx_fb_par *par;
 	resource_size_t len;
 	int ret, i;
+	unsigned long ulcm;
 
 	if (fb_pdata = NULL) {
 		dev_err(&device->dev, "Can not get platform data\n");
@@ -1209,7 +1211,8 @@ static int __devinit fb_probe(struct platform_device *device)
 
 	/* allocate frame buffer */
 	par->vram_size = lcdc_info->width * lcdc_info->height * lcd_cfg->bpp;
-	par->vram_size = PAGE_ALIGN(par->vram_size/8);
+	ulcm = lcm((lcdc_info->width * lcd_cfg->bpp)/8, PAGE_SIZE);
+	par->vram_size = roundup(par->vram_size/8, ulcm);
 	par->vram_size = par->vram_size * LCD_NUM_BUFFERS;
 
 	par->vram_virt = dma_alloc_coherent(NULL,
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 0/2] OMAPDSS: write-through caching support for omapfb
From: Tomi Valkeinen @ 2012-05-24  7:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1337716444-30139-1-git-send-email-siarhei.siamashka@gmail.com>

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

On Tue, 2012-05-22 at 22:54 +0300, Siarhei Siamashka wrote:
> This is a very simple few-liner patchset, which allows to optionally
> enable write-through caching for OMAP DSS framebuffer. The problem with
> the current writecombine cacheability attribute is that it only speeds
> up writes. Uncached reads are slow, even though the use of NEON mitigates
> this problem a bit.
> 
> Traditionally, xf86-video-fbdev DDX is using shadow framebuffer in the
> system memory. Which contains a copy of the framebuffer data for the
> purpose of providing fast read access to it when needed. Framebuffer
> read access is required not so often, but it still gets used for
> scrolling and moving windows around in Xorg server. And the users
> perceive their linux desktop as rather sluggish when these operations
> are not fast enough.
> 
> In the case of ARM hardware, framebuffer is typically physically
> located in the main memory. And the processors still support
> write-through cacheability attribute. According to ARM ARM, the writes
> done to write-through cached memory inside the level of cache are
> visible to all observers outside the level of cache without the need
> of explicit cache maintenance (same rule as for non-cached memory).
> So write-through cache is a perfect choice when only CPU is allowed
> to modify the data in the framebuffer and everyone else (screen
> refresh DMA) is only reading it. That is, assuming that write-through
> cached memory provides good performance and there are no quirks.
> As the framebuffer reads become fast, the need for shadow framebuffer
> disappears.

I ran my own fb perf test on omap3 overo board ("perf" test in
https://gitorious.org/linux-omap-dss2/omapfb-tests) :

vram_cache=n:

sequential_horiz_singlepixel_read: 25198080 pix, 4955475 us, 5084897 pix/s
sequential_horiz_singlepixel_write: 434634240 pix, 4081146 us, 106498086 pix/s
sequential_vert_singlepixel_read: 20106240 pix, 4970611 us, 4045023 pix/s
sequential_vert_singlepixel_write: 98572800 pix, 4985748 us, 19770915 pix/s
sequential_line_read: 40734720 pix, 4977906 us, 8183103 pix/s
sequential_line_write: 1058580480 pix, 5024628 us, 210678378 pix/s
nonsequential_singlepixel_write: 17625600 pix, 4992828 us, 3530183 pix/s
nonsequential_singlepixel_read: 9661440 pix, 4952973 us, 1950634 pix/s

vram_cache=y:

sequential_horiz_singlepixel_read: 270389760 pix, 4994154 us, 54141253 pix/s
sequential_horiz_singlepixel_write: 473149440 pix, 3932801 us, 120308512 pix/s
sequential_vert_singlepixel_read: 18147840 pix, 4976226 us, 3646908 pix/s
sequential_vert_singlepixel_write: 100661760 pix, 4993164 us, 20159914 pix/s
sequential_line_read: 285143040 pix, 4917267 us, 57988114 pix/s
sequential_line_write: 876710400 pix, 5012146 us, 174917171 pix/s
nonsequential_singlepixel_write: 17625600 pix, 4977967 us, 3540722 pix/s
nonsequential_singlepixel_read: 9661440 pix, 4944885 us, 1953825 pix/s

These also show quite a bit of improvement in some read cases.
Interestingly some of the write cases are also faster.

Reading pixels vertically is slower with vram_cache. I guess this is
because the cache causes some overhead, and we always miss the cache so
the caching is just wasted time.

I would've also presumed the difference in sequential_line_write would
be bigger. write-through is effectively no-cache for writes, right?

If the user of the fb just writes to the fb and vram_cache=y, it means
that the cache is filled with pixel data that is never used, thus
lowering the performance of all other programs?

I have to say I don't know much of the cpu caches, but the read speed
improvements are very big, so I think this is definitely interesting
patch. So if you get the first patch accepted I see no problem with
adding this to omapfb as an optional feature.

However, "vram_cache" is not a very good name for the option.
"vram_writethrough", or something?

Did you test this with VRFB (omap3) or TILER (omap4)? I wonder how those
are affected.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH 1/3] video: exynos_dp: use devm_ functions
From: Jingoo Han @ 2012-05-25  7:20 UTC (permalink / raw)
  To: linux-fbdev

The devm_ functions allocate memory that is released when a driver
detaches. This makes the code smaller and a bit simpler.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |   49 ++++++++-------------------------
 drivers/video/exynos/exynos_dp_core.h |    1 -
 2 files changed, 12 insertions(+), 38 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index ef4d1ab..aabd214 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -858,7 +858,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	dp = kzalloc(sizeof(struct exynos_dp_device), GFP_KERNEL);
+	dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device),
+				GFP_KERNEL);
 	if (!dp) {
 		dev_err(&pdev->dev, "no memory for device data\n");
 		return -ENOMEM;
@@ -869,8 +870,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 	dp->clock = clk_get(&pdev->dev, "dp");
 	if (IS_ERR(dp->clock)) {
 		dev_err(&pdev->dev, "failed to get clock\n");
-		ret = PTR_ERR(dp->clock);
-		goto err_dp;
+		return PTR_ERR(dp->clock);
 	}
 
 	clk_enable(dp->clock);
@@ -882,35 +882,25 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 		goto err_clock;
 	}
 
-	res = request_mem_region(res->start, resource_size(res),
-				dev_name(&pdev->dev));
-	if (!res) {
-		dev_err(&pdev->dev, "failed to request registers region\n");
-		ret = -EINVAL;
-		goto err_clock;
-	}
-
-	dp->res = res;
-
-	dp->reg_base = ioremap(res->start, resource_size(res));
+	dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
 	if (!dp->reg_base) {
 		dev_err(&pdev->dev, "failed to ioremap\n");
 		ret = -ENOMEM;
-		goto err_req_region;
+		goto err_clock;
 	}
 
 	dp->irq = platform_get_irq(pdev, 0);
 	if (!dp->irq) {
 		dev_err(&pdev->dev, "failed to get irq\n");
 		ret = -ENODEV;
-		goto err_ioremap;
+		goto err_clock;
 	}
 
-	ret = request_irq(dp->irq, exynos_dp_irq_handler, 0,
-			"exynos-dp", dp);
+	ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
+				"exynos-dp", dp);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request irq\n");
-		goto err_ioremap;
+		goto err_clock;
 	}
 
 	dp->video_info = pdata->video_info;
@@ -922,7 +912,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 	ret = exynos_dp_detect_hpd(dp);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to detect hpd\n");
-		goto err_irq;
+		goto err_clock;
 	}
 
 	exynos_dp_handle_edid(dp);
@@ -931,7 +921,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 				dp->video_info->link_rate);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to do link train\n");
-		goto err_irq;
+		goto err_clock;
 	}
 
 	exynos_dp_enable_scramble(dp, 1);
@@ -945,23 +935,15 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
 	ret = exynos_dp_config_video(dp, dp->video_info);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to config video\n");
-		goto err_irq;
+		goto err_clock;
 	}
 
 	platform_set_drvdata(pdev, dp);
 
 	return 0;
 
-err_irq:
-	free_irq(dp->irq, dp);
-err_ioremap:
-	iounmap(dp->reg_base);
-err_req_region:
-	release_mem_region(res->start, resource_size(res));
 err_clock:
 	clk_put(dp->clock);
-err_dp:
-	kfree(dp);
 
 	return ret;
 }
@@ -974,16 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
 	if (pdata && pdata->phy_exit)
 		pdata->phy_exit();
 
-	free_irq(dp->irq, dp);
-	iounmap(dp->reg_base);
-
 	clk_disable(dp->clock);
 	clk_put(dp->clock);
 
-	release_mem_region(dp->res->start, resource_size(dp->res));
-
-	kfree(dp);
-
 	return 0;
 }
 
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 519c3a6..8f596b9 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -26,7 +26,6 @@ struct link_train {
 
 struct exynos_dp_device {
 	struct device		*dev;
-	struct resource		*res;
 	struct clk		*clock;
 	unsigned int		irq;
 	void __iomem		*reg_base;
-- 
1.7.1



^ permalink raw reply related

* [PATCH 2/3] video: exynos_dp: move sw reset prioir to enabling sw defined function
From: Jingoo Han @ 2012-05-25  7:21 UTC (permalink / raw)
  To: linux-fbdev

The sw reset should be called prioir to enabling sw defined function,
according to datasheet.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |    2 ++
 drivers/video/exynos/exynos_dp_core.h |    1 +
 drivers/video/exynos/exynos_dp_reg.c  |    7 +++++--
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index aabd214..b446f7e 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -27,6 +27,8 @@ static int exynos_dp_init_dp(struct exynos_dp_device *dp)
 {
 	exynos_dp_reset(dp);
 
+	exynos_dp_swreset(dp);
+
 	/* SW defined function Normal operation */
 	exynos_dp_enable_sw_function(dp);
 
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 8f596b9..1e0f998 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -41,6 +41,7 @@ void exynos_dp_lane_swap(struct exynos_dp_device *dp, bool enable);
 void exynos_dp_init_analog_param(struct exynos_dp_device *dp);
 void exynos_dp_init_interrupt(struct exynos_dp_device *dp);
 void exynos_dp_reset(struct exynos_dp_device *dp);
+void exynos_dp_swreset(struct exynos_dp_device *dp);
 void exynos_dp_config_interrupt(struct exynos_dp_device *dp);
 u32 exynos_dp_get_pll_lock_status(struct exynos_dp_device *dp);
 void exynos_dp_set_pll_power_down(struct exynos_dp_device *dp, bool enable);
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 99bafb8..6ce76d5 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -109,8 +109,6 @@ void exynos_dp_reset(struct exynos_dp_device *dp)
 {
 	u32 reg;
 
-	writel(RESET_DP_TX, dp->reg_base + EXYNOS_DP_TX_SW_RESET);
-
 	exynos_dp_stop_video(dp);
 	exynos_dp_enable_video_mute(dp, 0);
 
@@ -155,6 +153,11 @@ void exynos_dp_reset(struct exynos_dp_device *dp)
 	exynos_dp_init_interrupt(dp);
 }
 
+void exynos_dp_swreset(struct exynos_dp_device *dp)
+{
+	writel(RESET_DP_TX, dp->reg_base + EXYNOS_DP_TX_SW_RESET);
+}
+
 void exynos_dp_config_interrupt(struct exynos_dp_device *dp)
 {
 	u32 reg;
-- 
1.7.1



^ permalink raw reply related

* [PATCH 3/3] video: exynos_dp: reduce delay time when configuring video setting
From: Jingoo Han @ 2012-05-25  7:21 UTC (permalink / raw)
  To: linux-fbdev

This patch reduces delay time when configuring video setting,
which is helpful to reduce wakeup time during resume.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/exynos/exynos_dp_core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index b446f7e..a36b2d2 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -770,7 +770,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp,
 			return -ETIMEDOUT;
 		}
 
-		mdelay(100);
+		udelay(1);
 	}
 
 	/* Set to use the register calculated M/N video */
@@ -804,7 +804,7 @@ static int exynos_dp_config_video(struct exynos_dp_device *dp,
 			return -ETIMEDOUT;
 		}
 
-		mdelay(100);
+		mdelay(1);
 	}
 
 	if (retval != 0)
-- 
1.7.1



^ permalink raw reply related

* Re: [PATCH 0/2] OMAPDSS: write-through caching support for omapfb
From: Siarhei Siamashka @ 2012-05-25  9:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1337845417.2909.31.camel@deskari>

On Thu, May 24, 2012 at 10:43 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Tue, 2012-05-22 at 22:54 +0300, Siarhei Siamashka wrote:
> I ran my own fb perf test on omap3 overo board ("perf" test in
> https://gitorious.org/linux-omap-dss2/omapfb-tests) :
>
> vram_cache=n:
>
> sequential_horiz_singlepixel_read: 25198080 pix, 4955475 us, 5084897 pix/s
> sequential_horiz_singlepixel_write: 434634240 pix, 4081146 us, 106498086 pix/s
> sequential_vert_singlepixel_read: 20106240 pix, 4970611 us, 4045023 pix/s
> sequential_vert_singlepixel_write: 98572800 pix, 4985748 us, 19770915 pix/s
> sequential_line_read: 40734720 pix, 4977906 us, 8183103 pix/s
> sequential_line_write: 1058580480 pix, 5024628 us, 210678378 pix/s
> nonsequential_singlepixel_write: 17625600 pix, 4992828 us, 3530183 pix/s
> nonsequential_singlepixel_read: 9661440 pix, 4952973 us, 1950634 pix/s
>
> vram_cache=y:
>
> sequential_horiz_singlepixel_read: 270389760 pix, 4994154 us, 54141253 pix/s
> sequential_horiz_singlepixel_write: 473149440 pix, 3932801 us, 120308512 pix/s
> sequential_vert_singlepixel_read: 18147840 pix, 4976226 us, 3646908 pix/s
> sequential_vert_singlepixel_write: 100661760 pix, 4993164 us, 20159914 pix/s
> sequential_line_read: 285143040 pix, 4917267 us, 57988114 pix/s
> sequential_line_write: 876710400 pix, 5012146 us, 174917171 pix/s
> nonsequential_singlepixel_write: 17625600 pix, 4977967 us, 3540722 pix/s
> nonsequential_singlepixel_read: 9661440 pix, 4944885 us, 1953825 pix/s
>
> These also show quite a bit of improvement in some read cases.
> Interestingly some of the write cases are also faster.
>
> Reading pixels vertically is slower with vram_cache. I guess this is
> because the cache causes some overhead, and we always miss the cache so
> the caching is just wasted time.

On a positive side, nobody is normally accessing memory in this way.
It is a well known performance anti-pattern.

> I would've also presumed the difference in sequential_line_write would
> be bigger. write-through is effectively no-cache for writes, right?

Write-through cache is still using write combining buffer for memory
writes. So I would actually expect the performance to be the same.

> If the user of the fb just writes to the fb and vram_cache=y, it means
> that the cache is filled with pixel data that is never used, thus
> lowering the performance of all other programs?

This is true only for write-allocate. We want the framebuffer to be
cacheable as write-through, allocate on read, no allocate on write.

Sure, when we are reading from the cached framebuffer, some useful
data may be evicted from cache. But if we are not caching the
framebuffer, any readers suffer from a huge performance penalty.
That's the reason why shadow framebuffer (a poor man's software
workaround) is implemented in Xorg server. And if we use a shadow
framebuffer (in normal write-back cached memory), we already have the
same or worse cache eviction problems when compared to the cached
framebuffer. I could not find any use case or benchmark where shadow
framebuffer would perform better than write-through cached framebuffer
on OMAP3 hardware.

Maybe a bit more details about the shadow framebuffer would be useful.
That's just one level above the framebuffer in the graphics stack for
linux desktop. And many of the performance issues happen exactly on
the boundary between different pieces of software when they are
developed independently. Knowing how the framebuffer is used in the
real world applications (and I assume X server is one of them) may
provide some insights about how to improve the framebuffer on the
kernel side.

Here the shadow framebuffer is initialized:
    http://cgit.freedesktop.org/xorg/xserver/tree/miext/shadow/shadow.c?id=xorg-server-1.12.1#n136
And it also enables damage extension to track all the drawing
operations performed with the shadow buffer in order to copy the
updated areas to the real framebuffer from time to time. The update
itself happens here:
    http://cgit.freedesktop.org/xorg/xserver/tree/miext/shadow/shpacked.c?id=xorg-server-1.12.1#n43
With the damage extension active, we go through damageCopyArea:
    http://cgit.freedesktop.org/xorg/xserver/tree/miext/damage/damage.c?id=xorg-server-1.12.1#n801
before reaching fbCopyArea:
    http://cgit.freedesktop.org/xorg/xserver/tree/fb/fbcopy.c?id=xorg-server-1.12.1#n265

While running x11perf test and doing copying/scrolling tests, the
function calls inside of X server look more or less like this:
    62. [00:29:27.779] fbCopyArea()
    63. [00:29:27.779] DamageReportDamage()
    64. [00:29:27.783] fbCopyArea()
    65. [00:29:27.783] DamageReportDamage()
    66. [00:29:27.787] fbCopyArea()
    67. [00:29:27.792] shadowUpdatePacked()
    68. [00:29:27.792] DamageReportDamage()
    69. [00:29:27.795] fbCopyArea()
    70. [00:29:27.795] DamageReportDamage()
    71. [00:29:27.799] fbCopyArea()
    72. [00:29:27.800] DamageReportDamage()
    73. [00:29:27.803] fbCopyArea()
    74. [00:29:27.803] DamageReportDamage()
    75. [00:29:27.807] fbCopyArea()
    76. [00:29:27.807] DamageReportDamage()
    77. [00:29:27.811] fbCopyArea()
    78. [00:29:27.811] DamageReportDamage()
    79. [00:29:27.815] fbCopyArea()
    80. [00:29:27.820] shadowUpdatePacked()

As can be seen in the log above, shadowUpdatePacked() is used much
less frequently than fbCopyArea(). Which means that shadow framebuffer
is cheating a bit by accumulating damage and updating the real
framebuffer much less frequently.

The write-through cached framebuffer beats the shadow framebuffer in every way:
- It needs less RAM
- No need for damage tracking overhead (important for small drawing operations)
- No screen updates are skipped, which means smoother animation

> I have to say I don't know much of the cpu caches, but the read speed
> improvements are very big, so I think this is definitely interesting
> patch.

Yes, it is definitely providing a significant performance improvement
for software rendering in Xorg server. But there is unfortunately no
free lunch. Using write-through cache means that if anything other
than CPU is writing to the framebuffer, then it must invalidate CPU
cache for this area. And this means that it makes sense to review how
the SGX integration is done and fix it if needed. I tried some tests
with X11WSEGL (a binary blob provided in GFX SDK for X11 integration)
and it did not seem to exhibit any obvious screen corruption issues,
but without having the sources it's hard to say for sure. I would
assume that the ideal setup for OMAP3 would be to use GFX plane
exclusively by the CPU for 2D graphics, and render SGX 3D graphics to
one of the VID planes. In this case DISPC can do the compositing and
CPU cache invalidate operations become unnecessary.

There is also DSP, ISP and maybe some other hardware blocks, but these
can be handled on case by case basis. My primary interest is a little
personal hobby project to get linux desktop running with acceptable
performance.

> So if you get the first patch accepted I see no problem with
> adding this to omapfb as an optional feature.

Yes, a review from ARM memory management subsystem experts is definitely needed.

> However, "vram_cache" is not a very good name for the option.
> "vram_writethrough", or something?

Still having "cache" in the name would be useful. Just in order to
imply that there might be coherency issues to consider. By the way,
vesafb actually uses some numeric codes for different types of caching
in mtrr:n option:
    https://github.com/torvalds/linux/blob/v3.4/Documentation/fb/vesafb.txt#L147

Write-through caching is bad on Cortex-A9 as promised in TRM and as
confirmed by tests. Still for Cortex-A9 it *might* be interesting to
experiment with enabling write-back cache for the framebuffer, but
instead of using shadow framebuffer just do CPU cache flushes based on
all the same damage tracking borrowed from shadow framebuffer code.
This even might be somehow related to the "manual" update mode :)
Except that we can't change framebuffer caching attributes at runtime.

> Did you test this with VRFB (omap3) or TILER (omap4)? I wonder how those
> are affected.

That's a good point. I tried VRFB on OMAP3 1GHz and got the following
results with x11perf (shadow framebuffer disabled in
xf86-video-fbdev):

------------------ rotate 90, write-through cached
  3500 trep @   8.0242 msec ( 125.0/sec): Scroll 500x500 pixels
  4000 trep @   8.7027 msec ( 115.0/sec): Copy 500x500 from window to window
  6000 trep @   5.4885 msec ( 182.0/sec): Copy 500x500 from pixmap to window
  6000 trep @   4.9806 msec ( 201.0/sec): Copy 500x500 from window to pixmap
  8000 trep @   3.4231 msec ( 292.0/sec): Copy 500x500 from pixmap to pixmap

------------------ rotate 90,  non-cached writecombining
  3000 trep @   8.9732 msec ( 111.0/sec): Scroll 500x500 pixels
  1000 trep @  26.3218 msec (  38.0/sec): Copy 500x500 from window to window
  6000 trep @   5.5002 msec ( 182.0/sec): Copy 500x500 from pixmap to window
  6000 trep @   6.2368 msec ( 160.0/sec): Copy 500x500 from window to pixmap
  8000 trep @   3.4219 msec ( 292.0/sec): Copy 500x500 from pixmap to pixmap

------------------ rotate 180, write-through cached
 10000 trep @   3.5219 msec ( 284.0/sec): Scroll 500x500 pixels
  6000 trep @   4.8829 msec ( 205.0/sec): Copy 500x500 from window to window
  8000 trep @   3.4772 msec ( 288.0/sec): Copy 500x500 from pixmap to window
  8000 trep @   3.2554 msec ( 307.0/sec): Copy 500x500 from window to pixmap
  8000 trep @   3.4196 msec ( 292.0/sec): Copy 500x500 from pixmap to pixmap

------------------ rotate 180, non-cached writecombining
 10000 trep @   4.5777 msec ( 218.0/sec): Scroll 500x500 pixels
  1000 trep @  24.9100 msec (  40.1/sec): Copy 500x500 from window to window
  8000 trep @   3.4763 msec ( 288.0/sec): Copy 500x500 from pixmap to window
  6000 trep @   4.8676 msec ( 205.0/sec): Copy 500x500 from window to pixmap
  8000 trep @   3.4205 msec ( 292.0/sec): Copy 500x500 from pixmap to pixmap

The 90 degrees rotation significantly reduces performance. Shadow
framebuffer is "faster", because it is skipping some work. But
write-through caching is at least not worse than the default
writecombine.

Regarding OMAP4. I only have an old pre-production Pandaboard EA1,
which only runs memory at half speed. It is useless for any
benchmarks.

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply

* (no subject)
From: robothroli company @ 2012-05-25 13:45 UTC (permalink / raw)



 i am robothroli, Purchase manager from roli Merchant Ltd. We are
Import/export Company based in taiwan. We are interested in purchasing
your product and I would like to make an inquiry. Please inform me on:

Sample availability and price
Minimum order quantity
FOB Prices

Sincerely
Purchase Manager
robothroli




^ permalink raw reply

* [PATCH] video: bfin_adv7393fb: Fix cleanup code
From: Emil Goode @ 2012-05-28 14:40 UTC (permalink / raw)
  To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, kernel-janitors, Emil Goode

This patch fixes the cleanup code of the bfin_adv7393_fb_probe
function by changing the order in which cleanup is performed
and by adding one label.

Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
 drivers/video/bfin_adv7393fb.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/video/bfin_adv7393fb.c b/drivers/video/bfin_adv7393fb.c
index 1a268a2..ddbd031 100644
--- a/drivers/video/bfin_adv7393fb.c
+++ b/drivers/video/bfin_adv7393fb.c
@@ -414,7 +414,7 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
 		if (ret) {
 			dev_err(&client->dev, "PPI0_FS3 GPIO request failed\n");
 			ret = -EBUSY;
-			goto out_8;
+			goto out_9;
 		}
 	}
 
@@ -528,15 +528,18 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
  out_3:
 	free_dma(CH_PPI);
  out_4:
-	dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
-			  fbdev->dma_handle);
- out_5:
 	fb_dealloc_cmap(&fbdev->info.cmap);
- out_6:
+ out_5:
 	kfree(fbdev->info.pseudo_palette);
+ out_6:
+	dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
+			  fbdev->dma_handle);
  out_7:
 	peripheral_free_list(ppi_pins);
  out_8:
+	if (ANOMALY_05000400)
+		gpio_free(P_IDENT(P_PPI0_FS3));
+ out_9:
 	kfree(fbdev);
 
 	return ret;
-- 
1.7.10


^ permalink raw reply related

* Re: [PATCH] video: bfin_adv7393fb: Fix cleanup code
From: Dan Carpenter @ 2012-05-28 15:26 UTC (permalink / raw)
  To: Emil Goode; +Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <1338216019-1288-1-git-send-email-emilgoode@gmail.com>

On Mon, May 28, 2012 at 04:40:19PM +0200, Emil Goode wrote:
> This patch fixes the cleanup code of the bfin_adv7393_fb_probe
> function by changing the order in which cleanup is performed
> and by adding one label.
> 

Could you list the actual bugs which were fixed:

1) The resources were not freed in the order that we allocated them
   so we call dma_free_coherent() before it was allocated.
2) The labels weren't in the right place which also meant that we
   freed resources that weren't allocated.
3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.

The new gpio_free() is more important than the added label.

Since you're changing all these, you may as well fix the label names
as well.  The out_9 style labels suck.  The right way is to give
them meaningful labels instead of GW-BASIC style labels.

Labels should be named after what happens when you arrive.  Some
people name them after the start of the journey but that's a
mistake.  It's just like the town of Chicago is still called Chicago
even though you might goto Chicago starting from Boston.  Like this:

err_ppi:
	free_dma(CH_PPI);
	fb_dealloc_cmap(&fbdev->info.cmap);
err_palette:
	kfree(fbdev->info.pseudo_palette);

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH] video: bfin_adv7393fb: Fix cleanup code
From: Emil Goode @ 2012-05-28 15:34 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20120528152601.GE4637@mwanda>

Thanks Dan, I'll resend with these modifications.

Best regards, 
Emil Goode
On Mon, 2012-05-28 at 18:26 +0300, Dan Carpenter wrote:
> On Mon, May 28, 2012 at 04:40:19PM +0200, Emil Goode wrote:
> > This patch fixes the cleanup code of the bfin_adv7393_fb_probe
> > function by changing the order in which cleanup is performed
> > and by adding one label.
> > 
> 
> Could you list the actual bugs which were fixed:
> 
> 1) The resources were not freed in the order that we allocated them
>    so we call dma_free_coherent() before it was allocated.
> 2) The labels weren't in the right place which also meant that we
>    freed resources that weren't allocated.
> 3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.
> 
> The new gpio_free() is more important than the added label.
> 
> Since you're changing all these, you may as well fix the label names
> as well.  The out_9 style labels suck.  The right way is to give
> them meaningful labels instead of GW-BASIC style labels.
> 
> Labels should be named after what happens when you arrive.  Some
> people name them after the start of the journey but that's a
> mistake.  It's just like the town of Chicago is still called Chicago
> even though you might goto Chicago starting from Boston.  Like this:
> 
> err_ppi:
> 	free_dma(CH_PPI);
> 	fb_dealloc_cmap(&fbdev->info.cmap);
> err_palette:
> 	kfree(fbdev->info.pseudo_palette);
> 
> regards,
> dan carpenter



^ permalink raw reply

* [PATCH v2] video: bfin_adv7393fb: Fix cleanup code
From: Emil Goode @ 2012-05-28 16:54 UTC (permalink / raw)
  To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, kernel-janitors, Emil Goode

This patch fixes the cleanup code of the bfin_adv7393_fb_probe
function.

1) The resources were not freed in the order that we allocated them
   so we call dma_free_coherent() before it was allocated.
2) The labels weren't in the right place which also meant that we
   freed resources that weren't allocated.
3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.
4) Lets change the label names into something more meaningful.

Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
v2: Changed the label names as well.

 drivers/video/bfin_adv7393fb.c |   43 +++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/video/bfin_adv7393fb.c b/drivers/video/bfin_adv7393fb.c
index 1a268a2..33ea874 100644
--- a/drivers/video/bfin_adv7393fb.c
+++ b/drivers/video/bfin_adv7393fb.c
@@ -414,14 +414,14 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
 		if (ret) {
 			dev_err(&client->dev, "PPI0_FS3 GPIO request failed\n");
 			ret = -EBUSY;
-			goto out_8;
+			goto free_fbdev;
 		}
 	}
 
 	if (peripheral_request_list(ppi_pins, DRIVER_NAME)) {
 		dev_err(&client->dev, "requesting PPI peripheral failed\n");
 		ret = -EFAULT;
-		goto out_8;
+		goto free_gpio;
 	}
 
 	fbdev->fb_mem @@ -432,7 +432,7 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
 		dev_err(&client->dev, "couldn't allocate dma buffer (%d bytes)\n",
 		       (u32) fbdev->fb_len);
 		ret = -ENOMEM;
-		goto out_7;
+		goto free_ppi_pins;
 	}
 
 	fbdev->info.screen_base = (void *)fbdev->fb_mem;
@@ -464,27 +464,27 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
 	if (!fbdev->info.pseudo_palette) {
 		dev_err(&client->dev, "failed to allocate pseudo_palette\n");
 		ret = -ENOMEM;
-		goto out_6;
+		goto free_fb_mem;
 	}
 
 	if (fb_alloc_cmap(&fbdev->info.cmap, BFIN_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
 		dev_err(&client->dev, "failed to allocate colormap (%d entries)\n",
 			   BFIN_LCD_NBR_PALETTE_ENTRIES);
 		ret = -EFAULT;
-		goto out_5;
+		goto free_palette;
 	}
 
 	if (request_dma(CH_PPI, "BF5xx_PPI_DMA") < 0) {
 		dev_err(&client->dev, "unable to request PPI DMA\n");
 		ret = -EFAULT;
-		goto out_4;
+		goto free_cmap;
 	}
 
 	if (request_irq(IRQ_PPI_ERROR, ppi_irq_error, 0,
 			"PPI ERROR", fbdev) < 0) {
 		dev_err(&client->dev, "unable to request PPI ERROR IRQ\n");
 		ret = -EFAULT;
-		goto out_3;
+		goto free_ch_ppi;
 	}
 
 	fbdev->open = 0;
@@ -494,14 +494,14 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
 
 	if (ret) {
 		dev_err(&client->dev, "i2c attach: init error\n");
-		goto out_1;
+		goto free_irq_ppi;
 	}
 
 
 	if (register_framebuffer(&fbdev->info) < 0) {
 		dev_err(&client->dev, "unable to register framebuffer\n");
 		ret = -EFAULT;
-		goto out_1;
+		goto free_irq_ppi;
 	}
 
 	dev_info(&client->dev, "fb%d: %s frame buffer device\n",
@@ -512,7 +512,7 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
 	if (!entry) {
 		dev_err(&client->dev, "unable to create /proc entry\n");
 		ret = -EFAULT;
-		goto out_0;
+		goto free_fb;
 	}
 
 	entry->read_proc = adv7393_read_proc;
@@ -521,22 +521,25 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
 
 	return 0;
 
- out_0:
+free_fb:
 	unregister_framebuffer(&fbdev->info);
- out_1:
+free_irq_ppi:
 	free_irq(IRQ_PPI_ERROR, fbdev);
- out_3:
+free_ch_ppi:
 	free_dma(CH_PPI);
- out_4:
-	dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
-			  fbdev->dma_handle);
- out_5:
+free_cmap:
 	fb_dealloc_cmap(&fbdev->info.cmap);
- out_6:
+free_palette:
 	kfree(fbdev->info.pseudo_palette);
- out_7:
+free_fb_mem:
+	dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
+			  fbdev->dma_handle);
+free_ppi_pins:
 	peripheral_free_list(ppi_pins);
- out_8:
+free_gpio:
+	if (ANOMALY_05000400)
+		gpio_free(P_IDENT(P_PPI0_FS3));
+free_fbdev:
 	kfree(fbdev);
 
 	return ret;
-- 
1.7.10


^ permalink raw reply related

* Re: [PATCH v2] video: bfin_adv7393fb: Fix cleanup code
From: Dan Carpenter @ 2012-05-29  6:21 UTC (permalink / raw)
  To: Emil Goode; +Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <1338224091-8322-1-git-send-email-emilgoode@gmail.com>

On Mon, May 28, 2012 at 06:54:51PM +0200, Emil Goode wrote:
> This patch fixes the cleanup code of the bfin_adv7393_fb_probe
> function.
> 
> 1) The resources were not freed in the order that we allocated them
>    so we call dma_free_coherent() before it was allocated.
> 2) The labels weren't in the right place which also meant that we
>    freed resources that weren't allocated.
> 3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.
> 4) Lets change the label names into something more meaningful.
> 
> Signed-off-by: Emil Goode <emilgoode@gmail.com>
> ---

Yep.  Looks great.

I see the goto free_ppi_pins;  I look up a couple lines.  I see that
yeah, it makes sense to free the ppi_pins.  I hit the '*' key in vim
I see that the label does exactly what it says.  Move on.

Bing.  Bang.  Boom.

regards,
dan carpenter


^ permalink raw reply

* Re: [GIT PULL] OMAP DSS for v3.5
From: Florian Tobias Schandinat @ 2012-05-29 15:05 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, Tony Lindgren
In-Reply-To: <1337681363.4125.10.camel@deskari>

On 05/22/2012 10:09 AM, Tomi Valkeinen wrote:
> Hi Florian,
> 
> Here are the OMAP DSS changes for 3.5.

Merged.


Thanks,

Florian Tobias Schandinat

> 
> I really tried this time to send the pull request early, but here I am
> again, sending it when the merge window has already opened... A few
> late-found bugs caused some unnecessary delays.
> 
> I'm using github this time instead of gitorious, as gitorious seems to
> be down. This is my first pull request with a signed tag, I hope
> everything is correct.
> 
> Note that there's a merge for small branch with omap board file changes
> (e4a9e94cc58ed6e4efb02b80be3a9bf57f448d07) that is also pulled by Tony
> for the linux-omap tree. This is meant to help avoid conflicts in the
> board files.
> 
>  Tomi
> 
> 
> The following changes since commit 66f75a5d028beaf67c931435fdc3e7823125730c:
> 
>   Linux 3.4-rc4 (2012-04-21 14:47:52 -0700)
> 
> are available in the git repository at:
> 
>   git://github.com/tomba/linux.git tags/omapdss-for-3.5
> 
> for you to fetch changes up to e92a5b28f71aea01b281f9c89d97a4bc5b24748f:
> 
>   OMAPDSS: HDMI: OMAP4: Update IRQ flags for the HPD IRQ request (2012-05-22 11:00:09 +0300)
> 
> ----------------------------------------------------------------
> Omapdss driver changes for 3.5 merge window.
> 
> Lots of normal development commits, but perhaps most notable changes are:
> 
> * HDMI rework to properly decouple the HDMI audio part from the HDMI video part.
> * Restructure omapdss core driver so that it's possible to implement device
>   tree support. This included changing how platform data is passed to the
>   drivers, changing display device registration and improving the panel driver's
>   ability to configure the underlying video output interface.
> * Basic support for DSI packet interleaving
> 
> ----------------------------------------------------------------
> Archit Taneja (19):
>       OMAPDSS: DISPC/RFBI: Use dispc_mgr_set_lcd_timings() for setting lcd size
>       OMAPDSS: DISPC: Use a common function to set manager timings
>       OMAPDSS: DISPC: Clean up manager timing/size functions
>       OMAPDSS: HDMI: Fix ti_hdmi_4xxx_core_dump
>       OMAPDSS: HDMI: define and dump CORE registers in correct order
>       OMAPDSS: Fix DSI_FCLK clock source selection
>       OMAPDSS: DISPC: Remove Fake VSYNC support
>       OMAPDSS: APPLY: Add manager timings as extra_info in private data
>       OMAPDSS: Apply manager timings instead of direct DISPC writes
>       OMAPDSS: MANAGER: Create a function to check manager timings
>       OMAPDSS: APPLY: Don't check manager settings if it is disabled
>       OMAPDSS: APPLY: Remove display dependency from overlay and manager checks
>       OMAPDSS: DPI/HDMI: Apply manager timings even if panel is disabled
>       OMAPDSS: APPLY: Remove an unnecessary omap_dss_device pointer
>       OMAPDSS: DISPC: Remove omap_dss_device pointer usage from dispc_mgr_pclk_rate()
>       OMAPDSS: DISPC: Remove usage of dispc_mgr_get_device()
>       OMAPDSS: DSI: Support command mode interleaving during video mode blanking periods
>       OMAPDSS: VENC/DISPC: Delay dividing Y resolution for managers connected to VENC
>       OMAPDSS: Apply VENC timings even if panel is disabled
> 
> Axel Castaneda Gonzalez (1):
>       OMAPDSS: HDMI: Decouple wrapper enable/disable and audio start/stop
> 
> Chandrabhanu Mahapatra (5):
>       OMAPDSS: DISPC: Enable predecimation
>       OMAPDSS: DISPC: Handle synclost errors in OMAP3
>       OMAPDSS: DISPC: Correct DISPC functional clock usage
>       OMAPDSS: DISPC: Update Accumulator configuration for chroma plane
>       OMAPDSS: DISPC: Support rotation through TILER
> 
> Grazvydas Ignotas (5):
>       OMAPDSS: OMAPFB: check for matching memory size early
>       OMAPDSS: provide default get_timings function for panels
>       OMAPDSS: TPO-TD03MTEA1: add set/check timing functions
>       OMAPDSS: VENC: allow switching venc output type at runtime
>       OMAPDSS: OMAPFB: always allow to configure overlay
> 
> Jan Weitzel (1):
>       ARM: OMAP2+ Add Primview displays to panel-generic
> 
> Mark Brown (4):
>       OMAP: DSS2: Remove suspicous and unused TAAL regulator API usage
>       OMAPDSS: VENC: Check for errors from regulator_enable()
>       OMAPDSS: TPO-TD03MTEA1: Check for errors from regulator_enable()
>       OMAPDSS: TPO-TD03MTEA1: Correct comment for power on delay
> 
> Peter Meerwald (1):
>       OMAPFB: remove unused FB_OMAP_BOOTLOADER_INIT config option
> 
> Ricardo Neri (15):
>       OMAPDSS: Provide an interface for audio support
>       OMAPDSS: HDMI: Split audio_enable into audio_enable/disable
>       OMAPDSS: HDMI: Split video_enable into video_enable/disable
>       OMAPDSS: HDMI: Remove ASoC codec
>       OMAPDSS: HDMI: OMAP4: Remove CEA-861 audio infoframe and IEC-60958 enums
>       OMAPDSS: HDMI: OMAP4: Remove invalid I2S settings
>       OMAPDSS: HDMI: Decouple HDMI audio from ASoC
>       OMAPDSS: HDMI: OMAP4: Expand configuration for IEC-60958 audio
>       OMAPDSS: HDMI: Relocate N/CTS calculation
>       OMAPDSS: HDMI: Add support for more audio sample rates in N/CTS calculation
>       OMAPDSS: HDMI: Add an audio configuration function
>       OMAPDSS: HDMI: OMAP4: Remap speaker order to match ALSA order
>       OMAPDSS: HDMI: Panel: Simplify the name of the HDMI mutex
>       OMAPDSS: HDMI: Implement DSS driver interface for audio
>       OMAPDSS: HDMI: OMAP4: Update IRQ flags for the HPD IRQ request
> 
> Russ Dill (1):
>       OMAPDSS: TFP410: use gpio_set_value_cansleep
> 
> Thomas Weber (2):
>       OMAPDSS: Add Mitsubishi AA084SB01 display support
>       OMAPDSS: Add EDT ET0500G0DH6 display support
> 
> Tomi Valkeinen (38):
>       OMAPDSS: add set_min_bus_tput pointer to omapdss's platform data
>       OMAPDSS: Ensure OPP100 when DSS is operational
>       OMAPDSS: DSI: remove option to use pck for DSI PLL clkin
>       OMAPDSS: panel-dvi: add PD gpio handling
>       OMAP: board-files: remove custom PD GPIO handling for DVI output
>       OMAPDSS: TFP410: rename dvi -> tfp410
>       OMAPDSS: TFP410: rename dvi files to tfp410
>       OMAPDSS: Taal: move reset gpio handling to taal driver
>       OMAPDSS: DSI: implement generic DSI pin config
>       Merge branch 'archit/set-timing-work'
>       Merge branch 'for-l-o-3.5'
>       OMAPFB: fix parsing of vram parameter
>       OMAPDSS: TFP410: pdata rewrite
>       OMAPDSS: DSI: use dsi_get_dsidev_id(dsidev) instead of dsidev->id
>       OMAPDSS: clean up the omapdss platform data mess
>       OMAPDSS: remove return from platform_driver_unreg
>       OMAPDSS: use platform_driver_probe for core/dispc/dss
>       OMAPDSS: create custom pdevs for DSS omap_devices
>       OMAPDSS: create DPI & SDI devices
>       OMAPDSS: create DPI & SDI drivers
>       OMAPDSS: remove uses of dss_runtime_get/put
>       OMAPDSS: handle output-driver reg/unreg more dynamically
>       OMAPDSS: move the creation of debugfs files
>       OMAPDSS: use platform_driver_probe for dsi/hdmi/rfbi/venc/dpi/sdi
>       OMAPDSS: add __init & __exit
>       OMAPFB: add __init & __exit
>       OMAPDSS: change default_device handling
>       OMAPDSS: interface drivers register their panel devices
>       OMAPDSS: init omap_dss_devices internally
>       OMAPDSS: DSI: improve DSI module id handling
>       OMAPDSS: separate pdata based initialization
>       Merge branch 'dss-devtree-cleanup'
>       Merge branch 'omapdss-hdmi-audio'
>       OMAPDSS: use DSI_FIFO_BUG workaround only for manual update displays
>       OMAPDSS: DISPC: fix usage of dispc_ovl_set_accu_uv
>       OMAPDSS: remove compiler warnings when CONFIG_BUG=n
>       OMAPFB: remove compiler warnings when CONFIG_BUG=n
>       OMAPDSS: VRFB: remove compiler warnings when CONFIG_BUG=n
> 
>  Documentation/arm/OMAP/DSS                         |   46 ++
>  arch/arm/mach-omap2/board-3430sdp.c                |   38 +-
>  arch/arm/mach-omap2/board-4430sdp.c                |   37 +-
>  arch/arm/mach-omap2/board-am3517evm.c              |   25 +-
>  arch/arm/mach-omap2/board-cm-t35.c                 |   30 +-
>  arch/arm/mach-omap2/board-devkit8000.c             |   30 +-
>  arch/arm/mach-omap2/board-igep0020.c               |   32 +-
>  arch/arm/mach-omap2/board-omap3beagle.c            |   37 +-
>  arch/arm/mach-omap2/board-omap3evm.c               |   29 +-
>  arch/arm/mach-omap2/board-omap3stalker.c           |   29 +-
>  arch/arm/mach-omap2/board-omap4panda.c             |   39 +-
>  arch/arm/mach-omap2/board-overo.c                  |   25 +-
>  arch/arm/mach-omap2/display.c                      |  196 ++++-
>  drivers/video/omap/Kconfig                         |    8 -
>  drivers/video/omap2/displays/Kconfig               |    8 +-
>  drivers/video/omap2/displays/Makefile              |    2 +-
>  drivers/video/omap2/displays/panel-acx565akm.c     |    7 -
>  drivers/video/omap2/displays/panel-generic-dpi.c   |  107 ++-
>  drivers/video/omap2/displays/panel-n8x0.c          |    8 -
>  drivers/video/omap2/displays/panel-taal.c          |  108 +--
>  .../omap2/displays/{panel-dvi.c => panel-tfp410.c} |  186 ++---
>  .../video/omap2/displays/panel-tpo-td043mtea1.c    |   22 +-
>  drivers/video/omap2/dss/Kconfig                    |   13 +-
>  drivers/video/omap2/dss/apply.c                    |  134 +++-
>  drivers/video/omap2/dss/core.c                     |  255 ++++---
>  drivers/video/omap2/dss/dispc.c                    |  747 ++++++++++++++------
>  drivers/video/omap2/dss/dispc.h                    |   72 ++
>  drivers/video/omap2/dss/display.c                  |   49 +-
>  drivers/video/omap2/dss/dpi.c                      |   75 +-
>  drivers/video/omap2/dss/dsi.c                      |  537 ++++++++------
>  drivers/video/omap2/dss/dss.c                      |   65 +-
>  drivers/video/omap2/dss/dss.h                      |  151 ++--
>  drivers/video/omap2/dss/dss_features.c             |   30 +-
>  drivers/video/omap2/dss/dss_features.h             |    5 +
>  drivers/video/omap2/dss/hdmi.c                     |  443 +++++-------
>  drivers/video/omap2/dss/hdmi_panel.c               |  236 ++++++-
>  drivers/video/omap2/dss/manager.c                  |   19 +-
>  drivers/video/omap2/dss/overlay.c                  |   16 +-
>  drivers/video/omap2/dss/rfbi.c                     |   84 ++-
>  drivers/video/omap2/dss/sdi.c                      |   63 +-
>  drivers/video/omap2/dss/ti_hdmi.h                  |   32 +-
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c          |  480 ++++++++-----
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h          |  161 +----
>  drivers/video/omap2/dss/venc.c                     |  133 +++-
>  drivers/video/omap2/omapfb/omapfb-ioctl.c          |   17 +-
>  drivers/video/omap2/omapfb/omapfb-main.c           |   12 +-
>  drivers/video/omap2/omapfb/omapfb.h                |    1 +
>  drivers/video/omap2/vrfb.c                         |    4 +-
>  include/video/omap-panel-nokia-dsi.h               |    3 +
>  .../{omap-panel-dvi.h => omap-panel-tfp410.h}      |   18 +-
>  include/video/omapdss.h                            |   75 +-
>  51 files changed, 2983 insertions(+), 1996 deletions(-)
>  rename drivers/video/omap2/displays/{panel-dvi.c => panel-tfp410.c} (56%)
>  rename include/video/{omap-panel-dvi.h => omap-panel-tfp410.h} (63%)
> 


^ permalink raw reply

* Re: [patch] fb: handle NULL pointers in framebuffer release
From: Florian Tobias Schandinat @ 2012-05-29 15:05 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20120514205837.GE20836@elgon.mountain>

On 05/14/2012 08:58 PM, Dan Carpenter wrote:
> This function is called with a potential NULL pointer in
> picolcd_init_framebuffer() and it causes a static checker warning.  This
> used to handle NULL pointers when the picolcd code was written, but a
> couple months later we added the "info->apertures" dereference.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
> index 67afa9c..a55e366 100644
> --- a/drivers/video/fbsysfs.c
> +++ b/drivers/video/fbsysfs.c
> @@ -80,6 +80,8 @@ EXPORT_SYMBOL(framebuffer_alloc);
>   */
>  void framebuffer_release(struct fb_info *info)
>  {
> +	if (!info)
> +		return;
>  	kfree(info->apertures);
>  	kfree(info);
>  }
> 


^ permalink raw reply

* Re: [PATCH 1/3] video: exynos_dp: use devm_ functions
From: Florian Tobias Schandinat @ 2012-05-29 15:06 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <001d01cd3a46$e68d8b10$b3a8a130$%han@samsung.com>

Applied all 3 patches.


Thanks,

Florian Tobias Schandinat


On 05/25/2012 07:20 AM, Jingoo Han wrote:
> The devm_ functions allocate memory that is released when a driver
> detaches. This makes the code smaller and a bit simpler.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/video/exynos/exynos_dp_core.c |   49 ++++++++-------------------------
>  drivers/video/exynos/exynos_dp_core.h |    1 -
>  2 files changed, 12 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index ef4d1ab..aabd214 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -858,7 +858,8 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	dp = kzalloc(sizeof(struct exynos_dp_device), GFP_KERNEL);
> +	dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device),
> +				GFP_KERNEL);
>  	if (!dp) {
>  		dev_err(&pdev->dev, "no memory for device data\n");
>  		return -ENOMEM;
> @@ -869,8 +870,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  	dp->clock = clk_get(&pdev->dev, "dp");
>  	if (IS_ERR(dp->clock)) {
>  		dev_err(&pdev->dev, "failed to get clock\n");
> -		ret = PTR_ERR(dp->clock);
> -		goto err_dp;
> +		return PTR_ERR(dp->clock);
>  	}
>  
>  	clk_enable(dp->clock);
> @@ -882,35 +882,25 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  		goto err_clock;
>  	}
>  
> -	res = request_mem_region(res->start, resource_size(res),
> -				dev_name(&pdev->dev));
> -	if (!res) {
> -		dev_err(&pdev->dev, "failed to request registers region\n");
> -		ret = -EINVAL;
> -		goto err_clock;
> -	}
> -
> -	dp->res = res;
> -
> -	dp->reg_base = ioremap(res->start, resource_size(res));
> +	dp->reg_base = devm_request_and_ioremap(&pdev->dev, res);
>  	if (!dp->reg_base) {
>  		dev_err(&pdev->dev, "failed to ioremap\n");
>  		ret = -ENOMEM;
> -		goto err_req_region;
> +		goto err_clock;
>  	}
>  
>  	dp->irq = platform_get_irq(pdev, 0);
>  	if (!dp->irq) {
>  		dev_err(&pdev->dev, "failed to get irq\n");
>  		ret = -ENODEV;
> -		goto err_ioremap;
> +		goto err_clock;
>  	}
>  
> -	ret = request_irq(dp->irq, exynos_dp_irq_handler, 0,
> -			"exynos-dp", dp);
> +	ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0,
> +				"exynos-dp", dp);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to request irq\n");
> -		goto err_ioremap;
> +		goto err_clock;
>  	}
>  
>  	dp->video_info = pdata->video_info;
> @@ -922,7 +912,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  	ret = exynos_dp_detect_hpd(dp);
>  	if (ret) {
>  		dev_err(&pdev->dev, "unable to detect hpd\n");
> -		goto err_irq;
> +		goto err_clock;
>  	}
>  
>  	exynos_dp_handle_edid(dp);
> @@ -931,7 +921,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  				dp->video_info->link_rate);
>  	if (ret) {
>  		dev_err(&pdev->dev, "unable to do link train\n");
> -		goto err_irq;
> +		goto err_clock;
>  	}
>  
>  	exynos_dp_enable_scramble(dp, 1);
> @@ -945,23 +935,15 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev)
>  	ret = exynos_dp_config_video(dp, dp->video_info);
>  	if (ret) {
>  		dev_err(&pdev->dev, "unable to config video\n");
> -		goto err_irq;
> +		goto err_clock;
>  	}
>  
>  	platform_set_drvdata(pdev, dp);
>  
>  	return 0;
>  
> -err_irq:
> -	free_irq(dp->irq, dp);
> -err_ioremap:
> -	iounmap(dp->reg_base);
> -err_req_region:
> -	release_mem_region(res->start, resource_size(res));
>  err_clock:
>  	clk_put(dp->clock);
> -err_dp:
> -	kfree(dp);
>  
>  	return ret;
>  }
> @@ -974,16 +956,9 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev)
>  	if (pdata && pdata->phy_exit)
>  		pdata->phy_exit();
>  
> -	free_irq(dp->irq, dp);
> -	iounmap(dp->reg_base);
> -
>  	clk_disable(dp->clock);
>  	clk_put(dp->clock);
>  
> -	release_mem_region(dp->res->start, resource_size(dp->res));
> -
> -	kfree(dp);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> index 519c3a6..8f596b9 100644
> --- a/drivers/video/exynos/exynos_dp_core.h
> +++ b/drivers/video/exynos/exynos_dp_core.h
> @@ -26,7 +26,6 @@ struct link_train {
>  
>  struct exynos_dp_device {
>  	struct device		*dev;
> -	struct resource		*res;
>  	struct clk		*clock;
>  	unsigned int		irq;
>  	void __iomem		*reg_base;


^ permalink raw reply

* Re: [PATCH v2] video: bfin_adv7393fb: Fix cleanup code
From: Florian Tobias Schandinat @ 2012-05-29 15:07 UTC (permalink / raw)
  To: Emil Goode; +Cc: linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <1338224091-8322-1-git-send-email-emilgoode@gmail.com>

On 05/28/2012 04:54 PM, Emil Goode wrote:
> This patch fixes the cleanup code of the bfin_adv7393_fb_probe
> function.
> 
> 1) The resources were not freed in the order that we allocated them
>    so we call dma_free_coherent() before it was allocated.
> 2) The labels weren't in the right place which also meant that we
>    freed resources that weren't allocated.
> 3) We should free gpio_free(P_IDENT(P_PPI0_FS3)) before returning.
> 4) Lets change the label names into something more meaningful.
> 
> Signed-off-by: Emil Goode <emilgoode@gmail.com>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
> v2: Changed the label names as well.
> 
>  drivers/video/bfin_adv7393fb.c |   43 +++++++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/video/bfin_adv7393fb.c b/drivers/video/bfin_adv7393fb.c
> index 1a268a2..33ea874 100644
> --- a/drivers/video/bfin_adv7393fb.c
> +++ b/drivers/video/bfin_adv7393fb.c
> @@ -414,14 +414,14 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
>  		if (ret) {
>  			dev_err(&client->dev, "PPI0_FS3 GPIO request failed\n");
>  			ret = -EBUSY;
> -			goto out_8;
> +			goto free_fbdev;
>  		}
>  	}
>  
>  	if (peripheral_request_list(ppi_pins, DRIVER_NAME)) {
>  		dev_err(&client->dev, "requesting PPI peripheral failed\n");
>  		ret = -EFAULT;
> -		goto out_8;
> +		goto free_gpio;
>  	}
>  
>  	fbdev->fb_mem > @@ -432,7 +432,7 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
>  		dev_err(&client->dev, "couldn't allocate dma buffer (%d bytes)\n",
>  		       (u32) fbdev->fb_len);
>  		ret = -ENOMEM;
> -		goto out_7;
> +		goto free_ppi_pins;
>  	}
>  
>  	fbdev->info.screen_base = (void *)fbdev->fb_mem;
> @@ -464,27 +464,27 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
>  	if (!fbdev->info.pseudo_palette) {
>  		dev_err(&client->dev, "failed to allocate pseudo_palette\n");
>  		ret = -ENOMEM;
> -		goto out_6;
> +		goto free_fb_mem;
>  	}
>  
>  	if (fb_alloc_cmap(&fbdev->info.cmap, BFIN_LCD_NBR_PALETTE_ENTRIES, 0) < 0) {
>  		dev_err(&client->dev, "failed to allocate colormap (%d entries)\n",
>  			   BFIN_LCD_NBR_PALETTE_ENTRIES);
>  		ret = -EFAULT;
> -		goto out_5;
> +		goto free_palette;
>  	}
>  
>  	if (request_dma(CH_PPI, "BF5xx_PPI_DMA") < 0) {
>  		dev_err(&client->dev, "unable to request PPI DMA\n");
>  		ret = -EFAULT;
> -		goto out_4;
> +		goto free_cmap;
>  	}
>  
>  	if (request_irq(IRQ_PPI_ERROR, ppi_irq_error, 0,
>  			"PPI ERROR", fbdev) < 0) {
>  		dev_err(&client->dev, "unable to request PPI ERROR IRQ\n");
>  		ret = -EFAULT;
> -		goto out_3;
> +		goto free_ch_ppi;
>  	}
>  
>  	fbdev->open = 0;
> @@ -494,14 +494,14 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
>  
>  	if (ret) {
>  		dev_err(&client->dev, "i2c attach: init error\n");
> -		goto out_1;
> +		goto free_irq_ppi;
>  	}
>  
>  
>  	if (register_framebuffer(&fbdev->info) < 0) {
>  		dev_err(&client->dev, "unable to register framebuffer\n");
>  		ret = -EFAULT;
> -		goto out_1;
> +		goto free_irq_ppi;
>  	}
>  
>  	dev_info(&client->dev, "fb%d: %s frame buffer device\n",
> @@ -512,7 +512,7 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
>  	if (!entry) {
>  		dev_err(&client->dev, "unable to create /proc entry\n");
>  		ret = -EFAULT;
> -		goto out_0;
> +		goto free_fb;
>  	}
>  
>  	entry->read_proc = adv7393_read_proc;
> @@ -521,22 +521,25 @@ static int __devinit bfin_adv7393_fb_probe(struct i2c_client *client,
>  
>  	return 0;
>  
> - out_0:
> +free_fb:
>  	unregister_framebuffer(&fbdev->info);
> - out_1:
> +free_irq_ppi:
>  	free_irq(IRQ_PPI_ERROR, fbdev);
> - out_3:
> +free_ch_ppi:
>  	free_dma(CH_PPI);
> - out_4:
> -	dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
> -			  fbdev->dma_handle);
> - out_5:
> +free_cmap:
>  	fb_dealloc_cmap(&fbdev->info.cmap);
> - out_6:
> +free_palette:
>  	kfree(fbdev->info.pseudo_palette);
> - out_7:
> +free_fb_mem:
> +	dma_free_coherent(NULL, fbdev->fb_len, fbdev->fb_mem,
> +			  fbdev->dma_handle);
> +free_ppi_pins:
>  	peripheral_free_list(ppi_pins);
> - out_8:
> +free_gpio:
> +	if (ANOMALY_05000400)
> +		gpio_free(P_IDENT(P_PPI0_FS3));
> +free_fbdev:
>  	kfree(fbdev);
>  
>  	return ret;


^ permalink raw reply

* [PATCH] video: bfin_adv7393fb: Convert to kstrtouint_from_user
From: Emil Goode @ 2012-05-29 16:57 UTC (permalink / raw)
  To: FlorianSchandinat; +Cc: linux-fbdev, linux-kernel, kernel-janitors, Emil Goode

This patch removes a call to the deprecated simple_strtoul function
and simplifies the code by replacing two function calls with one
call to kstrtouint_from_user.

-Simplify the adv7393_write_proc function by replacing the
 simple_strtoul and copy_from_user calls with one call
 to kstrtouint_from_user.

-Change the count parameter from unsigned long to size_t as
 this is the type that the kstrtouint_from_user function expects.
 (size_t is what will be passed to the adv7393_write_proc function
 by the proc write handler function proc_file_write anyway)

Signed-off-by: Emil Goode <emilgoode@gmail.com>
---
 drivers/video/bfin_adv7393fb.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/bfin_adv7393fb.c b/drivers/video/bfin_adv7393fb.c
index 1a268a2..8c6edfd 100644
--- a/drivers/video/bfin_adv7393fb.c
+++ b/drivers/video/bfin_adv7393fb.c
@@ -353,18 +353,16 @@ adv7393_read_proc(char *page, char **start, off_t off,
 
 static int
 adv7393_write_proc(struct file *file, const char __user * buffer,
-		   unsigned long count, void *data)
+		   size_t count, void *data)
 {
 	struct adv7393fb_device *fbdev = data;
-	char line[8];
 	unsigned int val;
 	int ret;
 
-	ret = copy_from_user(line, buffer, count);
+	ret = kstrtouint_from_user(buffer, count, 0, &val);
 	if (ret)
 		return -EFAULT;
 
-	val = simple_strtoul(line, NULL, 0);
 	adv7393_write(fbdev->client, val >> 8, val & 0xff);
 
 	return count;
-- 
1.7.10


^ permalink raw reply related

* [PATCH 1/1] mx3fb: support pan display with fb_set_var
From: Liu Ying @ 2012-05-30  4:14 UTC (permalink / raw)
  To: FlorianSchandinat; +Cc: lg, linux-fbdev, linux-kernel, Liu Ying

Users may call FBIOPUT_VSCREENINFO ioctrl to do pan display.
This ioctrl relies on fb_set_var() to do the job. fb_set_var()
calls custom fb_set_par() method and then calls custom
fb_pan_display() method. The current implementation of mx3fb
reinitializes IPU display controller every time the custom
fb_set_par() method is called, which makes the display flash
if fb_set_var() is called to do panning frequently. The custom
fb_pan_display() method checks if the current xoffset and
yoffset are different from previous ones before doing actual
panning, which prevents the panning from happening within the
fb_set_var() context. This patch checks new var info to decide
whether we really need to reinitialize IPU display controller.
We ignore xoffset and yoffset update because it doesn't require
to reinitialize the controller. Users may specify activate field
of var info with FB_ACTIVATE_NOW and FB_ACTIVATE_FORCE to
reinialize the controller by force. Meanwhile, this patch removes
the check in custom fb_pan_display() method mentioned before to
have the panning work within fb_set_var() context. It doesn't
hurt to do panning again if there is no update for xoffset and
yoffset.

Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
 drivers/video/mx3fb.c |   31 +++++++++++++++++++++++++++----
 1 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index e3406ab..86daabe 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -269,6 +269,8 @@ struct mx3fb_info {
 	struct scatterlist		sg[2];
 
 	u32				sync;	/* preserve var->sync flags */
+
+	struct fb_var_screeninfo	cur_var; /* current var info */
 };
 
 static void mx3fb_dma_done(void *);
@@ -721,6 +723,26 @@ static void mx3fb_dma_done(void *arg)
 	complete(&mx3_fbi->flip_cmpl);
 }
 
+static bool fb_need_not_to_set_par(struct fb_info *fbi)
+{
+	struct mx3fb_info *mx3_fbi = fbi->par;
+	struct fb_var_screeninfo old_var = mx3_fbi->cur_var;
+	struct fb_var_screeninfo new_var = fbi->var;
+
+	if ((fbi->var.activate & FB_ACTIVATE_FORCE) &&
+	    (fbi->var.activate & FB_ACTIVATE_MASK) = FB_ACTIVATE_NOW)
+		return false;
+
+	/*
+	 * Ignore xoffset and yoffset update,
+	 * because pan display handles this case.
+	 */
+	old_var.xoffset = new_var.xoffset;
+	old_var.yoffset = new_var.yoffset;
+
+	return !memcmp(&old_var, &new_var, sizeof(struct fb_var_screeninfo));
+}
+
 static int __set_par(struct fb_info *fbi, bool lock)
 {
 	u32 mem_len;
@@ -732,6 +754,9 @@ static int __set_par(struct fb_info *fbi, bool lock)
 	struct idmac_video_param *video = &ichan->params.video;
 	struct scatterlist *sg = mx3_fbi->sg;
 
+	if (fb_need_not_to_set_par(fbi))
+		return 0;
+
 	/* Total cleanup */
 	if (mx3_fbi->txd)
 		sdc_disable_channel(mx3_fbi);
@@ -808,6 +833,8 @@ static int __set_par(struct fb_info *fbi, bool lock)
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK)
 		sdc_enable_channel(mx3_fbi);
 
+	mx3_fbi->cur_var = fbi->var;
+
 	return 0;
 }
 
@@ -1068,10 +1095,6 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
 		return -EINVAL;
 	}
 
-	if (fbi->var.xoffset = var->xoffset &&
-	    fbi->var.yoffset = var->yoffset)
-		return 0;	/* No change, do nothing */
-
 	y_bottom = var->yoffset;
 
 	if (!(var->vmode & FB_VMODE_YWRAP))
-- 
1.7.1



^ permalink raw reply related

* [PATCH 1/1] mx3fb: support pan display with fb_set_var
From: Liu Ying @ 2012-05-30  4:27 UTC (permalink / raw)
  To: FlorianSchandinat; +Cc: lg, linux-fbdev, linux-kernel, Liu Ying
In-Reply-To: <1338351267-25561-1-git-send-email-Ying.Liu@freescale.com>

Users may call FBIOPUT_VSCREENINFO ioctrl to do pan display.
This ioctrl relies on fb_set_var() to do the job. fb_set_var()
calls custom fb_set_par() method and then calls custom
fb_pan_display() method. The current implementation of mx3fb
reinitializes IPU display controller every time the custom
fb_set_par() method is called, which makes the display flash
if fb_set_var() is called to do panning frequently. The custom
fb_pan_display() method checks if the current xoffset and
yoffset are different from previous ones before doing actual
panning, which prevents the panning from happening within the
fb_set_var() context. This patch checks new var info to decide
whether we really need to reinitialize IPU display controller.
We ignore xoffset and yoffset update because it doesn't require
to reinitialize the controller. Users may specify activate field
of var info with FB_ACTIVATE_NOW and FB_ACTIVATE_FORCE to
reinialize the controller by force. Meanwhile, this patch removes
the check in custom fb_pan_display() method mentioned before to
have the panning work within fb_set_var() context. It doesn't
hurt to do panning again if there is no update for xoffset and
yoffset.

Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
 drivers/video/mx3fb.c |   31 +++++++++++++++++++++++++++----
 1 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index e3406ab..238b9aa 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -269,6 +269,8 @@ struct mx3fb_info {
 	struct scatterlist		sg[2];
 
 	u32				sync;	/* preserve var->sync flags */
+
+	struct fb_var_screeninfo	cur_var; /* current var info */
 };
 
 static void mx3fb_dma_done(void *);
@@ -721,6 +723,26 @@ static void mx3fb_dma_done(void *arg)
 	complete(&mx3_fbi->flip_cmpl);
 }
 
+static bool mx3fb_need_not_to_set_par(struct fb_info *fbi)
+{
+	struct mx3fb_info *mx3_fbi = fbi->par;
+	struct fb_var_screeninfo old_var = mx3_fbi->cur_var;
+	struct fb_var_screeninfo new_var = fbi->var;
+
+	if ((fbi->var.activate & FB_ACTIVATE_FORCE) &&
+	    (fbi->var.activate & FB_ACTIVATE_MASK) = FB_ACTIVATE_NOW)
+		return false;
+
+	/*
+	 * Ignore xoffset and yoffset update,
+	 * because pan display handles this case.
+	 */
+	old_var.xoffset = new_var.xoffset;
+	old_var.yoffset = new_var.yoffset;
+
+	return !memcmp(&old_var, &new_var, sizeof(struct fb_var_screeninfo));
+}
+
 static int __set_par(struct fb_info *fbi, bool lock)
 {
 	u32 mem_len;
@@ -732,6 +754,9 @@ static int __set_par(struct fb_info *fbi, bool lock)
 	struct idmac_video_param *video = &ichan->params.video;
 	struct scatterlist *sg = mx3_fbi->sg;
 
+	if (mx3fb_need_not_to_set_par(fbi))
+		return 0;
+
 	/* Total cleanup */
 	if (mx3_fbi->txd)
 		sdc_disable_channel(mx3_fbi);
@@ -808,6 +833,8 @@ static int __set_par(struct fb_info *fbi, bool lock)
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK)
 		sdc_enable_channel(mx3_fbi);
 
+	mx3_fbi->cur_var = fbi->var;
+
 	return 0;
 }
 
@@ -1068,10 +1095,6 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
 		return -EINVAL;
 	}
 
-	if (fbi->var.xoffset = var->xoffset &&
-	    fbi->var.yoffset = var->yoffset)
-		return 0;	/* No change, do nothing */
-
 	y_bottom = var->yoffset;
 
 	if (!(var->vmode & FB_VMODE_YWRAP))
-- 
1.7.1



^ permalink raw reply related

* RE: [PATCH 1/1] mx3fb: support pan display with fb_set_var
From: Liu Ying-B17645 @ 2012-05-30  5:18 UTC (permalink / raw)
  To: FlorianSchandinat@gmx.de
  Cc: lg@denx.de, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1338351267-25561-1-git-send-email-Ying.Liu@freescale.com>

Please ignore this version.
I will send another updated one soon.

Liu Ying
-----Original Message-----
From: Liu Ying-B17645 
Sent: Wednesday, May 30, 2012 12:14 PM
To: FlorianSchandinat@gmx.de
Cc: lg@denx.de; linux-fbdev@vger.kernel.org; linux-kernel@vger.kernel.org; Liu Ying-B17645
Subject: [PATCH 1/1] mx3fb: support pan display with fb_set_var

Users may call FBIOPUT_VSCREENINFO ioctrl to do pan display.
This ioctrl relies on fb_set_var() to do the job. fb_set_var() calls custom fb_set_par() method and then calls custom
fb_pan_display() method. The current implementation of mx3fb reinitializes IPU display controller every time the custom
fb_set_par() method is called, which makes the display flash if fb_set_var() is called to do panning frequently. The custom
fb_pan_display() method checks if the current xoffset and yoffset are different from previous ones before doing actual panning, which prevents the panning from happening within the
fb_set_var() context. This patch checks new var info to decide whether we really need to reinitialize IPU display controller.
We ignore xoffset and yoffset update because it doesn't require to reinitialize the controller. Users may specify activate field of var info with FB_ACTIVATE_NOW and FB_ACTIVATE_FORCE to reinialize the controller by force. Meanwhile, this patch removes the check in custom fb_pan_display() method mentioned before to have the panning work within fb_set_var() context. It doesn't hurt to do panning again if there is no update for xoffset and yoffset.

Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
 drivers/video/mx3fb.c |   31 +++++++++++++++++++++++++++----
 1 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c index e3406ab..86daabe 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -269,6 +269,8 @@ struct mx3fb_info {
 	struct scatterlist		sg[2];
 
 	u32				sync;	/* preserve var->sync flags */
+
+	struct fb_var_screeninfo	cur_var; /* current var info */
 };
 
 static void mx3fb_dma_done(void *);
@@ -721,6 +723,26 @@ static void mx3fb_dma_done(void *arg)
 	complete(&mx3_fbi->flip_cmpl);
 }
 
+static bool fb_need_not_to_set_par(struct fb_info *fbi) {
+	struct mx3fb_info *mx3_fbi = fbi->par;
+	struct fb_var_screeninfo old_var = mx3_fbi->cur_var;
+	struct fb_var_screeninfo new_var = fbi->var;
+
+	if ((fbi->var.activate & FB_ACTIVATE_FORCE) &&
+	    (fbi->var.activate & FB_ACTIVATE_MASK) = FB_ACTIVATE_NOW)
+		return false;
+
+	/*
+	 * Ignore xoffset and yoffset update,
+	 * because pan display handles this case.
+	 */
+	old_var.xoffset = new_var.xoffset;
+	old_var.yoffset = new_var.yoffset;
+
+	return !memcmp(&old_var, &new_var, sizeof(struct fb_var_screeninfo)); 
+}
+
 static int __set_par(struct fb_info *fbi, bool lock)  {
 	u32 mem_len;
@@ -732,6 +754,9 @@ static int __set_par(struct fb_info *fbi, bool lock)
 	struct idmac_video_param *video = &ichan->params.video;
 	struct scatterlist *sg = mx3_fbi->sg;
 
+	if (fb_need_not_to_set_par(fbi))
+		return 0;
+
 	/* Total cleanup */
 	if (mx3_fbi->txd)
 		sdc_disable_channel(mx3_fbi);
@@ -808,6 +833,8 @@ static int __set_par(struct fb_info *fbi, bool lock)
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK)
 		sdc_enable_channel(mx3_fbi);
 
+	mx3_fbi->cur_var = fbi->var;
+
 	return 0;
 }
 
@@ -1068,10 +1095,6 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
 		return -EINVAL;
 	}
 
-	if (fbi->var.xoffset = var->xoffset &&
-	    fbi->var.yoffset = var->yoffset)
-		return 0;	/* No change, do nothing */
-
 	y_bottom = var->yoffset;
 
 	if (!(var->vmode & FB_VMODE_YWRAP))
--
1.7.1



^ permalink raw reply

* RE: [PATCH 1/1] mx3fb: support pan display with fb_set_var
From: Liu Ying-B17645 @ 2012-05-31  1:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1338352042-26522-1-git-send-email-Ying.Liu@freescale.com>

Add i.mx SoC maintainer Sacha and ARM maillist, and update Guennadi's email address.

Liu Ying
-----Original Message-----
From: Liu Ying-B17645 
Sent: Wednesday, May 30, 2012 12:27 PM
To: FlorianSchandinat@gmx.de
Cc: lg@denx.de; linux-fbdev@vger.kernel.org; linux-kernel@vger.kernel.org; Liu Ying-B17645
Subject: [PATCH 1/1] mx3fb: support pan display with fb_set_var

Users may call FBIOPUT_VSCREENINFO ioctrl to do pan display.
This ioctrl relies on fb_set_var() to do the job. fb_set_var() calls custom fb_set_par() method and then calls custom
fb_pan_display() method. The current implementation of mx3fb reinitializes IPU display controller every time the custom
fb_set_par() method is called, which makes the display flash if fb_set_var() is called to do panning frequently. The custom
fb_pan_display() method checks if the current xoffset and yoffset are different from previous ones before doing actual panning, which prevents the panning from happening within the
fb_set_var() context. This patch checks new var info to decide whether we really need to reinitialize IPU display controller.
We ignore xoffset and yoffset update because it doesn't require to reinitialize the controller. Users may specify activate field of var info with FB_ACTIVATE_NOW and FB_ACTIVATE_FORCE to reinialize the controller by force. Meanwhile, this patch removes the check in custom fb_pan_display() method mentioned before to have the panning work within fb_set_var() context. It doesn't hurt to do panning again if there is no update for xoffset and yoffset.

Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
 drivers/video/mx3fb.c |   31 +++++++++++++++++++++++++++----
 1 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c index e3406ab..238b9aa 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -269,6 +269,8 @@ struct mx3fb_info {
 	struct scatterlist		sg[2];
 
 	u32				sync;	/* preserve var->sync flags */
+
+	struct fb_var_screeninfo	cur_var; /* current var info */
 };
 
 static void mx3fb_dma_done(void *);
@@ -721,6 +723,26 @@ static void mx3fb_dma_done(void *arg)
 	complete(&mx3_fbi->flip_cmpl);
 }
 
+static bool mx3fb_need_not_to_set_par(struct fb_info *fbi) {
+	struct mx3fb_info *mx3_fbi = fbi->par;
+	struct fb_var_screeninfo old_var = mx3_fbi->cur_var;
+	struct fb_var_screeninfo new_var = fbi->var;
+
+	if ((fbi->var.activate & FB_ACTIVATE_FORCE) &&
+	    (fbi->var.activate & FB_ACTIVATE_MASK) = FB_ACTIVATE_NOW)
+		return false;
+
+	/*
+	 * Ignore xoffset and yoffset update,
+	 * because pan display handles this case.
+	 */
+	old_var.xoffset = new_var.xoffset;
+	old_var.yoffset = new_var.yoffset;
+
+	return !memcmp(&old_var, &new_var, sizeof(struct fb_var_screeninfo)); 
+}
+
 static int __set_par(struct fb_info *fbi, bool lock)  {
 	u32 mem_len;
@@ -732,6 +754,9 @@ static int __set_par(struct fb_info *fbi, bool lock)
 	struct idmac_video_param *video = &ichan->params.video;
 	struct scatterlist *sg = mx3_fbi->sg;
 
+	if (mx3fb_need_not_to_set_par(fbi))
+		return 0;
+
 	/* Total cleanup */
 	if (mx3_fbi->txd)
 		sdc_disable_channel(mx3_fbi);
@@ -808,6 +833,8 @@ static int __set_par(struct fb_info *fbi, bool lock)
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK)
 		sdc_enable_channel(mx3_fbi);
 
+	mx3_fbi->cur_var = fbi->var;
+
 	return 0;
 }
 
@@ -1068,10 +1095,6 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
 		return -EINVAL;
 	}
 
-	if (fbi->var.xoffset = var->xoffset &&
-	    fbi->var.yoffset = var->yoffset)
-		return 0;	/* No change, do nothing */
-
 	y_bottom = var->yoffset;
 
 	if (!(var->vmode & FB_VMODE_YWRAP))
--
1.7.1



^ permalink raw reply

* [PATCH 0/2] SH Mobile MERAM fix
From: Laurent Pinchart @ 2012-05-31 16:33 UTC (permalink / raw)
  To: linux-fbdev

Hello,

These two patches add MERAM operations wrappers. As the MERAM is used by a new
DRM driver I'm working on for the same hardware, I don't want the operations
to be accessed directly. This is a first step in getting rid of the MERAM
platform device pointer in the LCDC platform data.

Laurent Pinchart (2):
  fbdev: sh_mobile_meram: Add MERAM operations wrappers
  fbdev: sh_mobile_lcdc: Use MERAM operations wrappers

 drivers/video/sh_mobile_lcdcfb.c |   18 +++++++-----------
 drivers/video/sh_mobile_meram.c  |   24 ++++++++++++------------
 include/video/sh_mobile_meram.h  |   28 ++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 23 deletions(-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [PATCH 1/2] fbdev: sh_mobile_meram: Add MERAM operations wrappers
From: Laurent Pinchart @ 2012-05-31 16:33 UTC (permalink / raw)
  To: linux-fbdev

Remove direct dependency on the MERAM driver implementation by
introducing inline wrappers for MERAM operations.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_meram.c |   24 ++++++++++++------------
 include/video/sh_mobile_meram.h |   28 ++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 82ba830..c51849f 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -444,11 +444,11 @@ static void meram_deinit(struct sh_mobile_meram_priv *priv,
  * Registration/unregistration
  */
 
-static void *sh_mobile_meram_register(struct sh_mobile_meram_info *pdata,
-				      const struct sh_mobile_meram_cfg *cfg,
-				      unsigned int xres, unsigned int yres,
-				      unsigned int pixelformat,
-				      unsigned int *pitch)
+static void *__sh_mobile_meram_register(struct sh_mobile_meram_info *pdata,
+					const struct sh_mobile_meram_cfg *cfg,
+					unsigned int xres, unsigned int yres,
+					unsigned int pixelformat,
+					unsigned int *pitch)
 {
 	struct sh_mobile_meram_fb_cache *cache;
 	struct sh_mobile_meram_priv *priv = pdata->priv;
@@ -495,7 +495,7 @@ err:
 }
 
 static void
-sh_mobile_meram_unregister(struct sh_mobile_meram_info *pdata, void *data)
+__sh_mobile_meram_unregister(struct sh_mobile_meram_info *pdata, void *data)
 {
 	struct sh_mobile_meram_fb_cache *cache = data;
 	struct sh_mobile_meram_priv *priv = pdata->priv;
@@ -513,9 +513,9 @@ sh_mobile_meram_unregister(struct sh_mobile_meram_info *pdata, void *data)
 }
 
 static void
-sh_mobile_meram_update(struct sh_mobile_meram_info *pdata, void *data,
-		       unsigned long base_addr_y, unsigned long base_addr_c,
-		       unsigned long *icb_addr_y, unsigned long *icb_addr_c)
+__sh_mobile_meram_update(struct sh_mobile_meram_info *pdata, void *data,
+			 unsigned long base_addr_y, unsigned long base_addr_c,
+			 unsigned long *icb_addr_y, unsigned long *icb_addr_c)
 {
 	struct sh_mobile_meram_fb_cache *cache = data;
 	struct sh_mobile_meram_priv *priv = pdata->priv;
@@ -530,9 +530,9 @@ sh_mobile_meram_update(struct sh_mobile_meram_info *pdata, void *data,
 
 static struct sh_mobile_meram_ops sh_mobile_meram_ops = {
 	.module			= THIS_MODULE,
-	.meram_register		= sh_mobile_meram_register,
-	.meram_unregister	= sh_mobile_meram_unregister,
-	.meram_update		= sh_mobile_meram_update,
+	.meram_register		= __sh_mobile_meram_register,
+	.meram_unregister	= __sh_mobile_meram_unregister,
+	.meram_update		= __sh_mobile_meram_update,
 };
 
 /* -----------------------------------------------------------------------------
diff --git a/include/video/sh_mobile_meram.h b/include/video/sh_mobile_meram.h
index 29b2fd3..b03f823 100644
--- a/include/video/sh_mobile_meram.h
+++ b/include/video/sh_mobile_meram.h
@@ -60,4 +60,32 @@ struct sh_mobile_meram_ops {
 			     unsigned long *icb_addr_c);
 };
 
+static inline void *
+sh_mobile_meram_register(struct sh_mobile_meram_info *mdev,
+			 const struct sh_mobile_meram_cfg *cfg,
+			 unsigned int xres, unsigned int yres,
+			 unsigned int pixelformat, unsigned int *pitch)
+{
+	if (mdev = NULL || mdev->ops = NULL || cfg = NULL)
+		return NULL;
+
+	return mdev->ops->meram_register(mdev, cfg, xres, yres, pixelformat,
+					 pitch);
+}
+
+static inline void
+sh_mobile_meram_unregister(struct sh_mobile_meram_info *mdev, void *data)
+{
+	mdev->ops->meram_unregister(mdev, data);
+}
+
+static inline void
+sh_mobile_meram_update(struct sh_mobile_meram_info *mdev, void *data,
+		       unsigned long base_addr_y, unsigned long base_addr_c,
+		       unsigned long *icb_addr_y, unsigned long *icb_addr_c)
+{
+	mdev->ops->meram_update(mdev, data, base_addr_y, base_addr_c,
+				icb_addr_y, icb_addr_c);
+}
+
 #endif /* __VIDEO_SH_MOBILE_MERAM_H__  */
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH 2/2] fbdev: sh_mobile_lcdc: Use MERAM operations wrappers
From: Laurent Pinchart @ 2012-05-31 16:34 UTC (permalink / raw)
  To: linux-fbdev

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 7a0b301..671685d 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -857,7 +857,7 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 		 * re-initialize them.
 		 */
 		if (ch->meram) {
-			mdev->ops->meram_unregister(mdev, ch->meram);
+			sh_mobile_meram_unregister(mdev, ch->meram);
 			ch->meram = NULL;
 		}
 
@@ -880,11 +880,11 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 			break;
 		}
 
-		meram = mdev->ops->meram_register(mdev, ch->cfg->meram_cfg,
+		meram = sh_mobile_meram_register(mdev, ch->cfg->meram_cfg,
 					ch->pitch, ch->yres, pixelformat,
 					&ch->pitch);
 		if (!IS_ERR(meram)) {
-			mdev->ops->meram_update(mdev, meram,
+			sh_mobile_meram_update(mdev, meram,
 					ch->base_addr_y, ch->base_addr_c,
 					&ch->base_addr_y, &ch->base_addr_c);
 			ch->meram = meram;
@@ -1070,14 +1070,10 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 			base_addr_c += var->xoffset;
 	}
 
-	if (ch->meram) {
-		struct sh_mobile_meram_info *mdev;
-
-		mdev = priv->meram_dev;
-		mdev->ops->meram_update(mdev, ch->meram,
-					base_addr_y, base_addr_c,
-					&base_addr_y, &base_addr_c);
-	}
+	if (ch->meram)
+		sh_mobile_meram_update(priv->meram_dev, ch->meram,
+				       base_addr_y, base_addr_c,
+				       &base_addr_y, &base_addr_c);
 
 	ch->base_addr_y = base_addr_y;
 	ch->base_addr_c = base_addr_c;
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH 0/3] sh_mobile_lcdcfb overlays support
From: Laurent Pinchart @ 2012-05-31 17:21 UTC (permalink / raw)
  To: linux-fbdev

Hello,

These three patches implement overlays support for the sh_mobile_lcdcfb driver.

Compared to the previous version (posted as part of a larger patch set), the
sysfs entries are documented in Documentation/ABI and the DMA coherent memory
allocated for the planes is now freed at cleanup time.

Laurent Pinchart (3):
  fbdev: sh_mobile_lcdc: Constify sh_mobile_lcdc_fix structure
  fbdev: sh_mobile_lcdc: Rename fb operation handlers with a common
    prefix
  fbdev: sh_mobile_lcdc: Implement overlays support

 .../sysfs-devices-platform-sh_mobile_lcdc_fb       |   59 ++
 drivers/video/sh_mobile_lcdcfb.c                   | 1011 ++++++++++++++++++--
 include/video/sh_mobile_lcdc.h                     |    7 +
 3 files changed, 981 insertions(+), 96 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-sh_mobile_lcdc_fb

-- 
Regards,

Laurent Pinchart


^ 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