Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 0/3] sh_mobile_lcdc cleanup and fixes
From: Laurent Pinchart @ 2011-09-20 19:30 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1310766753-30314-1-git-send-email-laurent.pinchart@ideasonboard.com>

Hi Florian,

On Friday 15 July 2011 23:52:30 Laurent Pinchart wrote:
> Hi everybody,
> 
> When trying to understand the sh_mobile_lcdc driver, I found it hard to
> read statements that include hardcoded register values. I thus wrote a
> patch that replace them with macros, making the code more readable.
> 
> While doing so, I found two potential issues in the driver. See patches 2
> and 3 for detailed explanations and fixes.
> 
> Laurent Pinchart (3):
>   fbdev: sh_mobile_lcdc: Replace hardcoded register values with macros
>   fbdev: sh_mobile_lcdc: Don't acknowlege interrupts unintentionally
>   fbdev: sh_mobile_lcdc: Compute clock pattern using divider
>     denominator

Could you please pick these patches for v3.2 ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [PATCH v2 4/4] fbdev: sh_mobile_meram: Remove unneeded sh_mobile_meram.h
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
  To: linux-fbdev

The drivers/video/sh_mobile_meram.h header contains unused definitions
and declarations. Move the only used macro to sh_mobile_meram.c, and
remove the header.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |    2 +-
 drivers/video/sh_mobile_meram.c  |    5 +++--
 drivers/video/sh_mobile_meram.h  |   33 ---------------------------------
 3 files changed, 4 insertions(+), 36 deletions(-)
 delete mode 100644 drivers/video/sh_mobile_meram.h

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 0b7b492..088cb17 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -24,10 +24,10 @@
 #include <linux/backlight.h>
 #include <linux/gpio.h>
 #include <video/sh_mobile_lcdc.h>
+#include <video/sh_mobile_meram.h>
 #include <linux/atomic.h>
 
 #include "sh_mobile_lcdcfb.h"
-#include "sh_mobile_meram.h"
 
 #define SIDE_B_OFFSET 0x1000
 #define MIRROR_OFFSET 0x2000
diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 4ad083c..48c1828 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -16,8 +16,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
-
-#include "sh_mobile_meram.h"
+#include <video/sh_mobile_meram.h>
 
 /* meram registers */
 #define MEVCR1			0x0004
@@ -84,6 +83,8 @@
 	 ((yszm1) << MExxBSIZE_YSZM1_SHIFT) | \
 	 ((xszm1) << MExxBSIZE_XSZM1_SHIFT))
 
+#define SH_MOBILE_MERAM_ICB_NUM		32
+
 static unsigned long common_regs[] = {
 	MEVCR1,
 	MEQSEL1,
diff --git a/drivers/video/sh_mobile_meram.h b/drivers/video/sh_mobile_meram.h
deleted file mode 100644
index 1615204..0000000
--- a/drivers/video/sh_mobile_meram.h
+++ /dev/null
@@ -1,33 +0,0 @@
-#ifndef __sh_mobile_meram_h__
-#define __sh_mobile_meram_h__
-
-#include <linux/mutex.h>
-#include <video/sh_mobile_meram.h>
-
-/*
- * MERAM private
- */
-
-#define MERAM_ICB_Y 0x1
-#define MERAM_ICB_C 0x2
-
-/* MERAM cache size */
-#define SH_MOBILE_MERAM_ICB_NUM		32
-
-#define SH_MOBILE_MERAM_CACHE_OFFSET(p)	((p) >> 16)
-#define SH_MOBILE_MERAM_CACHE_SIZE(p)	((p) & 0xffff)
-
-int sh_mobile_meram_alloc_icb(const struct sh_mobile_meram_cfg *cfg,
-		   int xres,
-		   int yres,
-		   unsigned int base_addr,
-		   int yuv_mode,
-		   int *marker_icb,
-		   int *out_pitch);
-
-void sh_mobile_meram_free_icb(int marker_icb);
-
-#define SH_MOBILE_MERAM_START(ind, ab) \
-	(0xC0000000 | ((ab & 0x1) << 23) | ((ind & 0x1F) << 24))
-
-#endif /* !__sh_mobile_meram_h__ */
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 3/4] fbdev: sh_mobile_meram: Fix MExxCTL register save on runtime PM suspend
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
  To: linux-fbdev

To reset the ICB on resume the MExxCTL register needs to be OR'ed with
MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF, no set to that value. Fix this.

This fixes corruption at the bottom of the display when resuming from
runtime PM.

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

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index f7e77f6..4ad083c 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -552,7 +552,7 @@ static int sh_mobile_meram_runtime_suspend(struct device *dev)
 				meram_read_icb(priv->base, j, icb_regs[k]);
 			/* Reset ICB on resume */
 			if (icb_regs[k] = MExxCTL)
-				priv->icb_saved_regs[j * ICB_REGS_SIZE + k] +				priv->icb_saved_regs[j * ICB_REGS_SIZE + k] | 					MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF;
 		}
 	}
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 2/4] fbdev: sh_mobile_meram: Validate ICB configuration outside mutex
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
  To: linux-fbdev

Validate as much of the requested ICB configuration as possible outside
of the mutex-protected region when registering ICBs.

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

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 569888c..f7e77f6 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -413,24 +413,22 @@ static int sh_mobile_meram_register(struct sh_mobile_meram_info *pdata,
 		xres, yres, (!pixelformat) ? "yuv" : "rgb",
 		base_addr_y, base_addr_c);
 
-	mutex_lock(&priv->lock);
-
 	/* we can't handle wider than 8192px */
 	if (xres > 8192) {
 		dev_err(&pdev->dev, "width exceeding the limit (> 8192).");
-		error = -EINVAL;
-		goto err;
-	}
-
-	if (priv->used_meram_cache_regions + 2 > SH_MOBILE_MERAM_ICB_NUM) {
-		dev_err(&pdev->dev, "no more ICB available.");
-		error = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 	/* do we have at least one ICB config? */
 	if (cfg->icb[0].marker_icb < 0 || cfg->icb[0].cache_icb < 0) {
 		dev_err(&pdev->dev, "at least one ICB is required.");
+		return -EINVAL;
+	}
+
+	mutex_lock(&priv->lock);
+
+	if (priv->used_meram_cache_regions + 2 > SH_MOBILE_MERAM_ICB_NUM) {
+		dev_err(&pdev->dev, "no more ICB available.");
 		error = -EINVAL;
 		goto err;
 	}
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 1/4] fbdev: sh_mobile_meram: Replace hardcoded register values with macros
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
  To: linux-fbdev

Instead of hardcoding register values through the driver, define macros
for individual register bits using the register name and the bit name,
and use the macros.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_meram.c |  100 +++++++++++++++++++++++++++++----------
 1 files changed, 74 insertions(+), 26 deletions(-)

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 39f28a1..569888c 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -20,22 +20,69 @@
 #include "sh_mobile_meram.h"
 
 /* meram registers */
-#define MExxCTL 0x0
-#define MExxBSIZE 0x4
-#define MExxMNCF 0x8
-#define MExxSARA 0x10
-#define MExxSARB 0x14
-#define MExxSBSIZE 0x18
-
-#define MERAM_MExxCTL_VAL(ctl, next_icb, addr)	\
-	((ctl) | (((next_icb) & 0x1f) << 11) | (((addr) & 0x7ff) << 16))
-#define	MERAM_MExxBSIZE_VAL(a, b, c) \
-	(((a) << 28) | ((b) << 16) | (c))
-
-#define MEVCR1 0x4
-#define MEACTS 0x10
-#define MEQSEL1 0x40
-#define MEQSEL2 0x44
+#define MEVCR1			0x0004
+#define MEVCR1_RST		(1 << 31)
+#define MEVCR1_WD		(1 << 30)
+#define MEVCR1_AMD1		(1 << 29)
+#define MEVCR1_AMD0		(1 << 28)
+#define MEQSEL1			0x0040
+#define MEQSEL2			0x0044
+
+#define MExx_BASE		0x0400
+
+#define MExxCTL			0x0000
+#define MExxCTL_BV		(1 << 31)
+#define MExxCTL_BSZ_SHIFT	28
+#define MExxCTL_MSAR_MASK	(0x7ff << MExxCTL_MSAR_SHIFT)
+#define MExxCTL_MSAR_SHIFT	16
+#define MExxCTL_NXT_MASK	(0x1f << MExxCTL_NXT_SHIFT)
+#define MExxCTL_NXT_SHIFT	11
+#define MExxCTL_WD1		(1 << 10)
+#define MExxCTL_WD0		(1 << 9)
+#define MExxCTL_WS		(1 << 8)
+#define MExxCTL_CB		(1 << 7)
+#define MExxCTL_WBF		(1 << 6)
+#define MExxCTL_WF		(1 << 5)
+#define MExxCTL_RF		(1 << 4)
+#define MExxCTL_CM		(1 << 3)
+#define MExxCTL_MD_READ		(1 << 0)
+#define MExxCTL_MD_WRITE	(2 << 0)
+#define MExxCTL_MD_ICB_WB	(3 << 0)
+#define MExxCTL_MD_ICB		(4 << 0)
+#define MExxCTL_MD_FB		(7 << 0)
+#define MExxCTL_MD_MASK		(7 << 0)
+#define MExxBSIZE		0x0004
+#define MExxBSIZE_RCNT_SHIFT	28
+#define MExxBSIZE_YSZM1_SHIFT	16
+#define MExxBSIZE_XSZM1_SHIFT	0
+#define MExxMNCF		0x0008
+#define MExxMNCF_KWBNM_SHIFT	28
+#define MExxMNCF_KRBNM_SHIFT	24
+#define MExxMNCF_BNM_SHIFT	16
+#define MExxMNCF_XBV		(1 << 15)
+#define MExxMNCF_CPL_YCBCR444	(1 << 12)
+#define MExxMNCF_CPL_YCBCR420	(2 << 12)
+#define MExxMNCF_CPL_YCBCR422	(3 << 12)
+#define MExxMNCF_CPL_MSK	(3 << 12)
+#define MExxMNCF_BL		(1 << 2)
+#define MExxMNCF_LNM_SHIFT	0
+#define MExxSARA		0x0010
+#define MExxSARB		0x0014
+#define MExxSBSIZE		0x0018
+#define MExxSBSIZE_HDV		(1 << 31)
+#define MExxSBSIZE_HSZ16	(0 << 28)
+#define MExxSBSIZE_HSZ32	(1 << 28)
+#define MExxSBSIZE_HSZ64	(2 << 28)
+#define MExxSBSIZE_HSZ128	(3 << 28)
+#define MExxSBSIZE_SBSIZZ_SHIFT	0
+
+#define MERAM_MExxCTL_VAL(next, addr)	\
+	((((next) << MExxCTL_NXT_SHIFT) & MExxCTL_NXT_MASK) | \
+	 (((addr) << MExxCTL_MSAR_SHIFT) & MExxCTL_MSAR_MASK))
+#define	MERAM_MExxBSIZE_VAL(rcnt, yszm1, xszm1) \
+	(((rcnt) << MExxBSIZE_RCNT_SHIFT) | \
+	 ((yszm1) << MExxBSIZE_YSZM1_SHIFT) | \
+	 ((xszm1) << MExxBSIZE_XSZM1_SHIFT))
 
 static unsigned long common_regs[] = {
 	MEVCR1,
@@ -72,8 +119,8 @@ struct sh_mobile_meram_priv {
  * MERAM/ICB access functions
  */
 
-#define MERAM_ICB_OFFSET(base, idx, off)	\
-	((base) + (0x400 + ((idx) * 0x20) + (off)))
+#define MERAM_ICB_OFFSET(base, idx, off) \
+	(MExx_BASE + (base) + (off) + (idx) * 0x20)
 
 static inline void meram_write_icb(void __iomem *base, int idx, int off,
 	unsigned long val)
@@ -308,17 +355,18 @@ static int meram_init(struct sh_mobile_meram_priv *priv,
 	/*
 	 * Set MERAM for framebuffer
 	 *
-	 * 0x70f:  WD = 0x3, WS=0x1, CM=0x1, MDû mode
 	 * we also chain the cache_icb and the marker_icb.
 	 * we also split the allocated MERAM buffer between two ICBs.
 	 */
 	meram_write_icb(priv->base, icb->cache_icb, MExxCTL,
-			MERAM_MExxCTL_VAL(0x70f, icb->marker_icb,
-					  icb->meram_offset));
+			MERAM_MExxCTL_VAL(icb->marker_icb, icb->meram_offset) |
+			MExxCTL_WD1 | MExxCTL_WD0 | MExxCTL_WS | MExxCTL_CM |
+			MExxCTL_MD_FB);
 	meram_write_icb(priv->base, icb->marker_icb, MExxCTL,
-			MERAM_MExxCTL_VAL(0x70f, icb->cache_icb,
-					  icb->meram_offset +
-					  icb->meram_size / 2));
+			MERAM_MExxCTL_VAL(icb->cache_icb, icb->meram_offset +
+					  icb->meram_size / 2) |
+			MExxCTL_WD1 | MExxCTL_WD0 | MExxCTL_WS | MExxCTL_CM |
+			MExxCTL_MD_FB);
 
 	return 0;
 }
@@ -507,7 +555,7 @@ static int sh_mobile_meram_runtime_suspend(struct device *dev)
 			/* Reset ICB on resume */
 			if (icb_regs[k] = MExxCTL)
 				priv->icb_saved_regs[j * ICB_REGS_SIZE + k] -					0x70;
+					MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF;
 		}
 	}
 	return 0;
@@ -592,7 +640,7 @@ static int __devinit sh_mobile_meram_probe(struct platform_device *pdev)
 
 	/* initialize ICB addressing mode */
 	if (pdata->addr_mode = SH_MOBILE_MERAM_MODE1)
-		meram_write_reg(priv->base, MEVCR1, 1 << 29);
+		meram_write_reg(priv->base, MEVCR1, MEVCR1_AMD1);
 
 	pm_runtime_enable(&pdev->dev);
 
-- 
1.7.3.4


^ permalink raw reply related

* [PATCH v2 0/4] sh_mobile_meram cleanups and fixes
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
  To: linux-fbdev

Hi,

Here's the second version of the sh_mobile_meram and cleanup fixes. Compared
to v1, I've incorporated Damian's comments in the register definitions (first
patch).

Laurent Pinchart (4):
  fbdev: sh_mobile_meram: Replace hardcoded register values with macros
  fbdev: sh_mobile_meram: Validate ICB configuration outside mutex
  fbdev: sh_mobile_meram: Fix MExxCTL register save on runtime PM
    suspend
  fbdev: sh_mobile_meram: Remove unneeded sh_mobile_meram.h

 drivers/video/sh_mobile_lcdcfb.c |    2 +-
 drivers/video/sh_mobile_meram.c  |  125 ++++++++++++++++++++++++++------------
 drivers/video/sh_mobile_meram.h  |   33 ----------
 3 files changed, 87 insertions(+), 73 deletions(-)
 delete mode 100644 drivers/video/sh_mobile_meram.h

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Tomi Valkeinen @ 2011-09-20 18:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4E78C579.9060305@gmx.de>

On Tue, 2011-09-20 at 16:55 +0000, Florian Tobias Schandinat wrote:

> Did you have a look at the (existing) API [1] Laurent proposed for discovering
> the internal connections between the framebuffers (or with any other devices)?

I know the basics of media controller, but I haven't really looked at
the code.

> If you agree that it'd be a good idea to use it I feel that we should make the
> windowing API more compatible with it. So basically what we want to have as a

I can't say if MC should be used for fb or not, but I think something
similar should be used if we want to get overlays etc. supported. But
even with MC (or something else) there, I'm not quite sure how they fit
into the fb model.

I think the core problem here is that in a more complex setup (at least
as I see it for OMAP) the fb should be just a framebuffer in memory,
without any direct relation to hardware. The hardware part (an overlay,
or whichever is the corresponding element) will then use the framebuffer
as the pixel source.

However, the current fb model combines those two things into one. If we
manage to separate them, and add MC or similar, I think it'll work.

> window is one or more sunk pads so the pad-index should be also part of the
> interface. I'm still confused with how OMAP works when it does not have a "root"

Do you mean how the hardware works, or how I've designed omapfb driver?

> window/framebuffer. Normally I feel that the window position should be a
> property of the parent window as this is what the position is relative too. But
> if the parent is no framebuffer, should we also include the entity into the
> interface to allow configuring things that are nor even framebuffers?

Right, I think you're pondering the core problem here =).

On OMAP we have the display (the whole area shown on the panel/tv),
which has video timings and things like that. But no pixel data as such
(a configurable background color is there, though).

And then we have the overlays, which are somewhere on the display, and
the overlays get pixel data from memory (framebuffers).

So in a way we have a contentless root window, but presenting that with
an fb device doesn't feel right. And the fb device would logically be
fb0, and if it couldn't show any content it couldn't be used as default
framebuffer.

> Also I think we need a z-index in case overlays overlap (might happen or?) and
> enforcing that all z-indexes are different for the same entity.

Yes, free z-order is supported in OMAP4. Previous OMAPs had fixed
z-order, although in certain configuration (enable/disable alpha
blending) the fixed z-order does change...

> As I see it xres/yres (together with xoffset/yoffset) is always the visible part
> of the framebuffer. Typically that's also part of the timings as they define
> what is visible. With the introduction of overlays (and maybe even for some
> hardware anyway) it is no longer always true to have any timings at all. So on
> all framebuffer that do not have physical timings the timing interpretation is
> useless anyway (I'm thinking about adding a FB_CAP_NOTIMING) and what remains is
> the interpretation of xres/yres as visible screen region.

For a system where there's always a root window for every output, plus
variable size overlays, FB_CAP_NOTIMING makes sense. But it would still
leave the problem if there's no root window. How to change timings on a
system like OMAP?

 Tomi



^ permalink raw reply

* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Baruch Siach @ 2011-09-20 17:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAEC9eQOpwfmRfcxHoiA00xX0a=dxXj1xJDpW14DozGXKXPJ3Xg@mail.gmail.com>

Hi Ajay,

On Tue, Sep 20, 2011 at 08:56:57PM +0530, Ajay kumar wrote:
> Hi Baruch,
> On Tue, Sep 20, 2011 at 4:54 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> > Hi Ajay,
> >
> > On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
> >> This patch adds a data structure definiton to hold framebuffer windows/planes.
> >> An ioctl number is also added to provide user access
> >> to change window position dynamically.
> >
> > [snip]
> >
> >> +/* Window overlaying */
> >> +struct fb_overlay_win_pos {
> >> +     __u32 win_pos_x;        /* x-offset from LCD(0,0) where window starts */
> >> +     __u32 win_pos_y;        /* y-offset from LCD(0,0) where window starts */
> >> +};
> >
> > Why not allow negative offsets where the left or upper part of the framebuffer
> > is hidden?
> 
> Thanks for pointing it out. Are there drivers which place the overlay
> windows such that some part of the window is hidden from being
> displayed on the screen?

I don't know. However, since this is new userspace ABI which should stay 
compatible forever, we should make sure to do it right. Using __s32 instead of 
__u32 won't limit us in the future.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply

* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Florian Tobias Schandinat @ 2011-09-20 16:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316533193.18781.12.camel@deskari>

On 09/20/2011 03:39 PM, Tomi Valkeinen wrote:
> On Tue, 2011-09-20 at 20:16 +0530, Ajay kumar wrote:
>> Hi Tomi,
>>
>> On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
>>>> This patch adds a data structure definiton to hold framebuffer windows/planes.
>>>> An ioctl number is also added to provide user access
>>>> to change window position dynamically.

Ajay, do you need this urgently or can we delay this one merge window? I don't
think that a week or so is enough to get a consistent API that gets everything
right. So if you have a pressing need to have it within the 3.2 kernel I'd
prefer to do it only for your driver now and adjust it when we get the thing
done, probably in 3.3.

>>>>
>>>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>>>> Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
>>>> Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>>  include/linux/fb.h |    7 +++++++
>>>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>>>> index 1d6836c..2141941 100644
>>>> --- a/include/linux/fb.h
>>>> +++ b/include/linux/fb.h
>>>> @@ -39,6 +39,7 @@
>>>>  #define FBIOPUT_MODEINFO        0x4617
>>>>  #define FBIOGET_DISPINFO        0x4618
>>>>  #define FBIO_WAITFORVSYNC    _IOW('F', 0x20, __u32)
>>>> +#define FBIOPOS_OVERLAY_WIN  _IOW('F', 0x21, struct fb_overlay_win_pos)
>>>>
>>>>  #define FB_TYPE_PACKED_PIXELS                0       /* Packed Pixels        */
>>>>  #define FB_TYPE_PLANES                       1       /* Non interleaved planes */
>>>> @@ -366,6 +367,12 @@ struct fb_image {
>>>>       struct fb_cmap cmap;    /* color map info */
>>>>  };
>>>>
>>>> +/* Window overlaying */
>>>> +struct fb_overlay_win_pos {
>>>> +     __u32 win_pos_x;        /* x-offset from LCD(0,0) where window starts */
>>>> +     __u32 win_pos_y;        /* y-offset from LCD(0,0) where window starts */
>>>> +};
>>>
>>> Shouldn't this also include the window size (in case scaling is
>>> supported)?
>>
>> The "xres" and "yres" fields in fb_var_screeninfo are being used to
>> represent the size of the window (visible resolution). So we have,
>>
>> win_pos_x: x-offset from LCD(0,0) where window starts.
>> win_pos_y: y-offset from LCD(0,0) where window starts.
>> (win_pos_x + xres) : x-offset from LCD(0,0) where window ends.
>> (win_pos_y + yres) : y-offset from LCD(0,0) where window ends.
> 
> Sure, but the xres and yres tell the _input_ resolution, i.e. how many
> pixels are read from the memory. What is missing is the _output_
> resolution, which is the size of the window. These are not necessarily
> the same, if the system supports scaling.

I agree, scaling is an issue that should get solved on the way. So adding
u32 width, height;
with an initial/special value of 0 which means just take what the source
width/height is.

>>> This also won't work for setups where the same framebuffer is used by
>>> multiple overlays. For example, this is the case on OMAP when the same
>>> content is cloned to, say, LCD and TV, each of which is showing an
>>> overlay.
>>
>> These x and y position are used to configure the display controller
>> (for LCD only) and not to alter the data in physical buffer
>> (framebuffer). Could you elaborate the above use case you have
>> mentioned and how adding the x and y offsets would not meet that
>> requirement.
> 
> Nothing wrong with adding x/y offsets, but the problem is in configuring
> the two overlays. If the framebuffer data is used by two overlays, each
> overlay should be configured separately. And your ioctl does not have
> any way to define which overlay is being affected.

Did you have a look at the (existing) API [1] Laurent proposed for discovering
the internal connections between the framebuffers (or with any other devices)?
If you agree that it'd be a good idea to use it I feel that we should make the
windowing API more compatible with it. So basically what we want to have as a
window is one or more sunk pads so the pad-index should be also part of the
interface. I'm still confused with how OMAP works when it does not have a "root"
window/framebuffer. Normally I feel that the window position should be a
property of the parent window as this is what the position is relative too. But
if the parent is no framebuffer, should we also include the entity into the
interface to allow configuring things that are nor even framebuffers?
Also I think we need a z-index in case overlays overlap (might happen or?) and
enforcing that all z-indexes are different for the same entity.

> Of course, if we specify that a single framebuffer will ever go only to
> one output, the problem disappears.
> 
> However, even if we specify so, this will make the fbdev a bit weird:
> what is x/yres after this patch? In the current fbdev x/yres is the size
> of the output, and x/yres are part of video timings. After this patch
> this is no longer the case: x/yres will be the size of the overlay. But
> the old code will still use x/yres as part of video timings, making
> things confusing.

As I see it xres/yres (together with xoffset/yoffset) is always the visible part
of the framebuffer. Typically that's also part of the timings as they define
what is visible. With the introduction of overlays (and maybe even for some
hardware anyway) it is no longer always true to have any timings at all. So on
all framebuffer that do not have physical timings the timing interpretation is
useless anyway (I'm thinking about adding a FB_CAP_NOTIMING) and what remains is
the interpretation of xres/yres as visible screen region.

> And generally I can't really make my mind about adding these more
> complex features. On one hand it would be very nice to have fbdev
> supporting overlays and whatnot, but on the other hand, I can't figure
> out how to add them properly.

I don't see it as adding new features, rather unifying what is already there for
easier use. Sure it should be done in a consistent way.


Best regards,

Florian Tobias Schandinat


[1] http://linuxtv.org/downloads/v4l-dvb-apis/media_common.html

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Keith Packard @ 2011-09-20 15:55 UTC (permalink / raw)
  To: Patrik Jakobsson, Tomi Valkeinen
  Cc: Alan Cox, linux-fbdev, linux-kernel, dri-devel, linaro-dev,
	Clark, Rob, Archit Taneja
In-Reply-To: <CAMeQTsbP-_ut7BA3YQ+qgtqfM1tzM4qn3jxHT8B8_0hdPbP3Jg@mail.gmail.com>

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

On Tue, 20 Sep 2011 10:29:23 +0200, Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote:

> It would be nice to have a model that fits both DSI and SDVO, and the option
> to configure some of it from userspace.

> I thought the purpose of drm_encoder was to abstract hardware like this?

SDVO is entirely hidden by the drm_encoder interface; some of the
controls (like TV encoder parameters) are exposed through DRM
properties, others are used in the basic configuration of the device.

I'm not sure we need a new abstraction that subsumes both DSI and SDVO,
but we may need a DSI library that can be used by both DRM and other
parts of the kernel.

-- 
keith.packard@intel.com

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

^ permalink raw reply

* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Tomi Valkeinen @ 2011-09-20 15:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAEC9eQOTNaMcUWeSFN-DCTYrCJsVaDAO7uOZNQM=efjVdqtU2g@mail.gmail.com>

On Tue, 2011-09-20 at 20:16 +0530, Ajay kumar wrote:
> Hi Tomi,
> 
> On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
> >> This patch adds a data structure definiton to hold framebuffer windows/planes.
> >> An ioctl number is also added to provide user access
> >> to change window position dynamically.
> >>
> >> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> >> Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
> >> Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >>  include/linux/fb.h |    7 +++++++
> >>  1 files changed, 7 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/fb.h b/include/linux/fb.h
> >> index 1d6836c..2141941 100644
> >> --- a/include/linux/fb.h
> >> +++ b/include/linux/fb.h
> >> @@ -39,6 +39,7 @@
> >>  #define FBIOPUT_MODEINFO        0x4617
> >>  #define FBIOGET_DISPINFO        0x4618
> >>  #define FBIO_WAITFORVSYNC    _IOW('F', 0x20, __u32)
> >> +#define FBIOPOS_OVERLAY_WIN  _IOW('F', 0x21, struct fb_overlay_win_pos)
> >>
> >>  #define FB_TYPE_PACKED_PIXELS                0       /* Packed Pixels        */
> >>  #define FB_TYPE_PLANES                       1       /* Non interleaved planes */
> >> @@ -366,6 +367,12 @@ struct fb_image {
> >>       struct fb_cmap cmap;    /* color map info */
> >>  };
> >>
> >> +/* Window overlaying */
> >> +struct fb_overlay_win_pos {
> >> +     __u32 win_pos_x;        /* x-offset from LCD(0,0) where window starts */
> >> +     __u32 win_pos_y;        /* y-offset from LCD(0,0) where window starts */
> >> +};
> >
> > Shouldn't this also include the window size (in case scaling is
> > supported)?
> 
> The "xres" and "yres" fields in fb_var_screeninfo are being used to
> represent the size of the window (visible resolution). So we have,
> 
> win_pos_x: x-offset from LCD(0,0) where window starts.
> win_pos_y: y-offset from LCD(0,0) where window starts.
> (win_pos_x + xres) : x-offset from LCD(0,0) where window ends.
> (win_pos_y + yres) : y-offset from LCD(0,0) where window ends.

Sure, but the xres and yres tell the _input_ resolution, i.e. how many
pixels are read from the memory. What is missing is the _output_
resolution, which is the size of the window. These are not necessarily
the same, if the system supports scaling.

> > This also won't work for setups where the same framebuffer is used by
> > multiple overlays. For example, this is the case on OMAP when the same
> > content is cloned to, say, LCD and TV, each of which is showing an
> > overlay.
> 
> These x and y position are used to configure the display controller
> (for LCD only) and not to alter the data in physical buffer
> (framebuffer). Could you elaborate the above use case you have
> mentioned and how adding the x and y offsets would not meet that
> requirement.

Nothing wrong with adding x/y offsets, but the problem is in configuring
the two overlays. If the framebuffer data is used by two overlays, each
overlay should be configured separately. And your ioctl does not have
any way to define which overlay is being affected.

Of course, if we specify that a single framebuffer will ever go only to
one output, the problem disappears.

However, even if we specify so, this will make the fbdev a bit weird:
what is x/yres after this patch? In the current fbdev x/yres is the size
of the output, and x/yres are part of video timings. After this patch
this is no longer the case: x/yres will be the size of the overlay. But
the old code will still use x/yres as part of video timings, making
things confusing.

And generally I can't really make my mind about adding these more
complex features. On one hand it would be very nice to have fbdev
supporting overlays and whatnot, but on the other hand, I can't figure
out how to add them properly.

 Tomi



^ permalink raw reply

* Re: [PATCH 1/3] include: fb: Add definiton for window positioning structure
From: Ajay kumar @ 2011-09-20 15:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110920112427.GB19695@sapphire.tkos.co.il>

Hi Baruch,

On Tue, Sep 20, 2011 at 4:54 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Ajay,
>
> On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
>> This patch adds a data structure definiton to hold framebuffer windows/planes.
>> An ioctl number is also added to provide user access
>> to change window position dynamically.
>
> [snip]
>
>> +/* Window overlaying */
>> +struct fb_overlay_win_pos {
>> +     __u32 win_pos_x;        /* x-offset from LCD(0,0) where window starts */
>> +     __u32 win_pos_y;        /* y-offset from LCD(0,0) where window starts */
>> +};
>
> Why not allow negative offsets where the left or upper part of the framebuffer
> is hidden?

Thanks for pointing it out. Are there drivers which place the overlay
windows such that some part of the window is hidden from being
displayed on the screen?

> baruch
>
> --
>                                                     ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{>   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Ajay

^ permalink raw reply

* Re: [PATCH 1/3] include: fb: Add definiton for window positioning structure
From: Ajay kumar @ 2011-09-20 14:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316517021.1949.13.camel@deskari>

Hi Tomi,

On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
>> This patch adds a data structure definiton to hold framebuffer windows/planes.
>> An ioctl number is also added to provide user access
>> to change window position dynamically.
>>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
>> Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  include/linux/fb.h |    7 +++++++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index 1d6836c..2141941 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -39,6 +39,7 @@
>>  #define FBIOPUT_MODEINFO        0x4617
>>  #define FBIOGET_DISPINFO        0x4618
>>  #define FBIO_WAITFORVSYNC    _IOW('F', 0x20, __u32)
>> +#define FBIOPOS_OVERLAY_WIN  _IOW('F', 0x21, struct fb_overlay_win_pos)
>>
>>  #define FB_TYPE_PACKED_PIXELS                0       /* Packed Pixels        */
>>  #define FB_TYPE_PLANES                       1       /* Non interleaved planes */
>> @@ -366,6 +367,12 @@ struct fb_image {
>>       struct fb_cmap cmap;    /* color map info */
>>  };
>>
>> +/* Window overlaying */
>> +struct fb_overlay_win_pos {
>> +     __u32 win_pos_x;        /* x-offset from LCD(0,0) where window starts */
>> +     __u32 win_pos_y;        /* y-offset from LCD(0,0) where window starts */
>> +};
>
> Shouldn't this also include the window size (in case scaling is
> supported)?

The "xres" and "yres" fields in fb_var_screeninfo are being used to
represent the size of the window (visible resolution). So we have,

win_pos_x: x-offset from LCD(0,0) where window starts.
win_pos_y: y-offset from LCD(0,0) where window starts.
(win_pos_x + xres) : x-offset from LCD(0,0) where window ends.
(win_pos_y + yres) : y-offset from LCD(0,0) where window ends.

> This also won't work for setups where the same framebuffer is used by
> multiple overlays. For example, this is the case on OMAP when the same
> content is cloned to, say, LCD and TV, each of which is showing an
> overlay.

These x and y position are used to configure the display controller
(for LCD only) and not to alter the data in physical buffer
(framebuffer). Could you elaborate the above use case you have
mentioned and how adding the x and y offsets would not meet that
requirement.

>  Tomi
>

Ajay

^ permalink raw reply

* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Baruch Siach @ 2011-09-20 11:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316532641-2657-2-git-send-email-ajaykumar.rs@samsung.com>

Hi Ajay,

On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
> This patch adds a data structure definiton to hold framebuffer windows/planes.
> An ioctl number is also added to provide user access
> to change window position dynamically.

[snip]

> +/* Window overlaying */
> +struct fb_overlay_win_pos {
> +	__u32 win_pos_x;	/* x-offset from LCD(0,0) where window starts */
> +	__u32 win_pos_y;	/* y-offset from LCD(0,0) where window starts */
> +};

Why not allow negative offsets where the left or upper part of the framebuffer 
is hidden?

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply

* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Tomi Valkeinen @ 2011-09-20 11:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316532641-2657-2-git-send-email-ajaykumar.rs@samsung.com>

On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
> This patch adds a data structure definiton to hold framebuffer windows/planes.
> An ioctl number is also added to provide user access
> to change window position dynamically.
> 
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
> Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  include/linux/fb.h |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 1d6836c..2141941 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -39,6 +39,7 @@
>  #define FBIOPUT_MODEINFO        0x4617
>  #define FBIOGET_DISPINFO        0x4618
>  #define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
> +#define FBIOPOS_OVERLAY_WIN	_IOW('F', 0x21, struct fb_overlay_win_pos)
>  
>  #define FB_TYPE_PACKED_PIXELS		0	/* Packed Pixels	*/
>  #define FB_TYPE_PLANES			1	/* Non interleaved planes */
> @@ -366,6 +367,12 @@ struct fb_image {
>  	struct fb_cmap cmap;	/* color map info */
>  };
>  
> +/* Window overlaying */
> +struct fb_overlay_win_pos {
> +	__u32 win_pos_x;	/* x-offset from LCD(0,0) where window starts */
> +	__u32 win_pos_y;	/* y-offset from LCD(0,0) where window starts */
> +};

Shouldn't this also include the window size (in case scaling is
supported)?

This also won't work for setups where the same framebuffer is used by
multiple overlays. For example, this is the case on OMAP when the same
content is cloned to, say, LCD and TV, each of which is showing an
overlay.

 Tomi



^ permalink raw reply

* [PATCH 2/3] ARM: SAMSUNG: Embed window positioning data structure in
From: Ajay Kumar @ 2011-09-20  9:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316532641-2657-1-git-send-email-ajaykumar.rs@samsung.com>

This patch adds a new field in s3c_fb_pd_win structure,
thereby allowing the user to place window at the desired position.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/plat-samsung/include/plat/fb.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/fb.h b/arch/arm/plat-samsung/include/plat/fb.h
index 0fedf47..920decd 100644
--- a/arch/arm/plat-samsung/include/plat/fb.h
+++ b/arch/arm/plat-samsung/include/plat/fb.h
@@ -27,6 +27,7 @@
  * @win_mode: The display parameters to initialise (not for window 0)
  * @virtual_x: The virtual X size.
  * @virtual_y: The virtual Y size.
+ * @winpos: Position of overlayed window w.r.t the LCD screen.
  */
 struct s3c_fb_pd_win {
 	struct fb_videomode	win_mode;
@@ -35,6 +36,8 @@ struct s3c_fb_pd_win {
 	unsigned short		max_bpp;
 	unsigned short		virtual_x;
 	unsigned short		virtual_y;
+
+	struct fb_overlay_win_pos	winpos;
 };
 
 /**
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 3/3] video: s3c-fb: Modify s3c-fb driver to support window
From: Ajay Kumar @ 2011-09-20  9:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316532641-2657-1-git-send-email-ajaykumar.rs@samsung.com>

Positions the framebuffer window during driver initialization,
using the platform data.It also adds an ioctl definition
to change window position dynamically.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/video/s3c-fb.c |   37 ++++++++++++++++++++++++++++++++-----
 1 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 0fda252..b087a12 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -441,6 +441,7 @@ static int s3c_fb_set_par(struct fb_info *info)
 {
 	struct fb_var_screeninfo *var = &info->var;
 	struct s3c_fb_win *win = info->par;
+	struct fb_overlay_win_pos *winpos = &win->windata->winpos;
 	struct s3c_fb *sfb = win->parent;
 	void __iomem *regs = sfb->regs;
 	void __iomem *buf = regs;
@@ -539,12 +540,13 @@ static int s3c_fb_set_par(struct fb_info *info)
 
 	/* write 'OSD' registers to control position of framebuffer */
 
-	data = VIDOSDxA_TOPLEFT_X(0) | VIDOSDxA_TOPLEFT_Y(0);
+	data = VIDOSDxA_TOPLEFT_X(winpos->win_pos_x) |
+	       VIDOSDxA_TOPLEFT_Y(winpos->win_pos_y);
 	writel(data, regs + VIDOSD_A(win_no, sfb->variant));
 
-	data = VIDOSDxB_BOTRIGHT_X(s3c_fb_align_word(var->bits_per_pixel,
-						     var->xres - 1)) |
-	       VIDOSDxB_BOTRIGHT_Y(var->yres - 1);
+	data = VIDOSDxB_BOTRIGHT_X((s3c_fb_align_word(var->bits_per_pixel,
+	       (winpos->win_pos_x + var->xres - 1)))) |
+	       VIDOSDxB_BOTRIGHT_Y((winpos->win_pos_y + var->yres - 1));
 
 	writel(data, regs + VIDOSD_B(win_no, sfb->variant));
 
@@ -998,9 +1000,11 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
 			unsigned long arg)
 {
 	struct s3c_fb_win *win = info->par;
+	struct fb_overlay_win_pos *winpos = &win->windata->winpos;
 	struct s3c_fb *sfb = win->parent;
-	int ret;
+	int ret = 0;
 	u32 crtc;
+	u32 data;
 
 	switch (cmd) {
 	case FBIO_WAITFORVSYNC:
@@ -1011,6 +1015,29 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
 
 		ret = s3c_fb_wait_for_vsync(sfb, crtc);
 		break;
+	case FBIOPOS_OVERLAY_WIN:
+		if (copy_from_user(winpos, (u32 __user *)arg,
+					sizeof(struct fb_overlay_win_pos))) {
+			ret = -EFAULT;
+			break;
+		}
+
+		shadow_protect_win(win, 1);
+
+		/* write 'OSD' registers to set position of the window */
+		data = VIDOSDxA_TOPLEFT_X(winpos->win_pos_x) |
+		       VIDOSDxA_TOPLEFT_Y(winpos->win_pos_y);
+		writel(data, sfb->regs + VIDOSD_A(win->index, sfb->variant));
+
+		data = VIDOSDxB_BOTRIGHT_X(
+				s3c_fb_align_word(info->var.bits_per_pixel,
+				(winpos->win_pos_x + info->var.xres - 1)));
+		data |=	VIDOSDxB_BOTRIGHT_Y(winpos->win_pos_y +
+				info->var.yres - 1);
+		writel(data, sfb->regs + VIDOSD_B(win->index, sfb->variant));
+
+		shadow_protect_win(win, 0);
+		break;
 	default:
 		ret = -ENOTTY;
 	}
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/3] include: fb: Add definiton for window positioning structure
From: Ajay Kumar @ 2011-09-20  9:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1316532641-2657-1-git-send-email-ajaykumar.rs@samsung.com>

This patch adds a data structure definiton to hold framebuffer windows/planes.
An ioctl number is also added to provide user access
to change window position dynamically.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 include/linux/fb.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/linux/fb.h b/include/linux/fb.h
index 1d6836c..2141941 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -39,6 +39,7 @@
 #define FBIOPUT_MODEINFO        0x4617
 #define FBIOGET_DISPINFO        0x4618
 #define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
+#define FBIOPOS_OVERLAY_WIN	_IOW('F', 0x21, struct fb_overlay_win_pos)
 
 #define FB_TYPE_PACKED_PIXELS		0	/* Packed Pixels	*/
 #define FB_TYPE_PLANES			1	/* Non interleaved planes */
@@ -366,6 +367,12 @@ struct fb_image {
 	struct fb_cmap cmap;	/* color map info */
 };
 
+/* Window overlaying */
+struct fb_overlay_win_pos {
+	__u32 win_pos_x;	/* x-offset from LCD(0,0) where window starts */
+	__u32 win_pos_y;	/* y-offset from LCD(0,0) where window starts */
+};
+
 /*
  * hardware cursor control
  */
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 0/3] FB: Add window positioning support
From: Ajay Kumar @ 2011-09-20  9:28 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset adds support for positioning the overlay windows on the LCD.

These patches are based on the discussion about adding features to fbdev at:
http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg06292.html

To Florian:
  	include: fb: Add definiton for window positioning structure
  	video: s3c-fb: Modify s3c-fb driver to support window positioning

To Kukjin,
  	ARM: SAMSUNG: Embed window positioning data structure in s3c-fb plat
  		      data

 arch/arm/plat-samsung/include/plat/fb.h |    3 ++
 drivers/video/s3c-fb.c                  |   37 ++++++++++++++++++++++++++----
 include/linux/fb.h                      |    7 ++++++
 3 files changed, 42 insertions(+), 5 deletions(-)


^ permalink raw reply

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

On Mon, Sep 19, 2011 at 9:29 AM, Tomi Valkeinen wrote:
>> So DSI is more like i2c than the DisplayPort aux channel or DDC. That
>
> Well, not quite. DSI is like DisplayPort and DisplayPort aux combined;
> there's only one bus, DSI, which is used to transfer video data and
> commands.
>
> For DSI video mode, the transfer is somewhat like traditional displays,
> and video data is send according to a pixel clock as a constant stream.
> However, before the video stream is enabled the bus can be used in
> bi-directional communication. And even when the video stream is enabled,
> there can be other communication in the blanking periods.

This sounds a lot like SDVO. You communicate with the SDVO chip through
i2c and then do a bus switch to get to the DDC. You also have the GMBus
with interrupt support that can help you do the i2c transfers.

SDVO supports many connectors and can have multiple in and out channels
so some setups are a bit complicated.

It would be nice to have a model that fits both DSI and SDVO, and the option
to configure some of it from userspace.

I thought the purpose of drm_encoder was to abstract hardware like this?

-Patrik

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Tomi Valkeinen @ 2011-09-19  7:29 UTC (permalink / raw)
  To: Keith Packard
  Cc: Alan Cox, linux-fbdev, linux-kernel, dri-devel, linaro-dev,
	Clark, Rob, Archit Taneja
In-Reply-To: <yunfwjtauqg.fsf@aiko.keithp.com>

On Sun, 2011-09-18 at 23:53 -0700, Keith Packard wrote:
> On Mon, 19 Sep 2011 09:33:34 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 
> > I think it's a bit more complex than that. True, there are MIPI
> > standards, for the video there are DPI, DBI, DSI, and for the commands
> > there is DCS. And, as you mentioned, many panels need custom
> > initialization, or support only parts of the DCS, or have other
> > quirks.
> 
> So DSI is more like i2c than the DisplayPort aux channel or DDC. That

Well, not quite. DSI is like DisplayPort and DisplayPort aux combined;
there's only one bus, DSI, which is used to transfer video data and
commands.

For DSI video mode, the transfer is somewhat like traditional displays,
and video data is send according to a pixel clock as a constant stream.
However, before the video stream is enabled the bus can be used in
bi-directional communication. And even when the video stream is enabled,
there can be other communication in the blanking periods.

For DSI command mode the transfer is a bit like high speed i2c; messages
are sent when needed (when the userspace gives the command to update),
without any strict timings. In practice this means that the peripheral
needs to have a framebuffer memory of its own, which it uses to refresh
the actual panel (or send the pixels forward to another peripheral).

As the use patterns of these two types of displays are quite different,
we have the terms auto-update and manual-update displays for them.

> seems fine; you can create a DSI infrastructure like the i2c
> infrastructure and then just have your display drivers use it to talk to
> the panel. We might eventually end up with some shared DRM code to deal
> with common DSI functions for display devices, like the EDID code
> today, but that doesn't need to happen before you can write your first
> DSI-using display driver.

One difference with i2c and DSI is that i2c is independent of the video
path, so it's easy to keep that separate from DRM. But for DSI the data
is produced by the video hardware using the overlays, encoders etc. I
don't quite see how we could have an i2c-like separate DSI API, which
wasn't part of DRM.

And even in simpler case MIPI DPI, which is a traditional parallel RGB
interface, a panel may need custom configuration via, say, i2c or spi.
We can, of course, create a i2c device driver for the panel, but how is
that then connected to DRM? The i2c driver may need to know things like
when the display is enabled/disabled, about backlight changes, or any
other display related event.

Is there a way for the i2c driver to get these events, and add new
properties to the DRM (say, if the panel has a feature configured via
i2c, but we'd like it to be visible to the userspace via the DRM
driver)?

 Tomi



^ permalink raw reply

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

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

On Mon, 19 Sep 2011 09:33:34 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> I think it's a bit more complex than that. True, there are MIPI
> standards, for the video there are DPI, DBI, DSI, and for the commands
> there is DCS. And, as you mentioned, many panels need custom
> initialization, or support only parts of the DCS, or have other
> quirks.

So DSI is more like i2c than the DisplayPort aux channel or DDC. That
seems fine; you can create a DSI infrastructure like the i2c
infrastructure and then just have your display drivers use it to talk to
the panel. We might eventually end up with some shared DRM code to deal
with common DSI functions for display devices, like the EDID code
today, but that doesn't need to happen before you can write your first
DSI-using display driver.

-- 
keith.packard@intel.com

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

^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-19  6:50 UTC (permalink / raw)
  To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9upB3VUaRme84Fm6ewx9KkyOu4hCavMetkEeKcSHORXw@mail.gmail.com>

On Fri, 2011-09-16 at 18:11 +0530, K, Mythri P wrote:
> Hi,

> >> Sequence with HPD:
> >> 1.Register for HPD connect.
> >> 2.Enable display
> >> 3.Notify DRM/Audio/Kernel component that wants to listen to this event.
> >
> > Why would you enable the display even if there's no monitor connected?
> >
> > And when the DRM starts, how does DRM know if the display was already
> > connected? Would you send a HPD event when DRM registers to the event
> > even if there's no actual plug-in event done (i.e. user actually
> > connecting the cable)?
> >
> HPD event would be triggered only when the cable is connected , and
> the EDID is ready to be read by the monitor. So the question enabling
> display doesnt exist. When HDMI is enabled in the HPD mode it will be
> in minimal power mode.

I don't think that will work correctly. "enable" in the current driver
means that the DISPC output will be enabled, and the display is showing
an image.

How would you implement the case with your model where the user wants to
get the EDID information, but doesn't want to enable the display yet
(send an output signal)?

I think we need a new power state for the displays. DSI panels could use
it also, as there's a need to turn them on for configuration before
actually turning on the actual display.

 Tomi



^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Tomi Valkeinen @ 2011-09-19  6:33 UTC (permalink / raw)
  To: Alan Cox
  Cc: Keith Packard, linux-fbdev, linux-kernel, dri-devel, linaro-dev,
	Clark, Rob, Archit Taneja
In-Reply-To: <20110916175326.54567b14@lxorguk.ukuu.org.uk>

On Fri, 2011-09-16 at 17:53 +0100, Alan Cox wrote:
> > 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.
> 
> DSI and the various other MIPI bits tend to be horribly panel and device
> specific. In one sense yes its a standard with standard commands,
> processes, queries etc, on the other a lot of stuff is oriented around
> the 'its a fixed configuration unit we don't need to have queries' view.

I think it's a bit more complex than that. True, there are MIPI
standards, for the video there are DPI, DBI, DSI, and for the commands
there is DCS. And, as you mentioned, many panels need custom
initialization, or support only parts of the DCS, or have other quirks.

However, I think the biggest thing to realize here is that DSI
peripherals can be anything. It's not always so that you have a DSI bus
and a single panel connected to it. You can have hubs, buffer chips,
etc.

We need DSI peripheral device drivers for those, a single "DSI output"
driver cannot work. And this is also the most important thing in my
original proposition.

> There also tends to be a lot of vendor magic initialisation logic both
> chipset and device dependant, and often 'plumbing dependant' on SoC

While I have DSI experience with only one SoC, I do believe it'd be
possible to create a DSI API that would allow us to have platform
independent DSI peripheral drivers.

Can you share any SoC side DSI peculiarities on Intel's SoCs?

 Tomi



^ permalink raw reply

* linux-next: manual merge of the fbdev tree with the mips tree
From: Stephen Rothwell @ 2011-09-19  6:22 UTC (permalink / raw)
  To: Florian Tobias Schandinat, linux-fbdev
  Cc: linux-next, linux-kernel, Manuel Lauss, Ralf Baechle, Paul Mundt

Hi all,

Today's linux-next merge of the fbdev tree got a conflict in
drivers/video/Kconfig between commit 9d7dec27ace1 ("MIPS: Alchemy: remove
all CONFIG_SOC_AU1??? defines") from the mips tree and commit
4ee584615102 ("au1200fb: switch to FB_SYS helpers") from the fbdev tree.

Just context changes. I fixed it up (see below) and can carry the fix as
necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/video/Kconfig
index 55a7df4,8165c55..0000000
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@@ -1755,10 -1764,11 +1764,11 @@@ config FB_AU110
  
  config FB_AU1200
  	bool "Au1200 LCD Driver"
 -	depends on (FB = y) && MIPS && SOC_AU1200
 +	depends on (FB = y) && MIPS_ALCHEMY
- 	select FB_CFB_FILLRECT
- 	select FB_CFB_COPYAREA
- 	select FB_CFB_IMAGEBLIT
+ 	select FB_SYS_FILLRECT
+ 	select FB_SYS_COPYAREA
+ 	select FB_SYS_IMAGEBLIT
+ 	select FB_SYS_FOPS
  	help
  	  This is the framebuffer driver for the AMD Au1200 SOC.  It can drive
  	  various panels and CRTs by passing in kernel cmd line option

^ 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