* [PATCH v11 5/8] ARM: dts: omap: update usb_otg_hs data
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Updated the usb_otg_hs dt data to include the *phy* and *phy-names*
binding in order for the driver to use the new generic PHY framework.
Also updated the Documentation to include the binding information.
The PHY binding information can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +++++
Documentation/devicetree/bindings/usb/usb-phy.txt | 6 ++++++
arch/arm/boot/dts/omap3-beagle-xm.dts | 2 ++
arch/arm/boot/dts/omap3-evm.dts | 2 ++
arch/arm/boot/dts/omap3-overo.dtsi | 2 ++
arch/arm/boot/dts/omap4.dtsi | 3 +++
arch/arm/boot/dts/twl4030.dtsi | 1 +
7 files changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 57e71f6..825790d 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -19,6 +19,9 @@ OMAP MUSB GLUE
- power : Should be "50". This signifies the controller can supply up to
100mA when operating in host mode.
- usb-phy : the phandle for the PHY device
+ - phys : the phandle for the PHY device (used by generic PHY framework)
+ - phy-names : the names of the PHY corresponding to the PHYs present in the
+ *phy* phandle.
Optional properties:
- ctrl-module : phandle of the control module this glue uses to write to
@@ -33,6 +36,8 @@ usb_otg_hs: usb_otg_hs@4a0ab000 {
num-eps = <16>;
ram-bits = <12>;
ctrl-module = <&omap_control_usb>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
};
Board specific device node entry
diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 61496f5..c0245c8 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -5,6 +5,8 @@ OMAP USB2 PHY
Required properties:
- compatible: Should be "ti,omap-usb2"
- reg : Address and length of the register set for the device.
+ - #phy-cells: determine the number of cells that should be given in the
+ phandle while referencing this phy.
Optional properties:
- ctrl-module : phandle of the control module used by PHY driver to power on
@@ -16,6 +18,7 @@ usb2phy@4a0ad080 {
compatible = "ti,omap-usb2";
reg = <0x4a0ad080 0x58>;
ctrl-module = <&omap_control_usb>;
+ #phy-cells = <0>;
};
OMAP USB3 PHY
@@ -25,6 +28,8 @@ Required properties:
- reg : Address and length of the register set for the device.
- reg-names: The names of the register addresses corresponding to the registers
filled in "reg".
+ - #phy-cells: determine the number of cells that should be given in the
+ phandle while referencing this phy.
Optional properties:
- ctrl-module : phandle of the control module used by PHY driver to power on
@@ -39,4 +44,5 @@ usb3phy@4a084400 {
<0x4a084c00 0x40>;
reg-names = "phy_rx", "phy_tx", "pll_ctrl";
ctrl-module = <&omap_control_usb>;
+ #phy-cells = <0>;
};
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
index afdb164..533b2da 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -144,6 +144,8 @@
&usb_otg_hs {
interface-type = <0>;
usb-phy = <&usb2_phy>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
mode = <3>;
power = <50>;
};
diff --git a/arch/arm/boot/dts/omap3-evm.dts b/arch/arm/boot/dts/omap3-evm.dts
index 7d4329d..4134dd0 100644
--- a/arch/arm/boot/dts/omap3-evm.dts
+++ b/arch/arm/boot/dts/omap3-evm.dts
@@ -70,6 +70,8 @@
&usb_otg_hs {
interface-type = <0>;
usb-phy = <&usb2_phy>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
mode = <3>;
power = <50>;
};
diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
index 8f1abec..a461d2f 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -76,6 +76,8 @@
&usb_otg_hs {
interface-type = <0>;
usb-phy = <&usb2_phy>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
mode = <3>;
power = <50>;
};
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 22d9f2b..1e8e2fe 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -520,6 +520,7 @@
compatible = "ti,omap-usb2";
reg = <0x4a0ad080 0x58>;
ctrl-module = <&omap_control_usb>;
+ #phy-cells = <0>;
};
};
@@ -658,6 +659,8 @@
interrupt-names = "mc", "dma";
ti,hwmods = "usb_otg_hs";
usb-phy = <&usb2_phy>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
multipoint = <1>;
num-eps = <16>;
ram-bits = <12>;
diff --git a/arch/arm/boot/dts/twl4030.dtsi b/arch/arm/boot/dts/twl4030.dtsi
index ae6a17a..5aba238 100644
--- a/arch/arm/boot/dts/twl4030.dtsi
+++ b/arch/arm/boot/dts/twl4030.dtsi
@@ -86,6 +86,7 @@
usb1v8-supply = <&vusb1v8>;
usb3v1-supply = <&vusb3v1>;
usb_mode = <1>;
+ #phy-cells = <0>;
};
twl_pwm: pwm {
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 6/8] usb: musb: omap2430: use the new generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Use the generic PHY framework API to get the PHY. The usb_phy_set_resume
and usb_phy_set_suspend is replaced with power_on and
power_off to align with the new PHY framework.
musb->xceiv can't be removed as of now because musb core uses xceiv.state and
xceiv.otg. Once there is a separate state machine to handle otg, these can be
moved out of xceiv and then we can start using the generic PHY framework.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
drivers/usb/musb/Kconfig | 1 +
drivers/usb/musb/musb_core.h | 2 ++
drivers/usb/musb/omap2430.c | 26 ++++++++++++++++++++------
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 797e3fd..25e2e57 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -76,6 +76,7 @@ config USB_MUSB_TUSB6010
config USB_MUSB_OMAP2PLUS
tristate "OMAP2430 and onwards"
depends on ARCH_OMAP2PLUS
+ select GENERIC_PHY
config USB_MUSB_AM35X
tristate "AM35x"
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 7d341c3..6e567bd 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -46,6 +46,7 @@
#include <linux/usb.h>
#include <linux/usb/otg.h>
#include <linux/usb/musb.h>
+#include <linux/phy/phy.h>
struct musb;
struct musb_hw_ep;
@@ -346,6 +347,7 @@ struct musb {
u16 int_tx;
struct usb_phy *xceiv;
+ struct phy *phy;
int nIrq;
unsigned irq_wake:1;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index f44e8b5..063773a 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -348,11 +348,21 @@ static int omap2430_musb_init(struct musb *musb)
* up through ULPI. TWL4030-family PMICs include one,
* which needs a driver, drivers aren't always needed.
*/
- if (dev->parent->of_node)
+ if (dev->parent->of_node) {
+ musb->phy = devm_phy_get(dev->parent, "usb2-phy");
+
+ /* We can't totally remove musb->xceiv as of now because
+ * musb core uses xceiv.state and xceiv.otg. Once we have
+ * a separate state machine to handle otg, these can be moved
+ * out of xceiv and then we can start using the generic PHY
+ * framework
+ */
musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent,
"usb-phy", 0);
- else
+ } else {
musb->xceiv = devm_usb_get_phy_dev(dev, 0);
+ musb->phy = devm_phy_get(dev, "usb");
+ }
if (IS_ERR(musb->xceiv)) {
status = PTR_ERR(musb->xceiv);
@@ -364,6 +374,10 @@ static int omap2430_musb_init(struct musb *musb)
return -EPROBE_DEFER;
}
+ if (IS_ERR(musb->phy)) {
+ pr_err("HS USB OTG: no PHY configured\n");
+ return PTR_ERR(musb->phy);
+ }
musb->isr = omap2430_musb_interrupt;
status = pm_runtime_get_sync(dev);
@@ -397,7 +411,7 @@ static int omap2430_musb_init(struct musb *musb)
if (glue->status != OMAP_MUSB_UNKNOWN)
omap_musb_set_mailbox(glue);
- usb_phy_init(musb->xceiv);
+ phy_init(musb->phy);
pm_runtime_put_noidle(musb->controller);
return 0;
@@ -460,6 +474,7 @@ static int omap2430_musb_exit(struct musb *musb)
del_timer_sync(&musb_idle_timer);
omap2430_low_level_exit(musb);
+ phy_exit(musb->phy);
return 0;
}
@@ -638,7 +653,7 @@ static int omap2430_runtime_suspend(struct device *dev)
OTG_INTERFSEL);
omap2430_low_level_exit(musb);
- usb_phy_set_suspend(musb->xceiv, 1);
+ phy_power_off(musb->phy);
}
return 0;
@@ -653,8 +668,7 @@ static int omap2430_runtime_resume(struct device *dev)
omap2430_low_level_init(musb);
musb_writel(musb->mregs, OTG_INTERFSEL,
musb->context.otg_interfsel);
-
- usb_phy_set_suspend(musb->xceiv, 0);
+ phy_power_on(musb->phy);
}
return 0;
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 7/8] usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Now that omap-usb2 is adapted to the new generic PHY framework,
*set_suspend* ops can be removed from omap-usb2 driver.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
drivers/phy/phy-omap-usb2.c | 25 -------------------------
1 file changed, 25 deletions(-)
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 25e0f3c..dec3fab 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -97,29 +97,6 @@ static int omap_usb_set_peripheral(struct usb_otg *otg,
return 0;
}
-static int omap_usb2_suspend(struct usb_phy *x, int suspend)
-{
- u32 ret;
- struct omap_usb *phy = phy_to_omapusb(x);
-
- if (suspend && !phy->is_suspended) {
- omap_control_usb_phy_power(phy->control_dev, 0);
- pm_runtime_put_sync(phy->dev);
- phy->is_suspended = 1;
- } else if (!suspend && phy->is_suspended) {
- ret = pm_runtime_get_sync(phy->dev);
- if (ret < 0) {
- dev_err(phy->dev, "get_sync failed with err %d\n",
- ret);
- return ret;
- }
- omap_control_usb_phy_power(phy->control_dev, 1);
- phy->is_suspended = 0;
- }
-
- return 0;
-}
-
static int omap_usb_power_off(struct phy *x)
{
struct omap_usb *phy = phy_get_drvdata(x);
@@ -167,7 +144,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy->phy.dev = phy->dev;
phy->phy.label = "omap-usb2";
- phy->phy.set_suspend = omap_usb2_suspend;
phy->phy.otg = otg;
phy->phy.type = USB_PHY_TYPE_USB2;
@@ -182,7 +158,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
return -ENODEV;
}
- phy->is_suspended = 1;
omap_control_usb_phy_power(phy->control_dev, 0);
otg->set_host = omap_usb_set_host;
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 8/8] usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Now that twl4030-usb is adapted to the new generic PHY framework,
*set_suspend* and *phy_init* ops can be removed from twl4030-usb driver.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
drivers/phy/phy-twl4030-usb.c | 57 ++++++++++-------------------------------
1 file changed, 13 insertions(+), 44 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 494f107..c437211 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -422,25 +422,20 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
}
}
-static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
+static int twl4030_phy_power_off(struct phy *phy)
{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
if (twl->asleep)
- return;
+ return 0;
twl4030_phy_power(twl, 0);
twl->asleep = 1;
dev_dbg(twl->dev, "%s\n", __func__);
-}
-
-static int twl4030_phy_power_off(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- twl4030_phy_suspend(twl, 0);
return 0;
}
-static void __twl4030_phy_resume(struct twl4030_usb *twl)
+static void __twl4030_phy_power_on(struct twl4030_usb *twl)
{
twl4030_phy_power(twl, 1);
twl4030_i2c_access(twl, 1);
@@ -449,11 +444,13 @@ static void __twl4030_phy_resume(struct twl4030_usb *twl)
twl4030_i2c_access(twl, 0);
}
-static void twl4030_phy_resume(struct twl4030_usb *twl)
+static int twl4030_phy_power_on(struct phy *phy)
{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
if (!twl->asleep)
- return;
- __twl4030_phy_resume(twl);
+ return 0;
+ __twl4030_phy_power_on(twl);
twl->asleep = 0;
dev_dbg(twl->dev, "%s\n", __func__);
@@ -466,13 +463,6 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
cancel_delayed_work(&twl->id_workaround_work);
schedule_delayed_work(&twl->id_workaround_work, HZ);
}
-}
-
-static int twl4030_phy_power_on(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- twl4030_phy_resume(twl);
return 0;
}
@@ -604,9 +594,9 @@ static void twl4030_id_workaround_work(struct work_struct *work)
}
}
-static int twl4030_usb_phy_init(struct usb_phy *phy)
+static int twl4030_phy_init(struct phy *phy)
{
- struct twl4030_usb *twl = phy_to_twl(phy);
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
enum omap_musb_vbus_id_status status;
/*
@@ -621,32 +611,13 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
if (status = OMAP_MUSB_ID_GROUND || status = OMAP_MUSB_VBUS_VALID) {
omap_musb_mailbox(twl->linkstat);
- twl4030_phy_resume(twl);
+ twl4030_phy_power_on(phy);
}
sysfs_notify(&twl->dev->kobj, NULL, "vbus");
return 0;
}
-static int twl4030_phy_init(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- return twl4030_usb_phy_init(&twl->phy);
-}
-
-static int twl4030_set_suspend(struct usb_phy *x, int suspend)
-{
- struct twl4030_usb *twl = phy_to_twl(x);
-
- if (suspend)
- twl4030_phy_suspend(twl, 1);
- else
- twl4030_phy_resume(twl);
-
- return 0;
-}
-
static int twl4030_set_peripheral(struct usb_otg *otg,
struct usb_gadget *gadget)
{
@@ -719,8 +690,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
twl->phy.label = "twl4030";
twl->phy.otg = otg;
twl->phy.type = USB_PHY_TYPE_USB2;
- twl->phy.set_suspend = twl4030_set_suspend;
- twl->phy.init = twl4030_usb_phy_init;
otg->phy = &twl->phy;
otg->set_host = twl4030_set_host;
--
1.7.10.4
^ permalink raw reply related
* RE: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
From: Inki Dae @ 2013-08-21 8:40 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1376385576-9039-2-git-send-email-inki.dae@samsung.com>
Thanks for the review,
Inki Dae
> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Konrad Rzeszutek Wilk
> Sent: Wednesday, August 21, 2013 4:22 AM
> To: Inki Dae
> Cc: dri-devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-media@vger.kernel.org; linaro-
> kernel@lists.linaro.org; kyungmin.park@samsung.com;
> myungjoo.ham@samsung.com
> Subject: Re: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer
> synchronization framework
>
> On Tue, Aug 13, 2013 at 06:19:35PM +0900, Inki Dae wrote:
> > This patch adds a buffer synchronization framework based on DMA BUF[1]
> > and and based on ww-mutexes[2] for lock mechanism.
> >
> > The purpose of this framework is to provide not only buffer access
> control
> > to CPU and DMA but also easy-to-use interfaces for device drivers and
> > user application. This framework can be used for all dma devices using
> > system memory as dma buffer, especially for most ARM based SoCs.
> >
> > Changelog v6:
> > - Fix sync lock to multiple reads.
> > - Add select system call support.
> > . Wake up poll_wait when a dmabuf is unlocked.
> > - Remove unnecessary the use of mutex lock.
> > - Add private backend ops callbacks.
> > . This ops has one callback for device drivers to clean up their
> > sync object resource when the sync object is freed. For this,
> > device drivers should implement the free callback properly.
> > - Update document file.
> >
> > Changelog v5:
> > - Rmove a dependence on reservation_object: the reservation_object is
> used
> > to hook up to ttm and dma-buf for easy sharing of reservations across
> > devices. However, the dmabuf sync can be used for all dma devices;
> v4l2
> > and drm based drivers, so doesn't need the reservation_object anymore.
> > With regared to this, it adds 'void *sync' to dma_buf structure.
> > - All patches are rebased on mainline, Linux v3.10.
> >
> > Changelog v4:
> > - Add user side interface for buffer synchronization mechanism and
> update
> > descriptions related to the user side interface.
> >
> > Changelog v3:
> > - remove cache operation relevant codes and update document file.
> >
> > Changelog v2:
> > - use atomic_add_unless to avoid potential bug.
> > - add a macro for checking valid access type.
> > - code clean.
> >
> > The mechanism of this framework has the following steps,
> > 1. Register dmabufs to a sync object - A task gets a new sync object
> and
> > can add one or more dmabufs that the task wants to access.
> > This registering should be performed when a device context or an
> event
> > context such as a page flip event is created or before CPU accesses
a
> shared
> > buffer.
> >
> > dma_buf_sync_get(a sync object, a dmabuf);
> >
> > 2. Lock a sync object - A task tries to lock all dmabufs added in
its
> own
> > sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid
> dead
> > lock issue and for race condition between CPU and CPU, CPU and DMA,
> and DMA
> > and DMA. Taking a lock means that others cannot access all locked
> dmabufs
> > until the task that locked the corresponding dmabufs, unlocks all
the
> locked
> > dmabufs.
> > This locking should be performed before DMA or CPU accesses these
> dmabufs.
> >
> > dma_buf_sync_lock(a sync object);
> >
> > 3. Unlock a sync object - The task unlocks all dmabufs added in its
> own sync
> > object. The unlock means that the DMA or CPU accesses to the dmabufs
> have
> > been completed so that others may access them.
> > This unlocking should be performed after DMA or CPU has completed
> accesses
> > to the dmabufs.
> >
> > dma_buf_sync_unlock(a sync object);
> >
> > 4. Unregister one or all dmabufs from a sync object - A task
> unregisters
> > the given dmabufs from the sync object. This means that the task
> dosen't
> > want to lock the dmabufs.
> > The unregistering should be performed after DMA or CPU has completed
> > accesses to the dmabufs or when dma_buf_sync_lock() is failed.
> >
> > dma_buf_sync_put(a sync object, a dmabuf);
> > dma_buf_sync_put_all(a sync object);
> >
> > The described steps may be summarized as:
> > get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
> >
> > This framework includes the following two features.
> > 1. read (shared) and write (exclusive) locks - A task is required to
> declare
> > the access type when the task tries to register a dmabuf;
> > READ, WRITE, READ DMA, or WRITE DMA.
> >
> > The below is example codes,
> > struct dmabuf_sync *sync;
> >
> > sync = dmabuf_sync_init(...);
> > ...
> >
> > dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
> > ...
> >
> > And the below can be used as access types:
> > DMA_BUF_ACCESS_R - CPU will access a buffer for read.
> > DMA_BUF_ACCESS_W - CPU will access a buffer for read or
> write.
> > DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
> > DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or
> > write.
> >
> > 2. Mandatory resource releasing - a task cannot hold a lock
> indefinitely.
> > A task may never try to unlock a buffer after taking a lock to the
> buffer.
> > In this case, a timer handler to the corresponding sync object is
> called
> > in five (default) seconds and then the timed-out buffer is unlocked
> by work
> > queue handler to avoid lockups and to enforce resources of the
buffer.
> >
> > The below is how to use interfaces for device driver:
> > 1. Allocate and Initialize a sync object:
> > static void xxx_dmabuf_sync_free(void *priv)
> > {
> > struct xxx_context *ctx = priv;
> >
> > if (!ctx)
> > return;
> >
> > ctx->sync = NULL;
> > }
> > ...
> >
> > static struct dmabuf_sync_priv_ops driver_specific_ops = {
> > .free = xxx_dmabuf_sync_free,
> > };
> > ...
> >
> > struct dmabuf_sync *sync;
> >
> > sync = dmabuf_sync_init("test sync", &driver_specific_ops,
> ctx);
> > ...
> >
> > 2. Add a dmabuf to the sync object when setting up dma buffer
> relevant
> > registers:
> > dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
> > ...
> >
> > 3. Lock all dmabufs of the sync object before DMA or CPU accesses
> > the dmabufs:
> > dmabuf_sync_lock(sync);
> > ...
> >
> > 4. Now CPU or DMA can access all dmabufs locked in step 3.
> >
> > 5. Unlock all dmabufs added in a sync object after DMA or CPU
> access
> > to these dmabufs is completed:
> > dmabuf_sync_unlock(sync);
> >
> > And call the following functions to release all resources,
> > dmabuf_sync_put_all(sync);
> > dmabuf_sync_fini(sync);
> >
> > You can refer to actual example codes:
> > "drm/exynos: add dmabuf sync support for g2d driver" and
> > "drm/exynos: add dmabuf sync support for kms framework" from
> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/
> > drm-exynos.git/log/?h=dmabuf-sync
> >
> > And this framework includes fcntl system call[3] as interfaces exported
> > to user. As you know, user sees a buffer object as a dma-buf file
> descriptor.
> > So fcntl() call with the file descriptor means to lock some buffer
> region being
> > managed by the dma-buf object.
> >
> > The below is how to use interfaces for user application:
> >
> > fcntl system call:
> >
> > struct flock filelock;
> >
> > 1. Lock a dma buf:
> > filelock.l_type = F_WRLCK or F_RDLCK;
> >
> > /* lock entire region to the dma buf. */
> > filelock.lwhence = SEEK_CUR;
> > filelock.l_start = 0;
> > filelock.l_len = 0;
> >
> > fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> > ...
> > CPU access to the dma buf
> >
> > 2. Unlock a dma buf:
> > filelock.l_type = F_UNLCK;
> >
> > fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> >
> > close(dmabuf fd) call would also unlock the dma buf. And for
> more
> > detail, please refer to [3]
> >
> > select system call:
> >
> > fd_set wdfs or rdfs;
> >
> > FD_ZERO(&wdfs or &rdfs);
> > FD_SET(fd, &wdfs or &rdfs);
> >
> > select(fd + 1, &rdfs, NULL, NULL, NULL);
> > or
> > select(fd + 1, NULL, &wdfs, NULL, NULL);
> >
> > Every time select system call is called, a caller will wait for
> > the completion of DMA or CPU access to a shared buffer if there
> > is someone accessing the shared buffer; locked the shared buffer.
> > However, if no anyone then select system call will be returned
> > at once.
> >
> > References:
> > [1] http://lwn.net/Articles/470339/
> > [2] https://patchwork.kernel.org/patch/2625361/
> > [3] http://linux.die.net/man/2/fcntl
> >
> > Signed-off-by: Inki Dae <inki.dae@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > Documentation/dma-buf-sync.txt | 285 +++++++++++++++++
> > drivers/base/Kconfig | 7 +
> > drivers/base/Makefile | 1 +
> > drivers/base/dma-buf.c | 4 +
> > drivers/base/dmabuf-sync.c | 678
> ++++++++++++++++++++++++++++++++++++++++
> > include/linux/dma-buf.h | 16 +
> > include/linux/dmabuf-sync.h | 190 +++++++++++
> > 7 files changed, 1181 insertions(+), 0 deletions(-)
> > create mode 100644 Documentation/dma-buf-sync.txt
> > create mode 100644 drivers/base/dmabuf-sync.c
> > create mode 100644 include/linux/dmabuf-sync.h
> >
> > diff --git a/Documentation/dma-buf-sync.txt b/Documentation/dma-buf-
> sync.txt
> > new file mode 100644
> > index 0000000..8023d06
> > --- /dev/null
> > +++ b/Documentation/dma-buf-sync.txt
> > @@ -0,0 +1,285 @@
> > + DMA Buffer Synchronization Framework
> > + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > + Inki Dae
> > + <inki dot dae at samsung dot com>
> > + <daeinki at gmail dot com>
> > +
> > +This document is a guide for device-driver writers describing the DMA
> buffer
> > +synchronization API. This document also describes how to use the API to
> > +use buffer synchronization mechanism between DMA and DMA, CPU and DMA,
> and
> > +CPU and CPU.
> > +
> > +The DMA Buffer synchronization API provides buffer synchronization
> mechanism;
> > +i.e., buffer access control to CPU and DMA, and easy-to-use interfaces
> for
> > +device drivers and user application. And this API can be used for all
> dma
> > +devices using system memory as dma buffer, especially for most ARM
> based SoCs.
> > +
> > +
> > +Motivation
> > +----------
> > +
> > +Buffer synchronization issue between DMA and DMA:
> > + Sharing a buffer, a device cannot be aware of when the other device
> > + will access the shared buffer: a device may access a buffer
> containing
> > + wrong data if the device accesses the shared buffer while another
> > + device is still accessing the shared buffer.
> > + Therefore, a user process should have waited for the completion of
> DMA
> > + access by another device before a device tries to access the shared
> > + buffer.
> > +
> > +Buffer synchronization issue between CPU and DMA:
> > + A user process should consider that when having to send a buffer,
> filled
> > + by CPU, to a device driver for the device driver to access the
> buffer as
> > + a input buffer while CPU and DMA are sharing the buffer.
> > + This means that the user process needs to understand how the device
> > + driver is worked. Hence, the conventional mechanism not only makes
> > + user application complicated but also incurs performance overhead.
> > +
> > +Buffer synchronization issue between CPU and CPU:
> > + In case that two processes share one buffer; shared with DMA also,
> > + they may need some mechanism to allow process B to access the
> shared
> > + buffer after the completion of CPU access by process A.
> > + Therefore, process B should have waited for the completion of CPU
> access
> > + by process A using the mechanism before trying to access the shared
> > + buffer.
> > +
> > +What is the best way to solve these buffer synchronization issues?
> > + We may need a common object that a device driver and a user process
> > + notify the common object of when they try to access a shared buffer.
> > + That way we could decide when we have to allow or not to allow for
> CPU
> > + or DMA to access the shared buffer through the common object.
> > + If so, what could become the common object? Right, that's a dma-
> buf[1].
> > + Now we have already been using the dma-buf to share one buffer with
> > + other drivers.
> > +
> > +
> > +Basic concept
> > +-------------
> > +
> > +The mechanism of this framework has the following steps,
> > + 1. Register dmabufs to a sync object - A task gets a new sync
object
> and
> > + can add one or more dmabufs that the task wants to access.
> > + This registering should be performed when a device context or an
> event
> > + context such as a page flip event is created or before CPU accesses
> a shared
> > + buffer.
> > +
> > + dma_buf_sync_get(a sync object, a dmabuf);
> > +
> > + 2. Lock a sync object - A task tries to lock all dmabufs added in
> its own
> > + sync object. Basically, the lock mechanism uses ww-mutexes[2] to
> avoid dead
> > + lock issue and for race condition between CPU and CPU, CPU and DMA,
> and DMA
> > + and DMA. Taking a lock means that others cannot access all locked
> dmabufs
> > + until the task that locked the corresponding dmabufs, unlocks all
> the locked
> > + dmabufs.
> > + This locking should be performed before DMA or CPU accesses these
> dmabufs.
> > +
> > + dma_buf_sync_lock(a sync object);
> > +
> > + 3. Unlock a sync object - The task unlocks all dmabufs added in its
> own sync
> > + object. The unlock means that the DMA or CPU accesses to the
dmabufs
> have
> > + been completed so that others may access them.
> > + This unlocking should be performed after DMA or CPU has completed
> accesses
> > + to the dmabufs.
> > +
> > + dma_buf_sync_unlock(a sync object);
> > +
> > + 4. Unregister one or all dmabufs from a sync object - A task
> unregisters
> > + the given dmabufs from the sync object. This means that the task
> dosen't
> > + want to lock the dmabufs.
> > + The unregistering should be performed after DMA or CPU has
completed
> > + accesses to the dmabufs or when dma_buf_sync_lock() is failed.
> > +
> > + dma_buf_sync_put(a sync object, a dmabuf);
> > + dma_buf_sync_put_all(a sync object);
> > +
> > + The described steps may be summarized as:
> > + get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
> > +
> > +This framework includes the following two features.
> > + 1. read (shared) and write (exclusive) locks - A task is required
to
> declare
> > + the access type when the task tries to register a dmabuf;
> > + READ, WRITE, READ DMA, or WRITE DMA.
> > +
> > + The below is example codes,
> > + struct dmabuf_sync *sync;
> > +
> > + sync = dmabuf_sync_init(NULL, "test sync");
> > +
> > + dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
> > + ...
> > +
> > + 2. Mandatory resource releasing - a task cannot hold a lock
> indefinitely.
> > + A task may never try to unlock a buffer after taking a lock to the
> buffer.
> > + In this case, a timer handler to the corresponding sync object is
> called
> > + in five (default) seconds and then the timed-out buffer is unlocked
> by work
> > + queue handler to avoid lockups and to enforce resources of the
> buffer.
> > +
> > +
> > +Access types
> > +------------
> > +
> > +DMA_BUF_ACCESS_R - CPU will access a buffer for read.
> > +DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
> > +DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
> > +DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or write.
> > +
> > +
> > +Generic user interfaces
> > +-----------------------
> > +
> > +And this framework includes fcntl system call[3] as interfaces exported
> > +to user. As you know, user sees a buffer object as a dma-buf file
> descriptor.
> > +So fcntl() call with the file descriptor means to lock some buffer
> region being
> > +managed by the dma-buf object.
> > +
> > +
> > +API set
> > +-------
> > +
> > +bool is_dmabuf_sync_supported(void)
> > + - Check if dmabuf sync is supported or not.
> > +
> > +struct dmabuf_sync *dmabuf_sync_init(const char *name,
> > + struct dmabuf_sync_priv_ops *ops,
> > + void priv*)
> > + - Allocate and initialize a new sync object. The caller can get a
> new
> > + sync object for buffer synchronization. ops is used for device
> driver
> > + to clean up its own sync object. For this, each device driver
> should
> > + implement a free callback. priv is used for device driver to get
> its
> > + device context when free callback is called.
> > +
> > +void dmabuf_sync_fini(struct dmabuf_sync *sync)
> > + - Release all resources to the sync object.
> > +
> > +int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
> > + unsigned int type)
> > + - Get dmabuf sync object. Internally, this function allocates
> > + a dmabuf_sync object and adds a given dmabuf to it, and also takes
> > + a reference to the dmabuf. The caller can tie up multiple dmabufs
> > + into one sync object by calling this function several times.
> > +
> > +void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
> > + - Put dmabuf sync object to a given dmabuf. Internally, this
> function
> > + removes a given dmabuf from a sync object and remove the sync
> object.
> > + At this time, the dmabuf is putted.
> > +
> > +void dmabuf_sync_put_all(struct dmabuf_sync *sync)
> > + - Put dmabuf sync object to dmabufs. Internally, this function
> removes
> > + all dmabufs from a sync object and remove the sync object.
> > + At this time, all dmabufs are putted.
> > +
> > +int dmabuf_sync_lock(struct dmabuf_sync *sync)
> > + - Lock all dmabufs added in a sync object. The caller should call
> this
> > + function prior to CPU or DMA access to the dmabufs so that others
> can
> > + not access the dmabufs. Internally, this function avoids dead lock
> > + issue with ww-mutexes.
> > +
> > +int dmabuf_sync_single_lock(struct dma_buf *dmabuf)
> > + - Lock a dmabuf. The caller should call this
> > + function prior to CPU or DMA access to the dmabuf so that others
> can
> > + not access the dmabuf.
> > +
> > +int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> > + - Unlock all dmabufs added in a sync object. The caller should call
> > + this function after CPU or DMA access to the dmabufs is completed
> so
> > + that others can access the dmabufs.
> > +
> > +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> > + - Unlock a dmabuf. The caller should call this function after CPU
> or
> > + DMA access to the dmabuf is completed so that others can access
> > + the dmabuf.
> > +
> > +
> > +Tutorial for device driver
> > +--------------------------
> > +
> > +1. Allocate and Initialize a sync object:
> > + static void xxx_dmabuf_sync_free(void *priv)
> > + {
> > + struct xxx_context *ctx = priv;
> > +
> > + if (!ctx)
> > + return;
> > +
> > + ctx->sync = NULL;
> > + }
> > + ...
> > +
> > + static struct dmabuf_sync_priv_ops driver_specific_ops = {
> > + .free = xxx_dmabuf_sync_free,
> > + };
> > + ...
> > +
> > + struct dmabuf_sync *sync;
> > +
> > + sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
> > + ...
> > +
> > +2. Add a dmabuf to the sync object when setting up dma buffer relevant
> registers:
> > + dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
> > + ...
> > +
> > +3. Lock all dmabufs of the sync object before DMA or CPU accesses the
> dmabufs:
> > + dmabuf_sync_lock(sync);
> > + ...
> > +
> > +4. Now CPU or DMA can access all dmabufs locked in step 3.
> > +
> > +5. Unlock all dmabufs added in a sync object after DMA or CPU access to
> these
> > + dmabufs is completed:
> > + dmabuf_sync_unlock(sync);
> > +
> > + And call the following functions to release all resources,
> > + dmabuf_sync_put_all(sync);
> > + dmabuf_sync_fini(sync);
> > +
> > +
> > +Tutorial for user application
> > +-----------------------------
> > +fcntl system call:
> > +
> > + struct flock filelock;
> > +
> > +1. Lock a dma buf:
> > + filelock.l_type = F_WRLCK or F_RDLCK;
> > +
> > + /* lock entire region to the dma buf. */
> > + filelock.lwhence = SEEK_CUR;
> > + filelock.l_start = 0;
> > + filelock.l_len = 0;
> > +
> > + fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> > + ...
> > + CPU access to the dma buf
> > +
> > +2. Unlock a dma buf:
> > + filelock.l_type = F_UNLCK;
> > +
> > + fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> > +
> > + close(dmabuf fd) call would also unlock the dma buf. And for more
> > + detail, please refer to [3]
> > +
> > +
> > +select system call:
> > +
> > + fd_set wdfs or rdfs;
> > +
> > + FD_ZERO(&wdfs or &rdfs);
> > + FD_SET(fd, &wdfs or &rdfs);
> > +
> > + select(fd + 1, &rdfs, NULL, NULL, NULL);
> > + or
> > + select(fd + 1, NULL, &wdfs, NULL, NULL);
> > +
> > + Every time select system call is called, a caller will wait for
> > + the completion of DMA or CPU access to a shared buffer if there is
> > + someone accessing the shared buffer; locked the shared buffer.
> > + However, if no anyone then select system call will be returned
> > + at once.
> > +
> > +References:
> > +[1] http://lwn.net/Articles/470339/
> > +[2] https://patchwork.kernel.org/patch/2625361/
> > +[3] http://linux.die.net/man/2/fcntl
> > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> > index 5daa259..35e1518 100644
> > --- a/drivers/base/Kconfig
> > +++ b/drivers/base/Kconfig
> > @@ -200,6 +200,13 @@ config DMA_SHARED_BUFFER
> > APIs extension; the file's descriptor can then be passed on to
> other
> > driver.
> >
> > +config DMABUF_SYNC
> > + bool "DMABUF Synchronization Framework"
> > + depends on DMA_SHARED_BUFFER
> > + help
> > + This option enables dmabuf sync framework for buffer
> synchronization between
> > + DMA and DMA, CPU and DMA, and CPU and CPU.
> > +
> > config CMA
> > bool "Contiguous Memory Allocator"
> > depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK
> > diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> > index 48029aa..e06a5d7 100644
> > --- a/drivers/base/Makefile
> > +++ b/drivers/base/Makefile
> > @@ -11,6 +11,7 @@ obj-y += power/
> > obj-$(CONFIG_HAS_DMA) += dma-mapping.o
> > obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
> > obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o reservation.o
> > +obj-$(CONFIG_DMABUF_SYNC) += dmabuf-sync.o
> > obj-$(CONFIG_ISA) += isa.o
> > obj-$(CONFIG_FW_LOADER) += firmware_class.o
> > obj-$(CONFIG_NUMA) += node.o
> > diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> > index 6687ba7..4aca57a 100644
> > --- a/drivers/base/dma-buf.c
> > +++ b/drivers/base/dma-buf.c
> > @@ -29,6 +29,7 @@
> > #include <linux/export.h>
> > #include <linux/debugfs.h>
> > #include <linux/seq_file.h>
> > +#include <linux/dmabuf-sync.h>
> >
> > static inline int is_dma_buf_file(struct file *);
> >
> > @@ -56,6 +57,8 @@ static int dma_buf_release(struct inode *inode, struct
> file *file)
> > list_del(&dmabuf->list_node);
> > mutex_unlock(&db_list.lock);
> >
> > + dmabuf_sync_reservation_fini(dmabuf);
> > +
> > kfree(dmabuf);
> > return 0;
> > }
> > @@ -134,6 +137,7 @@ struct dma_buf *dma_buf_export_named(void *priv,
> const struct dma_buf_ops *ops,
> >
> > file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
> >
> > + dmabuf_sync_reservation_init(dmabuf);
> > dmabuf->file = file;
> >
> > mutex_init(&dmabuf->lock);
> > diff --git a/drivers/base/dmabuf-sync.c b/drivers/base/dmabuf-sync.c
> > new file mode 100644
> > index 0000000..fbe711c
> > --- /dev/null
> > +++ b/drivers/base/dmabuf-sync.c
> > @@ -0,0 +1,678 @@
> > +/*
> > + * Copyright (C) 2013 Samsung Electronics Co.Ltd
> > + * Authors:
> > + * Inki Dae <inki.dae@samsung.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify it
> > + * under the terms of the GNU General Public License as published by
> the
> > + * Free Software Foundation; either version 2 of the License, or (at
> your
> > + * option) any later version.
> > + *
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/uaccess.h>
> > +
> > +#include <linux/dmabuf-sync.h>
> > +
> > +#define MAX_SYNC_TIMEOUT 5 /* Second. */
> > +
> > +int dmabuf_sync_enabled = 1;
> > +
> > +MODULE_PARM_DESC(enabled, "Check if dmabuf sync is supported or not");
> > +module_param_named(enabled, dmabuf_sync_enabled, int, 0444);
> > +
> > +DEFINE_WW_CLASS(dmabuf_sync_ww_class);
> > +EXPORT_SYMBOL(dmabuf_sync_ww_class);
> > +
> > +static void dmabuf_sync_timeout_worker(struct work_struct *work)
> > +{
> > + struct dmabuf_sync *sync = container_of(work, struct dmabuf_sync,
> work);
> > + struct dmabuf_sync_object *sobj;
> > +
> > + mutex_lock(&sync->lock);
> > +
> > + list_for_each_entry(sobj, &sync->syncs, head) {
>
> You are using the 'sobj->robj' quite a lot. Why not just use a temp
> structure:
>
> struct dmabuf_sync_reservation *rsvp = sobj->robj;
>
> and use that in this function. It would make it easier to read I think.
Ok, will use the temp structure.
>
>
> > + BUG_ON(!sobj->robj);
> > +
> > + mutex_lock(&sobj->robj->lock);
> > +
> > + printk(KERN_WARNING "%s: timeout = 0x%x [type = %d:%d, " \
> > + "refcnt = %d, locked = %d]\n",
> > + sync->name, (u32)sobj->dmabuf,
> > + sobj->robj->accessed_type,
> > + sobj->access_type,
> > +
atomic_read(&sobj->robj->shared_cnt),
> > + sobj->robj->locked);
>
> pr_warn_ratelimited?
Will use pr_warn because the timeout worker handler isn't called so
frequently so the printk storm wouldn't be caused
>
> > +
> > + /* unlock only valid sync object. */
> > + if (!sobj->robj->locked) {
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + if (sobj->robj->polled) {
> > + sobj->robj->poll_event = true;
> > + sobj->robj->polled = false;
> > + wake_up_interruptible(&sobj->robj->poll_wait);
> > + }
> > +
> > + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > +
> > + ww_mutex_unlock(&sobj->robj->sync_lock);
> > +
> > + mutex_lock(&sobj->robj->lock);
> > + sobj->robj->locked = false;
> > +
> > + if (sobj->access_type & DMA_BUF_ACCESS_R)
> > + printk(KERN_WARNING "%s: r-unlocked = 0x%x\n",
> > + sync->name, (u32)sobj->dmabuf);
> > + else
> > + printk(KERN_WARNING "%s: w-unlocked = 0x%x\n",
> > + sync->name, (u32)sobj->dmabuf);
>
> How about using 'pr_warn'? And in it have:
Ok, will use it.
>
> sobj->access_type & DMA_BUF_ACCESS_R ? "r-" : "w-",
>
> and just have one printk.
>
> Why the (u32) casting? Don't you want %p ?
Right, I should had used %p instead. Will remove the casting and use %p
instead.
>
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > + }
> > +
> > + sync->status = 0;
> > + mutex_unlock(&sync->lock);
> > +
> > + dmabuf_sync_put_all(sync);
> > + dmabuf_sync_fini(sync);
> > +}
> > +
> > +static void dmabuf_sync_lock_timeout(unsigned long arg)
> > +{
> > + struct dmabuf_sync *sync = (struct dmabuf_sync *)arg;
> > +
> > + schedule_work(&sync->work);
> > +}
> > +
> > +static int dmabuf_sync_lock_objs(struct dmabuf_sync *sync,
> > + struct ww_acquire_ctx *ctx)
> > +{
> > + struct dmabuf_sync_object *contended_sobj = NULL;
> > + struct dmabuf_sync_object *res_sobj = NULL;
> > + struct dmabuf_sync_object *sobj = NULL;
> > + int ret;
> > +
> > + if (ctx)
> > + ww_acquire_init(ctx, &dmabuf_sync_ww_class);
> > +
> > +retry:
> > + list_for_each_entry(sobj, &sync->syncs, head) {
> > + if (WARN_ON(!sobj->robj))
> > + continue;
> > +
> > + mutex_lock(&sobj->robj->lock);
> > +
> > + /* Don't lock in case of read and read. */
> > + if (sobj->robj->accessed_type & DMA_BUF_ACCESS_R &&
> > + sobj->access_type & DMA_BUF_ACCESS_R) {
> > + atomic_inc(&sobj->robj->shared_cnt);
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + if (sobj = res_sobj) {
> > + res_sobj = NULL;
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > +
> > + ret = ww_mutex_lock(&sobj->robj->sync_lock, ctx);
> > + if (ret < 0) {
> > + contended_sobj = sobj;
> > +
> > + if (ret = -EDEADLK)
> > + printk(KERN_WARNING"%s: deadlock = 0x%x\n",
> > + sync->name, (u32)sobj->dmabuf);
>
> Again, why (u32) and not %p?
>
> > + goto err;
>
> This looks odd. You jump to err, which jumps back to 'retry'. Won't this
> cause an infinite loop? Perhaps you need to add a retry counter to only
> do this up to five times or so and then give up?
It jumps to err only if ww_mutex_lock returns -EDEADLK. This means that the
lock trying to a given sync object caused dead lock. So all robjs already
locked should be unlocked, and retried to take lock again going to err. So I
think the infinite loop isn't caused.
>
> > + }
> > +
> > + mutex_lock(&sobj->robj->lock);
> > + sobj->robj->locked = true;
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > + }
> > +
> > + if (ctx)
> > + ww_acquire_done(ctx);
> > +
> > + init_timer(&sync->timer);
> > +
> > + sync->timer.data = (unsigned long)sync;
> > + sync->timer.function = dmabuf_sync_lock_timeout;
> > + sync->timer.expires = jiffies + (HZ * MAX_SYNC_TIMEOUT);
> > +
> > + add_timer(&sync->timer);
> > +
> > + return 0;
> > +
> > +err:
> > + list_for_each_entry_continue_reverse(sobj, &sync->syncs, head) {
> > + mutex_lock(&sobj->robj->lock);
> > +
> > + /* Don't need to unlock in case of read and read. */
> > + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + ww_mutex_unlock(&sobj->robj->sync_lock);
> > + sobj->robj->locked = false;
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > + }
> > +
> > + if (res_sobj) {
> > + mutex_lock(&res_sobj->robj->lock);
> > +
> > + if (!atomic_add_unless(&res_sobj->robj->shared_cnt, -1, 1))
> {
> > + ww_mutex_unlock(&res_sobj->robj->sync_lock);
> > + res_sobj->robj->locked = false;
> > + }
> > +
> > + mutex_unlock(&res_sobj->robj->lock);
> > + }
> > +
> > + if (ret = -EDEADLK) {
> > + ww_mutex_lock_slow(&contended_sobj->robj->sync_lock, ctx);
> > + res_sobj = contended_sobj;
> > +
> > + goto retry;
> > + }
> > +
> > + if (ctx)
> > + ww_acquire_fini(ctx);
> > +
> > + return ret;
> > +}
> > +
> > +static void dmabuf_sync_unlock_objs(struct dmabuf_sync *sync,
> > + struct ww_acquire_ctx *ctx)
> > +{
> > + struct dmabuf_sync_object *sobj;
> > +
> > + if (list_empty(&sync->syncs))
> > + return;
> > +
> > + mutex_lock(&sync->lock);
> > +
> > + list_for_each_entry(sobj, &sync->syncs, head) {
> > + mutex_lock(&sobj->robj->lock);
> > +
> > + if (sobj->robj->polled) {
> > + sobj->robj->poll_event = true;
> > + sobj->robj->polled = false;
> > + wake_up_interruptible(&sobj->robj->poll_wait);
> > + }
> > +
> > + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > +
> > + ww_mutex_unlock(&sobj->robj->sync_lock);
> > +
> > + mutex_lock(&sobj->robj->lock);
> > + sobj->robj->locked = false;
> > + mutex_unlock(&sobj->robj->lock);
> > + }
> > +
> > + mutex_unlock(&sync->lock);
> > +
> > + if (ctx)
> > + ww_acquire_fini(ctx);
> > +
> > + del_timer(&sync->timer);
> > +}
> > +
> > +/**
> > + * is_dmabuf_sync_supported - Check if dmabuf sync is supported or not.
> > + */
> > +bool is_dmabuf_sync_supported(void)
> > +{
> > + return dmabuf_sync_enabled = 1;
> > +}
> > +EXPORT_SYMBOL(is_dmabuf_sync_supported);
>
> _GPL ?
>
> I would also prefix it with 'dmabuf_is_sync_supported' just to make
> all of the libraries call start with 'dmabuf'
>
Seems better. Will change it to dmabuf_is_sync_supported, and use
EXPORT_SYMBOL_GPL.
> > +
> > +/**
> > + * dmabuf_sync_init - Allocate and initialize a dmabuf sync.
> > + *
> > + * @priv: A device private data.
> > + * @name: A sync object name.
> > + *
> > + * This function should be called when a device context or an event
> > + * context such as a page flip event is created. And the created
> > + * dmabuf_sync object should be set to the context.
> > + * The caller can get a new sync object for buffer synchronization
> > + * through this function.
> > + */
> > +struct dmabuf_sync *dmabuf_sync_init(const char *name,
> > + struct dmabuf_sync_priv_ops *ops,
> > + void *priv)
> > +{
> > + struct dmabuf_sync *sync;
> > +
> > + sync = kzalloc(sizeof(*sync), GFP_KERNEL);
> > + if (!sync)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + strncpy(sync->name, name, ARRAY_SIZE(sync->name) - 1);
> > +
>
> That is odd usage of an ARRAY_SIZE, but I can see how you can use it.
> I would say you should just do a #define for the 64 line and use that
> instead.
>
Ok, will use the macro instead.
> > + sync->ops = ops;
> > + sync->priv = priv;
> > + INIT_LIST_HEAD(&sync->syncs);
> > + mutex_init(&sync->lock);
> > + INIT_WORK(&sync->work, dmabuf_sync_timeout_worker);
> > +
> > + return sync;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_init);
>
> _GPL ?
Sure.
> > +
> > +/**
> > + * dmabuf_sync_fini - Release a given dmabuf sync.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * This function should be called if some operation is failed after
> > + * dmabuf_sync_init call to release relevant resources, and after
> > + * dmabuf_sync_unlock function is called.
> > + */
> > +void dmabuf_sync_fini(struct dmabuf_sync *sync)
> > +{
> > + if (WARN_ON(!sync))
> > + return;
> > +
> > + if (sync->ops && sync->ops->free)
> > + sync->ops->free(sync->priv);
> > +
>
> No need to cancel the sync->work in case that is still
> running?
Right, the locks to all buffers should be canceled if dmabuf_sync_fini was
called without unlock call.
>
> > + kfree(sync);
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_fini);
>
> _GPL ?
> > +
> > +/*
> > + * dmabuf_sync_get_obj - Add a given object to syncs list.
>
> sync's list I think?
>
Ok, seems better.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + * @dmabuf: An object to dma_buf structure.
> > + * @type: A access type to a dma buf.
> > + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> > + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> > + * means that this dmabuf couldn't be accessed by others but would be
> > + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA
> can be
> > + * combined.
>
> Should this be an enum?
> > + *
> > + * This function creates and initializes a new dmabuf sync object and
> it adds
> > + * the dmabuf sync object to syncs list to track and manage all
dmabufs.
> > + */
> > +static int dmabuf_sync_get_obj(struct dmabuf_sync *sync, struct dma_buf
> *dmabuf,
> > + unsigned int type)
>
> enum for 'type'?
> > +{
> > + struct dmabuf_sync_object *sobj;
> > +
> > + if (!dmabuf->sync) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type))
> > + return -EINVAL;
> > +
> > + if ((type & DMA_BUF_ACCESS_RW) = DMA_BUF_ACCESS_RW)
> > + type &= ~DMA_BUF_ACCESS_R;
>
> Ah, that is why you are not using an enum.
>
> > +
> > + sobj = kzalloc(sizeof(*sobj), GFP_KERNEL);
> > + if (!sobj) {
> > + WARN_ON(1);
>
> I think you can skip that WARN_ON. Handling an -ENOMEM should be
> something fairly easy to handle by the calleer.
>
Ok, will remove it.
> > + return -ENOMEM;
> > + }
> > +
> > + get_dma_buf(dmabuf);
> > +
> > + sobj->dmabuf = dmabuf;
> > + sobj->robj = dmabuf->sync;
> > + sobj->access_type = type;
> > +
> > + mutex_lock(&sync->lock);
> > + list_add_tail(&sobj->head, &sync->syncs);
> > + mutex_unlock(&sync->lock);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * dmabuf_sync_put_obj - Release a given sync object.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * This function should be called if some operation is failed after
>
> s/is//
Sure.
> > + * dmabuf_sync_get_obj call to release a given sync object.
> > + */
> > +static void dmabuf_sync_put_obj(struct dmabuf_sync *sync,
> > + struct dma_buf *dmabuf)
> > +{
> > + struct dmabuf_sync_object *sobj;
> > +
> > + mutex_lock(&sync->lock);
> > +
> > + list_for_each_entry(sobj, &sync->syncs, head) {
> > + if (sobj->dmabuf != dmabuf)
> > + continue;
> > +
> > + dma_buf_put(sobj->dmabuf);
> > +
> > + list_del_init(&sobj->head);
> > + kfree(sobj);
> > + break;
> > + }
> > +
> > + if (list_empty(&sync->syncs))
> > + sync->status = 0;
> > +
> > + mutex_unlock(&sync->lock);
> > +}
> > +
> > +/*
> > + * dmabuf_sync_put_objs - Release all sync objects of dmabuf_sync.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * This function should be called if some operation is failed after
>
> s/is//
Sure.
>
> > + * dmabuf_sync_get_obj call to release all sync objects.
> > + */
> > +static void dmabuf_sync_put_objs(struct dmabuf_sync *sync)
> > +{
> > + struct dmabuf_sync_object *sobj, *next;
> > +
> > + mutex_lock(&sync->lock);
> > +
> > + list_for_each_entry_safe(sobj, next, &sync->syncs, head) {
> > + dma_buf_put(sobj->dmabuf);
> > +
> > + list_del_init(&sobj->head);
> > + kfree(sobj);
> > + }
> > +
> > + mutex_unlock(&sync->lock);
> > +
> > + sync->status = 0;
> > +}
> > +
> > +/**
> > + * dmabuf_sync_lock - lock all dmabufs added to syncs list.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * The caller should call this function prior to CPU or DMA access to
> > + * the dmabufs so that others can not access the dmabufs.
> > + * Internally, this function avoids dead lock issue with ww-mutex.
> > + */
> > +int dmabuf_sync_lock(struct dmabuf_sync *sync)
> > +{
> > + int ret;
> > +
> > + if (!sync) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + if (list_empty(&sync->syncs))
> > + return -EINVAL;
> > +
> > + if (sync->status != DMABUF_SYNC_GOT)
> > + return -EINVAL;
> > +
> > + ret = dmabuf_sync_lock_objs(sync, &sync->ctx);
> > + if (ret < 0) {
> > + WARN_ON(1);
>
> Perhaps also include the ret value in the WARN?
>
> > + return ret;
> > + }
> > +
> > + sync->status = DMABUF_SYNC_LOCKED;
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_lock);
>
> I think you know what I am going to say.
> > +
> > +/**
> > + * dmabuf_sync_unlock - unlock all objects added to syncs list.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * The caller should call this function after CPU or DMA access to
> > + * the dmabufs is completed so that others can access the dmabufs.
> > + */
> > +int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> > +{
> > + if (!sync) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + /* If current dmabuf sync object wasn't reserved then just return.
> */
> > + if (sync->status != DMABUF_SYNC_LOCKED)
> > + return -EAGAIN;
> > +
> > + dmabuf_sync_unlock_objs(sync, &sync->ctx);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_unlock);
> > +
> > +/**
> > + * dmabuf_sync_single_lock - lock a dma buf.
> > + *
> > + * @dmabuf: A dma buf object that tries to lock.
> > + * @type: A access type to a dma buf.
> > + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> > + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> > + * means that this dmabuf couldn't be accessed by others but would be
> > + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA
> can
> > + * be combined with other.
> > + * @wait: Indicate whether caller is blocked or not.
> > + * true means that caller will be blocked, and false means that this
> > + * function will return -EAGAIN if this caller can't take the lock
> > + * right now.
> > + *
> > + * The caller should call this function prior to CPU or DMA access to
> the dmabuf
> > + * so that others cannot access the dmabuf.
> > + */
> > +int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
> > + bool wait)
> > +{
> > + struct dmabuf_sync_reservation *robj;
> > +
> > + if (!dmabuf->sync) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type)) {
> > + WARN_ON(1);
> > + return -EINVAL;
> > + }
> > +
> > + get_dma_buf(dmabuf);
> > + robj = dmabuf->sync;
> > +
> > + mutex_lock(&robj->lock);
> > +
> > + /* Don't lock in case of read and read. */
> > + if (robj->accessed_type & DMA_BUF_ACCESS_R && type &
> DMA_BUF_ACCESS_R) {
> > + atomic_inc(&robj->shared_cnt);
> > + mutex_unlock(&robj->lock);
> > + return 0;
> > + }
> > +
> > + /*
> > + * In case of F_SETLK, just return -EAGAIN if this dmabuf has
> already
> > + * been locked.
> > + */
> > + if (!wait && robj->locked) {
> > + mutex_unlock(&robj->lock);
> > + dma_buf_put(dmabuf);
> > + return -EAGAIN;
> > + }
> > +
> > + mutex_unlock(&robj->lock);
> > +
> > + mutex_lock(&robj->sync_lock.base);
> > +
> > + mutex_lock(&robj->lock);
> > + robj->locked = true;
> > + mutex_unlock(&robj->lock);
>
> Are you missing an mutex_unlock on &robj->sync_lock.base?
> Oh wait, that is the purpose of this code. You might want
> to put a nice comment right above that and say: "Unlocked
> by dmabuf_sync_single_unlock"
Will add the comment.
>
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_single_lock);
> > +
> > +/**
> > + * dmabuf_sync_single_unlock - unlock a dma buf.
> > + *
> > + * @dmabuf: A dma buf object that tries to unlock.
> > + *
> > + * The caller should call this function after CPU or DMA access to
> > + * the dmabuf is completed so that others can access the dmabuf.
> > + */
> > +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> > +{
> > + struct dmabuf_sync_reservation *robj;
> > +
> > + if (!dmabuf->sync) {
> > + WARN_ON(1);
> > + return;
> > + }
> > +
> > + robj = dmabuf->sync;
> > +
> > + mutex_lock(&robj->lock);
> > +
> > + if (robj->polled) {
> > + robj->poll_event = true;
> > + robj->polled = false;
> > + wake_up_interruptible(&robj->poll_wait);
> > + }
> > +
> > + if (atomic_add_unless(&robj->shared_cnt, -1 , 1)) {
> > + mutex_unlock(&robj->lock);
> > + dma_buf_put(dmabuf);
> > + return;
> > + }
> > +
> > + mutex_unlock(&robj->lock);
> > +
> > + mutex_unlock(&robj->sync_lock.base);
> > +
> > + mutex_lock(&robj->lock);
> > + robj->locked = false;
> > + mutex_unlock(&robj->lock);
> > +
> > + dma_buf_put(dmabuf);
> > +
> > + return;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_single_unlock);
> > +
> > +/**
> > + * dmabuf_sync_get - Get dmabuf sync object.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + * @sync_buf: A dmabuf object to be synchronized with others.
> > + * @type: A access type to a dma buf.
> > + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> > + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> > + * means that this dmabuf couldn't be accessed by others but would be
> > + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA
> can
> > + * be combined with other.
> > + *
> > + * This function should be called after dmabuf_sync_init function is
> called.
> > + * The caller can tie up multiple dmabufs into one sync object by
> calling this
> > + * function several times. Internally, this function allocates
> > + * a dmabuf_sync_object and adds a given dmabuf to it, and also takes
> > + * a reference to a dmabuf.
> > + */
> > +int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf, unsigned
> int type)
> > +{
> > + int ret;
> > +
> > + if (!sync || !sync_buf) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + ret = dmabuf_sync_get_obj(sync, sync_buf, type);
> > + if (ret < 0) {
> > + WARN_ON(1);
> > + return ret;
> > + }
> > +
> > + sync->status = DMABUF_SYNC_GOT;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_get);
> > +
> > +/**
> > + * dmabuf_sync_put - Put dmabuf sync object to a given dmabuf.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + * @dmabuf: An dmabuf object.
> > + *
> > + * This function should be called if some operation is failed after
> > + * dmabuf_sync_get function is called to release the dmabuf, or
> > + * dmabuf_sync_unlock function is called. Internally, this function
> > + * removes a given dmabuf from a sync object and remove the sync
object.
> > + * At this time, the dmabuf is putted.
> > + */
> > +void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
> > +{
> > + if (!sync || !dmabuf) {
> > + WARN_ON(1);
> > + return;
> > + }
> > +
> > + if (list_empty(&sync->syncs))
> > + return;
> > +
> > + dmabuf_sync_put_obj(sync, dmabuf);
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_put);
> > +
> > +/**
> > + * dmabuf_sync_put_all - Put dmabuf sync object to dmabufs.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * This function should be called if some operation is failed after
> > + * dmabuf_sync_get function is called to release all sync objects, or
> > + * dmabuf_sync_unlock function is called. Internally, this function
> > + * removes dmabufs from a sync object and remove the sync object.
> > + * At this time, all dmabufs are putted.
> > + */
> > +void dmabuf_sync_put_all(struct dmabuf_sync *sync)
> > +{
> > + if (!sync) {
> > + WARN_ON(1);
> > + return;
> > + }
> > +
> > + if (list_empty(&sync->syncs))
> > + return;
> > +
> > + dmabuf_sync_put_objs(sync);
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_put_all);
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index dfac5ed..0109673 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -115,6 +115,7 @@ struct dma_buf_ops {
> > * @exp_name: name of the exporter; useful for debugging.
> > * @list_node: node for dma_buf accounting and debugging.
> > * @priv: exporter specific private data for this buffer object.
> > + * @sync: sync object linked to this dma-buf
> > */
> > struct dma_buf {
> > size_t size;
> > @@ -128,6 +129,7 @@ struct dma_buf {
> > const char *exp_name;
> > struct list_head list_node;
> > void *priv;
> > + void *sync;
> > };
> >
> > /**
> > @@ -148,6 +150,20 @@ struct dma_buf_attachment {
> > void *priv;
> > };
> >
> > +#define DMA_BUF_ACCESS_R 0x1
> > +#define DMA_BUF_ACCESS_W 0x2
> > +#define DMA_BUF_ACCESS_DMA 0x4
> > +#define DMA_BUF_ACCESS_RW (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_W)
> > +#define DMA_BUF_ACCESS_DMA_R (DMA_BUF_ACCESS_R |
DMA_BUF_ACCESS_DMA)
> > +#define DMA_BUF_ACCESS_DMA_W (DMA_BUF_ACCESS_W |
DMA_BUF_ACCESS_DMA)
> > +#define DMA_BUF_ACCESS_DMA_RW (DMA_BUF_ACCESS_DMA_R |
> DMA_BUF_ACCESS_DMA_W)
> > +#define IS_VALID_DMA_BUF_ACCESS_TYPE(t) (t = DMA_BUF_ACCESS_R || \
> > + t = DMA_BUF_ACCESS_W || \
> > + t = DMA_BUF_ACCESS_DMA_R || \
> > + t = DMA_BUF_ACCESS_DMA_W || \
> > + t = DMA_BUF_ACCESS_RW || \
> > + t = DMA_BUF_ACCESS_DMA_RW)
> > +
> > /**
> > * get_dma_buf - convenience wrapper for get_file.
> > * @dmabuf: [in] pointer to dma_buf
> > diff --git a/include/linux/dmabuf-sync.h b/include/linux/dmabuf-sync.h
> > new file mode 100644
> > index 0000000..9a3afc4
> > --- /dev/null
> > +++ b/include/linux/dmabuf-sync.h
> > @@ -0,0 +1,190 @@
> > +/*
> > + * Copyright (C) 2013 Samsung Electronics Co.Ltd
> > + * Authors:
> > + * Inki Dae <inki.dae@samsung.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify it
> > + * under the terms of the GNU General Public License as published by
> the
> > + * Free Software Foundation; either version 2 of the License, or (at
> your
> > + * option) any later version.
> > + *
> > + */
> > +
> > +#include <linux/mutex.h>
> > +#include <linux/sched.h>
> > +#include <linux/dma-buf.h>
> > +
> > +enum dmabuf_sync_status {
> > + DMABUF_SYNC_GOT = 1,
> > + DMABUF_SYNC_LOCKED,
> > +};
> > +
>
> No comment about this structure?
Will add comments.
>
> > +struct dmabuf_sync_reservation {
> > + struct ww_mutex sync_lock;
> > + struct mutex lock;
> > + wait_queue_head_t poll_wait;
> > + unsigned int poll_event;
> > + unsigned int polled;
> > + atomic_t shared_cnt;
> > + unsigned int accessed_type;
> > + unsigned int locked;
> > +};
> > +
> > +/*
> > + * A structure for dmabuf_sync_object.
> > + *
> > + * @head: A list head to be added to syncs list.
> > + * @robj: A reservation_object object.
> > + * @dma_buf: A dma_buf object.
> > + * @access_type: Indicate how a current task tries to access
> > + * a given buffer.
>
> Huh? What values are expected then? Is there some #define or enum
> for that?
>
Right, there are definitions for that. Will add more comments.
> > + */
> > +struct dmabuf_sync_object {
> > + struct list_head head;
> > + struct dmabuf_sync_reservation *robj;
> > + struct dma_buf *dmabuf;
> > + unsigned int access_type;
> > +};
> > +
> > +struct dmabuf_sync_priv_ops {
> > + void (*free)(void *priv);
> > +};
> > +
> > +/*
> > + * A structure for dmabuf_sync.
> > + *
> > + * @syncs: A list head to sync object and this is global to system.
> > + * @list: A list entry used as committed list node
> > + * @lock: A mutex lock to current sync object.
>
> You should say for which specific operations this mutex is needed.
> For everything? Or just for list operations.
Ok, will add more comments.
>
> > + * @ctx: A current context for ww mutex.
> > + * @work: A work struct to release resources at timeout.
> > + * @priv: A private data.
> > + * @name: A string to dmabuf sync owner.
> > + * @timer: A timer list to avoid lockup and release resources.
> > + * @status: Indicate current status (DMABUF_SYNC_GOT or
> DMABUF_SYNC_LOCKED).
> > + */
> > +struct dmabuf_sync {
> > + struct list_head syncs;
> > + struct list_head list;
> > + struct mutex lock;
> > + struct ww_acquire_ctx ctx;
> > + struct work_struct work;
> > + void *priv;
> > + struct dmabuf_sync_priv_ops *ops;
> > + char name[64];
>
> Perhaps a #define for the size?
Ok, will use macro instead.
>
> > + struct timer_list timer;
> > + unsigned int status;
> > +};
> > +
> > +#ifdef CONFIG_DMABUF_SYNC
> > +
> > +extern struct ww_class dmabuf_sync_ww_class;
> > +
> > +static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf)
> > +{
> > + struct dmabuf_sync_reservation *obj;
> > +
> > + obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> > + if (!obj)
> > + return;
> > +
> > + dmabuf->sync = obj;
> > +
> > + ww_mutex_init(&obj->sync_lock, &dmabuf_sync_ww_class);
> > +
> > + mutex_init(&obj->lock);
> > + atomic_set(&obj->shared_cnt, 1);
> > +
> > + init_waitqueue_head(&obj->poll_wait);
> > +}
> > +
> > +static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf)
> > +{
> > + struct dmabuf_sync_reservation *obj;
> > +
> > + if (!dmabuf->sync)
> > + return;
> > +
> > + obj = dmabuf->sync;
> > +
> > + ww_mutex_destroy(&obj->sync_lock);
> > +
> > + kfree(obj);
> > +}
> > +
> > +extern bool is_dmabuf_sync_supported(void);
> > +
> > +extern struct dmabuf_sync *dmabuf_sync_init(const char *name,
> > + struct dmabuf_sync_priv_ops *ops,
> > + void *priv);
> > +
> > +extern void dmabuf_sync_fini(struct dmabuf_sync *sync);
> > +
> > +extern int dmabuf_sync_lock(struct dmabuf_sync *sync);
> > +
> > +extern int dmabuf_sync_unlock(struct dmabuf_sync *sync);
> > +
> > +int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
> > + bool wait);
> > +
> > +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf);
> > +
> > +extern int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
> > + unsigned int type);
> > +
> > +extern void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf
> *dmabuf);
> > +
> > +extern void dmabuf_sync_put_all(struct dmabuf_sync *sync);
> > +
> > +#else
> > +
> > +static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf)
> { }
> > +
> > +static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf)
> { }
> > +
> > +static inline bool is_dmabuf_sync_supported(void) { return false; }
> > +
> > +static inline struct dmabuf_sync *dmabuf_sync_init(const char *name,
> > + struct dmabuf_sync_priv_ops *ops,
> > + void *priv)
> > +{
> > + return ERR_PTR(0);
> > +}
> > +
> > +static inline void dmabuf_sync_fini(struct dmabuf_sync *sync) { }
> > +
> > +static inline int dmabuf_sync_lock(struct dmabuf_sync *sync)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline int dmabuf_sync_single_lock(struct dma_buf *dmabuf,
> > + unsigned int type,
> > + bool wait)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> > +{
> > + return;
> > +}
> > +
> > +static inline int dmabuf_sync_get(struct dmabuf_sync *sync,
> > + void *sync_buf,
> > + unsigned int type)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void dmabuf_sync_put(struct dmabuf_sync *sync,
> > + struct dma_buf *dmabuf) { }
> > +
> > +static inline void dmabuf_sync_put_all(struct dmabuf_sync *sync) { }
> > +
> > +#endif
> > --
> > 1.7.5.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> 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
^ permalink raw reply
* Re: [PATCH] fbmem: move EXPORT_SYMBOL annotation next to symbol declarations
From: Daniel Mack @ 2013-08-21 9:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1376575975-13215-1-git-send-email-zonque@gmail.com>
On 15.08.2013 16:12, Daniel Mack wrote:
> Just a cosmetic thing to bring that file in line with others in the
> tree.
Jean-Christophe, in case you're fine with that patch, could you queue it
up for the next merge window?
Thanks,
Daniel
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
> drivers/video/fbmem.c | 29 +++++++++++++----------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 36e1fe2..dacaf74 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -43,8 +43,12 @@
> #define FBPIXMAPSIZE (1024 * 8)
>
> static DEFINE_MUTEX(registration_lock);
> +
> struct fb_info *registered_fb[FB_MAX] __read_mostly;
> +EXPORT_SYMBOL(registered_fb);
> +
> int num_registered_fb __read_mostly;
> +EXPORT_SYMBOL(num_registered_fb);
>
> static struct fb_info *get_fb_info(unsigned int idx)
> {
> @@ -182,6 +186,7 @@ char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size
>
> return addr;
> }
> +EXPORT_SYMBOL(fb_get_buffer_offset);
>
> #ifdef CONFIG_LOGO
>
> @@ -669,6 +674,7 @@ int fb_show_logo(struct fb_info *info, int rotate)
> int fb_prepare_logo(struct fb_info *info, int rotate) { return 0; }
> int fb_show_logo(struct fb_info *info, int rotate) { return 0; }
> #endif /* CONFIG_LOGO */
> +EXPORT_SYMBOL(fb_show_logo);
>
> static void *fb_seq_start(struct seq_file *m, loff_t *pos)
> {
> @@ -909,6 +915,7 @@ fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
> info->var.vmode &= ~FB_VMODE_YWRAP;
> return 0;
> }
> +EXPORT_SYMBOL(fb_pan_display);
>
> static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
> u32 activate)
> @@ -1042,6 +1049,7 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
> done:
> return ret;
> }
> +EXPORT_SYMBOL(fb_set_var);
>
> int
> fb_blank(struct fb_info *info, int blank)
> @@ -1073,6 +1081,7 @@ fb_blank(struct fb_info *info, int blank)
>
> return ret;
> }
> +EXPORT_SYMBOL(fb_blank);
>
> static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> unsigned long arg)
> @@ -1745,6 +1754,7 @@ register_framebuffer(struct fb_info *fb_info)
>
> return ret;
> }
> +EXPORT_SYMBOL(register_framebuffer);
>
> /**
> * unregister_framebuffer - releases a frame buffer device
> @@ -1773,6 +1783,7 @@ unregister_framebuffer(struct fb_info *fb_info)
>
> return ret;
> }
> +EXPORT_SYMBOL(unregister_framebuffer);
>
> /**
> * fb_set_suspend - low level driver signals suspend
> @@ -1796,6 +1807,7 @@ void fb_set_suspend(struct fb_info *info, int state)
> fb_notifier_call_chain(FB_EVENT_RESUME, &event);
> }
> }
> +EXPORT_SYMBOL(fb_set_suspend);
>
> /**
> * fbmem_init - init frame buffer subsystem
> @@ -1912,6 +1924,7 @@ int fb_get_options(const char *name, char **option)
>
> return retval;
> }
> +EXPORT_SYMBOL(fb_get_options);
>
> #ifndef MODULE
> /**
> @@ -1959,20 +1972,4 @@ static int __init video_setup(char *options)
> __setup("video=", video_setup);
> #endif
>
> - /*
> - * Visible symbols for modules
> - */
> -
> -EXPORT_SYMBOL(register_framebuffer);
> -EXPORT_SYMBOL(unregister_framebuffer);
> -EXPORT_SYMBOL(num_registered_fb);
> -EXPORT_SYMBOL(registered_fb);
> -EXPORT_SYMBOL(fb_show_logo);
> -EXPORT_SYMBOL(fb_set_var);
> -EXPORT_SYMBOL(fb_blank);
> -EXPORT_SYMBOL(fb_pan_display);
> -EXPORT_SYMBOL(fb_get_buffer_offset);
> -EXPORT_SYMBOL(fb_set_suspend);
> -EXPORT_SYMBOL(fb_get_options);
> -
> MODULE_LICENSE("GPL");
>
^ permalink raw reply
* [PATCH v7 0/2] Introduce buffer synchronization framework
From: Inki Dae @ 2013-08-21 10:33 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
Hi all,
This patch set introduces a buffer synchronization framework based
on DMA BUF[1] and based on ww-mutexes[2] for lock mechanism, and
has been rebased on linux-3.11-rc6.
The purpose of this framework is to provide not only buffer access
control to CPU and CPU, and CPU and DMA, and DMA and DMA but also
easy-to-use interfaces for device drivers and user application.
In addtion, this patch set suggests a way for enhancing performance.
Changelog v7:
Fix things pointed out by Konrad Rzeszutek Wilk,
- Use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL.
- Make sure to unlock and unreference all dmabuf objects
when dmabuf_sync_fini() is called.
- Add more comments.
- Code cleanups.
Changelog v6:
- Fix sync lock to multiple reads.
- Add select system call support.
. Wake up poll_wait when a dmabuf is unlocked.
- Remove unnecessary the use of mutex lock.
- Add private backend ops callbacks.
. This ops has one callback for device drivers to clean up their
sync object resource when the sync object is freed. For this,
device drivers should implement the free callback properly.
- Update document file.
Changelog v5:
- Rmove a dependence on reservation_object: the reservation_object is used
to hook up to ttm and dma-buf for easy sharing of reservations across
devices. However, the dmabuf sync can be used for all dma devices; v4l2
and drm based drivers, so doesn't need the reservation_object anymore.
With regared to this, it adds 'void *sync' to dma_buf structure.
- All patches are rebased on mainline, Linux v3.10.
Changelog v4:
- Add user side interface for buffer synchronization mechanism and update
descriptions related to the user side interface.
Changelog v3:
- remove cache operation relevant codes and update document file.
Changelog v2:
- use atomic_add_unless to avoid potential bug.
- add a macro for checking valid access type.
- code clean.
For generic user mode interface, we have used fcntl and select system
call[3]. As you know, user application sees a buffer object as a dma-buf
file descriptor. So fcntl() call with the file descriptor means to lock
some buffer region being managed by the dma-buf object. And select() call
means to wait for the completion of CPU or DMA access to the dma-buf
without locking. For more detail, you can refer to the dma-buf-sync.txt
in Documentation/
There are some cases we should use this buffer synchronization framework.
One of which is to primarily enhance GPU rendering performance on Tizen
platform in case of 3d app with compositing mode that 3d app draws
something in off-screen buffer, and Web app.
In case of 3d app with compositing mode which is not a full screen mode,
the app calls glFlush to submit 3d commands to GPU driver instead of
glFinish for more performance. The reason we call glFlush is that glFinish
blocks caller's task until the execution of the 2d commands is completed.
Thus, that makes GPU and CPU more idle. As result, 3d rendering performance
with glFinish is quite lower than glFlush. However, the use of glFlush has
one issue that the a buffer shared with GPU could be broken when CPU
accesses the buffer at once after glFlush because CPU cannot be aware of
the completion of GPU access to the buffer. Of course, the app can be aware
of that time using eglWaitGL but this function is valid only in case of the
same process.
The below summarizes how app's window is displayed on Tizen platform:
1. X client requests a window buffer to Xorg.
2. X client draws something in the window buffer using CPU.
3. X client requests SWAP to Xorg.
4. Xorg notifies a damage event to Composite Manager.
5. Composite Manager gets the window buffer (front buffer) through
DRI2GetBuffers.
6. Composite Manager composes the window buffer and its own back buffer
using GPU. At this time, eglSwapBuffers is called: internally, 3d
commands are flushed to gpu driver.
7. Composite Manager requests SWAP to Xorg.
8. Xorg performs drm page flip. At this time, the window buffer is
displayed on screen.
Web app based on HTML5 also has the same issue. Web browser and its web app
are different process. The web app draws something in its own pixmap buffer,
and then the web browser gets a window buffer from Xorg, and then composites
the pixmap buffer with the window buffer. And finally, page flip.
Thus, in such cases, a shared buffer could be broken as one process draws
something in pixmap buffer using CPU, when other process composites the
pixmap buffer with window buffer using GPU without any locking mechanism.
That is why we need user land locking interface, fcntl system call.
And last one is a deferred page flip issue. This issue is that a window
buffer rendered can be displayed on screen in about 32ms in worst case:
assume that the gpu rendering is completed within 16ms.
That can be incurred when compositing a pixmap buffer with a window buffer
using GPU and when vsync is just started. At this time, Xorg waits for
a vblank event to get a window buffer so 3d rendering will be delayed
up to about 16ms. As a result, the window buffer would be displayed in
about two vsyncs (about 32ms) and in turn, that would show slow
responsiveness.
For this, we could enhance the responsiveness with locking
mechanism: skipping one vblank wait. I guess in the similar reason,
Android, Chrome OS, and other platforms are using their own locking
mechanisms; Android sync driver, KDS, and DMA fence.
The below shows the deferred page flip issue in worst case,
|------------ <- vsync signal
|<------ DRI2GetBuffers
|
|
|
|------------ <- vsync signal
|<------ Request gpu rendering
time |
|
|<------ Request page flip (deferred)
|------------ <- vsync signal
|<------ Displayed on screen
|
|
|
|------------ <- vsync signal
Thanks,
Inki Dae
References:
[1] http://lwn.net/Articles/470339/
[2] https://patchwork.kernel.org/patch/2625361/
[3] http://linux.die.net/man/2/fcntl
Inki Dae (2):
dmabuf-sync: Add a buffer synchronization framework
dma-buf: Add user interfaces for dmabuf sync support
Documentation/dma-buf-sync.txt | 286 ++++++++++++++++
drivers/base/Kconfig | 7 +
drivers/base/Makefile | 1 +
drivers/base/dma-buf.c | 85 +++++
drivers/base/dmabuf-sync.c | 706 ++++++++++++++++++++++++++++++++++++++++
include/linux/dma-buf.h | 16 +
include/linux/dmabuf-sync.h | 236 ++++++++++++++
7 files changed, 1337 insertions(+), 0 deletions(-)
create mode 100644 Documentation/dma-buf-sync.txt
create mode 100644 drivers/base/dmabuf-sync.c
create mode 100644 include/linux/dmabuf-sync.h
--
1.7.5.4
^ permalink raw reply
* [PATCH v7 1/2] dmabuf-sync: Add a buffer synchronization framework
From: Inki Dae @ 2013-08-21 10:33 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
In-Reply-To: <1377081215-5948-1-git-send-email-inki.dae@samsung.com>
This patch adds a buffer synchronization framework based on DMA BUF[1]
and and based on ww-mutexes[2] for lock mechanism, and has been rebased
on linux-3.11-rc6.
The purpose of this framework is to provide not only buffer access control
to CPU and DMA but also easy-to-use interfaces for device drivers and
user application. This framework can be used for all dma devices using
system memory as dma buffer, especially for most ARM based SoCs.
Changelog v7:
Fix things pointed out by Konrad Rzeszutek Wilk,
- Use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL.
- Make sure to unlock and unreference all dmabuf objects
when dmabuf_sync_fini() is called.
- Add more comments.
- Code cleanups.
Changelog v6:
- Fix sync lock to multiple reads.
- Add select system call support.
. Wake up poll_wait when a dmabuf is unlocked.
- Remove unnecessary the use of mutex lock.
- Add private backend ops callbacks.
. This ops has one callback for device drivers to clean up their
sync object resource when the sync object is freed. For this,
device drivers should implement the free callback properly.
- Update document file.
Changelog v5:
- Rmove a dependence on reservation_object: the reservation_object is used
to hook up to ttm and dma-buf for easy sharing of reservations across
devices. However, the dmabuf sync can be used for all dma devices; v4l2
and drm based drivers, so doesn't need the reservation_object anymore.
With regared to this, it adds 'void *sync' to dma_buf structure.
- All patches are rebased on mainline, Linux v3.10.
Changelog v4:
- Add user side interface for buffer synchronization mechanism and update
descriptions related to the user side interface.
Changelog v3:
- remove cache operation relevant codes and update document file.
Changelog v2:
- use atomic_add_unless to avoid potential bug.
- add a macro for checking valid access type.
- code clean.
The mechanism of this framework has the following steps,
1. Register dmabufs to a sync object - A task gets a new sync object and
can add one or more dmabufs that the task wants to access.
This registering should be performed when a device context or an event
context such as a page flip event is created or before CPU accesses a shared
buffer.
dma_buf_sync_get(a sync object, a dmabuf);
2. Lock a sync object - A task tries to lock all dmabufs added in its own
sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid dead
lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
and DMA. Taking a lock means that others cannot access all locked dmabufs
until the task that locked the corresponding dmabufs, unlocks all the locked
dmabufs.
This locking should be performed before DMA or CPU accesses these dmabufs.
dma_buf_sync_lock(a sync object);
3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
object. The unlock means that the DMA or CPU accesses to the dmabufs have
been completed so that others may access them.
This unlocking should be performed after DMA or CPU has completed accesses
to the dmabufs.
dma_buf_sync_unlock(a sync object);
4. Unregister one or all dmabufs from a sync object - A task unregisters
the given dmabufs from the sync object. This means that the task dosen't
want to lock the dmabufs.
The unregistering should be performed after DMA or CPU has completed
accesses to the dmabufs or when dma_buf_sync_lock() is failed.
dma_buf_sync_put(a sync object, a dmabuf);
dma_buf_sync_put_all(a sync object);
The described steps may be summarized as:
get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
This framework includes the following two features.
1. read (shared) and write (exclusive) locks - A task is required to declare
the access type when the task tries to register a dmabuf;
READ, WRITE, READ DMA, or WRITE DMA.
The below is example codes,
struct dmabuf_sync *sync;
sync = dmabuf_sync_init(...);
...
dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
...
And the below can be used as access types:
DMA_BUF_ACCESS_R - CPU will access a buffer for read.
DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or
write.
2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
A task may never try to unlock a buffer after taking a lock to the buffer.
In this case, a timer handler to the corresponding sync object is called
in five (default) seconds and then the timed-out buffer is unlocked by work
queue handler to avoid lockups and to enforce resources of the buffer.
The below is how to use interfaces for device driver:
1. Allocate and Initialize a sync object:
static void xxx_dmabuf_sync_free(void *priv)
{
struct xxx_context *ctx = priv;
if (!ctx)
return;
ctx->sync = NULL;
}
...
static struct dmabuf_sync_priv_ops driver_specific_ops = {
.free = xxx_dmabuf_sync_free,
};
...
struct dmabuf_sync *sync;
sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
...
2. Add a dmabuf to the sync object when setting up dma buffer relevant
registers:
dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
...
3. Lock all dmabufs of the sync object before DMA or CPU accesses
the dmabufs:
dmabuf_sync_lock(sync);
...
4. Now CPU or DMA can access all dmabufs locked in step 3.
5. Unlock all dmabufs added in a sync object after DMA or CPU access
to these dmabufs is completed:
dmabuf_sync_unlock(sync);
And call the following functions to release all resources,
dmabuf_sync_put_all(sync);
dmabuf_sync_fini(sync);
You can refer to actual example codes:
"drm/exynos: add dmabuf sync support for g2d driver" and
"drm/exynos: add dmabuf sync support for kms framework" from
https://git.kernel.org/cgit/linux/kernel/git/daeinki/
drm-exynos.git/log/?h=dmabuf-sync
And this framework includes fcntl[3] and select system call as interfaces
exported to user. As you know, user sees a buffer object as a dma-buf file
descriptor. fcntl() call with the file descriptor means to lock some buffer
region being managed by the dma-buf object. And select() call with the file
descriptor means to poll the completion event of CPU or DMA access to
the dma-buf.
The below is how to use interfaces for user application:
fcntl system call:
struct flock filelock;
1. Lock a dma buf:
filelock.l_type = F_WRLCK or F_RDLCK;
/* lock entire region to the dma buf. */
filelock.lwhence = SEEK_CUR;
filelock.l_start = 0;
filelock.l_len = 0;
fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
...
CPU access to the dma buf
2. Unlock a dma buf:
filelock.l_type = F_UNLCK;
fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
close(dmabuf fd) call would also unlock the dma buf. And for more
detail, please refer to [3]
select system call:
fd_set wdfs or rdfs;
FD_ZERO(&wdfs or &rdfs);
FD_SET(fd, &wdfs or &rdfs);
select(fd + 1, &rdfs, NULL, NULL, NULL);
or
select(fd + 1, NULL, &wdfs, NULL, NULL);
Every time select system call is called, a caller will wait for
the completion of DMA or CPU access to a shared buffer if there
is someone accessing the shared buffer. If no anyone then select
system call will be returned at once.
References:
[1] http://lwn.net/Articles/470339/
[2] https://patchwork.kernel.org/patch/2625361/
[3] http://linux.die.net/man/2/fcntl
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Documentation/dma-buf-sync.txt | 286 ++++++++++++++++
drivers/base/Kconfig | 7 +
drivers/base/Makefile | 1 +
drivers/base/dma-buf.c | 4 +
drivers/base/dmabuf-sync.c | 706 ++++++++++++++++++++++++++++++++++++++++
include/linux/dma-buf.h | 16 +
include/linux/dmabuf-sync.h | 236 ++++++++++++++
7 files changed, 1256 insertions(+), 0 deletions(-)
create mode 100644 Documentation/dma-buf-sync.txt
create mode 100644 drivers/base/dmabuf-sync.c
create mode 100644 include/linux/dmabuf-sync.h
diff --git a/Documentation/dma-buf-sync.txt b/Documentation/dma-buf-sync.txt
new file mode 100644
index 0000000..5945c8a
--- /dev/null
+++ b/Documentation/dma-buf-sync.txt
@@ -0,0 +1,286 @@
+ DMA Buffer Synchronization Framework
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+ Inki Dae
+ <inki dot dae at samsung dot com>
+ <daeinki at gmail dot com>
+
+This document is a guide for device-driver writers describing the DMA buffer
+synchronization API. This document also describes how to use the API to
+use buffer synchronization mechanism between DMA and DMA, CPU and DMA, and
+CPU and CPU.
+
+The DMA Buffer synchronization API provides buffer synchronization mechanism;
+i.e., buffer access control to CPU and DMA, and easy-to-use interfaces for
+device drivers and user application. And this API can be used for all dma
+devices using system memory as dma buffer, especially for most ARM based SoCs.
+
+
+Motivation
+----------
+
+Buffer synchronization issue between DMA and DMA:
+ Sharing a buffer, a device cannot be aware of when the other device
+ will access the shared buffer: a device may access a buffer containing
+ wrong data if the device accesses the shared buffer while another
+ device is still accessing the shared buffer.
+ Therefore, a user process should have waited for the completion of DMA
+ access by another device before a device tries to access the shared
+ buffer.
+
+Buffer synchronization issue between CPU and DMA:
+ A user process should consider that when having to send a buffer, filled
+ by CPU, to a device driver for the device driver to access the buffer as
+ a input buffer while CPU and DMA are sharing the buffer.
+ This means that the user process needs to understand how the device
+ driver is worked. Hence, the conventional mechanism not only makes
+ user application complicated but also incurs performance overhead.
+
+Buffer synchronization issue between CPU and CPU:
+ In case that two processes share one buffer; shared with DMA also,
+ they may need some mechanism to allow process B to access the shared
+ buffer after the completion of CPU access by process A.
+ Therefore, process B should have waited for the completion of CPU access
+ by process A using the mechanism before trying to access the shared
+ buffer.
+
+What is the best way to solve these buffer synchronization issues?
+ We may need a common object that a device driver and a user process
+ notify the common object of when they try to access a shared buffer.
+ That way we could decide when we have to allow or not to allow for CPU
+ or DMA to access the shared buffer through the common object.
+ If so, what could become the common object? Right, that's a dma-buf[1].
+ Now we have already been using the dma-buf to share one buffer with
+ other drivers.
+
+
+Basic concept
+-------------
+
+The mechanism of this framework has the following steps,
+ 1. Register dmabufs to a sync object - A task gets a new sync object and
+ can add one or more dmabufs that the task wants to access.
+ This registering should be performed when a device context or an event
+ context such as a page flip event is created or before CPU accesses a shared
+ buffer.
+
+ dma_buf_sync_get(a sync object, a dmabuf);
+
+ 2. Lock a sync object - A task tries to lock all dmabufs added in its own
+ sync object. Basically, the lock mechanism uses ww-mutexes[2] to avoid dead
+ lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
+ and DMA. Taking a lock means that others cannot access all locked dmabufs
+ until the task that locked the corresponding dmabufs, unlocks all the locked
+ dmabufs.
+ This locking should be performed before DMA or CPU accesses these dmabufs.
+
+ dma_buf_sync_lock(a sync object);
+
+ 3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
+ object. The unlock means that the DMA or CPU accesses to the dmabufs have
+ been completed so that others may access them.
+ This unlocking should be performed after DMA or CPU has completed accesses
+ to the dmabufs.
+
+ dma_buf_sync_unlock(a sync object);
+
+ 4. Unregister one or all dmabufs from a sync object - A task unregisters
+ the given dmabufs from the sync object. This means that the task dosen't
+ want to lock the dmabufs.
+ The unregistering should be performed after DMA or CPU has completed
+ accesses to the dmabufs or when dma_buf_sync_lock() is failed.
+
+ dma_buf_sync_put(a sync object, a dmabuf);
+ dma_buf_sync_put_all(a sync object);
+
+ The described steps may be summarized as:
+ get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
+
+This framework includes the following two features.
+ 1. read (shared) and write (exclusive) locks - A task is required to declare
+ the access type when the task tries to register a dmabuf;
+ READ, WRITE, READ DMA, or WRITE DMA.
+
+ The below is example codes,
+ struct dmabuf_sync *sync;
+
+ sync = dmabuf_sync_init(NULL, "test sync");
+
+ dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
+ ...
+
+ 2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
+ A task may never try to unlock a buffer after taking a lock to the buffer.
+ In this case, a timer handler to the corresponding sync object is called
+ in five (default) seconds and then the timed-out buffer is unlocked by work
+ queue handler to avoid lockups and to enforce resources of the buffer.
+
+
+Access types
+------------
+
+DMA_BUF_ACCESS_R - CPU will access a buffer for read.
+DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
+DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
+DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or write.
+
+
+Generic user interfaces
+-----------------------
+
+And this framework includes fcntl[3] and select system calls as interfaces
+exported to user. As you know, user sees a buffer object as a dma-buf file
+descriptor. fcntl() call with the file descriptor means to lock some buffer
+region being managed by the dma-buf object. And select call with the file
+descriptor means to poll the completion event of CPU or DMA access to
+the dma-buf.
+
+
+API set
+-------
+
+bool is_dmabuf_sync_supported(void)
+ - Check if dmabuf sync is supported or not.
+
+struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void priv*)
+ - Allocate and initialize a new sync object. The caller can get a new
+ sync object for buffer synchronization. ops is used for device driver
+ to clean up its own sync object. For this, each device driver should
+ implement a free callback. priv is used for device driver to get its
+ device context when free callback is called.
+
+void dmabuf_sync_fini(struct dmabuf_sync *sync)
+ - Release all resources to the sync object.
+
+int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
+ unsigned int type)
+ - Get dmabuf sync object. Internally, this function allocates
+ a dmabuf_sync object and adds a given dmabuf to it, and also takes
+ a reference to the dmabuf. The caller can tie up multiple dmabufs
+ into one sync object by calling this function several times.
+
+void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
+ - Put dmabuf sync object to a given dmabuf. Internally, this function
+ removes a given dmabuf from a sync object and remove the sync object.
+ At this time, the dmabuf is putted.
+
+void dmabuf_sync_put_all(struct dmabuf_sync *sync)
+ - Put dmabuf sync object to dmabufs. Internally, this function removes
+ all dmabufs from a sync object and remove the sync object.
+ At this time, all dmabufs are putted.
+
+int dmabuf_sync_lock(struct dmabuf_sync *sync)
+ - Lock all dmabufs added in a sync object. The caller should call this
+ function prior to CPU or DMA access to the dmabufs so that others can
+ not access the dmabufs. Internally, this function avoids dead lock
+ issue with ww-mutexes.
+
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf)
+ - Lock a dmabuf. The caller should call this
+ function prior to CPU or DMA access to the dmabuf so that others can
+ not access the dmabuf.
+
+int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+ - Unlock all dmabufs added in a sync object. The caller should call
+ this function after CPU or DMA access to the dmabufs is completed so
+ that others can access the dmabufs.
+
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+ - Unlock a dmabuf. The caller should call this function after CPU or
+ DMA access to the dmabuf is completed so that others can access
+ the dmabuf.
+
+
+Tutorial for device driver
+--------------------------
+
+1. Allocate and Initialize a sync object:
+ static void xxx_dmabuf_sync_free(void *priv)
+ {
+ struct xxx_context *ctx = priv;
+
+ if (!ctx)
+ return;
+
+ ctx->sync = NULL;
+ }
+ ...
+
+ static struct dmabuf_sync_priv_ops driver_specific_ops = {
+ .free = xxx_dmabuf_sync_free,
+ };
+ ...
+
+ struct dmabuf_sync *sync;
+
+ sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
+ ...
+
+2. Add a dmabuf to the sync object when setting up dma buffer relevant registers:
+ dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
+ ...
+
+3. Lock all dmabufs of the sync object before DMA or CPU accesses the dmabufs:
+ dmabuf_sync_lock(sync);
+ ...
+
+4. Now CPU or DMA can access all dmabufs locked in step 3.
+
+5. Unlock all dmabufs added in a sync object after DMA or CPU access to these
+ dmabufs is completed:
+ dmabuf_sync_unlock(sync);
+
+ And call the following functions to release all resources,
+ dmabuf_sync_put_all(sync);
+ dmabuf_sync_fini(sync);
+
+
+Tutorial for user application
+-----------------------------
+fcntl system call:
+
+ struct flock filelock;
+
+1. Lock a dma buf:
+ filelock.l_type = F_WRLCK or F_RDLCK;
+
+ /* lock entire region to the dma buf. */
+ filelock.lwhence = SEEK_CUR;
+ filelock.l_start = 0;
+ filelock.l_len = 0;
+
+ fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
+ ...
+ CPU access to the dma buf
+
+2. Unlock a dma buf:
+ filelock.l_type = F_UNLCK;
+
+ fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
+
+ close(dmabuf fd) call would also unlock the dma buf. And for more
+ detail, please refer to [3]
+
+
+select system call:
+
+ fd_set wdfs or rdfs;
+
+ FD_ZERO(&wdfs or &rdfs);
+ FD_SET(fd, &wdfs or &rdfs);
+
+ select(fd + 1, &rdfs, NULL, NULL, NULL);
+ or
+ select(fd + 1, NULL, &wdfs, NULL, NULL);
+
+ Every time select system call is called, a caller will wait for
+ the completion of DMA or CPU access to a shared buffer if there
+ is someone accessing the shared buffer. If no anyone then select
+ system call will be returned at once.
+
+References:
+[1] http://lwn.net/Articles/470339/
+[2] https://patchwork.kernel.org/patch/2625361/
+[3] http://linux.die.net/man/2/fcntl
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 5daa259..35e1518 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -200,6 +200,13 @@ config DMA_SHARED_BUFFER
APIs extension; the file's descriptor can then be passed on to other
driver.
+config DMABUF_SYNC
+ bool "DMABUF Synchronization Framework"
+ depends on DMA_SHARED_BUFFER
+ help
+ This option enables dmabuf sync framework for buffer synchronization between
+ DMA and DMA, CPU and DMA, and CPU and CPU.
+
config CMA
bool "Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 48029aa..e06a5d7 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -11,6 +11,7 @@ obj-y += power/
obj-$(CONFIG_HAS_DMA) += dma-mapping.o
obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o reservation.o
+obj-$(CONFIG_DMABUF_SYNC) += dmabuf-sync.o
obj-$(CONFIG_ISA) += isa.o
obj-$(CONFIG_FW_LOADER) += firmware_class.o
obj-$(CONFIG_NUMA) += node.o
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 6687ba7..4aca57a 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -29,6 +29,7 @@
#include <linux/export.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/dmabuf-sync.h>
static inline int is_dma_buf_file(struct file *);
@@ -56,6 +57,8 @@ static int dma_buf_release(struct inode *inode, struct file *file)
list_del(&dmabuf->list_node);
mutex_unlock(&db_list.lock);
+ dmabuf_sync_reservation_fini(dmabuf);
+
kfree(dmabuf);
return 0;
}
@@ -134,6 +137,7 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
+ dmabuf_sync_reservation_init(dmabuf);
dmabuf->file = file;
mutex_init(&dmabuf->lock);
diff --git a/drivers/base/dmabuf-sync.c b/drivers/base/dmabuf-sync.c
new file mode 100644
index 0000000..5d69aef
--- /dev/null
+++ b/drivers/base/dmabuf-sync.c
@@ -0,0 +1,706 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics Co.Ltd
+ * Authors:
+ * Inki Dae <inki.dae@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/debugfs.h>
+#include <linux/uaccess.h>
+
+#include <linux/dmabuf-sync.h>
+
+#define MAX_SYNC_TIMEOUT 5 /* Second. */
+
+int dmabuf_sync_enabled = 1;
+
+MODULE_PARM_DESC(enabled, "Check if dmabuf sync is supported or not");
+module_param_named(enabled, dmabuf_sync_enabled, int, 0444);
+
+DEFINE_WW_CLASS(dmabuf_sync_ww_class);
+
+static void dmabuf_sync_timeout_worker(struct work_struct *work)
+{
+ struct dmabuf_sync *sync = container_of(work, struct dmabuf_sync, work);
+ struct dmabuf_sync_object *sobj;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ BUG_ON(!rsvp);
+
+ mutex_lock(&rsvp->lock);
+
+ pr_warn("%s: timeout = 0x%p [type = %d:%d, " \
+ "refcnt = %d, locked = %d]\n",
+ sync->name, sobj->dmabuf,
+ rsvp->accessed_type,
+ sobj->access_type,
+ atomic_read(&rsvp->shared_cnt),
+ rsvp->locked);
+
+ /* unlock only valid sync object. */
+ if (!rsvp->locked) {
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ if (rsvp->polled) {
+ rsvp->poll_event = true;
+ rsvp->polled = false;
+ wake_up_interruptible(&rsvp->poll_wait);
+ }
+
+ if (atomic_add_unless(&rsvp->shared_cnt, -1, 1)) {
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ mutex_unlock(&rsvp->lock);
+
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+
+ if (sobj->access_type & DMA_BUF_ACCESS_R)
+ pr_warn("%s: r-unlocked = 0x%p\n",
+ sync->name, sobj->dmabuf);
+ else
+ pr_warn("%s: w-unlocked = 0x%p\n",
+ sync->name, sobj->dmabuf);
+
+ mutex_unlock(&rsvp->lock);
+ }
+
+ sync->status = 0;
+ mutex_unlock(&sync->lock);
+
+ dmabuf_sync_put_all(sync);
+ dmabuf_sync_fini(sync);
+}
+
+static void dmabuf_sync_lock_timeout(unsigned long arg)
+{
+ struct dmabuf_sync *sync = (struct dmabuf_sync *)arg;
+
+ schedule_work(&sync->work);
+}
+
+static int dmabuf_sync_lock_objs(struct dmabuf_sync *sync,
+ struct ww_acquire_ctx *ctx)
+{
+ struct dmabuf_sync_object *contended_sobj = NULL;
+ struct dmabuf_sync_object *res_sobj = NULL;
+ struct dmabuf_sync_object *sobj = NULL;
+ int ret;
+
+ if (ctx)
+ ww_acquire_init(ctx, &dmabuf_sync_ww_class);
+
+retry:
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ if (WARN_ON(!rsvp))
+ continue;
+
+ mutex_lock(&rsvp->lock);
+
+ /* Don't lock in case of read and read. */
+ if (rsvp->accessed_type & DMA_BUF_ACCESS_R &&
+ sobj->access_type & DMA_BUF_ACCESS_R) {
+ atomic_inc(&rsvp->shared_cnt);
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ if (sobj = res_sobj) {
+ res_sobj = NULL;
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ mutex_unlock(&rsvp->lock);
+
+ ret = ww_mutex_lock(&rsvp->sync_lock, ctx);
+ if (ret < 0) {
+ contended_sobj = sobj;
+
+ if (ret = -EDEADLK)
+ pr_warn("%s: deadlock = 0x%p\n",
+ sync->name, sobj->dmabuf);
+ goto err;
+ }
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = true;
+
+ mutex_unlock(&rsvp->lock);
+ }
+
+ if (ctx)
+ ww_acquire_done(ctx);
+
+ init_timer(&sync->timer);
+
+ sync->timer.data = (unsigned long)sync;
+ sync->timer.function = dmabuf_sync_lock_timeout;
+ sync->timer.expires = jiffies + (HZ * MAX_SYNC_TIMEOUT);
+
+ add_timer(&sync->timer);
+
+ return 0;
+
+err:
+ list_for_each_entry_continue_reverse(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ mutex_lock(&rsvp->lock);
+
+ /* Don't need to unlock in case of read and read. */
+ if (atomic_add_unless(&rsvp->shared_cnt, -1, 1)) {
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ mutex_unlock(&rsvp->lock);
+
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+ mutex_unlock(&rsvp->lock);
+ }
+
+ if (res_sobj) {
+ struct dmabuf_sync_reservation *rsvp = res_sobj->robj;
+
+ mutex_lock(&rsvp->lock);
+
+ if (!atomic_add_unless(&rsvp->shared_cnt, -1, 1)) {
+ mutex_unlock(&rsvp->lock);
+
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+ }
+
+ mutex_unlock(&rsvp->lock);
+ }
+
+ if (ret = -EDEADLK) {
+ ww_mutex_lock_slow(&contended_sobj->robj->sync_lock, ctx);
+ res_sobj = contended_sobj;
+
+ goto retry;
+ }
+
+ if (ctx)
+ ww_acquire_fini(ctx);
+
+ return ret;
+}
+
+static void dmabuf_sync_unlock_objs(struct dmabuf_sync *sync,
+ struct ww_acquire_ctx *ctx)
+{
+ struct dmabuf_sync_object *sobj;
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ mutex_lock(&rsvp->lock);
+
+ if (rsvp->polled) {
+ rsvp->poll_event = true;
+ rsvp->polled = false;
+ wake_up_interruptible(&rsvp->poll_wait);
+ }
+
+ if (atomic_add_unless(&rsvp->shared_cnt, -1, 1)) {
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ mutex_unlock(&rsvp->lock);
+
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+ mutex_unlock(&rsvp->lock);
+ }
+
+ mutex_unlock(&sync->lock);
+
+ if (ctx)
+ ww_acquire_fini(ctx);
+
+ del_timer(&sync->timer);
+}
+
+/**
+ * dmabuf_sync_is_supported - Check if dmabuf sync is supported or not.
+ */
+bool dmabuf_sync_is_supported(void)
+{
+ return dmabuf_sync_enabled = 1;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_is_supported);
+
+/**
+ * dmabuf_sync_init - Allocate and initialize a dmabuf sync.
+ *
+ * @priv: A device private data.
+ * @name: A sync object name.
+ *
+ * This function should be called when a device context or an event
+ * context such as a page flip event is created. And the created
+ * dmabuf_sync object should be set to the context.
+ * The caller can get a new sync object for buffer synchronization
+ * through this function.
+ */
+struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv)
+{
+ struct dmabuf_sync *sync;
+
+ sync = kzalloc(sizeof(*sync), GFP_KERNEL);
+ if (!sync)
+ return ERR_PTR(-ENOMEM);
+
+ strncpy(sync->name, name, DMABUF_SYNC_NAME_SIZE);
+
+ sync->ops = ops;
+ sync->priv = priv;
+ INIT_LIST_HEAD(&sync->syncs);
+ mutex_init(&sync->lock);
+ INIT_WORK(&sync->work, dmabuf_sync_timeout_worker);
+
+ return sync;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_init);
+
+/**
+ * dmabuf_sync_fini - Release a given dmabuf sync.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_init call to release relevant resources, and after
+ * dmabuf_sync_unlock function is called.
+ */
+void dmabuf_sync_fini(struct dmabuf_sync *sync)
+{
+ struct dmabuf_sync_object *sobj;
+
+ if (WARN_ON(!sync))
+ return;
+
+ if (list_empty(&sync->syncs))
+ goto free_sync;
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ mutex_lock(&rsvp->lock);
+
+ if (rsvp->locked) {
+ mutex_unlock(&rsvp->lock);
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+ }
+
+ mutex_unlock(&rsvp->lock);
+ }
+
+ /*
+ * If !list_empty(&sync->syncs) then it means that dmabuf_sync_put()
+ * or dmabuf_sync_put_all() was never called. So unreference all
+ * dmabuf objects added to sync->syncs, and remove them from the syncs.
+ */
+ dmabuf_sync_put_all(sync);
+
+free_sync:
+ if (sync->ops && sync->ops->free)
+ sync->ops->free(sync->priv);
+
+ kfree(sync);
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_fini);
+
+/*
+ * dmabuf_sync_get_obj - Add a given object to sync's list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @dmabuf: An object to dma_buf structure.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can be
+ * combined.
+ *
+ * This function creates and initializes a new dmabuf sync object and it adds
+ * the dmabuf sync object to syncs list to track and manage all dmabufs.
+ */
+static int dmabuf_sync_get_obj(struct dmabuf_sync *sync, struct dma_buf *dmabuf,
+ unsigned int type)
+{
+ struct dmabuf_sync_object *sobj;
+
+ if (!dmabuf->sync)
+ return -EFAULT;
+
+ if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type))
+ return -EINVAL;
+
+ if ((type & DMA_BUF_ACCESS_RW) = DMA_BUF_ACCESS_RW)
+ type &= ~DMA_BUF_ACCESS_R;
+
+ sobj = kzalloc(sizeof(*sobj), GFP_KERNEL);
+ if (!sobj)
+ return -ENOMEM;
+
+ get_dma_buf(dmabuf);
+
+ sobj->dmabuf = dmabuf;
+ sobj->robj = dmabuf->sync;
+ sobj->access_type = type;
+
+ mutex_lock(&sync->lock);
+ list_add_tail(&sobj->head, &sync->syncs);
+ mutex_unlock(&sync->lock);
+
+ return 0;
+}
+
+/*
+ * dmabuf_sync_put_obj - Release a given sync object.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation failed after
+ * dmabuf_sync_get_obj call to release a given sync object.
+ */
+static void dmabuf_sync_put_obj(struct dmabuf_sync *sync,
+ struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_object *sobj;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ if (sobj->dmabuf != dmabuf)
+ continue;
+
+ dma_buf_put(sobj->dmabuf);
+
+ list_del_init(&sobj->head);
+ kfree(sobj);
+ break;
+ }
+
+ if (list_empty(&sync->syncs))
+ sync->status = 0;
+
+ mutex_unlock(&sync->lock);
+}
+
+/*
+ * dmabuf_sync_put_objs - Release all sync objects of dmabuf_sync.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation failed after
+ * dmabuf_sync_get_obj call to release all sync objects.
+ */
+static void dmabuf_sync_put_objs(struct dmabuf_sync *sync)
+{
+ struct dmabuf_sync_object *sobj, *next;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry_safe(sobj, next, &sync->syncs, head) {
+ dma_buf_put(sobj->dmabuf);
+
+ list_del_init(&sobj->head);
+ kfree(sobj);
+ }
+
+ mutex_unlock(&sync->lock);
+
+ sync->status = 0;
+}
+
+/**
+ * dmabuf_sync_lock - lock all dmabufs added to syncs list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * The caller should call this function prior to CPU or DMA access to
+ * the dmabufs so that others can not access the dmabufs.
+ * Internally, this function avoids dead lock issue with ww-mutex.
+ */
+int dmabuf_sync_lock(struct dmabuf_sync *sync)
+{
+ int ret;
+
+ if (!sync)
+ return -EFAULT;
+
+ if (list_empty(&sync->syncs))
+ return -EINVAL;
+
+ if (sync->status != DMABUF_SYNC_GOT)
+ return -EINVAL;
+
+ ret = dmabuf_sync_lock_objs(sync, &sync->ctx);
+ if (ret < 0)
+ return ret;
+
+ sync->status = DMABUF_SYNC_LOCKED;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_lock);
+
+/**
+ * dmabuf_sync_unlock - unlock all objects added to syncs list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * The caller should call this function after CPU or DMA access to
+ * the dmabufs is completed so that others can access the dmabufs.
+ */
+int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+{
+ if (!sync)
+ return -EFAULT;
+
+ /* If current dmabuf sync object wasn't reserved then just return. */
+ if (sync->status != DMABUF_SYNC_LOCKED)
+ return -EAGAIN;
+
+ dmabuf_sync_unlock_objs(sync, &sync->ctx);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_unlock);
+
+/**
+ * dmabuf_sync_single_lock - lock a dma buf.
+ *
+ * @dmabuf: A dma buf object that tries to lock.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can
+ * be combined with other.
+ * @wait: Indicate whether caller is blocked or not.
+ * true means that caller will be blocked, and false means that this
+ * function will return -EAGAIN if this caller can't take the lock
+ * right now.
+ *
+ * The caller should call this function prior to CPU or DMA access to the dmabuf
+ * so that others cannot access the dmabuf.
+ */
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
+ bool wait)
+{
+ struct dmabuf_sync_reservation *robj;
+
+ if (!dmabuf->sync)
+ return -EFAULT;
+
+ if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type))
+ return -EINVAL;
+
+ get_dma_buf(dmabuf);
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ /* Don't lock in case of read and read. */
+ if (robj->accessed_type & DMA_BUF_ACCESS_R && type & DMA_BUF_ACCESS_R) {
+ atomic_inc(&robj->shared_cnt);
+ mutex_unlock(&robj->lock);
+ return 0;
+ }
+
+ /*
+ * In case of F_SETLK, just return -EAGAIN if this dmabuf has already
+ * been locked.
+ */
+ if (!wait && robj->locked) {
+ mutex_unlock(&robj->lock);
+ dma_buf_put(dmabuf);
+ return -EAGAIN;
+ }
+
+ mutex_unlock(&robj->lock);
+
+ /* Unlocked by dmabuf_sync_single_unlock or dmabuf_sync_unlock. */
+ mutex_lock(&robj->sync_lock.base);
+
+ mutex_lock(&robj->lock);
+ robj->locked = true;
+ mutex_unlock(&robj->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_single_lock);
+
+/**
+ * dmabuf_sync_single_unlock - unlock a dma buf.
+ *
+ * @dmabuf: A dma buf object that tries to unlock.
+ *
+ * The caller should call this function after CPU or DMA access to
+ * the dmabuf is completed so that others can access the dmabuf.
+ */
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *robj;
+
+ if (!dmabuf->sync) {
+ WARN_ON(1);
+ return;
+ }
+
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ if (robj->polled) {
+ robj->poll_event = true;
+ robj->polled = false;
+ wake_up_interruptible(&robj->poll_wait);
+ }
+
+ if (atomic_add_unless(&robj->shared_cnt, -1 , 1)) {
+ mutex_unlock(&robj->lock);
+ dma_buf_put(dmabuf);
+ return;
+ }
+
+ mutex_unlock(&robj->lock);
+
+ mutex_unlock(&robj->sync_lock.base);
+
+ mutex_lock(&robj->lock);
+ robj->locked = false;
+ mutex_unlock(&robj->lock);
+
+ dma_buf_put(dmabuf);
+
+ return;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_single_unlock);
+
+/**
+ * dmabuf_sync_get - Get dmabuf sync object.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @sync_buf: A dmabuf object to be synchronized with others.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can
+ * be combined with other.
+ *
+ * This function should be called after dmabuf_sync_init function is called.
+ * The caller can tie up multiple dmabufs into one sync object by calling this
+ * function several times. Internally, this function allocates
+ * a dmabuf_sync_object and adds a given dmabuf to it, and also takes
+ * a reference to a dmabuf.
+ */
+int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf, unsigned int type)
+{
+ int ret;
+
+ if (!sync || !sync_buf)
+ return -EFAULT;
+
+ ret = dmabuf_sync_get_obj(sync, sync_buf, type);
+ if (ret < 0)
+ return ret;
+
+ sync->status = DMABUF_SYNC_GOT;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_get);
+
+/**
+ * dmabuf_sync_put - Put dmabuf sync object to a given dmabuf.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @dmabuf: An dmabuf object.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_get function is called to release the dmabuf, or
+ * dmabuf_sync_unlock function is called. Internally, this function
+ * removes a given dmabuf from a sync object and remove the sync object.
+ * At this time, the dmabuf is putted.
+ */
+void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
+{
+ if (!sync || !dmabuf) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ dmabuf_sync_put_obj(sync, dmabuf);
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_put);
+
+/**
+ * dmabuf_sync_put_all - Put dmabuf sync object to dmabufs.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_get function is called to release all sync objects, or
+ * dmabuf_sync_unlock function is called. Internally, this function
+ * removes dmabufs from a sync object and remove the sync object.
+ * At this time, all dmabufs are putted.
+ */
+void dmabuf_sync_put_all(struct dmabuf_sync *sync)
+{
+ if (!sync) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ dmabuf_sync_put_objs(sync);
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_put_all);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index dfac5ed..0109673 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -115,6 +115,7 @@ struct dma_buf_ops {
* @exp_name: name of the exporter; useful for debugging.
* @list_node: node for dma_buf accounting and debugging.
* @priv: exporter specific private data for this buffer object.
+ * @sync: sync object linked to this dma-buf
*/
struct dma_buf {
size_t size;
@@ -128,6 +129,7 @@ struct dma_buf {
const char *exp_name;
struct list_head list_node;
void *priv;
+ void *sync;
};
/**
@@ -148,6 +150,20 @@ struct dma_buf_attachment {
void *priv;
};
+#define DMA_BUF_ACCESS_R 0x1
+#define DMA_BUF_ACCESS_W 0x2
+#define DMA_BUF_ACCESS_DMA 0x4
+#define DMA_BUF_ACCESS_RW (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_W)
+#define DMA_BUF_ACCESS_DMA_R (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_DMA)
+#define DMA_BUF_ACCESS_DMA_W (DMA_BUF_ACCESS_W | DMA_BUF_ACCESS_DMA)
+#define DMA_BUF_ACCESS_DMA_RW (DMA_BUF_ACCESS_DMA_R | DMA_BUF_ACCESS_DMA_W)
+#define IS_VALID_DMA_BUF_ACCESS_TYPE(t) (t = DMA_BUF_ACCESS_R || \
+ t = DMA_BUF_ACCESS_W || \
+ t = DMA_BUF_ACCESS_DMA_R || \
+ t = DMA_BUF_ACCESS_DMA_W || \
+ t = DMA_BUF_ACCESS_RW || \
+ t = DMA_BUF_ACCESS_DMA_RW)
+
/**
* get_dma_buf - convenience wrapper for get_file.
* @dmabuf: [in] pointer to dma_buf
diff --git a/include/linux/dmabuf-sync.h b/include/linux/dmabuf-sync.h
new file mode 100644
index 0000000..91c9111
--- /dev/null
+++ b/include/linux/dmabuf-sync.h
@@ -0,0 +1,236 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics Co.Ltd
+ * Authors:
+ * Inki Dae <inki.dae@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/ww_mutex.h>
+#include <linux/sched.h>
+#include <linux/dma-buf.h>
+
+#define DMABUF_SYNC_NAME_SIZE 64
+
+/*
+ * Status to a dmabuf_sync object.
+ *
+ * @DMABUF_SYNC_GOT: Indicate that one more dmabuf objects have been added
+ * to a sync's list.
+ * @DMABUF_SYNC_LOCKED: Indicate that all dmabuf objects in a sync's list
+ * have been locked.
+ */
+enum dmabuf_sync_status {
+ DMABUF_SYNC_GOT = 1,
+ DMABUF_SYNC_LOCKED,
+};
+
+/*
+ * A structure for dmabuf_sync_reservation.
+ *
+ * @sync_lock: This provides read or write lock to a dmabuf.
+ * Except in the below cases, a task will be blocked if the task
+ * tries to lock a dmabuf for CPU or DMA access when other task
+ * already locked the dmabuf.
+ *
+ * Before After
+ * --------------------------
+ * CPU read CPU read
+ * CPU read DMA read
+ * DMA read CPU read
+ * DMA read DMA read
+ *
+ * @lock: Protecting a dmabuf_sync_reservation object.
+ * @poll_wait: A wait queue object to poll a dmabuf object.
+ * @poll_event: Indicate whether a dmabuf object - being polled -
+ * was unlocked or not. If true, a blocked task will be out
+ * of select system call.
+ * @poll: Indicate whether the polling to a dmabuf object was requested
+ * or not by userspace.
+ * @shared_cnt: Shared count to a dmabuf object.
+ * @accessed_type: Indicate how and who a dmabuf object was accessed by.
+ * One of the below types could be set.
+ * DMA_BUF_ACCESS_R -> CPU access for read.
+ * DMA_BUF_ACCRSS_W -> CPU access for write.
+ * DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_DMA -> DMA access for read.
+ * DMA_BUF_ACCESS_W | DMA_BUF_ACCESS_DMA -> DMA access for write.
+ * @locked: Indicate whether a dmabuf object has been locked or not.
+ *
+ */
+struct dmabuf_sync_reservation {
+ struct ww_mutex sync_lock;
+ struct mutex lock;
+ wait_queue_head_t poll_wait;
+ unsigned int poll_event;
+ unsigned int polled;
+ atomic_t shared_cnt;
+ unsigned int accessed_type;
+ unsigned int locked;
+};
+
+/*
+ * A structure for dmabuf_sync_object.
+ *
+ * @head: A list head to be added to syncs list.
+ * @robj: A reservation_object object.
+ * @dma_buf: A dma_buf object.
+ * @access_type: Indicate how a current task tries to access
+ * a given buffer, and one of the below types could be set.
+ * DMA_BUF_ACCESS_R -> CPU access for read.
+ * DMA_BUF_ACCRSS_W -> CPU access for write.
+ * DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_DMA -> DMA access for read.
+ * DMA_BUF_ACCESS_W | DMA_BUF_ACCESS_DMA -> DMA access for write.
+ */
+struct dmabuf_sync_object {
+ struct list_head head;
+ struct dmabuf_sync_reservation *robj;
+ struct dma_buf *dmabuf;
+ unsigned int access_type;
+};
+
+struct dmabuf_sync_priv_ops {
+ void (*free)(void *priv);
+};
+
+/*
+ * A structure for dmabuf_sync.
+ *
+ * @syncs: A list head to sync object and this is global to system.
+ * @list: A list entry used as committed list node
+ * @lock: Protecting a dmabuf_sync object.
+ * @ctx: A current context for ww mutex.
+ * @work: A work struct to release resources at timeout.
+ * @priv: A private data.
+ * @name: A string to dmabuf sync owner.
+ * @timer: A timer list to avoid lockup and release resources.
+ * @status: Indicate current status (DMABUF_SYNC_GOT or DMABUF_SYNC_LOCKED).
+ */
+struct dmabuf_sync {
+ struct list_head syncs;
+ struct list_head list;
+ struct mutex lock;
+ struct ww_acquire_ctx ctx;
+ struct work_struct work;
+ void *priv;
+ struct dmabuf_sync_priv_ops *ops;
+ char name[DMABUF_SYNC_NAME_SIZE];
+ struct timer_list timer;
+ unsigned int status;
+};
+
+#ifdef CONFIG_DMABUF_SYNC
+
+extern struct ww_class dmabuf_sync_ww_class;
+
+static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *obj;
+
+ obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+ if (!obj)
+ return;
+
+ dmabuf->sync = obj;
+
+ ww_mutex_init(&obj->sync_lock, &dmabuf_sync_ww_class);
+
+ mutex_init(&obj->lock);
+ atomic_set(&obj->shared_cnt, 1);
+
+ init_waitqueue_head(&obj->poll_wait);
+}
+
+static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *obj;
+
+ if (!dmabuf->sync)
+ return;
+
+ obj = dmabuf->sync;
+
+ ww_mutex_destroy(&obj->sync_lock);
+
+ kfree(obj);
+}
+
+extern bool dmabuf_sync_is_supported(void);
+
+extern struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv);
+
+extern void dmabuf_sync_fini(struct dmabuf_sync *sync);
+
+extern int dmabuf_sync_lock(struct dmabuf_sync *sync);
+
+extern int dmabuf_sync_unlock(struct dmabuf_sync *sync);
+
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
+ bool wait);
+
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf);
+
+extern int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
+ unsigned int type);
+
+extern void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf);
+
+extern void dmabuf_sync_put_all(struct dmabuf_sync *sync);
+
+#else
+
+static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf) { }
+
+static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf) { }
+
+static inline bool dmabuf_sync_is_supported(void) { return false; }
+
+static inline struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv)
+{
+ return ERR_PTR(0);
+}
+
+static inline void dmabuf_sync_fini(struct dmabuf_sync *sync) { }
+
+static inline int dmabuf_sync_lock(struct dmabuf_sync *sync)
+{
+ return 0;
+}
+
+static inline int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+{
+ return 0;
+}
+
+static inline int dmabuf_sync_single_lock(struct dma_buf *dmabuf,
+ unsigned int type,
+ bool wait)
+{
+ return 0;
+}
+
+static inline void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+{
+ return;
+}
+
+static inline int dmabuf_sync_get(struct dmabuf_sync *sync,
+ void *sync_buf,
+ unsigned int type)
+{
+ return 0;
+}
+
+static inline void dmabuf_sync_put(struct dmabuf_sync *sync,
+ struct dma_buf *dmabuf) { }
+
+static inline void dmabuf_sync_put_all(struct dmabuf_sync *sync) { }
+
+#endif
--
1.7.5.4
^ permalink raw reply related
* [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync support
From: Inki Dae @ 2013-08-21 10:33 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
In-Reply-To: <1377081215-5948-1-git-send-email-inki.dae@samsung.com>
This patch adds lock and poll callbacks to dma buf file operations,
and these callbacks will be called by fcntl and select system calls.
fcntl and select system calls can be used to wait for the completion
of DMA or CPU access to a shared dmabuf. The difference of them is
fcntl system call takes a lock after the completion but select system
call doesn't. So in case of fcntl system call, it's useful when a task
wants to access a shared dmabuf without any broken. On the other hand,
it's useful when a task wants to just wait for the completion.
Changelog v2:
- Add select system call support.
. The purpose of this feature is to wait for the completion of DMA or
CPU access to a dmabuf without that caller locks the dmabuf again
after the completion.
That is useful when caller wants to be aware of the completion of
DMA access to the dmabuf, and the caller doesn't use intefaces for
the DMA device driver.
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/base/dma-buf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 81 insertions(+), 0 deletions(-)
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 4aca57a..f16a396 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -29,6 +29,7 @@
#include <linux/export.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/poll.h>
#include <linux/dmabuf-sync.h>
static inline int is_dma_buf_file(struct file *);
@@ -80,9 +81,89 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
return dmabuf->ops->mmap(dmabuf, vma);
}
+static unsigned int dma_buf_poll(struct file *filp,
+ struct poll_table_struct *poll)
+{
+ struct dma_buf *dmabuf;
+ struct dmabuf_sync_reservation *robj;
+ int ret = 0;
+
+ if (!is_dma_buf_file(filp))
+ return POLLERR;
+
+ dmabuf = filp->private_data;
+ if (!dmabuf || !dmabuf->sync)
+ return POLLERR;
+
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ robj->polled = true;
+
+ /*
+ * CPU or DMA access to this buffer has been completed, and
+ * the blocked task has been waked up. Return poll event
+ * so that the task can get out of select().
+ */
+ if (robj->poll_event) {
+ robj->poll_event = false;
+ mutex_unlock(&robj->lock);
+ return POLLIN | POLLOUT;
+ }
+
+ /*
+ * There is no anyone accessing this buffer so just return.
+ */
+ if (!robj->locked) {
+ mutex_unlock(&robj->lock);
+ return POLLIN | POLLOUT;
+ }
+
+ poll_wait(filp, &robj->poll_wait, poll);
+
+ mutex_unlock(&robj->lock);
+
+ return ret;
+}
+
+static int dma_buf_lock(struct file *file, int cmd, struct file_lock *fl)
+{
+ struct dma_buf *dmabuf;
+ unsigned int type;
+ bool wait = false;
+
+ if (!is_dma_buf_file(file))
+ return -EINVAL;
+
+ dmabuf = file->private_data;
+
+ if ((fl->fl_type & F_UNLCK) = F_UNLCK) {
+ dmabuf_sync_single_unlock(dmabuf);
+ return 0;
+ }
+
+ /* convert flock type to dmabuf sync type. */
+ if ((fl->fl_type & F_WRLCK) = F_WRLCK)
+ type = DMA_BUF_ACCESS_W;
+ else if ((fl->fl_type & F_RDLCK) = F_RDLCK)
+ type = DMA_BUF_ACCESS_R;
+ else
+ return -EINVAL;
+
+ if (fl->fl_flags & FL_SLEEP)
+ wait = true;
+
+ /* TODO. the locking to certain region should also be considered. */
+
+ return dmabuf_sync_single_lock(dmabuf, type, wait);
+}
+
static const struct file_operations dma_buf_fops = {
.release = dma_buf_release,
.mmap = dma_buf_mmap_internal,
+ .poll = dma_buf_poll,
+ .lock = dma_buf_lock,
};
/*
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync support
From: David Herrmann @ 2013-08-21 13:17 UTC (permalink / raw)
To: Inki Dae
Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-arm-kernel, linux-media, linaro-kernel, Maarten Lankhorst,
Sumit Semwal, kyungmin.park, myungjoo.ham
In-Reply-To: <1377081215-5948-3-git-send-email-inki.dae@samsung.com>
Hi
On Wed, Aug 21, 2013 at 12:33 PM, Inki Dae <inki.dae@samsung.com> wrote:
> This patch adds lock and poll callbacks to dma buf file operations,
> and these callbacks will be called by fcntl and select system calls.
>
> fcntl and select system calls can be used to wait for the completion
> of DMA or CPU access to a shared dmabuf. The difference of them is
> fcntl system call takes a lock after the completion but select system
> call doesn't. So in case of fcntl system call, it's useful when a task
> wants to access a shared dmabuf without any broken. On the other hand,
> it's useful when a task wants to just wait for the completion.
1)
So how is that supposed to work in user-space? I don't want to block
on a buffer, but get notified once I can lock it. So I do:
select(..dmabuf..)
Once it is finished, I want to use it:
flock(..dmabuf..)
However, how can I guarantee the flock will not block? Some other
process might have locked it in between. So I do a non-blocking
flock() and if it fails I wait again? Looks ugly and un-predictable.
2)
What do I do if some user-space program holds a lock and dead-locks?
3)
How do we do modesetting in atomic-context in the kernel? There is no
way to lock the object. But this is required for panic-handlers and
more importantly the kdb debugging hooks.
Ok, I can live with that being racy, but would still be nice to be considered.
4)
Why do we need locks? Aren't fences enough? That is, in which
situation is a lock really needed?
If we assume we have two writers A and B (DMA, CPU, GPU, whatever) and
they have no synchronization on their own. What do we win by
synchronizing their writes? Ok, yeah, we end up with either A or B and
not a mixture of both. But if we cannot predict whether we get A or B,
I don't know why we care at all? It's random, so a mixture would be
fine, too, wouldn't it?
So if user-space doesn't have any synchronization on its own, I don't
see why we need an implicit sync on a dma-buf. Could you describe a
more elaborate use-case?
I think the problems we need to fix are read/write syncs. So we have a
write that issues the DMA+write plus a fence and passes the buf plus
fence to the reader. The reader waits for the fence and then issues
the read plus fence. It passes the fence back to the writer. The
writer waits for the fence again and then issues the next write if
required.
This has the following advantages:
- fences are _guaranteed_ to finish in a given time period. Locks, on
the other hand, might never be freed (of the holder dead-locks, for
instance)
- you avoid any stalls. That is, if a writer releases a buffer and
immediately locks it again, the reader side might stall if it didn't
lock it in exactly the given window. You have no control to guarantee
the reader ever gets access. You would need a synchronization in
user-space between the writer and reader to guarantee that. This makes
the whole lock useles, doesn't it?
Cheers
David
> Changelog v2:
> - Add select system call support.
> . The purpose of this feature is to wait for the completion of DMA or
> CPU access to a dmabuf without that caller locks the dmabuf again
> after the completion.
> That is useful when caller wants to be aware of the completion of
> DMA access to the dmabuf, and the caller doesn't use intefaces for
> the DMA device driver.
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> drivers/base/dma-buf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 81 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 4aca57a..f16a396 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -29,6 +29,7 @@
> #include <linux/export.h>
> #include <linux/debugfs.h>
> #include <linux/seq_file.h>
> +#include <linux/poll.h>
> #include <linux/dmabuf-sync.h>
>
> static inline int is_dma_buf_file(struct file *);
> @@ -80,9 +81,89 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
> return dmabuf->ops->mmap(dmabuf, vma);
> }
>
> +static unsigned int dma_buf_poll(struct file *filp,
> + struct poll_table_struct *poll)
> +{
> + struct dma_buf *dmabuf;
> + struct dmabuf_sync_reservation *robj;
> + int ret = 0;
> +
> + if (!is_dma_buf_file(filp))
> + return POLLERR;
> +
> + dmabuf = filp->private_data;
> + if (!dmabuf || !dmabuf->sync)
> + return POLLERR;
> +
> + robj = dmabuf->sync;
> +
> + mutex_lock(&robj->lock);
> +
> + robj->polled = true;
> +
> + /*
> + * CPU or DMA access to this buffer has been completed, and
> + * the blocked task has been waked up. Return poll event
> + * so that the task can get out of select().
> + */
> + if (robj->poll_event) {
> + robj->poll_event = false;
> + mutex_unlock(&robj->lock);
> + return POLLIN | POLLOUT;
> + }
> +
> + /*
> + * There is no anyone accessing this buffer so just return.
> + */
> + if (!robj->locked) {
> + mutex_unlock(&robj->lock);
> + return POLLIN | POLLOUT;
> + }
> +
> + poll_wait(filp, &robj->poll_wait, poll);
> +
> + mutex_unlock(&robj->lock);
> +
> + return ret;
> +}
> +
> +static int dma_buf_lock(struct file *file, int cmd, struct file_lock *fl)
> +{
> + struct dma_buf *dmabuf;
> + unsigned int type;
> + bool wait = false;
> +
> + if (!is_dma_buf_file(file))
> + return -EINVAL;
> +
> + dmabuf = file->private_data;
> +
> + if ((fl->fl_type & F_UNLCK) = F_UNLCK) {
> + dmabuf_sync_single_unlock(dmabuf);
> + return 0;
> + }
> +
> + /* convert flock type to dmabuf sync type. */
> + if ((fl->fl_type & F_WRLCK) = F_WRLCK)
> + type = DMA_BUF_ACCESS_W;
> + else if ((fl->fl_type & F_RDLCK) = F_RDLCK)
> + type = DMA_BUF_ACCESS_R;
> + else
> + return -EINVAL;
> +
> + if (fl->fl_flags & FL_SLEEP)
> + wait = true;
> +
> + /* TODO. the locking to certain region should also be considered. */
> +
> + return dmabuf_sync_single_lock(dmabuf, type, wait);
> +}
> +
> static const struct file_operations dma_buf_fops = {
> .release = dma_buf_release,
> .mmap = dma_buf_mmap_internal,
> + .poll = dma_buf_poll,
> + .lock = dma_buf_lock,
> };
>
> /*
> --
> 1.7.5.4
>
> --
> 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
^ permalink raw reply
* Re: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
From: Konrad Rzeszutek Wilk @ 2013-08-21 14:27 UTC (permalink / raw)
To: Inki Dae
Cc: linux-fbdev, linaro-kernel, dri-devel, kyungmin.park,
myungjoo.ham, linux-arm-kernel, linux-media
In-Reply-To: <008201ce9e4a$08d1d1a0$1a7574e0$%dae@samsung.com>
> > > +EXPORT_SYMBOL(is_dmabuf_sync_supported);
> >
> > _GPL ?
> >
> > I would also prefix it with 'dmabuf_is_sync_supported' just to make
> > all of the libraries call start with 'dmabuf'
> >
>
> Seems better. Will change it to dmabuf_is_sync_supported, and use
> EXPORT_SYMBOL_GPL.
One thing thought - while I suggest that you use GPL variant
I think you should check who the consumers are. As in, if nvidia
wants to use it it might make their lawyers unhappy - and in turn
means that their engineers won't be able to use these symbols.
So - if there is a strong argument to not have it GPL - then please
say so.
^ permalink raw reply
* Re: [RFC 00/22] OMAPDSS: DT support
From: Laurent Pinchart @ 2013-08-21 21:07 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
Nishanth Menon, Felipe Balbi, Santosh Shilimkar, Tony Lindgren
In-Reply-To: <1376037547-10859-1-git-send-email-tomi.valkeinen@ti.com>
Hi Tomi,
I'm back from holidays and have finally found time to review your patch set.
On Friday 09 August 2013 11:38:45 Tomi Valkeinen wrote:
> Hi,
>
> This is an RFC for OMAP Display DT support. The patches work fine, at least
> for me, but they are not perfect. I mostly don't have any clear questions
> about specific issues, but I would like to get feedback on the selected
> approaches in general, and also ideas how to proceed with the series.
>
> This series contains the following:
>
> DT support for the following OMAP's display subsystem devices:
> - DSS
> - DISPC
> - DPI
> - HDMI
> - VENC
> - DSI
> - (DBI/RFBI DT is not yet implemented)
>
> DT support for the following external display devices:
> - panel-dsi-cm (Generic DSI command mode panel)
> - encoder-tfp410 (DPI-to-DVI encoder)
> - connector-dvi
> - encoder-tpd12s015 (HDMI level-shifter & ESD protection)
> - hdmi-connector
> - panel-dpi (Generic DPI panel)
> - connector-analog-tv (S-Video & Composite)
>
> DT support for the following boards:
> - OMAP4 PandaBoard
> - OMAP4 SDP
> - OMAP3 BeagleBoard
> - OMAP3 Overo with Palo43 LCD expansion-board
>
> The patches are not final, and many contain quite brief descriptions.
> Binding descriptions are also still missing. The code and bindings in the
> patches should be pretty straightforward, though.
>
> The series is based on v3.11-rc2 + a couple of non-DSS fixes. The series
> can also be found from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git
> work/dss-dev-model-dt
>
> Vocabulary
> =====
>
> Display Entity - a hardware entity that takes one or more video streams as
> input and outputs one or more video streams.
>
> Upstream Entity - A display entity in the "upstream" side of the video
> stream, i.e. closer to the original video source.
>
> Downstream Entity - A display entity in the "downstream" side of the video
> stream, i.e. closer to the panel or monitor.
>
> Video pipeline - A chain of multiple display entities, starting from the
> SoC, going to the display device the user sees.
>
> Display or Panel - A display entity showing the pixels to the user
>
> Encoder - A display entity that takes video as an input and (usually)
> outputs it in some other format.
>
> Connector - HDMI/DVI/etc Connector on the board, to which an external
> monitor is connected.
>
> About Stable DT Bindings
> ============
>
> Generally speaking, the DT bindings should be stable. This brings the
> following problems:
>
> We already have DT bindings for some OMAP4 and OMAP3 boards, and OMAP4
> boards do not even have board files anymore. There are no display bindings
> for those OMAP4 boards, but the display support is currently enabled as a
> hack, by calling board-file-like code to add the display devices for the
> selected boards. So, when we add the display bindings, we should still
> support the current DT files which do not have the display bindings. Which
> would mean that we'd need to keep the hacky code forever. Considering the
> fact that the hacky code does not work quite correct in all cases, I don't
> see keeping it as a very good option.
>
> CDF (Common Display Framework) is in the works, and will most likely have
> different or more detailed bindings. Moving to CDF would mean we'd somehow
> need to still support the old OMAP bindings. In theory the display DT
> bindings should stay the same, as they represent the HW, not any SW
> constructs, but in practice I find it hard to believe the OMAP display
> bindings would be so perfect that there would be no need for changes.
>
> We most likely should somehow represent DSS clock tree in DT. That is not a
> simple task, and when we manage to do it, it again means supporting the DT
> files without clock tree data.
>
> All in all, I'm a bit scared to push the display bindings, as it's already
> clear there are changes coming. Then again, supporting the current hack for
> OMAP4 based boards is not nice at all, and has issues, so it would be really
> nice to get OMAP4 boards use proper display bindings.
Getting bindings right on the very first try is a very difficult, if not
impossible, task. We now have dedicated DT bindings reviewers and maintainers
to help here, and all DT bindings in mainline will likely go through extensive
review in the not too distant future. I don't know if there has been any
formal agreement on that, but one idea that seemed generally well approved was
to first mark new DT bindings as experimental before promoting them to stable.
On the other hand, having experimental bindings in mainline should help
stabilizing them, but that's obviously not a reason not to think them properly
from the start.
> General description of the DT bindings
> ===================
>
> All the display entities are represented as DT nodes of their own, and have
> a matching Linux driver. The display entities are organized by their control
> bus; that is, if a display entity is not controlled via any bus, it's a
> platform device, and if, say, it's controlled via i2c device, it's an i2c
> device.
I very much like that approach, and I think it's generally agreed upon.
> The exception to the above are DSI and DBI. DSI and DBI are combined control
> and video busses, but the use of the busses for control purposes is not
> independent of the video stream. Also, the the busses are, in practice,
> one-to-one links. And last, DSI and DBI display entities are often also
> controllable via, say, i2c. For these reasons there is no real Linux bus
> for DSI and DBI and thus the DSI and DBI devices are either platform
> devices or i2c devices.
That's something I'm less convinced about, but I don't have a too strong
feeling. The best implementation should get to mainline, I won't nack this
architecture just for theoretical reasons.
> The display entities are connected via "video-source" property. The video-
> source points to the upstream display entity where the video data comes
> from, and a chain of display entities thus form a full video pipeline. All
> video pipelines end with either a panel or a connector.
This is the part that bothers me the most, as I find it too limiting. In a
purely linear pipeline entities will have at most one video source, but linear
pipelines won't be enough for the future (and are actually not enough today
already). We need to support entities with more than one sink pad in the DT
bindings.
I've posted the third version of the Common Display Framework RFC
(https://lwn.net/Articles/563157/). While not formally documented, the DT
bindings are described in the cover letter. They're based on the V4L2 DT
bindings and can express arbitrary display pipelines in DT. The patch set also
provides helper functions to parse the DT bindings.
Could you please have a look at the bindings and tell me whether you can use
them for the OMAPDSS (even if you don't use the CDF helpers to start with) ?
> All the data related to a display entity, and how it is connected on the
> given board, is defined in the DT node of the display entity. This means
> that the DT node of the upstream entity does not have to be modified when
> adding support for new boards.
>
> As an example, consider OMAP3's DPI and two boards using it for panels.
> omap3.dtsi contains a node for the DPI, and the board dts files contain
> nodes for their panels. The board dts files do not change anything in the
> included DPI node. So:
>
> omap3.dtsi:
>
> dpi: encoder@0 {
> compatible = "ti,omap3-dpi";
> };
>
> omap3-board1.dts:
>
> lcd0: display@0 {
> compatible = "samsung,lte430wq-f0c", "panel-dpi";
> video-source = <&dpi>;
> data-lines = <24>;
> };
>
> omap3-board2.dts:
>
> lcd0: display@0 {
> compatible = "samsung,lte430wq-f0c", "panel-dpi";
> video-source = <&dpi>;
> data-lines = <16>;
> };
>
> The logic here is that the boards may have multiple panels that are
> connected to the same source, even if the panels can only be used one at a
> time. Each panel may thus have different properties for the bus, like the
> number of data-lines.
>
> Video bus properties
> ==========
>
> One question I've been pondering for a long time is related to the bus
> between two display entities. As an example, DPI (parallel RGB) requires
> configuring the number of datalines used. As described above, the properties
> of the video bus are represented in the downstream entity.
>
> This approach has one drawback: how to represent features specific to the
> upstream entity? Say, if OMAP's DSI has a bus-related foo-feature, which can
> be used in some scenarios, the only place where this foo-feature can be
> specified is the OMAP DSI's properties. Not in the DSI Panel's properties,
> which in the current model contains properties related to the bus.
>
> So Laurent has proposed to use V4L2-like ports, as described in
> Documentation/devicetree/bindings/media/video-interfaces.txt. I have not
> implemented such feature for OMAP DSS for the following reasons:
>
> - The current supported displays we use work fine with the current method
> - If I were to implement such system, it'd most certainly be different than
> what CDF will have.
>
> That said, the port based approach does sound good, and it would also remove
> the design issue with OMAP DPI and SDI modules as described later. So maybe
> I should just go forward with it and hope that CDF will do it in similar
> manner.
I believe we should go for the port-based approach, even if you don't use the
CDF helpers (possibly at first only).
> DSI Module ID
> ======>
> On OMAP4 we have two DSI modules. To configure the clock routing and pin
> muxing, we need to know the hardware module ID for the DSI device, i.e. is
> this Linux platform device DSI1 or DSI2. The same issue exists with other
> SoCs with multiple outputs of the same kind.
>
> With non-DT case, we used the platform device's ID directly. With DT, that
> doesn't work. I don't currently have a good solution for this, so as a
> temporary solution the DSI driver contains a table, from which it can find
> the HW module ID by using the device's base address.
You could add two output ports to the DSS, one for each DSI, and link the DSI
modules to the DSS output ports in DT. That would represent the hardware
topology, and would allow the DSI driver to know its ID based on the DSS
output port it's connected to. It's still a bit hackish in the sense that the
DSI driver will use information provided by the DSS (the output port number),
but not more than the current method.
> I believe, but I am not totally sure, that we can remove the concept of DSI
> module ID if we have a good representation of the DSS clock tree and DSI pin
> muxing in DT. The clock tree is probably a huge undertaking, but the pin
> muxing should be much easier. However, pinmuxing also is some
> complications, like the pins requiring a regulator to be enabled.
>
> Display names and aliases
> ============>
> With the board-file based model each display was given a name in the board
> file. Additionally, each display was given an alias in the style "displayX",
> where X is in incrementing number.
>
> The name could be used by the user to see which display device is what, i.e.
> on Pandaboard there are displays names "dvi" and "hdmi".
>
> The DT bindings do not have such a name. It would be simple to add a
> "nickname" property to each display node, but it just looked rather ugly so
> I have left it out.
>
> Additionally, as there's no clear order in which the displays are created,
> and thus the number in "displayX" alias could change semi-randomly, I added
> the displays to "aliases" node. This keeps the display number the same over
> boots, and also gives us some way to define a default display, i.e. which
> display to use initially if the user has not specified it.
>
> omapdss virtual device
> ===========
>
> In addition to the DSS devices matching to DSS hardware modules, we have a
> "virtual" omapdss device which does not match to any actual HW module. The
> device is there mostly for legacy reasons, but it has also allowed us to
> easily pass platform callbacks. The same device is also present in DT
> solution. It is created in code, and not present in DT bindings.
>
> Obviously, this omapdss virtual device is on the hack side, and nobody would
> mind if it would disappear.
>
> The following data is passed via omapdss device's platform data:
>
> - OMAP DSS version. In theory, the DSS revision registers should tell us
> which features the HW supports. In practice, the HW people have not
> bothered to change the revision number each time they've made changes. So
> we pass a DSS version from the platform code, based on OMAP revision
> number.
>
> - omap_dss_set_min_bus_tput() and omap_pm_get_dev_context_loss_count() to
> manage PM
>
> - DSI pin muxing functions.
>
> I have some ideas how to deduce the DSS version by poking to certain DSS
> registers, but it is not yet tested so I don't know if it will work.
That might be a stupid question, but can't you just encode the version in the
compatible string of the DSS DT node (the one currently compatible with
"ti,omap[34]-dss") ?
Version information might also need to be split/duplicated in several of the
DSS DT nodes. A quick grep through the driver source code shows that the
version is used by the submodules to infer SoC glue information. For instance
dsi_get_channel() uses the version to find the DSI module channel ID. That
information could possibly be retrieved from the links between the DSS DT
nodes.
> We could do altogether without omap_pm_get_dev_context_loss_count(), so that
> should be removable with some work. I am not sure if
> omap_dss_set_min_bus_tput() is supported via standard PM calls or not.
>
> However, the use of set_min_bus_tput() is actually a hack, as we're not
> really setting min bus tput. What we want to do is prevent OPP50. DSS clocks
> have different maximums on OPP100 and OPP50. So if DSS uses a clock over
> OPP50 limit, OPP50 cannot be entered. We prevent OPP50 by setting an
> arbitrarily high min bus tput.
>
> The DSI pin muxing should also be solvable with DT based solution, but is
> not the most trivial task and needs some work.
>
> So, I presume that at some point we can remove the omapdss device, but in
> the current solution it exists for the above reasons.
>
> DSS submodules in DT bindings
> ==============>
> The OMAP DSS modules are accessed via L4 or L3, and in that sense they
> should be on the same level in the DT bindings. However, we do not have them
> in the same level, but there is a main "dss" node, under which all the
> other DSS modules reside. The main reason for this is that the main DSS
> device needs to be enabled for the other modules to work properly, and
> having this structure makes runtime PM handle enabling DSS automatically.
>
> If I recall right, somebody (Paul?) mentioned that in the hardware there is
> actually some DSS internal bus, and thus the DT structure is good in that
> sense also.
>
> We also have nodes (and matching Linux devices) for DPI and SDI. Neither of
> them are actuall a separate IP block in the hardware, but are really more
> parts of DSS and maybe DISPC. They are still represented in the same way as
> the other DSS modules, to have similar architecture for all DSS outputs. But
> as they do not have an address-space of their own, the DT nodes use 0 and 1
> as "addresses", i.e. "dpi: encoder@0".
>
> That is rather ugly, and maybe could use some cleaning up. A V4L2 port style
> approach would probably allow us to add DPI and SDI ports as part of DSS.
I think that would really be a good idea. The DPI and SDI code could be moved
to the DSS module (it can of course still be kept in separate files as today),
and the DISPC (if I'm not mistaken) entity would just have several output
ports.
> Some of the DSS modules are actually a combination of multiple smaller
> modules. For example, the DSI module contains DSI protocol, DSI PHY and DSI
> PLL modules, each in their own address space. These could perhaps be
> presented as separate DT nodes and Linux devices, but I am not sure if that
> is a good approach or not.
What are the chances that one of those block will be upgraded and/or replaced
independently of the others in the future (I know it's a tricky question) ? It
might not be worth it going to a too fine-grained approach at the moment, but
we need to make sure that the DT bindings will allow an easy path forward if
needed.
> DT Node Names
> ======>
> I have used the following naming conventions with DT nodes:
>
> - Panels are named "display"
> - Connectors are named "connector"
> - Encoders are named "encoder"
> - DSS and DISPC are "dss" and "dispc", as they don't really match any of the
> above
>
> When appropriate, the address part of the node is the base address, as in
> "dsi1: encoder@58004000". For platform devices, I have used an increasing
> number for the address, as in "tpd12s015: encoder@1".
>
> Final words
> =====>
> So as is evident, I have things in my mind that should be improved. Maybe
> the most important question for short term future is:
>
> Can we add DSS DT bindings for OMAP4 as unstable bindings? It would give us
> some proper testing of the related code, and would also allow us to remove
> the related hacks (which don't even work quite right). However, I have no
> idea yet when the unstable DSS bindings would turn stable.
>
> If we shouldn't add the bindings as unstable, when should the bindings be
> added? Wait until CDF is in the mainline, and use that?
What about using the CDF bindings, without waiting for CDF to be in mainline ?
I believe the bindings should be upstreamed as unstable to start with anyway.
> I have not explained every piece of DSS in detail, as that would result in a
> book instead of this email, so feel free to ask for more details about any
> part.
>
> And last but not least (for me, at least =), I'm on vacation for the next
> two weeks, so answers may be delayed.
Enjoy your holidays and recharge your batteries, you will need energy when
you'll come back :-)
> Tomi Valkeinen (22):
> ARM: OMAP: remove DSS DT hack
> OMAPDSS: remove DT hacks for regulators
> ARM: OMAP2+: add omapdss_init_of()
> OMAPDSS: if dssdev->name=NULL, use alias
> OMAPDSS: get dssdev->alias from DT alias
> OMAPFB: clean up default display search
> OMAPFB: search for default display with DT alias
> OMAPDSS: Add DT support to DSS, DISPC, DPI, HDMI, VENC
> OMAPDSS: Add DT support to DSI
> ARM: omap3.dtsi: add omapdss information
> ARM: omap4.dtsi: add omapdss information
> ARM: omap4-panda.dts: add display information
> ARM: omap4-sdp.dts: add display information
> ARM: omap3-tobi.dts: add lcd (TEST)
> ARM: omap3-beagle.dts: add display information
> OMAPDSS: panel-dsi-cm: Add DT support
> OMAPDSS: encoder-tfp410: Add DT support
> OMAPDSS: connector-dvi: Add DT support
> OMAPDSS: encoder-tpd12s015: Add DT support
> OMAPDSS: hdmi-connector: Add DT support
> OMAPDSS: panel-dpi: Add DT support
> OMAPDSS: connector-analog-tv: Add DT support
>
> arch/arm/boot/dts/omap3-beagle.dts | 29 ++++++++
> arch/arm/boot/dts/omap3-tobi.dts | 33 ++++++++
> arch/arm/boot/dts/omap3.dtsi | 43 +++++++++++
> arch/arm/boot/dts/omap4-panda-common.dtsi | 48 ++++++++++++
> arch/arm/boot/dts/omap4-sdp.dts | 70 +++++++++++++++++
> arch/arm/boot/dts/omap4.dtsi | 59 +++++++++++++++
> arch/arm/mach-omap2/board-generic.c | 13 +---
> arch/arm/mach-omap2/common.h | 2 +
> arch/arm/mach-omap2/display.c | 34 +++++++++
> arch/arm/mach-omap2/dss-common.c | 23 ------
> arch/arm/mach-omap2/dss-common.h | 2 -
> .../video/omap2/displays-new/connector-analog-tv.c | 70 +++++++++++++++++
> drivers/video/omap2/displays-new/connector-dvi.c | 49 ++++++++++++
> drivers/video/omap2/displays-new/connector-hdmi.c | 36 +++++++++
> drivers/video/omap2/displays-new/encoder-tfp410.c | 54 ++++++++++++++
> .../video/omap2/displays-new/encoder-tpd12s015.c | 62 +++++++++++++++
> drivers/video/omap2/displays-new/panel-dpi.c | 75 +++++++++++++++++++
> drivers/video/omap2/displays-new/panel-dsi-cm.c | 87 +++++++++++++++++++
> drivers/video/omap2/dss/dispc.c | 7 ++
> drivers/video/omap2/dss/display.c | 23 +++++-
> drivers/video/omap2/dss/dpi.c | 8 ++
> drivers/video/omap2/dss/dsi.c | 58 +++++++++++++--
> drivers/video/omap2/dss/dss.c | 10 +++
> drivers/video/omap2/dss/hdmi.c | 11 +--
> drivers/video/omap2/dss/venc.c | 7 ++
> drivers/video/omap2/omapfb/omapfb-main.c | 67 ++++++++++++-----
> 26 files changed, 915 insertions(+), 65 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v2] video: imxfb: Fix retrieve values from DT
From: Sascha Hauer @ 2013-08-22 7:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1376120037-23777-1-git-send-email-shc_work@mail.ru>
On Sat, Aug 10, 2013 at 11:33:57AM +0400, Alexander Shiyan wrote:
> Display settings should be retrieved from "display" node, not from
> root fb node. This patch fix this bug.
As Documentation/devicetree/bindings/video/fsl,imx-fb.txt states
fsl,dmacr and fsl,lscr1 should be read from the imxfb node, not from the
display node. This is what the current code does.
BTW it seems cmap-greyscale, cmap-inverse and cmap-static are
undocumented. I'm not really sure whether they should exist at all.
Sascha
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
> drivers/video/imxfb.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index 38733ac..8e104c4 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -753,12 +753,12 @@ static int imxfb_resume(struct platform_device *dev)
> #define imxfb_resume NULL
> #endif
>
> -static int imxfb_init_fbinfo(struct platform_device *pdev)
> +static int imxfb_init_fbinfo(struct platform_device *pdev,
> + struct device_node *np)
> {
> struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
> struct fb_info *info = dev_get_drvdata(&pdev->dev);
> struct imxfb_info *fbi = info->par;
> - struct device_node *np;
>
> pr_debug("%s\n",__func__);
>
> @@ -799,7 +799,6 @@ static int imxfb_init_fbinfo(struct platform_device *pdev)
> fbi->lcd_power = pdata->lcd_power;
> fbi->backlight_power = pdata->backlight_power;
> } else {
> - np = pdev->dev.of_node;
> info->var.grayscale = of_property_read_bool(np,
> "cmap-greyscale");
> fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
> @@ -858,6 +857,7 @@ static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
>
> static int imxfb_probe(struct platform_device *pdev)
> {
> + struct device_node *display_np = NULL;
> struct imxfb_info *fbi;
> struct fb_info *info;
> struct imx_fb_platform_data *pdata;
> @@ -887,7 +887,17 @@ static int imxfb_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, info);
>
> - ret = imxfb_init_fbinfo(pdev);
> + if (pdev->dev.of_node) {
> + display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> + if (!display_np) {
> + dev_err(&pdev->dev,
> + "No display defined in devicetree\n");
> + ret = -EINVAL;
> + goto failed_init;
> + }
> + }
> +
> + ret = imxfb_init_fbinfo(pdev, display_np);
> if (ret < 0)
> goto failed_init;
>
> @@ -898,16 +908,8 @@ static int imxfb_probe(struct platform_device *pdev)
> fbi->mode = pdata->mode;
> fbi->num_modes = pdata->num_modes;
> } else {
> - struct device_node *display_np;
> fb_mode = NULL;
>
> - display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> - if (!display_np) {
> - dev_err(&pdev->dev, "No display defined in devicetree\n");
> - ret = -EINVAL;
> - goto failed_of_parse;
> - }
> -
> /*
> * imxfb does not support more modes, we choose only the native
> * mode.
> --
> 1.8.1.5
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* RE: [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync support
From: Inki Dae @ 2013-08-22 8:46 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1377081215-5948-3-git-send-email-inki.dae@samsung.com>
Thanks for your comments,
Inki Dae
> -----Original Message-----
> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> Sent: Wednesday, August 21, 2013 10:17 PM
> To: Inki Dae
> Cc: dri-devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-media@vger.kernel.org; linaro-
> kernel@lists.linaro.org; Maarten Lankhorst; Sumit Semwal;
> kyungmin.park@samsung.com; myungjoo.ham@samsung.com
> Subject: Re: [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync
> support
>
> Hi
>
> On Wed, Aug 21, 2013 at 12:33 PM, Inki Dae <inki.dae@samsung.com> wrote:
> > This patch adds lock and poll callbacks to dma buf file operations,
> > and these callbacks will be called by fcntl and select system calls.
> >
> > fcntl and select system calls can be used to wait for the completion
> > of DMA or CPU access to a shared dmabuf. The difference of them is
> > fcntl system call takes a lock after the completion but select system
> > call doesn't. So in case of fcntl system call, it's useful when a task
> > wants to access a shared dmabuf without any broken. On the other hand,
> > it's useful when a task wants to just wait for the completion.
>
> 1)
> So how is that supposed to work in user-space? I don't want to block
> on a buffer, but get notified once I can lock it. So I do:
> select(..dmabuf..)
> Once it is finished, I want to use it:
> flock(..dmabuf..)
> However, how can I guarantee the flock will not block? Some other
> process might have locked it in between. So I do a non-blocking
> flock() and if it fails I wait again?
s/flock/fcntl
Yes, it does if you wanted to do a non-blocking fcntl. The fcntl() call will
return -EAGAIN if some other process have locked first. So user process
could retry to lock or do other work. This user process called fcntl() with
non-blocking mode so in this case, I think the user should consider two
things. One is that the fcntl() call couldn't be failed, and other is that
the call could take a lock successfully. Isn't fcntl() with a other fd also,
not dmabuf, take a same action?
>Looks ugly and un-predictable.
>
So I think this is reasonable. However, for select system call, I'm not sure
that this system call is needed yet. So I can remove it if unnecessary.
> 2)
> What do I do if some user-space program holds a lock and dead-locks?
>
I think fcntl call with a other fd also could lead same situation, and the
lock will be unlocked once the user-space program is killed because when the
process is killed, all file descriptors of the process are closed.
> 3)
> How do we do modesetting in atomic-context in the kernel? There is no
> way to lock the object. But this is required for panic-handlers and
> more importantly the kdb debugging hooks.
> Ok, I can live with that being racy, but would still be nice to be
> considered.
Yes, The lock shouldn't be called in the atomic-context. For this, will add
comments enough.
>
> 4)
> Why do we need locks? Aren't fences enough? That is, in which
> situation is a lock really needed?
> If we assume we have two writers A and B (DMA, CPU, GPU, whatever) and
> they have no synchronization on their own. What do we win by
> synchronizing their writes? Ok, yeah, we end up with either A or B and
> not a mixture of both. But if we cannot predict whether we get A or B,
> I don't know why we care at all? It's random, so a mixture would be
> fine, too, wouldn't it?
I think not so. There are some cases that the mixture wouldn't be fine. For
this, will describe it at below.
>
> So if user-space doesn't have any synchronization on its own, I don't
> see why we need an implicit sync on a dma-buf. Could you describe a
> more elaborate use-case?
Ok, first, I think I described that enough though [PATCH 0/2]. For this, you
can refer to the below link,
http://lwn.net/Articles/564208/
Anyway, there are some cases that user-space process needs the
synchronization on its own. In case of Tizen platform[1], one is between X
Client and X Server; actually, Composite Manager. Other is between Web app
based on HTML5 and Web Browser.
Please, assume that X Client draws something in a window buffer using CPU,
and then the X Client requests SWAP to X Server. And then X Server notifies
a damage event to Composite Manager. And then Composite Manager composes the
window buffer with its own back buffer using GPU. In this case, Composite
Manager calls eglSwapBuffers; internally, flushing GL commands instead of
finishing them for more performance.
As you may know, the flushing doesn't wait for the complete event from GPU
driver. And at the same time, the X Client could do other work, and also
draw something in the same buffer again. At this time, The buffer could be
broken. Because the X Client can't aware of when GPU access to the buffer is
completed without out-of-band hand shaking; the out-of-band hand shaking is
quite big overhead. That is why we need user-space locking interface, fcntl
system call.
And also there is same issue in case of Web app: Web app draws something in
a buffer using CPU, and Web browser composes the buffer with its own buffer
using GPU. At the above, you mentioned that a mixture would be fine but it's
not fine with such reasons.
[1] https://www.tizen.org/
>
> I think the problems we need to fix are read/write syncs. So we have a
> write that issues the DMA+write plus a fence and passes the buf plus
> fence to the reader. The reader waits for the fence and then issues
> the read plus fence. It passes the fence back to the writer. The
> writer waits for the fence again and then issues the next write if
> required.
>
> This has the following advantages:
> - fences are _guaranteed_ to finish in a given time period. Locks, on
> the other hand, might never be freed (of the holder dead-locks, for
> instance)
Not so. If never unlock since locked then the buffer will be unlocked by
timeout worker queue handler in a given time period.
> - you avoid any stalls. That is, if a writer releases a buffer and
> immediately locks it again, the reader side might stall if it didn't
> lock it in exactly the given window.
> You have no control to guarantee
> the reader ever gets access. You would need a synchronization in
> user-space between the writer and reader to guarantee that. This makes
> the whole lock useles, doesn't it?
>
Ah..... right. The lock mechanism cannot guarantee the write-and-then-read
order. When the writer tries to take a lock again after the reader tried to
take a lock for read but blocked, the writer don't know the fact that the
reader tried to take a lock for read so in turn, the writer would take a
lock, and the reader side would stall as you mentioned above.
I think we wouldn't need the synchronization in user-space between the write
and reader to guarantee that if we use a mechanism such as DMA fence
additionally; wound/wait mutex to avoid dead lock issue, and DMA fence to
guarantee the write-and-then-read order. For this, I will consider using the
DMA fence. Maybe it takes much time.
Thanks,
Inki Dae
> Cheers
> David
>
> > Changelog v2:
> > - Add select system call support.
> > . The purpose of this feature is to wait for the completion of DMA or
> > CPU access to a dmabuf without that caller locks the dmabuf again
> > after the completion.
> > That is useful when caller wants to be aware of the completion of
> > DMA access to the dmabuf, and the caller doesn't use intefaces for
> > the DMA device driver.
> >
> > Signed-off-by: Inki Dae <inki.dae@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > drivers/base/dma-buf.c | 81
> ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 81 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> > index 4aca57a..f16a396 100644
> > --- a/drivers/base/dma-buf.c
> > +++ b/drivers/base/dma-buf.c
> > @@ -29,6 +29,7 @@
> > #include <linux/export.h>
> > #include <linux/debugfs.h>
> > #include <linux/seq_file.h>
> > +#include <linux/poll.h>
> > #include <linux/dmabuf-sync.h>
> >
> > static inline int is_dma_buf_file(struct file *);
> > @@ -80,9 +81,89 @@ static int dma_buf_mmap_internal(struct file *file,
> struct vm_area_struct *vma)
> > return dmabuf->ops->mmap(dmabuf, vma);
> > }
> >
> > +static unsigned int dma_buf_poll(struct file *filp,
> > + struct poll_table_struct *poll)
> > +{
> > + struct dma_buf *dmabuf;
> > + struct dmabuf_sync_reservation *robj;
> > + int ret = 0;
> > +
> > + if (!is_dma_buf_file(filp))
> > + return POLLERR;
> > +
> > + dmabuf = filp->private_data;
> > + if (!dmabuf || !dmabuf->sync)
> > + return POLLERR;
> > +
> > + robj = dmabuf->sync;
> > +
> > + mutex_lock(&robj->lock);
> > +
> > + robj->polled = true;
> > +
> > + /*
> > + * CPU or DMA access to this buffer has been completed, and
> > + * the blocked task has been waked up. Return poll event
> > + * so that the task can get out of select().
> > + */
> > + if (robj->poll_event) {
> > + robj->poll_event = false;
> > + mutex_unlock(&robj->lock);
> > + return POLLIN | POLLOUT;
> > + }
> > +
> > + /*
> > + * There is no anyone accessing this buffer so just return.
> > + */
> > + if (!robj->locked) {
> > + mutex_unlock(&robj->lock);
> > + return POLLIN | POLLOUT;
> > + }
> > +
> > + poll_wait(filp, &robj->poll_wait, poll);
> > +
> > + mutex_unlock(&robj->lock);
> > +
> > + return ret;
> > +}
> > +
> > +static int dma_buf_lock(struct file *file, int cmd, struct file_lock
> *fl)
> > +{
> > + struct dma_buf *dmabuf;
> > + unsigned int type;
> > + bool wait = false;
> > +
> > + if (!is_dma_buf_file(file))
> > + return -EINVAL;
> > +
> > + dmabuf = file->private_data;
> > +
> > + if ((fl->fl_type & F_UNLCK) = F_UNLCK) {
> > + dmabuf_sync_single_unlock(dmabuf);
> > + return 0;
> > + }
> > +
> > + /* convert flock type to dmabuf sync type. */
> > + if ((fl->fl_type & F_WRLCK) = F_WRLCK)
> > + type = DMA_BUF_ACCESS_W;
> > + else if ((fl->fl_type & F_RDLCK) = F_RDLCK)
> > + type = DMA_BUF_ACCESS_R;
> > + else
> > + return -EINVAL;
> > +
> > + if (fl->fl_flags & FL_SLEEP)
> > + wait = true;
> > +
> > + /* TODO. the locking to certain region should also be
considered.
> */
> > +
> > + return dmabuf_sync_single_lock(dmabuf, type, wait);
> > +}
> > +
> > static const struct file_operations dma_buf_fops = {
> > .release = dma_buf_release,
> > .mmap = dma_buf_mmap_internal,
> > + .poll = dma_buf_poll,
> > + .lock = dma_buf_lock,
> > };
> >
> > /*
> > --
> > 1.7.5.4
> >
> > --
> > 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
^ permalink raw reply
* [RFC v2 0/1] Adding DT support to video/da8xx-fb.c
From: Darren Etheridge @ 2013-08-22 21:21 UTC (permalink / raw)
To: devicetree, tomi.valkeinen, plagnioj, linux-fbdev, detheridge; +Cc: afzal
This is part of a larger series of patches to upgrade the da8xx-fb.c driver
to support the Texas Instruments AM335x device. As part of this upgrade
we also want to add devicetree support for both the original da8xx and the
am335x. Tomi Valkeinen has reviewed the fbdev changes but he suggested
that it was prudent to extract the dt pieces and run it through the
devicetree mailing list for review.
v2: incorporates -
* review comment changes from Prabhakar Lad
* changes the .compatible naming to match that of the exiting drm
driver for am335x devices.
* Renames the devicetree bindings documentation file to match the
actual driver filename
Thanks,
Darren
Darren Etheridge (1):
video: da8xx-fb: adding dt support
.../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
^ permalink raw reply
* [PATCH] video: da8xx-fb: adding dt support
From: Darren Etheridge @ 2013-08-22 21:21 UTC (permalink / raw)
To: devicetree, tomi.valkeinen, plagnioj, linux-fbdev, detheridge; +Cc: afzal
In-Reply-To: <1377206501-30519-1-git-send-email-detheridge@ti.com>
Enhancing driver to enable probe triggered by a corresponding dt entry.
Add da8xx-fb.txt documentation to devicetree section.
Obtain fb_videomode details for the connected lcd panel using the
display timing details present in DT.
Ensure that platform data is present before checking whether platform
callback is present (the one used to control backlight). So far this
was not an issue as driver was purely non-DT triggered, but now DT
support has been added this check must be performed.
v2: squashing multiple commits from Afzal Mohammed (afzal@ti.com)
v3: remove superfluous cast
v4: expose both ti,am3352-lcdc and ti,da830-lcdc for .compatible
as driver can use enhanced features of all version of the
silicon block.
v5: addressed review comments from Prabhakar Lad
v6: Changed the .compatible naming to match the existing drm bindings
for am33xx devices
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
.../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
diff --git a/Documentation/devicetree/bindings/video/da8xx-fb.txt b/Documentation/devicetree/bindings/video/da8xx-fb.txt
new file mode 100644
index 0000000..575bb4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/da8xx-fb.txt
@@ -0,0 +1,42 @@
+TI LCD Controller on DA830/DA850/AM335x SoC's
+
+Required properties:
+- compatible:
+ DA830 - "ti,da8xx-tilcdc"
+ AM335x SoC's - "ti,am33xx-tilcdc"
+- reg: Address range of lcdc register set
+- interrupts: lcdc interrupt
+- display-timings: typical videomode of lcd panel, represented as child.
+ Refer Documentation/devicetree/bindings/video/display-timing.txt for
+ display timing binding details. If multiple videomodes are mentioned
+ in display timings node, typical videomode has to be mentioned as the
+ native mode or it has to be first child (driver cares only for native
+ videomode).
+
+Recommended properties:
+- ti,hwmods: Name of the hwmod associated to the LCDC
+
+Example:
+
+lcdc@4830e000 {
+ compatible = "ti,am33xx-tilcdc";
+ reg = <0x4830e000 0x1000>;
+ interrupts = <36>;
+ ti,hwmods = "lcdc";
+ status = "okay";
+ display-timings {
+ 800x480p62 {
+ clock-frequency = <30000000>;
+ hactive = <800>;
+ vactive = <480>;
+ hfront-porch = <39>;
+ hback-porch = <39>;
+ hsync-len = <47>;
+ vback-porch = <29>;
+ vfront-porch = <13>;
+ vsync-len = <2>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ };
+ };
+};
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index ea9771f..1d9aff1 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -36,6 +36,7 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/lcm.h>
+#include <video/of_display_timing.h>
#include <video/da8xx-fb.h>
#include <asm/div64.h>
@@ -1297,12 +1298,54 @@ static struct fb_ops da8xx_fb_ops = {
.fb_blank = cfb_blank,
};
+static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev)
+{
+ struct lcd_ctrl_config *cfg;
+
+ cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL);
+ if (!cfg)
+ return NULL;
+
+ /* default values */
+
+ if (lcd_revision = LCD_VERSION_1)
+ cfg->bpp = 16;
+ else
+ cfg->bpp = 32;
+
+ /*
+ * For panels so far used with this LCDC, below statement is sufficient.
+ * For new panels, if required, struct lcd_ctrl_cfg fields to be updated
+ * with additional/modified values. Those values would have to be then
+ * obtained from dt(requiring new dt bindings).
+ */
+
+ cfg->panel_shade = COLOR_ACTIVE;
+
+ return cfg;
+}
+
static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
{
struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
struct fb_videomode *lcdc_info;
+ struct device_node *np = dev->dev.of_node;
int i;
+ if (np) {
+ lcdc_info = devm_kzalloc(&dev->dev,
+ sizeof(struct fb_videomode),
+ GFP_KERNEL);
+ if (!lcdc_info)
+ return NULL;
+
+ if (of_get_fb_videomode(np, lcdc_info, OF_USE_NATIVE_MODE)) {
+ dev_err(&dev->dev, "timings not available in DT\n");
+ return NULL;
+ }
+ return lcdc_info;
+ }
+
for (i = 0, lcdc_info = known_lcd_panels;
i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
@@ -1331,7 +1374,7 @@ static int fb_probe(struct platform_device *device)
int ret;
unsigned long ulcm;
- if (fb_pdata = NULL) {
+ if (fb_pdata = NULL && !device->dev.of_node) {
dev_err(&device->dev, "Can not get platform data\n");
return -ENOENT;
}
@@ -1371,7 +1414,10 @@ static int fb_probe(struct platform_device *device)
break;
}
- lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
+ if (device->dev.of_node)
+ lcd_cfg = da8xx_fb_create_cfg(device);
+ else
+ lcd_cfg = fb_pdata->controller_data;
if (!lcd_cfg) {
ret = -EINVAL;
@@ -1390,7 +1436,7 @@ static int fb_probe(struct platform_device *device)
par->dev = &device->dev;
par->lcdc_clk = tmp_lcdc_clk;
par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
- if (fb_pdata->panel_power_ctrl) {
+ if (fb_pdata && fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
par->panel_power_ctrl(1);
}
@@ -1638,6 +1684,19 @@ static int fb_resume(struct platform_device *dev)
#define fb_resume NULL
#endif
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id da8xx_fb_of_match[] = {
+ /*
+ * this driver supports version 1 and version 2 of the
+ * Texas Instruments lcd controller (lcdc) hardware block
+ */
+ {.compatible = "ti,da8xx-tilcdc", },
+ {.compatible = "ti,am33xx-tilcdc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
+#endif
+
static struct platform_driver da8xx_fb_driver = {
.probe = fb_probe,
.remove = fb_remove,
@@ -1646,6 +1705,7 @@ static struct platform_driver da8xx_fb_driver = {
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(da8xx_fb_of_match),
},
};
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] video: da8xx-fb: adding dt support
From: Prabhakar Lad @ 2013-08-23 7:44 UTC (permalink / raw)
To: Darren Etheridge; +Cc: devicetree, Tomi Valkeinen, plagnioj, LFBDEV, afzal
In-Reply-To: <1377206501-30519-2-git-send-email-detheridge@ti.com>
Hi Darren,
Thanks for the patch.
On Fri, Aug 23, 2013 at 2:51 AM, Darren Etheridge <detheridge@ti.com> wrote:
> Enhancing driver to enable probe triggered by a corresponding dt entry.
>
> Add da8xx-fb.txt documentation to devicetree section.
>
> Obtain fb_videomode details for the connected lcd panel using the
> display timing details present in DT.
>
> Ensure that platform data is present before checking whether platform
> callback is present (the one used to control backlight). So far this
> was not an issue as driver was purely non-DT triggered, but now DT
> support has been added this check must be performed.
>
> v2: squashing multiple commits from Afzal Mohammed (afzal@ti.com)
> v3: remove superfluous cast
> v4: expose both ti,am3352-lcdc and ti,da830-lcdc for .compatible
> as driver can use enhanced features of all version of the
> silicon block.
> v5: addressed review comments from Prabhakar Lad
> v6: Changed the .compatible naming to match the existing drm bindings
> for am33xx devices
>
> Signed-off-by: Darren Etheridge <detheridge@ti.com>
> ---
> .../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
> drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
> 2 files changed, 105 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
>
> diff --git a/Documentation/devicetree/bindings/video/da8xx-fb.txt b/Documentation/devicetree/bindings/video/da8xx-fb.txt
> new file mode 100644
> index 0000000..575bb4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/da8xx-fb.txt
> @@ -0,0 +1,42 @@
> +TI LCD Controller on DA830/DA850/AM335x SoC's
> +
> +Required properties:
> +- compatible:
> + DA830 - "ti,da8xx-tilcdc"
> + AM335x SoC's - "ti,am33xx-tilcdc"
what about DA850 ?
Rest of the patch looks good, with that change you can add my,
Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Regards,
--Prabhakar Lad
^ permalink raw reply
* [PATCH v3 0/1] Adding DT support to video/da8xx-fb.c
From: Darren Etheridge @ 2013-08-23 18:17 UTC (permalink / raw)
To: prabhakar.csengg, devicetree, tomi.valkeinen, plagnioj,
linux-fbdev, detheridge
Cc: afzal
This is part of a larger series of patches to upgrade the da8xx-fb.c driver
to support the Texas Instruments AM335x device. As part of this upgrade
we also want to add devicetree support for both the original da8xx and the
am335x. Tomi Valkeinen has reviewed the fbdev changes but he suggested
that it was prudent to extract the dt pieces and run it through the
devicetree mailing list for review.
v2: incorporates -
* review comment changes from Prabhakar Lad
* changes the .compatible naming to match that of the exiting drm
driver for am335x devices.
* Renames the devicetree bindings documentation file to match the
actual driver filename
v3: updated documentation to make clear which compatible to use for DA850
SoC devices.
Thanks,
Darren
Darren Etheridge (1):
video: da8xx-fb: adding dt support
.../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
^ permalink raw reply
* [PATCH v3 1/1] video: da8xx-fb: adding dt support
From: Darren Etheridge @ 2013-08-23 18:17 UTC (permalink / raw)
To: prabhakar.csengg, devicetree, tomi.valkeinen, plagnioj,
linux-fbdev, detheridge
Cc: afzal
In-Reply-To: <1377281822-29659-1-git-send-email-detheridge@ti.com>
Enhancing driver to enable probe triggered by a corresponding dt entry.
Add da8xx-fb.txt documentation to devicetree section.
Obtain fb_videomode details for the connected lcd panel using the
display timing details present in DT.
Ensure that platform data is present before checking whether platform
callback is present (the one used to control backlight). So far this
was not an issue as driver was purely non-DT triggered, but now DT
support has been added this check must be performed.
v2: squashing multiple commits from Afzal Mohammed (afzal@ti.com)
v3: remove superfluous cast
v4: expose both ti,am3352-lcdc and ti,da830-lcdc for .compatible
as driver can use enhanced features of all version of the
silicon block.
v5: addressed review comments from Prabhakar Lad
v6: Changed the .compatible naming to match the existing drm bindings
for am33xx devices
v7: clarify which compatible to use in the documentation for DA850
Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
.../devicetree/bindings/video/da8xx-fb.txt | 42 ++++++++++++
drivers/video/da8xx-fb.c | 66 +++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/da8xx-fb.txt
diff --git a/Documentation/devicetree/bindings/video/da8xx-fb.txt b/Documentation/devicetree/bindings/video/da8xx-fb.txt
new file mode 100644
index 0000000..d86afe7
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/da8xx-fb.txt
@@ -0,0 +1,42 @@
+TI LCD Controller on DA830/DA850/AM335x SoC's
+
+Required properties:
+- compatible:
+ DA830, DA850 - "ti,da8xx-tilcdc"
+ AM335x SoC's - "ti,am33xx-tilcdc"
+- reg: Address range of lcdc register set
+- interrupts: lcdc interrupt
+- display-timings: typical videomode of lcd panel, represented as child.
+ Refer Documentation/devicetree/bindings/video/display-timing.txt for
+ display timing binding details. If multiple videomodes are mentioned
+ in display timings node, typical videomode has to be mentioned as the
+ native mode or it has to be first child (driver cares only for native
+ videomode).
+
+Recommended properties:
+- ti,hwmods: Name of the hwmod associated to the LCDC
+
+Example for am335x SoC's:
+
+lcdc@4830e000 {
+ compatible = "ti,am33xx-tilcdc";
+ reg = <0x4830e000 0x1000>;
+ interrupts = <36>;
+ ti,hwmods = "lcdc";
+ status = "okay";
+ display-timings {
+ 800x480p62 {
+ clock-frequency = <30000000>;
+ hactive = <800>;
+ vactive = <480>;
+ hfront-porch = <39>;
+ hback-porch = <39>;
+ hsync-len = <47>;
+ vback-porch = <29>;
+ vfront-porch = <13>;
+ vsync-len = <2>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ };
+ };
+};
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index ea9771f..1d9aff1 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -36,6 +36,7 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/lcm.h>
+#include <video/of_display_timing.h>
#include <video/da8xx-fb.h>
#include <asm/div64.h>
@@ -1297,12 +1298,54 @@ static struct fb_ops da8xx_fb_ops = {
.fb_blank = cfb_blank,
};
+static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev)
+{
+ struct lcd_ctrl_config *cfg;
+
+ cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL);
+ if (!cfg)
+ return NULL;
+
+ /* default values */
+
+ if (lcd_revision = LCD_VERSION_1)
+ cfg->bpp = 16;
+ else
+ cfg->bpp = 32;
+
+ /*
+ * For panels so far used with this LCDC, below statement is sufficient.
+ * For new panels, if required, struct lcd_ctrl_cfg fields to be updated
+ * with additional/modified values. Those values would have to be then
+ * obtained from dt(requiring new dt bindings).
+ */
+
+ cfg->panel_shade = COLOR_ACTIVE;
+
+ return cfg;
+}
+
static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
{
struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
struct fb_videomode *lcdc_info;
+ struct device_node *np = dev->dev.of_node;
int i;
+ if (np) {
+ lcdc_info = devm_kzalloc(&dev->dev,
+ sizeof(struct fb_videomode),
+ GFP_KERNEL);
+ if (!lcdc_info)
+ return NULL;
+
+ if (of_get_fb_videomode(np, lcdc_info, OF_USE_NATIVE_MODE)) {
+ dev_err(&dev->dev, "timings not available in DT\n");
+ return NULL;
+ }
+ return lcdc_info;
+ }
+
for (i = 0, lcdc_info = known_lcd_panels;
i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
@@ -1331,7 +1374,7 @@ static int fb_probe(struct platform_device *device)
int ret;
unsigned long ulcm;
- if (fb_pdata = NULL) {
+ if (fb_pdata = NULL && !device->dev.of_node) {
dev_err(&device->dev, "Can not get platform data\n");
return -ENOENT;
}
@@ -1371,7 +1414,10 @@ static int fb_probe(struct platform_device *device)
break;
}
- lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
+ if (device->dev.of_node)
+ lcd_cfg = da8xx_fb_create_cfg(device);
+ else
+ lcd_cfg = fb_pdata->controller_data;
if (!lcd_cfg) {
ret = -EINVAL;
@@ -1390,7 +1436,7 @@ static int fb_probe(struct platform_device *device)
par->dev = &device->dev;
par->lcdc_clk = tmp_lcdc_clk;
par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
- if (fb_pdata->panel_power_ctrl) {
+ if (fb_pdata && fb_pdata->panel_power_ctrl) {
par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
par->panel_power_ctrl(1);
}
@@ -1638,6 +1684,19 @@ static int fb_resume(struct platform_device *dev)
#define fb_resume NULL
#endif
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id da8xx_fb_of_match[] = {
+ /*
+ * this driver supports version 1 and version 2 of the
+ * Texas Instruments lcd controller (lcdc) hardware block
+ */
+ {.compatible = "ti,da8xx-tilcdc", },
+ {.compatible = "ti,am33xx-tilcdc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
+#endif
+
static struct platform_driver da8xx_fb_driver = {
.probe = fb_probe,
.remove = fb_remove,
@@ -1646,6 +1705,7 @@ static struct platform_driver da8xx_fb_driver = {
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(da8xx_fb_of_match),
},
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH 0/4] video: da8xx-fb: numerous bugfixes
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
In developing a driver for an HDMI encoder to be attached to the LCD controller
on TI AM335x SoC I ran across a number of bugs in the da8xx-fb.c fbdev driver.
Two of them appear to have been in the driver for a while (02 and 04), one of
them is a deficiency where some LCD controller version 2 features have not been
utilized (03) and the last one was introduced in the last patch series that I
submitted for this driver (01).
These patches do not change the behavior of the AM335x EVM's platforms LCD
panel even though the current values set by the driver are wrong. I guess LCD
panels (at least the one used on AM335x EVM) must be fairly immune to bad
timings coming from the LCD controller. However the HDMI encoder simply will
not work without these fixes, and quite possibly other LCD panels could behave
the same way.
These patches apply on top of the patch series called:
"video/da8xx-fb fbdev driver enhance to support TI am335x SoC"
that was submitted by me.
Darren Etheridge (4):
video: da8xx-fb fixing incorrect porch mappings
video: da8xx-fb: fixing timing off by one errors
video: da8xx-fb: support lcdc v2 timing register expansion
video: da8xx-fb: fix the polarities of the hsync/vsync pulse
drivers/video/da8xx-fb.c | 35 +++++++++++++++++++++++++----------
1 files changed, 25 insertions(+), 10 deletions(-)
^ permalink raw reply
* [PATCH 1/4] video: da8xx-fb fixing incorrect porch mappings
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
The driver was mapping the wrong fbdev margins to the
front porch / back porch for both vertical and horizontal
timings.
This patch corrects it so that:
hfp = right margin
hbp = left margin
vbp = upper margin
vfp = lower margin
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 1d9aff1..0333a0b 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -784,10 +784,10 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
return ret;
/* Configure the vertical and horizontal sync properties. */
- lcd_cfg_vertical_sync(panel->lower_margin, panel->vsync_len,
- panel->upper_margin);
- lcd_cfg_horizontal_sync(panel->right_margin, panel->hsync_len,
- panel->left_margin);
+ lcd_cfg_vertical_sync(panel->upper_margin, panel->vsync_len,
+ panel->lower_margin);
+ lcd_cfg_horizontal_sync(panel->left_margin, panel->hsync_len,
+ panel->right_margin);
/* Configure for disply */
ret = lcd_cfg_display(cfg, panel);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/4] video: da8xx-fb: fixing timing off by one errors
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
The LCD controller represents some of the timing fields with a 0
in the register representing 1. This was not taken into account
when these registers were being set. Interestingly enough not
all of the LCDC controller timing registers implement this representation
so carefully went through the technical reference manual to only "fix"
the correct timings.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0333a0b..b131a6c 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -408,9 +408,9 @@ static void lcd_cfg_horizontal_sync(int back_porch, int pulse_width,
u32 reg;
reg = lcdc_read(LCD_RASTER_TIMING_0_REG) & 0xf;
- reg |= ((back_porch & 0xff) << 24)
- | ((front_porch & 0xff) << 16)
- | ((pulse_width & 0x3f) << 10);
+ reg |= (((back_porch-1) & 0xff) << 24)
+ | (((front_porch-1) & 0xff) << 16)
+ | (((pulse_width-1) & 0x3f) << 10);
lcdc_write(reg, LCD_RASTER_TIMING_0_REG);
}
@@ -422,7 +422,7 @@ static void lcd_cfg_vertical_sync(int back_porch, int pulse_width,
reg = lcdc_read(LCD_RASTER_TIMING_1_REG) & 0x3ff;
reg |= ((back_porch & 0xff) << 24)
| ((front_porch & 0xff) << 16)
- | ((pulse_width & 0x3f) << 10);
+ | (((pulse_width-1) & 0x3f) << 10);
lcdc_write(reg, LCD_RASTER_TIMING_1_REG);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/4] video: da8xx-fb: support lcdc v2 timing register expansion
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
TI LCD controller version 2 adds some extra bits in a register to
increase the available size to represent horizontal timings. This
patch allows the fbdev driver to utilize those extra bits.
This will become important for driving an HDMI encoder from the lcd
controller where some of the VESA/CEA modes require quite large porches.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index b131a6c..278b7d7 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -412,6 +412,21 @@ static void lcd_cfg_horizontal_sync(int back_porch, int pulse_width,
| (((front_porch-1) & 0xff) << 16)
| (((pulse_width-1) & 0x3f) << 10);
lcdc_write(reg, LCD_RASTER_TIMING_0_REG);
+
+ /*
+ * LCDC Version 2 adds some extra bits that increase the allowable
+ * size of the horizontal timing registers.
+ * remember that the registers use 0 to represent 1 so all values
+ * that get set into register need to be decremented by 1
+ */
+ if(lcd_revision = LCD_VERSION_2) {
+ /* Mask off the bits we want to change */
+ reg = lcdc_read(LCD_RASTER_TIMING_2_REG) & ~0x780000ff;
+ reg |= ((front_porch-1) & 0x300) >> 8;
+ reg |= ((back_porch-1) & 0x300) >> 4;
+ reg |= ((pulse_width-1) & 0x3c0) << 21;
+ lcdc_write(reg, LCD_RASTER_TIMING_2_REG);
+ }
}
static void lcd_cfg_vertical_sync(int back_porch, int pulse_width,
--
1.7.0.4
^ permalink raw reply related
* [PATCH 4/4] video: da8xx-fb: fix the polarities of the hsync/vsync pulse
From: Darren Etheridge @ 2013-08-23 21:52 UTC (permalink / raw)
To: linux-fbdev
The polarities were being set to active low when fbdev was requesting active
high. This patch reverses it so that what is set into the LCD controller is
correct.
Signed-off-by: Darren Etheridge <detheridge@ti.com>
---
drivers/video/da8xx-fb.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 278b7d7..f768791 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -494,12 +494,12 @@ static int lcd_cfg_display(const struct lcd_ctrl_config *cfg,
else
reg &= ~LCD_SYNC_EDGE;
- if (panel->sync & FB_SYNC_HOR_HIGH_ACT)
+ if ((panel->sync & FB_SYNC_HOR_HIGH_ACT) = 0)
reg |= LCD_INVERT_LINE_CLOCK;
else
reg &= ~LCD_INVERT_LINE_CLOCK;
- if (panel->sync & FB_SYNC_VERT_HIGH_ACT)
+ if ((panel->sync & FB_SYNC_VERT_HIGH_ACT) = 0)
reg |= LCD_INVERT_FRAME_CLOCK;
else
reg &= ~LCD_INVERT_FRAME_CLOCK;
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v11 0/8] PHY framework
From: Kishon Vijay Abraham I @ 2013-08-26 8:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Hi Greg,
On Wednesday 21 August 2013 11:16 AM, Kishon Vijay Abraham I wrote:
> Added a generic PHY framework that provides a set of APIs for the PHY drivers
> to create/destroy a PHY and APIs for the PHY users to obtain a reference to
> the PHY with or without using phandle.
>
> This framework will be of use only to devices that uses external PHY (PHY
> functionality is not embedded within the controller).
>
> The intention of creating this framework is to bring the phy drivers spread
> all over the Linux kernel to drivers/phy to increase code re-use and to
> increase code maintainability.
>
> Comments to make PHY as bus wasn't done because PHY devices can be part of
> other bus and making a same device attached to multiple bus leads to bad
> design.
>
> If the PHY driver has to send notification on connect/disconnect, the PHY
> driver should make use of the extcon framework. Using this susbsystem
> to use extcon framwork will have to be analysed.
>
> You can find this patch series @
> git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing
Looks like there are not further comments on this series. Can you take this in
your misc tree?
Thanks
Kishon
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox