* [PATCH 0/7] video phy's adaptation to *generic phy framework*
From: Kishon Vijay Abraham I @ 2013-10-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Greg,
This series includes video PHY adaptation to Generic PHY Framework.
With the adaptation they were able to get rid of plat data callbacks.
Since you've taken the Generic PHY Framework, I think this series should
also go into your tree.
We should thank Sylwester for actively testing and giving comments from
the initial versions of Generic Phy Framework. Both Sylwester and Jingoo
had been floating many revisions of their adaptation to Generic PHY
Framework.
This has been in my repo for quite some time and has got acks from
samsung maintainer and video maintainer.
If you want me to change anything, please let me know.
This patch series can be found @
git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing
Jingoo Han (3):
phy: Add driver for Exynos DP PHY
video: exynos_dp: remove non-DT support for Exynos Display Port
video: exynos_dp: Use the generic PHY driver
Sylwester Nawrocki (4):
phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
exynos4-is: Use the generic MIPI CSIS PHY driver
video: exynos_mipi_dsim: Use the generic PHY driver
ARM: Samsung: Remove the MIPI PHY setup code
.../devicetree/bindings/phy/samsung-phy.txt | 22 +++
.../devicetree/bindings/video/exynos_dp.txt | 17 +-
arch/arm/mach-exynos/include/mach/regs-pmu.h | 5 -
arch/arm/mach-s5pv210/include/mach/regs-clock.h | 4 -
arch/arm/plat-samsung/Kconfig | 5 -
arch/arm/plat-samsung/Makefile | 1 -
arch/arm/plat-samsung/setup-mipiphy.c | 60 -------
drivers/media/platform/exynos4-is/Kconfig | 2 +-
drivers/media/platform/exynos4-is/mipi-csis.c | 13 +-
drivers/phy/Kconfig | 13 ++
drivers/phy/Makefile | 8 +-
drivers/phy/phy-exynos-dp-video.c | 111 ++++++++++++
drivers/phy/phy-exynos-mipi-video.c | 176 ++++++++++++++++++++
drivers/video/exynos/Kconfig | 3 +-
drivers/video/exynos/exynos_dp_core.c | 132 ++++-----------
drivers/video/exynos/exynos_dp_core.h | 110 ++++++++++++
drivers/video/exynos/exynos_dp_reg.c | 2 -
drivers/video/exynos/exynos_mipi_dsi.c | 19 ++-
include/linux/platform_data/mipi-csis.h | 9 -
include/video/exynos_dp.h | 131 ---------------
include/video/exynos_mipi_dsim.h | 5 +-
21 files changed, 508 insertions(+), 340 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/samsung-phy.txt
delete mode 100644 arch/arm/plat-samsung/setup-mipiphy.c
create mode 100644 drivers/phy/phy-exynos-dp-video.c
create mode 100644 drivers/phy/phy-exynos-mipi-video.c
delete mode 100644 include/video/exynos_dp.h
--
1.7.10.4
^ permalink raw reply
* [PATCH 1/7] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Kishon Vijay Abraham I @ 2013-10-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1381940896-9355-1-git-send-email-kishon@ti.com>
From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
Add a PHY provider driver for the Samsung S5P/Exynos SoC MIPI CSI-2
receiver and MIPI DSI transmitter DPHYs.
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
.../devicetree/bindings/phy/samsung-phy.txt | 14 ++
drivers/phy/Kconfig | 6 +
drivers/phy/Makefile | 7 +-
drivers/phy/phy-exynos-mipi-video.c | 176 ++++++++++++++++++++
4 files changed, 200 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/samsung-phy.txt
create mode 100644 drivers/phy/phy-exynos-mipi-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
new file mode 100644
index 0000000..5ff208c
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -0,0 +1,14 @@
+Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,s5pv210-mipi-video-phy";
+- reg : offset and length of the MIPI DPHY register set;
+- #phy-cells : from the generic phy bindings, must be 1;
+
+For "samsung,s5pv210-mipi-video-phy" compatible PHYs the second cell in
+the PHY specifier identifies the PHY and its meaning is as follows:
+ 0 - MIPI CSIS 0,
+ 1 - MIPI DSIM 0,
+ 2 - MIPI CSIS 1,
+ 3 - MIPI DSIM 1.
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index ac239ac..0062d7e 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,6 +15,12 @@ config GENERIC_PHY
phy users can obtain reference to the PHY. All the users of this
framework should select this config.
+config PHY_EXYNOS_MIPI_VIDEO
+ tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
+ help
+ Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
+ and EXYNOS SoCs.
+
config OMAP_USB2
tristate "OMAP USB2 PHY Driver"
depends on ARCH_OMAP2PLUS
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 0dd8a98..6344053 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -2,6 +2,7 @@
# Makefile for the phy drivers.
#
-obj-$(CONFIG_GENERIC_PHY) += phy-core.o
-obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
-obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
+obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
+obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
+obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
diff --git a/drivers/phy/phy-exynos-mipi-video.c b/drivers/phy/phy-exynos-mipi-video.c
new file mode 100644
index 0000000..b73b86a
--- /dev/null
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -0,0 +1,176 @@
+/*
+ * Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+/* MIPI_PHYn_CONTROL register offset: n = 0..1 */
+#define EXYNOS_MIPI_PHY_CONTROL(n) ((n) * 4)
+#define EXYNOS_MIPI_PHY_ENABLE (1 << 0)
+#define EXYNOS_MIPI_PHY_SRESETN (1 << 1)
+#define EXYNOS_MIPI_PHY_MRESETN (1 << 2)
+#define EXYNOS_MIPI_PHY_RESET_MASK (3 << 1)
+
+enum exynos_mipi_phy_id {
+ EXYNOS_MIPI_PHY_ID_CSIS0,
+ EXYNOS_MIPI_PHY_ID_DSIM0,
+ EXYNOS_MIPI_PHY_ID_CSIS1,
+ EXYNOS_MIPI_PHY_ID_DSIM1,
+ EXYNOS_MIPI_PHYS_NUM
+};
+
+#define is_mipi_dsim_phy_id(id) \
+ ((id) = EXYNOS_MIPI_PHY_ID_DSIM0 || (id) = EXYNOS_MIPI_PHY_ID_DSIM1)
+
+struct exynos_mipi_video_phy {
+ spinlock_t slock;
+ struct video_phy_desc {
+ struct phy *phy;
+ unsigned int index;
+ } phys[EXYNOS_MIPI_PHYS_NUM];
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_mipi_video_phy *state,
+ enum exynos_mipi_phy_id id, unsigned int on)
+{
+ void __iomem *addr;
+ u32 reg, reset;
+
+ addr = state->regs + EXYNOS_MIPI_PHY_CONTROL(id / 2);
+
+ if (is_mipi_dsim_phy_id(id))
+ reset = EXYNOS_MIPI_PHY_MRESETN;
+ else
+ reset = EXYNOS_MIPI_PHY_SRESETN;
+
+ spin_lock(&state->slock);
+ reg = readl(addr);
+ if (on)
+ reg |= reset;
+ else
+ reg &= ~reset;
+ writel(reg, addr);
+
+ /* Clear ENABLE bit only if MRESETN, SRESETN bits are not set. */
+ if (on)
+ reg |= EXYNOS_MIPI_PHY_ENABLE;
+ else if (!(reg & EXYNOS_MIPI_PHY_RESET_MASK))
+ reg &= ~EXYNOS_MIPI_PHY_ENABLE;
+
+ writel(reg, addr);
+ spin_unlock(&state->slock);
+ return 0;
+}
+
+#define to_mipi_video_phy(desc) \
+ container_of((desc), struct exynos_mipi_video_phy, phys[(desc)->index]);
+
+static int exynos_mipi_video_phy_power_on(struct phy *phy)
+{
+ struct video_phy_desc *phy_desc = phy_get_drvdata(phy);
+ struct exynos_mipi_video_phy *state = to_mipi_video_phy(phy_desc);
+
+ return __set_phy_state(state, phy_desc->index, 1);
+}
+
+static int exynos_mipi_video_phy_power_off(struct phy *phy)
+{
+ struct video_phy_desc *phy_desc = phy_get_drvdata(phy);
+ struct exynos_mipi_video_phy *state = to_mipi_video_phy(phy_desc);
+
+ return __set_phy_state(state, phy_desc->index, 1);
+}
+
+static struct phy *exynos_mipi_video_phy_xlate(struct device *dev,
+ struct of_phandle_args *args)
+{
+ struct exynos_mipi_video_phy *state = dev_get_drvdata(dev);
+
+ if (WARN_ON(args->args[0] > EXYNOS_MIPI_PHYS_NUM))
+ return ERR_PTR(-ENODEV);
+
+ return state->phys[args->args[0]].phy;
+}
+
+static struct phy_ops exynos_mipi_video_phy_ops = {
+ .power_on = exynos_mipi_video_phy_power_on,
+ .power_off = exynos_mipi_video_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int exynos_mipi_video_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_mipi_video_phy *state;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ unsigned int i;
+
+ state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ state->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(state->regs))
+ return PTR_ERR(state->regs);
+
+ dev_set_drvdata(dev, state);
+ spin_lock_init(&state->slock);
+
+ phy_provider = devm_of_phy_provider_register(dev,
+ exynos_mipi_video_phy_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ for (i = 0; i < EXYNOS_MIPI_PHYS_NUM; i++) {
+ struct phy *phy = devm_phy_create(dev,
+ &exynos_mipi_video_phy_ops, NULL);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create PHY %d\n", i);
+ return PTR_ERR(phy);
+ }
+
+ state->phys[i].phy = phy;
+ state->phys[i].index = i;
+ phy_set_drvdata(phy, &state->phys[i]);
+ }
+
+ return 0;
+}
+
+static const struct of_device_id exynos_mipi_video_phy_of_match[] = {
+ { .compatible = "samsung,s5pv210-mipi-video-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, exynos_mipi_video_phy_of_match);
+
+static struct platform_driver exynos_mipi_video_phy_driver = {
+ .probe = exynos_mipi_video_phy_probe,
+ .driver = {
+ .of_match_table = exynos_mipi_video_phy_of_match,
+ .name = "exynos-mipi-video-phy",
+ .owner = THIS_MODULE,
+ }
+};
+module_platform_driver(exynos_mipi_video_phy_driver);
+
+MODULE_DESCRIPTION("Samsung S5P/EXYNOS SoC MIPI CSI-2/DSI PHY driver");
+MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
+MODULE_LICENSE("GPL v2");
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/7] exynos4-is: Use the generic MIPI CSIS PHY driver
From: Kishon Vijay Abraham I @ 2013-10-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1381940896-9355-1-git-send-email-kishon@ti.com>
From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
Use the generic PHY API instead of the platform callback
to control the MIPI CSIS DPHY.
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/media/platform/exynos4-is/Kconfig | 2 +-
drivers/media/platform/exynos4-is/mipi-csis.c | 13 ++++++++++---
include/linux/platform_data/mipi-csis.h | 9 ---------
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
index 53ad0f0..d2d3b4b 100644
--- a/drivers/media/platform/exynos4-is/Kconfig
+++ b/drivers/media/platform/exynos4-is/Kconfig
@@ -29,7 +29,7 @@ config VIDEO_S5P_FIMC
config VIDEO_S5P_MIPI_CSIS
tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
depends on REGULATOR
- select S5P_SETUP_MIPIPHY
+ select GENERIC_PHY
help
This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
receiver (MIPI-CSIS) devices.
diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
index 0914230..9fc2af6 100644
--- a/drivers/media/platform/exynos4-is/mipi-csis.c
+++ b/drivers/media/platform/exynos4-is/mipi-csis.c
@@ -20,6 +20,7 @@
#include <linux/memory.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/phy/phy.h>
#include <linux/platform_data/mipi-csis.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
@@ -180,6 +181,7 @@ struct csis_drvdata {
* @sd: v4l2_subdev associated with CSIS device instance
* @index: the hardware instance index
* @pdev: CSIS platform device
+ * @phy: pointer to the CSIS generic PHY
* @regs: mmaped I/O registers memory
* @supplies: CSIS regulator supplies
* @clock: CSIS clocks
@@ -203,6 +205,7 @@ struct csis_state {
struct v4l2_subdev sd;
u8 index;
struct platform_device *pdev;
+ struct phy *phy;
void __iomem *regs;
struct regulator_bulk_data supplies[CSIS_NUM_SUPPLIES];
struct clk *clock[NUM_CSIS_CLOCKS];
@@ -779,8 +782,8 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
"samsung,csis-wclk");
state->num_lanes = endpoint.bus.mipi_csi2.num_data_lanes;
-
of_node_put(node);
+
return 0;
}
#else
@@ -829,6 +832,10 @@ static int s5pcsis_probe(struct platform_device *pdev)
return -EINVAL;
}
+ state->phy = devm_phy_get(dev, "csis");
+ if (IS_ERR(state->phy))
+ return PTR_ERR(state->phy);
+
mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
state->regs = devm_ioremap_resource(dev, mem_res);
if (IS_ERR(state->regs))
@@ -922,7 +929,7 @@ static int s5pcsis_pm_suspend(struct device *dev, bool runtime)
mutex_lock(&state->lock);
if (state->flags & ST_POWERED) {
s5pcsis_stop_stream(state);
- ret = s5p_csis_phy_enable(state->index, false);
+ ret = phy_power_off(state->phy);
if (ret)
goto unlock;
ret = regulator_bulk_disable(CSIS_NUM_SUPPLIES,
@@ -958,7 +965,7 @@ static int s5pcsis_pm_resume(struct device *dev, bool runtime)
state->supplies);
if (ret)
goto unlock;
- ret = s5p_csis_phy_enable(state->index, true);
+ ret = phy_power_on(state->phy);
if (!ret) {
state->flags |= ST_POWERED;
} else {
diff --git a/include/linux/platform_data/mipi-csis.h b/include/linux/platform_data/mipi-csis.h
index bf34e17..c2fd902 100644
--- a/include/linux/platform_data/mipi-csis.h
+++ b/include/linux/platform_data/mipi-csis.h
@@ -25,13 +25,4 @@ struct s5p_platform_mipi_csis {
u8 hs_settle;
};
-/**
- * s5p_csis_phy_enable - global MIPI-CSI receiver D-PHY control
- * @id: MIPI-CSIS harware instance index (0...1)
- * @on: true to enable D-PHY and deassert its reset
- * false to disable D-PHY
- * @return: 0 on success, or negative error code on failure
- */
-int s5p_csis_phy_enable(int id, bool on);
-
#endif /* __PLAT_SAMSUNG_MIPI_CSIS_H_ */
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/7] video: exynos_mipi_dsim: Use the generic PHY driver
From: Kishon Vijay Abraham I @ 2013-10-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1381940896-9355-1-git-send-email-kishon@ti.com>
From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
Use the generic PHY API instead of the platform callback
for the MIPI DSIM DPHY enable/reset control.
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Acked-by: Donghwa Lee <dh09.lee@samsung.com>
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/video/exynos/Kconfig | 1 +
drivers/video/exynos/exynos_mipi_dsi.c | 19 ++++++++++---------
include/video/exynos_mipi_dsim.h | 5 ++---
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
index 1b035b2..976594d 100644
--- a/drivers/video/exynos/Kconfig
+++ b/drivers/video/exynos/Kconfig
@@ -16,6 +16,7 @@ if EXYNOS_VIDEO
config EXYNOS_MIPI_DSI
bool "EXYNOS MIPI DSI driver support."
depends on ARCH_S5PV210 || ARCH_EXYNOS
+ select GENERIC_PHY
help
This enables support for MIPI-DSI device.
diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index 32e5406..00b3a52 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -30,6 +30,7 @@
#include <linux/interrupt.h>
#include <linux/kthread.h>
#include <linux/notifier.h>
+#include <linux/phy/phy.h>
#include <linux/regulator/consumer.h>
#include <linux/pm_runtime.h>
#include <linux/err.h>
@@ -156,8 +157,7 @@ static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim->pd->phy_enable)
- dsim->pd->phy_enable(pdev, true);
+ phy_power_on(dsim->phy);
clk_enable(dsim->clock);
@@ -373,6 +373,10 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
return ret;
}
+ dsim->phy = devm_phy_get(&pdev->dev, "dsim");
+ if (IS_ERR(dsim->phy))
+ return PTR_ERR(dsim->phy);
+
dsim->clock = devm_clk_get(&pdev->dev, "dsim0");
if (IS_ERR(dsim->clock)) {
dev_err(&pdev->dev, "failed to get dsim clock source\n");
@@ -439,8 +443,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim->pd->phy_enable)
- dsim->pd->phy_enable(pdev, true);
+ phy_power_on(dsim->phy);
exynos_mipi_update_cfg(dsim);
@@ -504,9 +507,8 @@ static int exynos_mipi_dsi_suspend(struct device *dev)
if (client_drv && client_drv->suspend)
client_drv->suspend(client_dev);
- /* enable MIPI-DSI PHY. */
- if (dsim->pd->phy_enable)
- dsim->pd->phy_enable(pdev, false);
+ /* disable MIPI-DSI PHY. */
+ phy_power_off(dsim->phy);
clk_disable(dsim->clock);
@@ -536,8 +538,7 @@ static int exynos_mipi_dsi_resume(struct device *dev)
exynos_mipi_regulator_enable(dsim);
/* enable MIPI-DSI PHY. */
- if (dsim->pd->phy_enable)
- dsim->pd->phy_enable(pdev, true);
+ phy_power_on(dsim->phy);
clk_enable(dsim->clock);
diff --git a/include/video/exynos_mipi_dsim.h b/include/video/exynos_mipi_dsim.h
index 89dc88a..6a578f8 100644
--- a/include/video/exynos_mipi_dsim.h
+++ b/include/video/exynos_mipi_dsim.h
@@ -216,6 +216,7 @@ struct mipi_dsim_config {
* automatically.
* @e_clk_src: select byte clock source.
* @pd: pointer to MIPI-DSI driver platform data.
+ * @phy: pointer to the MIPI-DSI PHY
*/
struct mipi_dsim_device {
struct device *dev;
@@ -236,6 +237,7 @@ struct mipi_dsim_device {
bool suspended;
struct mipi_dsim_platform_data *pd;
+ struct phy *phy;
};
/*
@@ -248,7 +250,6 @@ struct mipi_dsim_device {
* @enabled: indicate whether mipi controller got enabled or not.
* @lcd_panel_info: pointer for lcd panel specific structure.
* this structure specifies width, height, timing and polarity and so on.
- * @phy_enable: pointer to a callback controlling D-PHY enable/reset
*/
struct mipi_dsim_platform_data {
char lcd_panel_name[PANEL_NAME_SIZE];
@@ -256,8 +257,6 @@ struct mipi_dsim_platform_data {
struct mipi_dsim_config *dsim_config;
unsigned int enabled;
void *lcd_panel_info;
-
- int (*phy_enable)(struct platform_device *pdev, bool on);
};
/*
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/7] ARM: Samsung: Remove the MIPI PHY setup code
From: Kishon Vijay Abraham I @ 2013-10-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1381940896-9355-1-git-send-email-kishon@ti.com>
From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
Generic PHY drivers are used to handle the MIPI CSIS and MIPI DSIM
DPHYs so we can remove now unused code at arch/arm/plat-samsung.
In case there is any board file for S5PV210 platforms using MIPI
CSIS/DSIM (not any upstream currently) it should use the generic
PHY API to bind the PHYs to respective PHY consumer drivers and
a platform device for the PHY provider should be defined.
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Acked-by: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
arch/arm/mach-exynos/include/mach/regs-pmu.h | 5 --
arch/arm/mach-s5pv210/include/mach/regs-clock.h | 4 --
arch/arm/plat-samsung/Kconfig | 5 --
arch/arm/plat-samsung/Makefile | 1 -
arch/arm/plat-samsung/setup-mipiphy.c | 60 -----------------------
5 files changed, 75 deletions(-)
delete mode 100644 arch/arm/plat-samsung/setup-mipiphy.c
diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h b/arch/arm/mach-exynos/include/mach/regs-pmu.h
index 57344b7..2cdb63e 100644
--- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
+++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
@@ -44,11 +44,6 @@
#define S5P_DAC_PHY_CONTROL S5P_PMUREG(0x070C)
#define S5P_DAC_PHY_ENABLE (1 << 0)
-#define S5P_MIPI_DPHY_CONTROL(n) S5P_PMUREG(0x0710 + (n) * 4)
-#define S5P_MIPI_DPHY_ENABLE (1 << 0)
-#define S5P_MIPI_DPHY_SRESETN (1 << 1)
-#define S5P_MIPI_DPHY_MRESETN (1 << 2)
-
#define S5P_INFORM0 S5P_PMUREG(0x0800)
#define S5P_INFORM1 S5P_PMUREG(0x0804)
#define S5P_INFORM2 S5P_PMUREG(0x0808)
diff --git a/arch/arm/mach-s5pv210/include/mach/regs-clock.h b/arch/arm/mach-s5pv210/include/mach/regs-clock.h
index 032de66..e345584 100644
--- a/arch/arm/mach-s5pv210/include/mach/regs-clock.h
+++ b/arch/arm/mach-s5pv210/include/mach/regs-clock.h
@@ -147,10 +147,6 @@
#define S5P_HDMI_PHY_CONTROL S5P_CLKREG(0xE804)
#define S5P_USB_PHY_CONTROL S5P_CLKREG(0xE80C)
#define S5P_DAC_PHY_CONTROL S5P_CLKREG(0xE810)
-#define S5P_MIPI_DPHY_CONTROL(x) S5P_CLKREG(0xE814)
-#define S5P_MIPI_DPHY_ENABLE (1 << 0)
-#define S5P_MIPI_DPHY_SRESETN (1 << 1)
-#define S5P_MIPI_DPHY_MRESETN (1 << 2)
#define S5P_INFORM0 S5P_CLKREG(0xF000)
#define S5P_INFORM1 S5P_CLKREG(0xF004)
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index 7dfba93..ec882ad 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -395,11 +395,6 @@ config S3C24XX_PWM
Support for exporting the PWM timer blocks via the pwm device
system
-config S5P_SETUP_MIPIPHY
- bool
- help
- Compile in common setup code for MIPI-CSIS and MIPI-DSIM devices
-
config S3C_SETUP_CAMIF
bool
help
diff --git a/arch/arm/plat-samsung/Makefile b/arch/arm/plat-samsung/Makefile
index 498c7c2..9267d29 100644
--- a/arch/arm/plat-samsung/Makefile
+++ b/arch/arm/plat-samsung/Makefile
@@ -38,7 +38,6 @@ obj-$(CONFIG_S5P_DEV_UART) += s5p-dev-uart.o
obj-$(CONFIG_SAMSUNG_DEV_BACKLIGHT) += dev-backlight.o
obj-$(CONFIG_S3C_SETUP_CAMIF) += setup-camif.o
-obj-$(CONFIG_S5P_SETUP_MIPIPHY) += setup-mipiphy.o
# DMA support
diff --git a/arch/arm/plat-samsung/setup-mipiphy.c b/arch/arm/plat-samsung/setup-mipiphy.c
deleted file mode 100644
index 66df315..0000000
--- a/arch/arm/plat-samsung/setup-mipiphy.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright (C) 2011 Samsung Electronics Co., Ltd.
- *
- * S5P - Helper functions for MIPI-CSIS and MIPI-DSIM D-PHY control
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/export.h>
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-#include <linux/io.h>
-#include <linux/spinlock.h>
-#include <mach/regs-clock.h>
-
-static int __s5p_mipi_phy_control(int id, bool on, u32 reset)
-{
- static DEFINE_SPINLOCK(lock);
- void __iomem *addr;
- unsigned long flags;
- u32 cfg;
-
- id = max(0, id);
- if (id > 1)
- return -EINVAL;
-
- addr = S5P_MIPI_DPHY_CONTROL(id);
-
- spin_lock_irqsave(&lock, flags);
-
- cfg = __raw_readl(addr);
- cfg = on ? (cfg | reset) : (cfg & ~reset);
- __raw_writel(cfg, addr);
-
- if (on) {
- cfg |= S5P_MIPI_DPHY_ENABLE;
- } else if (!(cfg & (S5P_MIPI_DPHY_SRESETN |
- S5P_MIPI_DPHY_MRESETN) & ~reset)) {
- cfg &= ~S5P_MIPI_DPHY_ENABLE;
- }
-
- __raw_writel(cfg, addr);
- spin_unlock_irqrestore(&lock, flags);
-
- return 0;
-}
-
-int s5p_csis_phy_enable(int id, bool on)
-{
- return __s5p_mipi_phy_control(id, on, S5P_MIPI_DPHY_SRESETN);
-}
-EXPORT_SYMBOL(s5p_csis_phy_enable);
-
-int s5p_dsim_phy_enable(struct platform_device *pdev, bool on)
-{
- return __s5p_mipi_phy_control(pdev->id, on, S5P_MIPI_DPHY_MRESETN);
-}
-EXPORT_SYMBOL(s5p_dsim_phy_enable);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/7] phy: Add driver for Exynos DP PHY
From: Kishon Vijay Abraham I @ 2013-10-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1381940896-9355-1-git-send-email-kishon@ti.com>
From: Jingoo Han <jg1.han@samsung.com>
Add a PHY provider driver for the Samsung Exynos SoC Display Port PHY.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
.../devicetree/bindings/phy/samsung-phy.txt | 8 ++
drivers/phy/Kconfig | 7 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-exynos-dp-video.c | 111 ++++++++++++++++++++
4 files changed, 127 insertions(+)
create mode 100644 drivers/phy/phy-exynos-dp-video.c
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt
index 5ff208c..c0fccaa 100644
--- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
+++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
1 - MIPI DSIM 0,
2 - MIPI CSIS 1,
3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series Display Port PHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the Display Port PHY register set;
+- #phy-cells : from the generic PHY bindings, must be 0;
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 0062d7e..a344f3d 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -44,4 +44,11 @@ config TWL4030_USB
This transceiver supports high and full speed devices plus,
in host mode, low speed.
+config PHY_EXYNOS_DP_VIDEO
+ tristate "EXYNOS SoC series Display Port PHY driver"
+ depends on OF
+ select GENERIC_PHY
+ help
+ Support for Display Port PHY found on Samsung EXYNOS SoCs.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 6344053..d0caae9 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,6 +3,7 @@
#
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
diff --git a/drivers/phy/phy-exynos-dp-video.c b/drivers/phy/phy-exynos-dp-video.c
new file mode 100644
index 0000000..1dbe6ce
--- /dev/null
+++ b/drivers/phy/phy-exynos-dp-video.c
@@ -0,0 +1,111 @@
+/*
+ * Samsung EXYNOS SoC series Display Port PHY driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han <jg1.han@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE (1 << 0)
+
+struct exynos_dp_video_phy {
+ void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+ u32 reg;
+
+ reg = readl(state->regs);
+ if (on)
+ reg |= EXYNOS_DPTX_PHY_ENABLE;
+ else
+ reg &= ~EXYNOS_DPTX_PHY_ENABLE;
+ writel(reg, state->regs);
+
+ return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+ struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+ return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+ .power_on = exynos_dp_video_phy_power_on,
+ .power_off = exynos_dp_video_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+ struct exynos_dp_video_phy *state;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ struct phy *phy;
+
+ state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ state->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(state->regs))
+ return PTR_ERR(state->regs);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy = devm_phy_create(dev, &exynos_dp_video_phy_ops, NULL);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create Display Port PHY\n");
+ return PTR_ERR(phy);
+ }
+ phy_set_drvdata(phy, state);
+
+ return 0;
+}
+
+static const struct of_device_id exynos_dp_video_phy_of_match[] = {
+ { .compatible = "samsung,exynos5250-dp-video-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
+
+static struct platform_driver exynos_dp_video_phy_driver = {
+ .probe = exynos_dp_video_phy_probe,
+ .driver = {
+ .name = "exynos-dp-video-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = exynos_dp_video_phy_of_match,
+ }
+};
+module_platform_driver(exynos_dp_video_phy_driver);
+
+MODULE_AUTHOR("Jingoo Han <jg1.han@samsung.com>");
+MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_LICENSE("GPL v2");
--
1.7.10.4
^ permalink raw reply related
* [PATCH 6/7] video: exynos_dp: remove non-DT support for Exynos Display Port
From: Kishon Vijay Abraham I @ 2013-10-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1381940896-9355-1-git-send-email-kishon@ti.com>
From: Jingoo Han <jg1.han@samsung.com>
Exynos Display Port can be used only for Exynos SoCs. In addition,
non-DT for EXYNOS SoCs is not supported from v3.11; thus, there is
no need to support non-DT for Exynos Display Port.
The 'include/video/exynos_dp.h' file has been used for non-DT
support and the content of file include/video/exynos_dp.h is moved
to drivers/video/exynos/exynos_dp_core.h. Thus, the 'exynos_dp.h'
file is removed. Also, 'struct exynos_dp_platdata' is removed,
because it is not used any more.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/video/exynos/Kconfig | 2 +-
drivers/video/exynos/exynos_dp_core.c | 116 +++++++----------------------
drivers/video/exynos/exynos_dp_core.h | 109 +++++++++++++++++++++++++++
drivers/video/exynos/exynos_dp_reg.c | 2 -
include/video/exynos_dp.h | 131 ---------------------------------
5 files changed, 135 insertions(+), 225 deletions(-)
delete mode 100644 include/video/exynos_dp.h
diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig
index 976594d..1129d0e 100644
--- a/drivers/video/exynos/Kconfig
+++ b/drivers/video/exynos/Kconfig
@@ -30,7 +30,7 @@ config EXYNOS_LCD_S6E8AX0
config EXYNOS_DP
bool "EXYNOS DP driver support"
- depends on ARCH_EXYNOS
+ depends on OF && ARCH_EXYNOS
default n
help
This enables support for DP device.
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 12bbede..05fed7d 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -20,8 +20,6 @@
#include <linux/delay.h>
#include <linux/of.h>
-#include <video/exynos_dp.h>
-
#include "exynos_dp_core.h"
static int exynos_dp_init_dp(struct exynos_dp_device *dp)
@@ -894,26 +892,17 @@ static void exynos_dp_hotplug(struct work_struct *work)
dev_err(dp->dev, "unable to config video\n");
}
-#ifdef CONFIG_OF
-static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
+static struct video_info *exynos_dp_dt_parse_pdata(struct device *dev)
{
struct device_node *dp_node = dev->of_node;
- struct exynos_dp_platdata *pd;
struct video_info *dp_video_config;
- pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
- if (!pd) {
- dev_err(dev, "memory allocation for pdata failed\n");
- return ERR_PTR(-ENOMEM);
- }
dp_video_config = devm_kzalloc(dev,
sizeof(*dp_video_config), GFP_KERNEL);
-
if (!dp_video_config) {
dev_err(dev, "memory allocation for video config failed\n");
return ERR_PTR(-ENOMEM);
}
- pd->video_info = dp_video_config;
dp_video_config->h_sync_polarity of_property_read_bool(dp_node, "hsync-active-high");
@@ -960,7 +949,7 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
return ERR_PTR(-EINVAL);
}
- return pd;
+ return dp_video_config;
}
static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
@@ -1003,48 +992,30 @@ err:
static void exynos_dp_phy_init(struct exynos_dp_device *dp)
{
- u32 reg;
+ if (dp->phy_addr) {
+ u32 reg;
- reg = __raw_readl(dp->phy_addr);
- reg |= dp->enable_mask;
- __raw_writel(reg, dp->phy_addr);
+ reg = __raw_readl(dp->phy_addr);
+ reg |= dp->enable_mask;
+ __raw_writel(reg, dp->phy_addr);
+ }
}
static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
{
- u32 reg;
-
- reg = __raw_readl(dp->phy_addr);
- reg &= ~(dp->enable_mask);
- __raw_writel(reg, dp->phy_addr);
-}
-#else
-static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
-{
- return NULL;
-}
-
-static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
-{
- return -EINVAL;
-}
-
-static void exynos_dp_phy_init(struct exynos_dp_device *dp)
-{
- return;
-}
+ if (dp->phy_addr) {
+ u32 reg;
-static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
-{
- return;
+ reg = __raw_readl(dp->phy_addr);
+ reg &= ~(dp->enable_mask);
+ __raw_writel(reg, dp->phy_addr);
+ }
}
-#endif /* CONFIG_OF */
static int exynos_dp_probe(struct platform_device *pdev)
{
struct resource *res;
struct exynos_dp_device *dp;
- struct exynos_dp_platdata *pdata;
int ret = 0;
@@ -1057,21 +1028,13 @@ static int exynos_dp_probe(struct platform_device *pdev)
dp->dev = &pdev->dev;
- if (pdev->dev.of_node) {
- pdata = exynos_dp_dt_parse_pdata(&pdev->dev);
- if (IS_ERR(pdata))
- return PTR_ERR(pdata);
+ dp->video_info = exynos_dp_dt_parse_pdata(&pdev->dev);
+ if (IS_ERR(dp->video_info))
+ return PTR_ERR(dp->video_info);
- ret = exynos_dp_dt_parse_phydata(dp);
- if (ret)
- return ret;
- } else {
- pdata = pdev->dev.platform_data;
- if (!pdata) {
- dev_err(&pdev->dev, "no platform data\n");
- return -EINVAL;
- }
- }
+ ret = exynos_dp_dt_parse_phydata(dp);
+ if (ret)
+ return ret;
dp->clock = devm_clk_get(&pdev->dev, "dp");
if (IS_ERR(dp->clock)) {
@@ -1095,15 +1058,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
- dp->video_info = pdata->video_info;
-
- if (pdev->dev.of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_init(dp);
- } else {
- if (pdata->phy_init)
- pdata->phy_init();
- }
+ exynos_dp_phy_init(dp);
exynos_dp_init_dp(dp);
@@ -1121,18 +1076,11 @@ static int exynos_dp_probe(struct platform_device *pdev)
static int exynos_dp_remove(struct platform_device *pdev)
{
- struct exynos_dp_platdata *pdata = pdev->dev.platform_data;
struct exynos_dp_device *dp = platform_get_drvdata(pdev);
flush_work(&dp->hotplug_work);
- if (pdev->dev.of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_exit(dp);
- } else {
- if (pdata->phy_exit)
- pdata->phy_exit();
- }
+ exynos_dp_phy_exit(dp);
clk_disable_unprepare(dp->clock);
@@ -1143,20 +1091,13 @@ static int exynos_dp_remove(struct platform_device *pdev)
#ifdef CONFIG_PM_SLEEP
static int exynos_dp_suspend(struct device *dev)
{
- struct exynos_dp_platdata *pdata = dev->platform_data;
struct exynos_dp_device *dp = dev_get_drvdata(dev);
disable_irq(dp->irq);
flush_work(&dp->hotplug_work);
- if (dev->of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_exit(dp);
- } else {
- if (pdata->phy_exit)
- pdata->phy_exit();
- }
+ exynos_dp_phy_exit(dp);
clk_disable_unprepare(dp->clock);
@@ -1165,16 +1106,9 @@ static int exynos_dp_suspend(struct device *dev)
static int exynos_dp_resume(struct device *dev)
{
- struct exynos_dp_platdata *pdata = dev->platform_data;
struct exynos_dp_device *dp = dev_get_drvdata(dev);
- if (dev->of_node) {
- if (dp->phy_addr)
- exynos_dp_phy_init(dp);
- } else {
- if (pdata->phy_init)
- pdata->phy_init();
- }
+ exynos_dp_phy_init(dp);
clk_prepare_enable(dp->clock);
@@ -1203,7 +1137,7 @@ static struct platform_driver exynos_dp_driver = {
.name = "exynos-dp",
.owner = THIS_MODULE,
.pm = &exynos_dp_pm_ops,
- .of_match_table = of_match_ptr(exynos_dp_match),
+ .of_match_table = exynos_dp_match,
},
};
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 6c567bbf..56cfec8 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -13,6 +13,99 @@
#ifndef _EXYNOS_DP_CORE_H
#define _EXYNOS_DP_CORE_H
+#define DP_TIMEOUT_LOOP_COUNT 100
+#define MAX_CR_LOOP 5
+#define MAX_EQ_LOOP 5
+
+enum link_rate_type {
+ LINK_RATE_1_62GBPS = 0x06,
+ LINK_RATE_2_70GBPS = 0x0a
+};
+
+enum link_lane_count_type {
+ LANE_COUNT1 = 1,
+ LANE_COUNT2 = 2,
+ LANE_COUNT4 = 4
+};
+
+enum link_training_state {
+ START,
+ CLOCK_RECOVERY,
+ EQUALIZER_TRAINING,
+ FINISHED,
+ FAILED
+};
+
+enum voltage_swing_level {
+ VOLTAGE_LEVEL_0,
+ VOLTAGE_LEVEL_1,
+ VOLTAGE_LEVEL_2,
+ VOLTAGE_LEVEL_3,
+};
+
+enum pre_emphasis_level {
+ PRE_EMPHASIS_LEVEL_0,
+ PRE_EMPHASIS_LEVEL_1,
+ PRE_EMPHASIS_LEVEL_2,
+ PRE_EMPHASIS_LEVEL_3,
+};
+
+enum pattern_set {
+ PRBS7,
+ D10_2,
+ TRAINING_PTN1,
+ TRAINING_PTN2,
+ DP_NONE
+};
+
+enum color_space {
+ COLOR_RGB,
+ COLOR_YCBCR422,
+ COLOR_YCBCR444
+};
+
+enum color_depth {
+ COLOR_6,
+ COLOR_8,
+ COLOR_10,
+ COLOR_12
+};
+
+enum color_coefficient {
+ COLOR_YCBCR601,
+ COLOR_YCBCR709
+};
+
+enum dynamic_range {
+ VESA,
+ CEA
+};
+
+enum pll_status {
+ PLL_UNLOCKED,
+ PLL_LOCKED
+};
+
+enum clock_recovery_m_value_type {
+ CALCULATED_M,
+ REGISTER_M
+};
+
+enum video_timing_recognition_type {
+ VIDEO_TIMING_FROM_CAPTURE,
+ VIDEO_TIMING_FROM_REGISTER
+};
+
+enum analog_power_block {
+ AUX_BLOCK,
+ CH0_BLOCK,
+ CH1_BLOCK,
+ CH2_BLOCK,
+ CH3_BLOCK,
+ ANALOG_TOTAL,
+ POWER_ALL
+};
+
enum dp_irq_type {
DP_IRQ_TYPE_HP_CABLE_IN,
DP_IRQ_TYPE_HP_CABLE_OUT,
@@ -20,6 +113,22 @@ enum dp_irq_type {
DP_IRQ_TYPE_UNKNOWN,
};
+struct video_info {
+ char *name;
+
+ bool h_sync_polarity;
+ bool v_sync_polarity;
+ bool interlaced;
+
+ enum color_space color_space;
+ enum dynamic_range dynamic_range;
+ enum color_coefficient ycbcr_coeff;
+ enum color_depth color_depth;
+
+ enum link_rate_type link_rate;
+ enum link_lane_count_type lane_count;
+};
+
struct link_train {
int eq_loop;
int cr_loop[4];
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 29d9d03..b70da50 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -14,8 +14,6 @@
#include <linux/io.h>
#include <linux/delay.h>
-#include <video/exynos_dp.h>
-
#include "exynos_dp_core.h"
#include "exynos_dp_reg.h"
diff --git a/include/video/exynos_dp.h b/include/video/exynos_dp.h
deleted file mode 100644
index bd8cabd..0000000
--- a/include/video/exynos_dp.h
+++ /dev/null
@@ -1,131 +0,0 @@
-/*
- * Samsung SoC DP device support
- *
- * Copyright (C) 2012 Samsung Electronics Co., Ltd.
- * Author: Jingoo Han <jg1.han@samsung.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#ifndef _EXYNOS_DP_H
-#define _EXYNOS_DP_H
-
-#define DP_TIMEOUT_LOOP_COUNT 100
-#define MAX_CR_LOOP 5
-#define MAX_EQ_LOOP 5
-
-enum link_rate_type {
- LINK_RATE_1_62GBPS = 0x06,
- LINK_RATE_2_70GBPS = 0x0a
-};
-
-enum link_lane_count_type {
- LANE_COUNT1 = 1,
- LANE_COUNT2 = 2,
- LANE_COUNT4 = 4
-};
-
-enum link_training_state {
- START,
- CLOCK_RECOVERY,
- EQUALIZER_TRAINING,
- FINISHED,
- FAILED
-};
-
-enum voltage_swing_level {
- VOLTAGE_LEVEL_0,
- VOLTAGE_LEVEL_1,
- VOLTAGE_LEVEL_2,
- VOLTAGE_LEVEL_3,
-};
-
-enum pre_emphasis_level {
- PRE_EMPHASIS_LEVEL_0,
- PRE_EMPHASIS_LEVEL_1,
- PRE_EMPHASIS_LEVEL_2,
- PRE_EMPHASIS_LEVEL_3,
-};
-
-enum pattern_set {
- PRBS7,
- D10_2,
- TRAINING_PTN1,
- TRAINING_PTN2,
- DP_NONE
-};
-
-enum color_space {
- COLOR_RGB,
- COLOR_YCBCR422,
- COLOR_YCBCR444
-};
-
-enum color_depth {
- COLOR_6,
- COLOR_8,
- COLOR_10,
- COLOR_12
-};
-
-enum color_coefficient {
- COLOR_YCBCR601,
- COLOR_YCBCR709
-};
-
-enum dynamic_range {
- VESA,
- CEA
-};
-
-enum pll_status {
- PLL_UNLOCKED,
- PLL_LOCKED
-};
-
-enum clock_recovery_m_value_type {
- CALCULATED_M,
- REGISTER_M
-};
-
-enum video_timing_recognition_type {
- VIDEO_TIMING_FROM_CAPTURE,
- VIDEO_TIMING_FROM_REGISTER
-};
-
-enum analog_power_block {
- AUX_BLOCK,
- CH0_BLOCK,
- CH1_BLOCK,
- CH2_BLOCK,
- CH3_BLOCK,
- ANALOG_TOTAL,
- POWER_ALL
-};
-
-struct video_info {
- char *name;
-
- bool h_sync_polarity;
- bool v_sync_polarity;
- bool interlaced;
-
- enum color_space color_space;
- enum dynamic_range dynamic_range;
- enum color_coefficient ycbcr_coeff;
- enum color_depth color_depth;
-
- enum link_rate_type link_rate;
- enum link_lane_count_type lane_count;
-};
-
-struct exynos_dp_platdata {
- struct video_info *video_info;
-
- void (*phy_init)(void);
- void (*phy_exit)(void);
-};
-
-#endif /* _EXYNOS_DP_H */
--
1.7.10.4
^ permalink raw reply related
* [PATCH 7/7] video: exynos_dp: Use the generic PHY driver
From: Kishon Vijay Abraham I @ 2013-10-16 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1381940896-9355-1-git-send-email-kishon@ti.com>
From: Jingoo Han <jg1.han@samsung.com>
Use the generic PHY API to control the DP PHY.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
Documentation/devicetree/bindings/video/exynos_dp.txt | 17 +++++++++--------
drivers/video/exynos/exynos_dp_core.c | 16 ++++++++++++----
drivers/video/exynos/exynos_dp_core.h | 1 +
3 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt
index 84f10c1..3289d76 100644
--- a/Documentation/devicetree/bindings/video/exynos_dp.txt
+++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
@@ -6,10 +6,10 @@ We use two nodes:
-dptx-phy node(defined inside dp-controller node)
For the DP-PHY initialization, we use the dptx-phy node.
-Required properties for dptx-phy:
- -reg:
+Required properties for dptx-phy: deprecated, use phys and phy-names
+ -reg: deprecated
Base address of DP PHY register.
- -samsung,enable-mask:
+ -samsung,enable-mask: deprecated
The bit-mask used to enable/disable DP PHY.
For the Panel initialization, we read data from dp-controller node.
@@ -27,6 +27,10 @@ Required properties for dp-controller:
from common clock binding: Shall be "dp".
-interrupt-parent:
phandle to Interrupt combiner node.
+ -phys:
+ from general PHY binding: the phandle for the PHY device.
+ -phy-names:
+ from general PHY binding: Should be "dp".
-samsung,color-space:
input video data format.
COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
@@ -68,11 +72,8 @@ SOC specific portion:
clocks = <&clock 342>;
clock-names = "dp";
- dptx-phy {
- reg = <0x10040720>;
- samsung,enable-mask = <1>;
- };
-
+ phys = <&dp_phy>;
+ phy-names = "dp";
};
Board Specific portion:
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 05fed7d..5e1a715 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -19,6 +19,7 @@
#include <linux/interrupt.h>
#include <linux/delay.h>
#include <linux/of.h>
+#include <linux/phy/phy.h>
#include "exynos_dp_core.h"
@@ -960,8 +961,11 @@ static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp)
dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
if (!dp_phy_node) {
- dev_err(dp->dev, "could not find dptx-phy node\n");
- return -ENODEV;
+ dp->phy = devm_phy_get(dp->dev, "dp");
+ if (IS_ERR(dp->phy))
+ return PTR_ERR(dp->phy);
+ else
+ return 0;
}
if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
@@ -992,7 +996,9 @@ err:
static void exynos_dp_phy_init(struct exynos_dp_device *dp)
{
- if (dp->phy_addr) {
+ if (dp->phy) {
+ phy_power_on(dp->phy);
+ } else if (dp->phy_addr) {
u32 reg;
reg = __raw_readl(dp->phy_addr);
@@ -1003,7 +1009,9 @@ static void exynos_dp_phy_init(struct exynos_dp_device *dp)
static void exynos_dp_phy_exit(struct exynos_dp_device *dp)
{
- if (dp->phy_addr) {
+ if (dp->phy) {
+ phy_power_off(dp->phy);
+ } else if (dp->phy_addr) {
u32 reg;
reg = __raw_readl(dp->phy_addr);
diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
index 56cfec8..607e36d 100644
--- a/drivers/video/exynos/exynos_dp_core.h
+++ b/drivers/video/exynos/exynos_dp_core.h
@@ -151,6 +151,7 @@ struct exynos_dp_device {
struct video_info *video_info;
struct link_train link_train;
struct work_struct hotplug_work;
+ struct phy *phy;
};
/* exynos_dp_reg.c */
--
1.7.10.4
^ permalink raw reply related
* [RFR 0/2] DRM display panel support
From: Thierry Reding @ 2013-10-16 18:25 UTC (permalink / raw)
To: Laurent Pinchart, Tomi Valkeinen, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
Hi everyone,
This mini series adds panel support for the DRM subsystem and a sample
implementation for three panels found on hardware that I have access to.
Stephen wanted be to get an OK from both device tree binding maintainers
and people involved with CDF before merging the related DTS patches into
the Tegra tree. This comes awfully late, for which I take all the blame,
because Stephen needs to include the patches in his arm-soc pull request
that he will send out on Thursday (US time).
In a nutshell the important bits that Stephen needs acknowledged are the
device tree bindings, because that needs to remain stable once merged. I
should mention that these patches have been acked by Dave Airlie for
inclusion in DRM, so besides the device tree bindings there's nothing
holding this up from getting merged.
From the CDF folks it'd be good to know if they consider these bindings
compatible with what they're working on. Note that these are simple
panels with no need for an explicit control channel or anything like
that. I'm fully aware that this doesn't cover all the use-cases that CDF
is designed to cover, but I don't think it needs to. It gives us an easy
and simple solution for all the trivial setups out there.
Stephen, have I summarized your concerns correctly in the above? If not
please correct me.
Thanks,
Thierry
Thierry Reding (2):
drm: Add panel support
drm/panel: Add simple panel support
.../devicetree/bindings/panel/auo,b101aw03.txt | 7 +
.../bindings/panel/chunghwa,claa101wb03.txt | 7 +
.../bindings/panel/panasonic,vvx10f004b00.txt | 7 +
.../devicetree/bindings/panel/simple-panel.txt | 21 ++
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 2 +
drivers/gpu/drm/drm_panel.c | 96 ++++++
drivers/gpu/drm/panel/Kconfig | 17 ++
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-simple.c | 335 +++++++++++++++++++++
include/drm/drm_panel.h | 78 +++++
11 files changed, 573 insertions(+)
create mode 100644 Documentation/devicetree/bindings/panel/auo,b101aw03.txt
create mode 100644 Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
create mode 100644 Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
create mode 100644 Documentation/devicetree/bindings/panel/simple-panel.txt
create mode 100644 drivers/gpu/drm/drm_panel.c
create mode 100644 drivers/gpu/drm/panel/Kconfig
create mode 100644 drivers/gpu/drm/panel/Makefile
create mode 100644 drivers/gpu/drm/panel/panel-simple.c
create mode 100644 include/drm/drm_panel.h
--
1.8.4
^ permalink raw reply
* [RFR 1/2] drm: Add panel support
From: Thierry Reding @ 2013-10-16 18:25 UTC (permalink / raw)
To: Laurent Pinchart, Tomi Valkeinen, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1381947912-11741-1-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Add a very simple framework to register and lookup panels. Panel drivers
can initialize a DRM panel and register it with the framework, allowing
them to be retrieved and used by display drivers. Currently only support
for DPMS and obtaining panel modes is provided. However it should be
sufficient to enable a large number of panels. The framework should also
be easily extensible to support more sophisticated kinds of panels such
as DSI.
The framework hasn't been tied into the DRM core, even though it should
be easily possible to do so if that's what we want. In the current
implementation, display drivers can simple make use of it to retrieve a
panel, obtain its modes and control its DPMS mode.
Note that this is currently only tested on systems that boot from a
device tree. No glue code has been written yet for systems that use
platform data, but it should be easy to add.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/drm_panel.c | 96 +++++++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/panel/Kconfig | 4 ++
include/drm/drm_panel.h | 78 +++++++++++++++++++++++++++++++++++
5 files changed, 181 insertions(+)
create mode 100644 drivers/gpu/drm/drm_panel.c
create mode 100644 drivers/gpu/drm/panel/Kconfig
create mode 100644 include/drm/drm_panel.h
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 290e2ac..a1d4f8e 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -238,3 +238,5 @@ source "drivers/gpu/drm/qxl/Kconfig"
source "drivers/gpu/drm/msm/Kconfig"
source "drivers/gpu/drm/tegra/Kconfig"
+
+source "drivers/gpu/drm/panel/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e39cd03..81363a9 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -18,6 +18,7 @@ drm-y := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
drm-$(CONFIG_COMPAT) += drm_ioc32.o
drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
drm-$(CONFIG_PCI) += ati_pcigart.o
+drm-$(CONFIG_DRM_PANEL) += drm_panel.o
drm-usb-y := drm_usb.o
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
new file mode 100644
index 0000000..ff6e459
--- /dev/null
+++ b/drivers/gpu/drm/drm_panel.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2013, NVIDIA Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/err.h>
+#include <linux/export.h>
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_panel.h>
+
+static DEFINE_MUTEX(panel_lock);
+static LIST_HEAD(panel_list);
+
+void drm_panel_init(struct drm_panel *panel)
+{
+ INIT_LIST_HEAD(&panel->list);
+}
+EXPORT_SYMBOL(drm_panel_init);
+
+int drm_panel_add(struct drm_panel *panel)
+{
+ mutex_lock(&panel_lock);
+ list_add_tail(&panel->list, &panel_list);
+ mutex_unlock(&panel_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_panel_add);
+
+void drm_panel_remove(struct drm_panel *panel)
+{
+ mutex_lock(&panel_lock);
+ list_del_init(&panel->list);
+ mutex_unlock(&panel_lock);
+}
+EXPORT_SYMBOL(drm_panel_remove);
+
+int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector)
+{
+ if (panel->connector)
+ return -EBUSY;
+
+ panel->connector = connector;
+ panel->drm = connector->dev;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_panel_attach);
+
+int drm_panel_detach(struct drm_panel *panel)
+{
+ panel->connector = NULL;
+ panel->drm = NULL;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_panel_detach);
+
+#ifdef CONFIG_OF
+struct drm_panel *of_drm_find_panel(struct device_node *np)
+{
+ struct drm_panel *panel;
+
+ mutex_lock(&panel_lock);
+
+ list_for_each_entry(panel, &panel_list, list) {
+ if (panel->dev->of_node = np) {
+ mutex_unlock(&panel_lock);
+ return panel;
+ }
+ }
+
+ mutex_unlock(&panel_lock);
+ return NULL;
+}
+EXPORT_SYMBOL(of_drm_find_panel);
+#endif
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
new file mode 100644
index 0000000..2ddd5bd
--- /dev/null
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -0,0 +1,4 @@
+menuconfig DRM_PANEL
+ bool "DRM panel support"
+ help
+ Panel registration and lookup framework.
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
new file mode 100644
index 0000000..4c8760b
--- /dev/null
+++ b/include/drm/drm_panel.h
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2013, NVIDIA Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __DRM_PANEL_H__
+#define __DRM_PANEL_H__
+
+#include <linux/list.h>
+
+struct drm_connector;
+struct drm_device;
+struct drm_panel;
+
+struct drm_panel_funcs {
+ void (*disable)(struct drm_panel *panel);
+ void (*enable)(struct drm_panel *panel);
+ int (*get_modes)(struct drm_panel *panel);
+};
+
+struct drm_panel {
+ struct drm_device *drm;
+ struct drm_connector *connector;
+ struct device *dev;
+
+ const struct drm_panel_funcs *funcs;
+
+ struct list_head list;
+};
+
+static inline void drm_panel_disable(struct drm_panel *panel)
+{
+ if (panel && panel->funcs && panel->funcs->disable)
+ panel->funcs->disable(panel);
+}
+
+static inline void drm_panel_enable(struct drm_panel *panel)
+{
+ if (panel && panel->funcs && panel->funcs->enable)
+ panel->funcs->enable(panel);
+}
+
+void drm_panel_init(struct drm_panel *panel);
+
+int drm_panel_add(struct drm_panel *panel);
+void drm_panel_remove(struct drm_panel *panel);
+
+int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector);
+int drm_panel_detach(struct drm_panel *panel);
+
+#ifdef CONFIG_OF
+struct drm_panel *of_drm_find_panel(struct device_node *np);
+#else
+static inline struct drm_panel *of_drm_find_panel(struct device_node *np)
+{
+ return NULL;
+}
+#endif
+
+#endif
--
1.8.4
^ permalink raw reply related
* [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-16 18:25 UTC (permalink / raw)
To: Laurent Pinchart, Tomi Valkeinen, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1381947912-11741-1-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Add a driver for simple panels. Such panels can have a regulator that
provides the supply voltage and a separate GPIO to enable the panel.
Optionally the panels can have a backlight associated with them so it
can be enabled or disabled according to the panel's power management
mode.
Support is added for three panels: An AU Optronics 10.1" WSVGA, a
Chunghwa Picture Tubes 10.1" WXGA and a Panasonic 10.1 WUXGA TFT LCD
panel.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
.../devicetree/bindings/panel/auo,b101aw03.txt | 7 +
.../bindings/panel/chunghwa,claa101wb03.txt | 7 +
.../bindings/panel/panasonic,vvx10f004b00.txt | 7 +
.../devicetree/bindings/panel/simple-panel.txt | 21 ++
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/panel/Kconfig | 13 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-simple.c | 335 +++++++++++++++++++++
8 files changed, 392 insertions(+)
create mode 100644 Documentation/devicetree/bindings/panel/auo,b101aw03.txt
create mode 100644 Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
create mode 100644 Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
create mode 100644 Documentation/devicetree/bindings/panel/simple-panel.txt
create mode 100644 drivers/gpu/drm/panel/Makefile
create mode 100644 drivers/gpu/drm/panel/panel-simple.c
diff --git a/Documentation/devicetree/bindings/panel/auo,b101aw03.txt b/Documentation/devicetree/bindings/panel/auo,b101aw03.txt
new file mode 100644
index 0000000..72e088a
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/auo,b101aw03.txt
@@ -0,0 +1,7 @@
+AU Optronics Corporation 10.1" WSVGA TFT LCD panel
+
+Required properties:
+- compatible: should be "auo,b101aw03"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt b/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
new file mode 100644
index 0000000..0ab2c05
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
@@ -0,0 +1,7 @@
+Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel
+
+Required properties:
+- compatible: should be "chunghwa,claa101wb03"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt b/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
new file mode 100644
index 0000000..d328b03
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
@@ -0,0 +1,7 @@
+Panasonic Corporation 10.1" WUXGA TFT LCD panel
+
+Required properties:
+- compatible: should be "panasonic,vvx10f004b00"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/Documentation/devicetree/bindings/panel/simple-panel.txt b/Documentation/devicetree/bindings/panel/simple-panel.txt
new file mode 100644
index 0000000..1341bbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/simple-panel.txt
@@ -0,0 +1,21 @@
+Simple display panel
+
+Required properties:
+- power-supply: regulator to provide the supply voltage
+
+Optional properties:
+- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
+- enable-gpios: GPIO pin to enable or disable the panel
+- backlight: phandle of the backlight device attached to the panel
+
+Example:
+
+ panel: panel {
+ compatible = "cptt,claa101wb01";
+ ddc-i2c-bus = <&panelddc>;
+
+ power-supply = <&vdd_pnl_reg>;
+ enable-gpios = <&gpio 90 0>;
+
+ backlight = <&backlight>;
+ };
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 81363a9..a9859e0 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_DRM_QXL) += qxl/
obj-$(CONFIG_DRM_MSM) += msm/
obj-$(CONFIG_DRM_TEGRA) += tegra/
obj-y += i2c/
+obj-y += panel/
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 2ddd5bd..843087b 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -2,3 +2,16 @@ menuconfig DRM_PANEL
bool "DRM panel support"
help
Panel registration and lookup framework.
+
+if DRM_PANEL
+
+config DRM_PANEL_SIMPLE
+ bool "support for simple panels"
+ depends on OF
+ help
+ DRM panel driver for dumb panels that need at most a regulator and
+ a GPIO to be powered up. Optionally a backlight can be attached so
+ that it can be automatically turned off when the panel goes into a
+ low power state.
+
+endif
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
new file mode 100644
index 0000000..af9dfa2
--- /dev/null
+++ b/drivers/gpu/drm/panel/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
new file mode 100644
index 0000000..def3e75
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -0,0 +1,335 @@
+/*
+ * Copyright (C) 2013, NVIDIA Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/backlight.h>
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_panel.h>
+
+struct panel_desc {
+ const struct drm_display_mode *modes;
+ unsigned int num_modes;
+
+ struct {
+ unsigned int width;
+ unsigned int height;
+ } size;
+};
+
+/* TODO: convert to gpiod_*() API once it's been merged */
+#define GPIO_ACTIVE_LOW (1 << 0)
+
+struct panel_simple {
+ struct drm_panel base;
+ bool enabled;
+
+ const struct panel_desc *desc;
+
+ struct backlight_device *backlight;
+ struct regulator *supply;
+
+ unsigned long enable_gpio_flags;
+ int enable_gpio;
+};
+
+static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
+{
+ return container_of(panel, struct panel_simple, base);
+}
+
+static void panel_simple_disable(struct drm_panel *panel)
+{
+ struct panel_simple *p = to_panel_simple(panel);
+
+ if (!p->enabled)
+ return;
+
+ if (p->backlight) {
+ p->backlight->props.power = FB_BLANK_POWERDOWN;
+ backlight_update_status(p->backlight);
+ }
+
+ if (gpio_is_valid(p->enable_gpio)) {
+ if (p->enable_gpio_flags & GPIO_ACTIVE_LOW)
+ gpio_set_value(p->enable_gpio, 1);
+ else
+ gpio_set_value(p->enable_gpio, 0);
+ }
+
+ regulator_disable(p->supply);
+ p->enabled = false;
+}
+
+static void panel_simple_enable(struct drm_panel *panel)
+{
+ struct panel_simple *p = to_panel_simple(panel);
+ int err;
+
+ if (p->enabled)
+ return;
+
+ err = regulator_enable(p->supply);
+ if (err < 0)
+ dev_err(panel->dev, "failed to enable supply: %d\n", err);
+
+ if (gpio_is_valid(p->enable_gpio)) {
+ if (p->enable_gpio_flags & GPIO_ACTIVE_LOW)
+ gpio_set_value(p->enable_gpio, 0);
+ else
+ gpio_set_value(p->enable_gpio, 1);
+ }
+
+ if (p->backlight) {
+ p->backlight->props.power = FB_BLANK_UNBLANK;
+ backlight_update_status(p->backlight);
+ }
+
+ p->enabled = true;
+}
+
+static int panel_simple_get_modes(struct drm_panel *panel)
+{
+ struct panel_simple *p = to_panel_simple(panel);
+ struct drm_display_mode *mode;
+ unsigned int i;
+
+ for (i = 0; i < p->desc->num_modes; i++) {
+ mode = drm_mode_duplicate(panel->drm, &p->desc->modes[i]);
+ if (!mode)
+ return -ENOMEM;
+
+ drm_mode_set_name(mode);
+
+ drm_mode_probed_add(panel->connector, mode);
+ }
+
+ return p->desc->num_modes;
+}
+
+static const struct drm_panel_funcs panel_simple_funcs = {
+ .disable = panel_simple_disable,
+ .enable = panel_simple_enable,
+ .get_modes = panel_simple_get_modes,
+};
+
+static const struct drm_display_mode auo_b101aw03_mode = {
+ .clock = 51450,
+ .hdisplay = 1024,
+ .hsync_start = 1024 + 156,
+ .hsync_end = 1024 + 156 + 8,
+ .htotal = 1024 + 156 + 8 + 156,
+ .vdisplay = 600,
+ .vsync_start = 600 + 16,
+ .vsync_end = 600 + 16 + 6,
+ .vtotal = 600 + 16 + 6 + 16,
+ .vrefresh = 60,
+};
+
+static const struct panel_desc auo_b101aw03 = {
+ .modes = &auo_b101aw03_mode,
+ .num_modes = 1,
+ .size = {
+ .width = 223,
+ .height = 125,
+ },
+};
+
+static const struct drm_display_mode chunghwa_claa101wb01_mode = {
+ .clock = 69300,
+ .hdisplay = 1366,
+ .hsync_start = 1366 + 48,
+ .hsync_end = 1366 + 48 + 32,
+ .htotal = 1366 + 48 + 32 + 20,
+ .vdisplay = 768,
+ .vsync_start = 768 + 16,
+ .vsync_end = 768 + 16 + 8,
+ .vtotal = 768 + 16 + 8 + 16,
+ .vrefresh = 60,
+};
+
+static const struct panel_desc chunghwa_claa101wb01 = {
+ .modes = &chunghwa_claa101wb01_mode,
+ .num_modes = 1,
+ .size = {
+ .width = 223,
+ .height = 125,
+ },
+};
+
+static const struct drm_display_mode panasonic_vvx10f004b00_mode = {
+ .clock = 154700,
+ .hdisplay = 1920,
+ .hsync_start = 1920 + 154,
+ .hsync_end = 1920 + 154 + 16,
+ .htotal = 1920 + 154 + 16 + 32,
+ .vdisplay = 1200,
+ .vsync_start = 1200 + 17,
+ .vsync_end = 1200 + 17 + 2,
+ .vtotal = 1200 + 17 + 2 + 16,
+ .vrefresh = 60,
+};
+
+static const struct panel_desc panasonic_vvx10f004b00 = {
+ .modes = &panasonic_vvx10f004b00_mode,
+ .num_modes = 1,
+ .size = {
+ .width = 217,
+ .height = 136,
+ },
+};
+
+static const struct of_device_id panel_simple_of_match[] = {
+ {
+ .compatible = "auo,b101aw03",
+ .data = &auo_b101aw03,
+ }, {
+ .compatible = "chunghwa,claa101wb01",
+ .data = &chunghwa_claa101wb01
+ }, {
+ .compatible = "panasonic,vvx10f004b00",
+ .data = &panasonic_vvx10f004b00
+ }, {
+ /* sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(of, panel_simple_of_match);
+
+static int panel_simple_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *id;
+ struct device_node *backlight;
+ struct panel_simple *panel;
+ enum of_gpio_flags flags;
+ int err;
+
+ panel = devm_kzalloc(&pdev->dev, sizeof(*panel), GFP_KERNEL);
+ if (!panel)
+ return -ENOMEM;
+
+ id = of_match_node(panel_simple_of_match, pdev->dev.of_node);
+ if (!id)
+ return -ENODEV;
+
+ panel->desc = id->data;
+ panel->enabled = false;
+
+ panel->supply = devm_regulator_get(&pdev->dev, "power");
+ if (IS_ERR(panel->supply))
+ return PTR_ERR(panel->supply);
+
+ panel->enable_gpio = of_get_named_gpio_flags(pdev->dev.of_node,
+ "enable-gpios", 0,
+ &flags);
+ if (gpio_is_valid(panel->enable_gpio)) {
+ unsigned int value;
+
+ if (flags & OF_GPIO_ACTIVE_LOW)
+ panel->enable_gpio_flags |= GPIO_ACTIVE_LOW;
+
+ err = gpio_request(panel->enable_gpio, "enable");
+ if (err < 0) {
+ dev_err(&pdev->dev, "failed to request GPIO#%u: %d\n",
+ panel->enable_gpio, err);
+ return err;
+ }
+
+ value = (panel->enable_gpio_flags & GPIO_ACTIVE_LOW) != 0;
+
+ err = gpio_direction_output(panel->enable_gpio, value);
+ if (err < 0) {
+ dev_err(&pdev->dev, "failed to setup GPIO%u: %d\n",
+ panel->enable_gpio, err);
+ goto free_gpio;
+ }
+ }
+
+ backlight = of_parse_phandle(pdev->dev.of_node, "backlight", 0);
+ if (backlight) {
+ panel->backlight = of_find_backlight_by_node(backlight);
+ if (!panel->backlight) {
+ err = -EPROBE_DEFER;
+ goto free_gpio;
+ }
+
+ of_node_put(backlight);
+ }
+
+ drm_panel_init(&panel->base);
+ panel->base.dev = &pdev->dev;
+ panel->base.funcs = &panel_simple_funcs;
+
+ err = drm_panel_add(&panel->base);
+ if (err < 0)
+ goto free_gpio;
+
+ platform_set_drvdata(pdev, panel);
+
+ return 0;
+
+free_gpio:
+ if (gpio_is_valid(panel->enable_gpio))
+ gpio_free(panel->enable_gpio);
+
+ return err;
+}
+
+static int panel_simple_remove(struct platform_device *pdev)
+{
+ struct panel_simple *panel = platform_get_drvdata(pdev);
+
+ drm_panel_detach(&panel->base);
+ drm_panel_remove(&panel->base);
+
+ panel_simple_disable(&panel->base);
+
+ if (panel->backlight)
+ put_device(&panel->backlight->dev);
+
+ if (gpio_is_valid(panel->enable_gpio))
+ gpio_free(panel->enable_gpio);
+
+ regulator_disable(panel->supply);
+
+ return 0;
+}
+
+static struct platform_driver panel_simple_driver = {
+ .driver = {
+ .name = "panel-simple",
+ .owner = THIS_MODULE,
+ .of_match_table = panel_simple_of_match,
+ },
+ .probe = panel_simple_probe,
+ .remove = panel_simple_remove,
+};
+module_platform_driver(panel_simple_driver);
+
+MODULE_DESCRIPTION("DRM Driver for Simple Panels");
+MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
+MODULE_LICENSE("GPL v2");
--
1.8.4
^ permalink raw reply related
* Re: [PATCH 0/7] video phy's adaptation to *generic phy framework*
From: Greg KH @ 2013-10-16 20:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1381940896-9355-1-git-send-email-kishon@ti.com>
On Wed, Oct 16, 2013 at 09:58:09PM +0530, Kishon Vijay Abraham I wrote:
> Hi Greg,
>
> This series includes video PHY adaptation to Generic PHY Framework.
> With the adaptation they were able to get rid of plat data callbacks.
>
> Since you've taken the Generic PHY Framework, I think this series should
> also go into your tree.
All now applied, thanks.
greg k-h
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Laurent Pinchart @ 2013-10-17 0:47 UTC (permalink / raw)
To: Thierry Reding
Cc: Laurent Pinchart, Tomi Valkeinen, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1381947912-11741-3-git-send-email-treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Hi Thierry,
Thank you for the patch.
On Wednesday 16 October 2013 20:25:12 Thierry Reding wrote:
> Add a driver for simple panels. Such panels can have a regulator that
> provides the supply voltage and a separate GPIO to enable the panel.
> Optionally the panels can have a backlight associated with them so it
> can be enabled or disabled according to the panel's power management
> mode.
>
> Support is added for three panels: An AU Optronics 10.1" WSVGA, a
> Chunghwa Picture Tubes 10.1" WXGA and a Panasonic 10.1 WUXGA TFT LCD
> panel.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> .../devicetree/bindings/panel/auo,b101aw03.txt | 7 +
> .../bindings/panel/chunghwa,claa101wb03.txt | 7 +
> .../bindings/panel/panasonic,vvx10f004b00.txt | 7 +
> .../devicetree/bindings/panel/simple-panel.txt | 21 ++
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/panel/Kconfig | 13 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-simple.c | 335 ++++++++++++++++++
> 8 files changed, 392 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> create mode 100644
> Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt create
> mode 100644
> Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt create
> mode 100644 Documentation/devicetree/bindings/panel/simple-panel.txt create
> mode 100644 drivers/gpu/drm/panel/Makefile
> create mode 100644 drivers/gpu/drm/panel/panel-simple.c
>
> diff --git a/Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> b/Documentation/devicetree/bindings/panel/auo,b101aw03.txt new file mode
> 100644
> index 0000000..72e088a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> @@ -0,0 +1,7 @@
> +AU Optronics Corporation 10.1" WSVGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "auo,b101aw03"
> +
> +This binding is compatible with the simple-panel binding, which is
> specified +in simple-panel.txt in this directory.
> diff --git
> a/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
> b/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt new file
> mode 100644
> index 0000000..0ab2c05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
> @@ -0,0 +1,7 @@
> +Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "chunghwa,claa101wb03"
> +
> +This binding is compatible with the simple-panel binding, which is
> specified +in simple-panel.txt in this directory.
> diff --git
> a/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
> b/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt new
> file mode 100644
> index 0000000..d328b03
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
> @@ -0,0 +1,7 @@
> +Panasonic Corporation 10.1" WUXGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "panasonic,vvx10f004b00"
> +
> +This binding is compatible with the simple-panel binding, which is
> specified +in simple-panel.txt in this directory.
> diff --git a/Documentation/devicetree/bindings/panel/simple-panel.txt
> b/Documentation/devicetree/bindings/panel/simple-panel.txt new file mode
> 100644
> index 0000000..1341bbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/simple-panel.txt
> @@ -0,0 +1,21 @@
> +Simple display panel
> +
> +Required properties:
> +- power-supply: regulator to provide the supply voltage
> +
> +Optional properties:
> +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> +- enable-gpios: GPIO pin to enable or disable the panel
> +- backlight: phandle of the backlight device attached to the panel
> +
> +Example:
> +
> + panel: panel {
> + compatible = "cptt,claa101wb01";
> + ddc-i2c-bus = <&panelddc>;
> +
> + power-supply = <&vdd_pnl_reg>;
> + enable-gpios = <&gpio 90 0>;
> +
> + backlight = <&backlight>;
> + };
My biggest concern here is that this will not be compatible with the CDF DT
bindings. They're not complete yet, but they will require connections between
entities to be described in DT, in a way very similar (or actually identical)
to the V4L2 DT bindings, documented in
Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a
look at that ? Please ignore all optional properties beside remote-endpoint,
as they're V4L2 specific.
I also plan to specify video bus parameters in DT for CDF, but this hasn't
been finalized yet.
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 81363a9..a9859e0 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -58,3 +58,4 @@ obj-$(CONFIG_DRM_QXL) += qxl/
> obj-$(CONFIG_DRM_MSM) += msm/
> obj-$(CONFIG_DRM_TEGRA) += tegra/
> obj-y += i2c/
> +obj-y += panel/
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 2ddd5bd..843087b 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -2,3 +2,16 @@ menuconfig DRM_PANEL
> bool "DRM panel support"
> help
> Panel registration and lookup framework.
> +
> +if DRM_PANEL
> +
> +config DRM_PANEL_SIMPLE
> + bool "support for simple panels"
> + depends on OF
> + help
> + DRM panel driver for dumb panels that need at most a regulator and
> + a GPIO to be powered up. Optionally a backlight can be attached so
> + that it can be automatically turned off when the panel goes into a
> + low power state.
> +
> +endif
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> new file mode 100644
> index 0000000..af9dfa2
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c new file mode 100644
> index 0000000..def3e75
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -0,0 +1,335 @@
[snip]
> +/*
> + * Copyright (C) 2013, NVIDIA Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sub
> license,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
This contradicts MODULE_LICENSE("GPL v2") below.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/gpio.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_panel.h>
> +
> +struct panel_desc {
> + const struct drm_display_mode *modes;
> + unsigned int num_modes;
> +
> + struct {
> + unsigned int width;
> + unsigned int height;
> + } size;
> +};
> +
> +/* TODO: convert to gpiod_*() API once it's been merged */
> +#define GPIO_ACTIVE_LOW (1 << 0)
Why can't you just #include <dt-bindings/gpio/gpio.h> ?
> +
> +struct panel_simple {
> + struct drm_panel base;
> + bool enabled;
> +
> + const struct panel_desc *desc;
> +
> + struct backlight_device *backlight;
> + struct regulator *supply;
> +
> + unsigned long enable_gpio_flags;
> + int enable_gpio;
> +};
> +
> +static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
> +{
> + return container_of(panel, struct panel_simple, base);
> +}
> +
> +static void panel_simple_disable(struct drm_panel *panel)
> +{
> + struct panel_simple *p = to_panel_simple(panel);
> +
> + if (!p->enabled)
> + return;
> +
> + if (p->backlight) {
> + p->backlight->props.power = FB_BLANK_POWERDOWN;
At some point we should decouple the backlight API from the fbdev API. That's
somewhere on my to-do list after completing CDF (I'd be happy to trade CDF for
that task ;-)).
> + backlight_update_status(p->backlight);
> + }
> +
> + if (gpio_is_valid(p->enable_gpio)) {
> + if (p->enable_gpio_flags & GPIO_ACTIVE_LOW)
> + gpio_set_value(p->enable_gpio, 1);
> + else
> + gpio_set_value(p->enable_gpio, 0);
> + }
> +
> + regulator_disable(p->supply);
> + p->enabled = false;
> +}
> +
> +static void panel_simple_enable(struct drm_panel *panel)
> +{
> + struct panel_simple *p = to_panel_simple(panel);
> + int err;
> +
> + if (p->enabled)
> + return;
> +
> + err = regulator_enable(p->supply);
> + if (err < 0)
> + dev_err(panel->dev, "failed to enable supply: %d\n", err);
Is that really a non-fatal error ? Shouldn't the enable operation return an
int ?
> +
> + if (gpio_is_valid(p->enable_gpio)) {
> + if (p->enable_gpio_flags & GPIO_ACTIVE_LOW)
> + gpio_set_value(p->enable_gpio, 0);
> + else
> + gpio_set_value(p->enable_gpio, 1);
> + }
> +
> + if (p->backlight) {
> + p->backlight->props.power = FB_BLANK_UNBLANK;
> + backlight_update_status(p->backlight);
> + }
> +
> + p->enabled = true;
> +}
> +
> +static int panel_simple_get_modes(struct drm_panel *panel)
> +{
> + struct panel_simple *p = to_panel_simple(panel);
> + struct drm_display_mode *mode;
> + unsigned int i;
> +
> + for (i = 0; i < p->desc->num_modes; i++) {
> + mode = drm_mode_duplicate(panel->drm, &p->desc->modes[i]);
> + if (!mode)
> + return -ENOMEM;
> +
> + drm_mode_set_name(mode);
> +
> + drm_mode_probed_add(panel->connector, mode);
> + }
> +
> + return p->desc->num_modes;
> +}
> +
> +static const struct drm_panel_funcs panel_simple_funcs = {
> + .disable = panel_simple_disable,
> + .enable = panel_simple_enable,
> + .get_modes = panel_simple_get_modes,
> +};
> +
> +static const struct drm_display_mode auo_b101aw03_mode = {
> + .clock = 51450,
> + .hdisplay = 1024,
> + .hsync_start = 1024 + 156,
> + .hsync_end = 1024 + 156 + 8,
> + .htotal = 1024 + 156 + 8 + 156,
> + .vdisplay = 600,
> + .vsync_start = 600 + 16,
> + .vsync_end = 600 + 16 + 6,
> + .vtotal = 600 + 16 + 6 + 16,
> + .vrefresh = 60,
> +};
Instead of hardcoding the modes in the driver, which would then require to be
updated for every new simple panel model (and we know there are lots of them),
why don't you specify the modes in the panel DT node ? The simple panel driver
would then become much more generic. It would also allow board designers to
tweak h/v sync timings depending on the system requirements.
> +
> +static const struct panel_desc auo_b101aw03 = {
> + .modes = &auo_b101aw03_mode,
> + .num_modes = 1,
> + .size = {
> + .width = 223,
> + .height = 125,
> + },
> +};
> +
> +static const struct drm_display_mode chunghwa_claa101wb01_mode = {
> + .clock = 69300,
> + .hdisplay = 1366,
> + .hsync_start = 1366 + 48,
> + .hsync_end = 1366 + 48 + 32,
> + .htotal = 1366 + 48 + 32 + 20,
> + .vdisplay = 768,
> + .vsync_start = 768 + 16,
> + .vsync_end = 768 + 16 + 8,
> + .vtotal = 768 + 16 + 8 + 16,
> + .vrefresh = 60,
> +};
> +
> +static const struct panel_desc chunghwa_claa101wb01 = {
> + .modes = &chunghwa_claa101wb01_mode,
> + .num_modes = 1,
> + .size = {
> + .width = 223,
> + .height = 125,
> + },
> +};
> +
> +static const struct drm_display_mode panasonic_vvx10f004b00_mode = {
> + .clock = 154700,
> + .hdisplay = 1920,
> + .hsync_start = 1920 + 154,
> + .hsync_end = 1920 + 154 + 16,
> + .htotal = 1920 + 154 + 16 + 32,
> + .vdisplay = 1200,
> + .vsync_start = 1200 + 17,
> + .vsync_end = 1200 + 17 + 2,
> + .vtotal = 1200 + 17 + 2 + 16,
> + .vrefresh = 60,
> +};
> +
> +static const struct panel_desc panasonic_vvx10f004b00 = {
> + .modes = &panasonic_vvx10f004b00_mode,
> + .num_modes = 1,
> + .size = {
> + .width = 217,
> + .height = 136,
> + },
> +};
> +
> +static const struct of_device_id panel_simple_of_match[] = {
> + {
> + .compatible = "auo,b101aw03",
> + .data = &auo_b101aw03,
> + }, {
> + .compatible = "chunghwa,claa101wb01",
> + .data = &chunghwa_claa101wb01
> + }, {
> + .compatible = "panasonic,vvx10f004b00",
> + .data = &panasonic_vvx10f004b00
> + }, {
> + /* sentinel */
> + }
> +};
> +MODULE_DEVICE_TABLE(of, panel_simple_of_match);
> +
> +static int panel_simple_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *id;
> + struct device_node *backlight;
> + struct panel_simple *panel;
> + enum of_gpio_flags flags;
> + int err;
> +
> + panel = devm_kzalloc(&pdev->dev, sizeof(*panel), GFP_KERNEL);
> + if (!panel)
> + return -ENOMEM;
> +
> + id = of_match_node(panel_simple_of_match, pdev->dev.of_node);
> + if (!id)
> + return -ENODEV;
> +
> + panel->desc = id->data;
> + panel->enabled = false;
> +
> + panel->supply = devm_regulator_get(&pdev->dev, "power");
> + if (IS_ERR(panel->supply))
> + return PTR_ERR(panel->supply);
> +
> + panel->enable_gpio = of_get_named_gpio_flags(pdev->dev.of_node,
> + "enable-gpios", 0,
> + &flags);
> + if (gpio_is_valid(panel->enable_gpio)) {
> + unsigned int value;
> +
> + if (flags & OF_GPIO_ACTIVE_LOW)
> + panel->enable_gpio_flags |= GPIO_ACTIVE_LOW;
> +
> + err = gpio_request(panel->enable_gpio, "enable");
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to request GPIO#%u: %d\n",
> + panel->enable_gpio, err);
> + return err;
> + }
> +
> + value = (panel->enable_gpio_flags & GPIO_ACTIVE_LOW) != 0;
> +
> + err = gpio_direction_output(panel->enable_gpio, value);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to setup GPIO%u: %d\n",
> + panel->enable_gpio, err);
> + goto free_gpio;
> + }
> + }
> +
> + backlight = of_parse_phandle(pdev->dev.of_node, "backlight", 0);
> + if (backlight) {
> + panel->backlight = of_find_backlight_by_node(backlight);
> + if (!panel->backlight) {
> + err = -EPROBE_DEFER;
> + goto free_gpio;
> + }
> +
> + of_node_put(backlight);
> + }
> +
> + drm_panel_init(&panel->base);
> + panel->base.dev = &pdev->dev;
> + panel->base.funcs = &panel_simple_funcs;
> +
> + err = drm_panel_add(&panel->base);
> + if (err < 0)
> + goto free_gpio;
> +
> + platform_set_drvdata(pdev, panel);
> +
> + return 0;
> +
> +free_gpio:
> + if (gpio_is_valid(panel->enable_gpio))
> + gpio_free(panel->enable_gpio);
> +
> + return err;
> +}
> +
> +static int panel_simple_remove(struct platform_device *pdev)
> +{
> + struct panel_simple *panel = platform_get_drvdata(pdev);
> +
> + drm_panel_detach(&panel->base);
> + drm_panel_remove(&panel->base);
> +
> + panel_simple_disable(&panel->base);
> +
> + if (panel->backlight)
> + put_device(&panel->backlight->dev);
> +
> + if (gpio_is_valid(panel->enable_gpio))
> + gpio_free(panel->enable_gpio);
> +
> + regulator_disable(panel->supply);
> +
> + return 0;
> +}
> +
> +static struct platform_driver panel_simple_driver = {
> + .driver = {
> + .name = "panel-simple",
> + .owner = THIS_MODULE,
> + .of_match_table = panel_simple_of_match,
> + },
> + .probe = panel_simple_probe,
> + .remove = panel_simple_remove,
> +};
> +module_platform_driver(panel_simple_driver);
> +
> +MODULE_DESCRIPTION("DRM Driver for Simple Panels");
> +MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
> +MODULE_LICENSE("GPL v2");
--
Regards,
Laurent Pinchart
^ permalink raw reply
* How to set fops in "struct platform_pwm_backlight_data"?
From: Mark Zhang @ 2013-10-17 6:49 UTC (permalink / raw)
To: thierry.reding, rpurdie, jg1.han,
Jean-Christophe PLAGNIOL-VILLARD, tomi.valkeinen
Cc: linux-pwm, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi,
This is the first time I send mail to linux-pwm, I didn't read through
the mails in this list, so if somebody already asked this question, I'm
sorry about that.
I wanna set some fops in "struct platform_pwm_backlight_data". But the
currrent probe function in pwm_bl.c says:
-------
if (!data) {
ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
if (ret < 0) {
dev_err(&pdev->dev, "failed to find platform data\n");
return ret;
}
data = &defdata;
}
-------
This looks like if we set the platform data for pwm backlight device,
"pwm_backlight_parse_dt" will never have a chance to be called, which
means the stuffs I defined in backlight DT node will be ignored.
If I don't set the platform data for pwm backlight device, according to
the pwm_backlight_probe, I will never have a chance to set some fops
which I need(like "notify", "check_fb"...).
So, what I suppose to do now? Maybe there is a way to set function
pointers in DT?
Mark
^ permalink raw reply
* Re: How to set fops in "struct platform_pwm_backlight_data"?
From: Thierry Reding @ 2013-10-17 7:14 UTC (permalink / raw)
To: Mark Zhang
Cc: rpurdie, jg1.han, Jean-Christophe PLAGNIOL-VILLARD,
tomi.valkeinen, linux-pwm, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <525F8895.5010806@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]
On Thu, Oct 17, 2013 at 02:49:57PM +0800, Mark Zhang wrote:
> Hi,
>
> This is the first time I send mail to linux-pwm, I didn't read through
> the mails in this list, so if somebody already asked this question, I'm
> sorry about that.
>
> I wanna set some fops in "struct platform_pwm_backlight_data". But the
> currrent probe function in pwm_bl.c says:
>
> -------
> if (!data) {
> ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
> if (ret < 0) {
> dev_err(&pdev->dev, "failed to find platform data\n");
> return ret;
> }
>
> data = &defdata;
> }
> -------
>
> This looks like if we set the platform data for pwm backlight device,
> "pwm_backlight_parse_dt" will never have a chance to be called, which
> means the stuffs I defined in backlight DT node will be ignored.
>
> If I don't set the platform data for pwm backlight device, according to
> the pwm_backlight_probe, I will never have a chance to set some fops
> which I need(like "notify", "check_fb"...).
>
> So, what I suppose to do now? Maybe there is a way to set function
> pointers in DT?
Perhaps you could describe in more detail what you need the functions
for.
Generally you're not supposed to mix DT and platform data. Without more
info that's about all I can say.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Andrzej Hajda @ 2013-10-17 7:48 UTC (permalink / raw)
To: Tomi Valkeinen, Laurent Pinchart
Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <52580EFF.2020401@ti.com>
Hi Tomi,
Sorry for delayed response.
On 10/11/2013 04:45 PM, Tomi Valkeinen wrote:
> On 11/10/13 17:16, Andrzej Hajda wrote:
>
>> Picture size, content and format is the same on input and on output of DSI.
>> The same bits which enters DSI appears on the output. Internally bits
>> order can
>> be different but practically you are configuring DSI master and slave
>> with the same format.
>>
>> If you create DSI entity you will have to always set the same format and
>> size on DSI input, DSI output and encoder input.
>> If you skip creating DSI entity you loose nothing, and you do not need
>> to take care of it.
> Well, this is really a different question from the bus problem. But
> nothing says the DSI master cannot change the format or even size. For
> sure it can change the video timings. The DSI master could even take two
> parallel inputs, and combine them into one DSI output. You don't can't
> what all the possible pieces of hardware do =)
> If you have a bigger IP block that internally contains the DISPC and the
> DSI, then, yes, you can combine them into one display entity. I don't
> think that's correct, though. And if the DISPC and DSI are independent
> blocks, then especially I think there must be an entity for the DSI
> block, which will enable the powers, clocks, etc, when needed.
The main function of DSI is to transport pixels from one IP to another IP
and this function IMO should not be modeled by display entity.
"Power, clocks, etc" will be performed via control bus according to
panel demands.
If 'DSI chip' has additional functions for video processing they can
be modeled by CDF entity if it makes sense.
>>> Well, one point of the endpoints is also to allow "switching" of video
>>> devices.
>>>
>>> For example, I could have a board with a SoC's DSI output, connected to
>>> two DSI panels. There would be some kind of mux between, so that I can
>>> select which of the panels is actually connected to the SoC.
>>>
>>> Here the first panel could use 2 datalanes, the second one 4. Thus, the
>>> DSI master would have two endpoints, the other one using 2 and the other
>>> 4 datalanes.
>>>
>>> If we decide that kind of support is not needed, well, is there even
>>> need for the V4L2 endpoints in the DT data at all?
>> Hmm, both panels connected to one endpoint of dispc ?
>> The problem I see is which driver should handle panel switching,
>> but this is question about hardware design as well. If this is realized
>> by dispc I have told already the solution. If this is realized by other
>> device I do not see a problem to create corresponding CDF entity,
>> or maybe it can be handled by "Pipeline Controller" ???
> Well the switching could be automatic, when the panel power is enabled,
> the DSI mux is switched for that panel. It's not relevant.
>
> We still have two different endpoint configurations for the same
> DSI-master port. If that configuration is in the DSI-master's port node,
> not inside an endpoint data, then that can't be supported.
I am not sure if I understand it correctly. But it seems quite simple:
when panel starts/resumes it request DSI (via control bus) to fulfill
its configuration settings.
Of course there are some settings which are not panel dependent and those
should reside in DSI node.
>>>>> I agree that having DSI/DBI control and video separated would be
>>>>> elegant. But I'd like to hear what is the technical benefit of that? At
>>>>> least to me it's clearly more complex to separate them than to keep them
>>>>> together (to the extent that I don't yet see how it is even possible),
>>>>> so there must be a good reason for the separation. I don't understand
>>>>> that reason. What is it?
>>>> Roughly speaking it is a question where is the more convenient place to
>>>> put bunch
>>>> of opses, technically both solutions can be somehow implemented.
>>> Well, it's also about dividing a single physical bus into two separate
>>> interfaces to it. It sounds to me that it would be much more complex
>>> with locking. With a single API, we can just say "the caller handles
>>> locking". With two separate interfaces, there must be locking at the
>>> lower level.
>> We say then: callee handles locking :)
> Sure, but my point was that the caller handling the locking is much
> simpler than the callee handling locking. And the latter causes
> atomicity issues, as the other API could be invoked in between two calls
> for the first API.
>
>
Could you describe such scenario?
> But note that I'm not saying we should not implement bus model just
> because it's more complex. We should go for bus model if it's better. I
> just want to bring up these complexities, which I feel are quite more
> difficult than with the simpler model.
>
>>>> Pros of mipi bus:
>>>> - no fake entity in CDF, with fake opses, I have to use similar entities
>>>> in MIPI-CSI
>>>> camera pipelines and it complicates life without any benefit(at least
>>>> from user side),
>>> You mean the DSI-master? I don't see how it's "fake", it's a video
>>> processing unit that has to be configured. Even if we forget the control
>>> side, and just think about plain video stream with DSI video mode,
>>> there's are things to configure with it.
>>>
>>> What kind of issues you have in the CSI side, then?
>> Not real issues, just needless calls to configure CSI entity pads,
>> with the same format and picture sizes as in camera.
> Well, the output of a component A is surely the same as the input of
> component B, if B receives the data from A. So that does sound useless.
> I don't do that kind of calls in my model.
>
>>>> - CDF models only video buses, control bus is a domain of Linux buses,
>>> Yes, but in this case the buses are the same. It makes me a bit nervous
>>> to have two separate ways (video and control) to use the same bus, in a
>>> case like video where timing is critical.
>>>
>>> So yes, we can consider video and control buses as "virtual" buses, and
>>> the actual transport is the DSI bus. Maybe it can be done. It just makes
>>> me a bit nervous =).
>>>
>>>> - less platform_bus abusing,
>>> Well, platform.txt says
>>>
>>> "This pseudo-bus
>>> is used to connect devices on busses with minimal infrastructure,
>>> like those used to integrate peripherals on many system-on-chip
>>> processors, or some "legacy" PC interconnects; as opposed to large
>>> formally specified ones like PCI or USB."
>>>
>>> I don't think DSI and DBI as platform bus is that far from the
>>> description. They are "simple", no probing point-to-point (in practice)
>>> buses. There's not much "bus" to speak of, just a point-to-point link.
>> Next section:
>>
>> Platform devices
>> ~~~~~~~~~~~~~~~~
>> Platform devices are devices that typically appear as autonomous
>> entities in the system. This includes legacy port-based devices and
>> host bridges to peripheral buses, and most controllers integrated
>> into system-on-chip platforms. What they usually have in common
>> is direct addressing from a CPU bus. Rarely, a platform_device will
>> be connected through a segment of some other kind of bus; but its
>> registers will still be directly addressable.
> Yep, "typically" and "rarely" =). I agree, it's not clear. I think there
> are things with DBI/DSI that clearly point to a platform device, but
> also the other way.
Just to be sure, we are talking here about DSI-slaves, ie. for example
about panels,
where direct accessing from CPU bus usually is not possible.
Andrzej
>>>> - better device tree topology (at least for common cases),
>>> Even if we use platform devices for DSI peripherals, we can have them
>>> described under the DSI master node.
>> Sorry, I meant rather Linux device tree topology, not DT.
> We can have the DSI peripheral platform devices as children of the
> DSI-master device.
>
> Tomi
>
>
^ permalink raw reply
* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Tomi Valkeinen @ 2013-10-17 8:18 UTC (permalink / raw)
To: Andrzej Hajda, Laurent Pinchart
Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <525F9660.3010408@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 4411 bytes --]
On 17/10/13 10:48, Andrzej Hajda wrote:
> The main function of DSI is to transport pixels from one IP to another IP
> and this function IMO should not be modeled by display entity.
> "Power, clocks, etc" will be performed via control bus according to
> panel demands.
> If 'DSI chip' has additional functions for video processing they can
> be modeled by CDF entity if it makes sense.
Now I don't follow. What do you mean with "display entity" and with "CDF
entity"? Are they the same?
Let me try to clarify my point:
On OMAP SoC we have a DSI encoder, which takes input from the display
controller in parallel RGB format, and outputs DSI.
Then there are external encoders that take MIPI DPI as input, and output
DSI.
The only difference with the above two components is that the first one
is embedded into the SoC. I see no reason to represent them in different
ways (i.e. as you suggested, not representing the SoC's DSI at all).
Also, if you use DSI burst mode, you will have to have different video
timings in the DSI encoder's input and output. And depending on the
buffering of the DSI encoder, you could have different timings in any case.
Furthermore, both components could have extra processing. I know the
external encoders sometimes do have features like scaling.
>> We still have two different endpoint configurations for the same
>> DSI-master port. If that configuration is in the DSI-master's port node,
>> not inside an endpoint data, then that can't be supported.
> I am not sure if I understand it correctly. But it seems quite simple:
> when panel starts/resumes it request DSI (via control bus) to fulfill
> its configuration settings.
> Of course there are some settings which are not panel dependent and those
> should reside in DSI node.
Exactly. And when the two panels require different non-panel-dependent
settings, how do you represent them in the DT data?
>>> We say then: callee handles locking :)
>> Sure, but my point was that the caller handling the locking is much
>> simpler than the callee handling locking. And the latter causes
>> atomicity issues, as the other API could be invoked in between two calls
>> for the first API.
>>
>>
> Could you describe such scenario?
If we have two independent APIs, ctrl and video, that affect the same
underlying hardware, the DSI bus, we could have a scenario like this:
thread 1:
ctrl->op_foo();
ctrl->op_bar();
thread 2:
video->op_baz();
Even if all those ops do locking properly internally, the fact that
op_baz() can be called in between op_foo() and op_bar() may cause problems.
To avoid that issue with two APIs we'd need something like:
thread 1:
ctrl->lock();
ctrl->op_foo();
ctrl->op_bar();
ctrl->unlock();
thread 2:
video->lock();
video->op_baz();
video->unlock();
>>> Platform devices
>>> ~~~~~~~~~~~~~~~~
>>> Platform devices are devices that typically appear as autonomous
>>> entities in the system. This includes legacy port-based devices and
>>> host bridges to peripheral buses, and most controllers integrated
>>> into system-on-chip platforms. What they usually have in common
>>> is direct addressing from a CPU bus. Rarely, a platform_device will
>>> be connected through a segment of some other kind of bus; but its
>>> registers will still be directly addressable.
>> Yep, "typically" and "rarely" =). I agree, it's not clear. I think there
>> are things with DBI/DSI that clearly point to a platform device, but
>> also the other way.
> Just to be sure, we are talking here about DSI-slaves, ie. for example
> about panels,
> where direct accessing from CPU bus usually is not possible.
Yes. My point is that with DBI/DSI there's not much bus there (if a
normal bus would be PCI/USB/i2c etc), it's just a point to point link
without probing or a clearly specified setup sequence.
If DSI/DBI was used only for control, a linux bus would probably make
sense. But DSI/DBI is mainly a video transport channel, with the
control-part being "secondary".
And when considering that the video and control data are sent over the
same channel (i.e. there's no separate, independent ctrl channel), and
the strict timing restrictions with video, my gut feeling is just that
all the extra complexity brought with separating the control to a
separate bus is not worth it.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Dave Airlie @ 2013-10-17 8:28 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Thierry Reding, Laurent Pinchart, Tomi Valkeinen, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <1695169.rzbDl9PeRX@avalon>
>
> My biggest concern here is that this will not be compatible with the CDF DT
> bindings. They're not complete yet, but they will require connections between
> entities to be described in DT, in a way very similar (or actually identical)
> to the V4L2 DT bindings, documented in
> Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a
> look at that ? Please ignore all optional properties beside remote-endpoint,
> as they're V4L2 specific.
>
> I also plan to specify video bus parameters in DT for CDF, but this hasn't
> been finalized yet.
>
While I understand this, I don't see why CDF can't enhance these
bindings if it has
requirements > than they have without disturbing the panel ones,
is DT really that inflexible?
It seems that have a simple description for basic panels like Thierry wants
to support, that can be enhanced for the other cases in the future should
suffice, I really don't like blocking stuff that makes things work on the chance
of something that isn't upstream yet, its sets a bad precedent, its also breaks
the perfect is the enemy of good rule
Dave.
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17 8:49 UTC (permalink / raw)
To: Dave Airlie, Laurent Pinchart
Cc: Thierry Reding, Laurent Pinchart, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <CAPM=9tzU36cwcM0pH0J_Vgc6UOj5MHdVNDvn3wpGNuihbMqdQQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2282 bytes --]
Hi,
On 17/10/13 11:28, Dave Airlie wrote:
>>
>> My biggest concern here is that this will not be compatible with the CDF DT
>> bindings. They're not complete yet, but they will require connections between
>> entities to be described in DT, in a way very similar (or actually identical)
>> to the V4L2 DT bindings, documented in
>> Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a
>> look at that ? Please ignore all optional properties beside remote-endpoint,
>> as they're V4L2 specific.
>>
>> I also plan to specify video bus parameters in DT for CDF, but this hasn't
>> been finalized yet.
>>
>
> While I understand this, I don't see why CDF can't enhance these
> bindings if it has
> requirements > than they have without disturbing the panel ones,
>
> is DT really that inflexible?
>
> It seems that have a simple description for basic panels like Thierry wants
> to support, that can be enhanced for the other cases in the future should
> suffice, I really don't like blocking stuff that makes things work on the chance
> of something that isn't upstream yet, its sets a bad precedent, its also breaks
> the perfect is the enemy of good rule
Just my opinion, but yes, DT is that inflexible. DT bindings are like a
syscall. Once they are there, you need to support them forever. That
support can, of course, include some kind of DT data converters or such,
that try to convert old bindings to new bindings at kernel boot.
In practice even such converter may be enough, if some important piece
of data in the new bindings is missing, and this data was implicit in a
driver. In that case the conversion, or parts of it, must be done later,
in that specific driver.
Even that may be difficult, if the piece of data should actually be
handled by some other driver. In that case there's probably need for
some kind of global look-up table or such, so that the drivers can work
around the problem of missing information.
I've been working with DT bindings for OMAP display subsystem for
something like 1.5 years. Still not merged, as they are not perfect, and
I'm scared to push them in non-perfect form, as that could result in
some kind of DT-binding-converter-hell described above.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17 8:53 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Laurent Pinchart, Tomi Valkeinen, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <1695169.rzbDl9PeRX@avalon>
[-- Attachment #1: Type: text/plain, Size: 10918 bytes --]
Adding the dri-devel mailing list on Cc, since this discussion is
outgrowing its original intent.
On Thu, Oct 17, 2013 at 02:47:52AM +0200, Laurent Pinchart wrote:
> Hi Thierry,
>
> Thank you for the patch.
>
> On Wednesday 16 October 2013 20:25:12 Thierry Reding wrote:
> > Add a driver for simple panels. Such panels can have a regulator that
> > provides the supply voltage and a separate GPIO to enable the panel.
> > Optionally the panels can have a backlight associated with them so it
> > can be enabled or disabled according to the panel's power management
> > mode.
> >
> > Support is added for three panels: An AU Optronics 10.1" WSVGA, a
> > Chunghwa Picture Tubes 10.1" WXGA and a Panasonic 10.1 WUXGA TFT LCD
> > panel.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > .../devicetree/bindings/panel/auo,b101aw03.txt | 7 +
> > .../bindings/panel/chunghwa,claa101wb03.txt | 7 +
> > .../bindings/panel/panasonic,vvx10f004b00.txt | 7 +
> > .../devicetree/bindings/panel/simple-panel.txt | 21 ++
> > drivers/gpu/drm/Makefile | 1 +
> > drivers/gpu/drm/panel/Kconfig | 13 +
> > drivers/gpu/drm/panel/Makefile | 1 +
> > drivers/gpu/drm/panel/panel-simple.c | 335 ++++++++++++++++++
> > 8 files changed, 392 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> > create mode 100644
> > Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt create
> > mode 100644
> > Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt create
> > mode 100644 Documentation/devicetree/bindings/panel/simple-panel.txt create
> > mode 100644 drivers/gpu/drm/panel/Makefile
> > create mode 100644 drivers/gpu/drm/panel/panel-simple.c
> >
> > diff --git a/Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> > b/Documentation/devicetree/bindings/panel/auo,b101aw03.txt new file mode
> > 100644
> > index 0000000..72e088a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/auo,b101aw03.txt
> > @@ -0,0 +1,7 @@
> > +AU Optronics Corporation 10.1" WSVGA TFT LCD panel
> > +
> > +Required properties:
> > +- compatible: should be "auo,b101aw03"
> > +
> > +This binding is compatible with the simple-panel binding, which is
> > specified +in simple-panel.txt in this directory.
> > diff --git
> > a/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
> > b/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt new file
> > mode 100644
> > index 0000000..0ab2c05
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/chunghwa,claa101wb03.txt
> > @@ -0,0 +1,7 @@
> > +Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel
> > +
> > +Required properties:
> > +- compatible: should be "chunghwa,claa101wb03"
> > +
> > +This binding is compatible with the simple-panel binding, which is
> > specified +in simple-panel.txt in this directory.
> > diff --git
> > a/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
> > b/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt new
> > file mode 100644
> > index 0000000..d328b03
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/panasonic,vvx10f004b00.txt
> > @@ -0,0 +1,7 @@
> > +Panasonic Corporation 10.1" WUXGA TFT LCD panel
> > +
> > +Required properties:
> > +- compatible: should be "panasonic,vvx10f004b00"
> > +
> > +This binding is compatible with the simple-panel binding, which is
> > specified +in simple-panel.txt in this directory.
> > diff --git a/Documentation/devicetree/bindings/panel/simple-panel.txt
> > b/Documentation/devicetree/bindings/panel/simple-panel.txt new file mode
> > 100644
> > index 0000000..1341bbf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/panel/simple-panel.txt
> > @@ -0,0 +1,21 @@
> > +Simple display panel
> > +
> > +Required properties:
> > +- power-supply: regulator to provide the supply voltage
> > +
> > +Optional properties:
> > +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> > +- enable-gpios: GPIO pin to enable or disable the panel
> > +- backlight: phandle of the backlight device attached to the panel
> > +
> > +Example:
> > +
> > + panel: panel {
> > + compatible = "cptt,claa101wb01";
> > + ddc-i2c-bus = <&panelddc>;
> > +
> > + power-supply = <&vdd_pnl_reg>;
> > + enable-gpios = <&gpio 90 0>;
> > +
> > + backlight = <&backlight>;
> > + };
>
> My biggest concern here is that this will not be compatible with the CDF DT
> bindings. They're not complete yet, but they will require connections between
> entities to be described in DT, in a way very similar (or actually identical)
> to the V4L2 DT bindings, documented in
> Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a
> look at that ? Please ignore all optional properties beside remote-endpoint,
> as they're V4L2 specific.
Okay, so if I understand correctly, translating those bindings to panel
nodes would look somewhat like this:
dc: display-controller {
ports {
port@0 {
remote-endpoint = <&panel>;
};
};
};
panel: panel {
ports {
port@0 {
remote-endpoint = <&dc>;
};
};
};
The above leaves out any of the other, non-relevant properties. Does
that sound about right? That seems like an overly complex amount of data
to describe just a simple panel where the only connection between it and
the display hardware is the data bus and I was sort of expecting the CDF
to provide some sort of shortcut for setups where the connection is 1:1
with no means to perform any configuration of the bus.
> I also plan to specify video bus parameters in DT for CDF, but this hasn't
> been finalized yet.
That effectively blocks any of the code that I've been working on until
CDF has been merged. It's been discussed for over a year now, and there
has even been significant pushback on using CDF in DRM, so even if CDF
is eventually merged, that will still leave DRM with no way to turn on
a simple panel.
I keep wondering why we absolutely must have compatibility between CDF
and this simple panel binding. DT content is supposed to concern itself
with the description of hardware only. What about the following isn't an
accurate description of the panel hardware?
panel: panel {
compatible = "cptt,claa101wb01";
power-supply = <&vdd_pnl_reg>;
enable-gpios = <&gpio 90 0>;
backlight = <&backlight>;
};
dsi-controller {
panel = <&panel>;
};
Note how the above is also not incompatible with anything that the CDF
(to be) mandates, so it could easily be extended with any nodes or
properties that CDF needs to work properly without breaking backwards-
compatibility.
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c
[...]
> > +/*
> > + * Copyright (C) 2013, NVIDIA Corporation. All rights reserved.
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sub
> > license,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial portions
> > + * of the Software.
>
> This contradicts MODULE_LICENSE("GPL v2") below.
True, I'll fix that up.
> > +/* TODO: convert to gpiod_*() API once it's been merged */
> > +#define GPIO_ACTIVE_LOW (1 << 0)
>
> Why can't you just #include <dt-bindings/gpio/gpio.h> ?
This is supposed to be something completely driver internal. Keeping it
here make that more explicit. It shouldn't matter much anyway, since by
now these patches have about 0 chance of making it into 3.13, so I'll
just convert them to gpiod.
> > + backlight_update_status(p->backlight);
> > + }
> > +
> > + if (gpio_is_valid(p->enable_gpio)) {
> > + if (p->enable_gpio_flags & GPIO_ACTIVE_LOW)
> > + gpio_set_value(p->enable_gpio, 1);
> > + else
> > + gpio_set_value(p->enable_gpio, 0);
> > + }
> > +
> > + regulator_disable(p->supply);
> > + p->enabled = false;
> > +}
> > +
> > +static void panel_simple_enable(struct drm_panel *panel)
> > +{
> > + struct panel_simple *p = to_panel_simple(panel);
> > + int err;
> > +
> > + if (p->enabled)
> > + return;
> > +
> > + err = regulator_enable(p->supply);
> > + if (err < 0)
> > + dev_err(panel->dev, "failed to enable supply: %d\n", err);
>
> Is that really a non-fatal error ? Shouldn't the enable operation return an
> int ?
There's no way to propagate this in DRM, so why go through the trouble
of returning the error? Furthermore, there's nothing that the caller
could do to remedy the situation anyway.
> > +static const struct drm_display_mode auo_b101aw03_mode = {
> > + .clock = 51450,
> > + .hdisplay = 1024,
> > + .hsync_start = 1024 + 156,
> > + .hsync_end = 1024 + 156 + 8,
> > + .htotal = 1024 + 156 + 8 + 156,
> > + .vdisplay = 600,
> > + .vsync_start = 600 + 16,
> > + .vsync_end = 600 + 16 + 6,
> > + .vtotal = 600 + 16 + 6 + 16,
> > + .vrefresh = 60,
> > +};
>
> Instead of hardcoding the modes in the driver, which would then require to be
> updated for every new simple panel model (and we know there are lots of them),
> why don't you specify the modes in the panel DT node ? The simple panel driver
> would then become much more generic. It would also allow board designers to
> tweak h/v sync timings depending on the system requirements.
Sigh... we keep second-guessing ourselves. Back at the time when power
sequences were designed (and NAKed at the last minute), we all decided
that the right thing to do would be to use specific compatible values
for individual panels, because that would allow us to encode the power
sequencing within the driver. And when we already have the panel model
encoded in the compatible value, we might just as well encode the mode
within the driver for that panel. Otherwise we'll have to keep adding
the same mode timings for every board that uses the same panel.
I do agree though that it might be useful to tweak the mode in case the
default one doesn't work. How about we provide a means to override the
mode encoded in the driver using one specified in the DT? That's
obviously a backwards-compatible change, so it could be added if or when
it becomes necessary.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17 9:16 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Dave Airlie, Laurent Pinchart, Laurent Pinchart, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <525FA4B0.8060504-l0cyMroinI0@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3471 bytes --]
On Thu, Oct 17, 2013 at 11:49:52AM +0300, Tomi Valkeinen wrote:
> Hi,
>
> On 17/10/13 11:28, Dave Airlie wrote:
> >>
> >> My biggest concern here is that this will not be compatible with the CDF DT
> >> bindings. They're not complete yet, but they will require connections between
> >> entities to be described in DT, in a way very similar (or actually identical)
> >> to the V4L2 DT bindings, documented in
> >> Documentation/devicetree/bindings/media/video-interfaces.txt. Could you have a
> >> look at that ? Please ignore all optional properties beside remote-endpoint,
> >> as they're V4L2 specific.
> >>
> >> I also plan to specify video bus parameters in DT for CDF, but this hasn't
> >> been finalized yet.
> >>
> >
> > While I understand this, I don't see why CDF can't enhance these
> > bindings if it has
> > requirements > than they have without disturbing the panel ones,
> >
> > is DT really that inflexible?
> >
> > It seems that have a simple description for basic panels like Thierry wants
> > to support, that can be enhanced for the other cases in the future should
> > suffice, I really don't like blocking stuff that makes things work on the chance
> > of something that isn't upstream yet, its sets a bad precedent, its also breaks
> > the perfect is the enemy of good rule
>
> Just my opinion, but yes, DT is that inflexible. DT bindings are like a
> syscall. Once they are there, you need to support them forever. That
> support can, of course, include some kind of DT data converters or such,
> that try to convert old bindings to new bindings at kernel boot.
That's not entirely true. DT bindings are allowed to change, the only
restriction being that they don't break backwards compatibility. So if
there's ever a need to support new functionality, be it in the form of
new nodes or properties to support something like CDF, that's not a
problem as long as the code continues to work with existing bindings.
> In practice even such converter may be enough, if some important piece
> of data in the new bindings is missing, and this data was implicit in a
> driver. In that case the conversion, or parts of it, must be done later,
> in that specific driver.
>
> Even that may be difficult, if the piece of data should actually be
> handled by some other driver. In that case there's probably need for
> some kind of global look-up table or such, so that the drivers can work
> around the problem of missing information.
>
> I've been working with DT bindings for OMAP display subsystem for
> something like 1.5 years. Still not merged, as they are not perfect, and
> I'm scared to push them in non-perfect form, as that could result in
> some kind of DT-binding-converter-hell described above.
Am I the only one refusing to accept that we can't provide even the most
basic functionality simply because we haven't been able to come up with
the ultimately "perfect" DT binding yet? By definition it's not very
likely that any binding will ever be perfect.
I don't think we're doing ourselves any good by letting DT actively
hinder us in merging any of these features. I would personally prefer
having to support several bindings rather than not be able to provide
the functionality at all.
For crying out loud, we can use the 3D engine to render to a framebuffer
but we can't look at the result because we can't make the bloody panel
light up! Seriously!?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] tmio: call tmiofb_set_par in tmiofb_probe
From: Dmitry Eremin-Solenikov @ 2013-10-17 9:50 UTC (permalink / raw)
To: linux-fbdev
Call tmiofb_set_par() to initlaize fixed variable before registering
framebuffer. This is necessary for current kexecboot.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
drivers/video/tmiofb.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/video/tmiofb.c b/drivers/video/tmiofb.c
index deb8733..69fd0f7 100644
--- a/drivers/video/tmiofb.c
+++ b/drivers/video/tmiofb.c
@@ -774,6 +774,10 @@ static int tmiofb_probe(struct platform_device *dev)
if (retval)
goto err_hw_init;
+ retval = tmiofb_set_par(info);
+ if (retval)
+ goto err_set_par;
+
fb_videomode_to_modelist(data->modes, data->num_modes,
&info->modelist);
@@ -787,7 +791,7 @@ static int tmiofb_probe(struct platform_device *dev)
return 0;
err_register_framebuffer:
-/*err_set_par:*/
+err_set_par:
tmiofb_hw_stop(dev);
err_hw_init:
if (cell->disable)
--
1.8.4.rc3
^ permalink raw reply related
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17 9:52 UTC (permalink / raw)
To: Thierry Reding
Cc: Dave Airlie, Laurent Pinchart, Laurent Pinchart, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <20131017091614.GC2502-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3444 bytes --]
On 17/10/13 12:16, Thierry Reding wrote:
> On Thu, Oct 17, 2013 at 11:49:52AM +0300, Tomi Valkeinen wrote:
>> Just my opinion, but yes, DT is that inflexible. DT bindings are like a
>> syscall. Once they are there, you need to support them forever. That
>> support can, of course, include some kind of DT data converters or such,
>> that try to convert old bindings to new bindings at kernel boot.
>
> That's not entirely true. DT bindings are allowed to change, the only
> restriction being that they don't break backwards compatibility. So if
True, but afaik the same goes for syscalls. But yes, it's probably
easier to add, say, new properties to DT bindings than adding new flags
to a syscall.
> Am I the only one refusing to accept that we can't provide even the most
> basic functionality simply because we haven't been able to come up with
> the ultimately "perfect" DT binding yet? By definition it's not very
> likely that any binding will ever be perfect.
With "perfect" I meant without any obvious features missing. I agree
that DT bindings will never be perfect, but I at least would like them
to be good enough to cover all the cases we know of.
> I don't think we're doing ourselves any good by letting DT actively
> hinder us in merging any of these features. I would personally prefer
> having to support several bindings rather than not be able to provide
> the functionality at all.
I'd say the driver writer is of course free to create basic bindings for
the device in question as long as the bindings are sane, and he agrees
to later create any needed converters. I don't see any problem with
having a driver supporting several versions of the bindings.
But I don't think it's fair to push a driver with basic bindings,
knowing that the bindings won't be enough in the future, and leave all
the converter-mess to other people (not saying that's the case here).
As a fbdev maintainer, I'm ok with adding DT bindings to device specific
fb drivers, as those can be just left as they are. Even when CDF is
merged, the specific fb drivers just work with the old bindings. If the
driver maintainer wants to use CDF, it's up to him to create any needed
binding-converters.
But a generic driver for panels is more problematic, as that one should
be maintained and refreshed to new features like CDF. If such was
proposed to be merged to fbdev, I'd be very reluctant to merge it if
there were any concerns about it.
I guess one option there would be to implement a new driver for the same
panel devices, one that supports CDF, and leave the old one as is. Not
so nice option, though.
> For crying out loud, we can use the 3D engine to render to a framebuffer
> but we can't look at the result because we can't make the bloody panel
> light up! Seriously!?
Ah, but do you have DT bindings for the 3D engine yet, which represent
how different internal blocks in the 3D engine are connected? If not,
better start designing it! Then the problem is solved, you won't even
have working 3D engine. ;)
But seriously speaking, I'm with you. I don't get this DT stuff.
And the funny thing is, DT should just represent the hardware, it
doesn't matter what kind of SW drivers there are, so in a sense talking
about DT problems with a SW framework like CDF is a bit silly. How hard
can it be to describe the hardware properly?
Very, obviously =).
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Tomi Valkeinen @ 2013-10-17 10:22 UTC (permalink / raw)
To: Thierry Reding, Laurent Pinchart
Cc: Laurent Pinchart, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie
In-Reply-To: <20131017085342.GB2502-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3623 bytes --]
On 17/10/13 11:53, Thierry Reding wrote:
> I keep wondering why we absolutely must have compatibility between CDF
> and this simple panel binding. DT content is supposed to concern itself
> with the description of hardware only. What about the following isn't an
> accurate description of the panel hardware?
>
> panel: panel {
> compatible = "cptt,claa101wb01";
>
> power-supply = <&vdd_pnl_reg>;
> enable-gpios = <&gpio 90 0>;
>
> backlight = <&backlight>;
> };
>
> dsi-controller {
> panel = <&panel>;
> };
That's quite similar to how my current out-of-mainline OMAP DSS DT
bindings work. The difference is that I have a reference from the panel
node to the video source (dsi-controller), instead of the other way
around. I just find it more natural. It works the same way as, say,
regulators, gpios, etc.
However, one thing unclear is where the panel node should be. You seem
to have a DSI panel here. If the panel is truly not controlled in any
way, maybe having the panel as a normal platform device is ok. But if
the DSI panel is controlled with DSI messages, should it be a child of
the dsi-controller node, the same way i2c peripherals are children of
i2c master?
>>> +static const struct drm_display_mode auo_b101aw03_mode = {
>>> + .clock = 51450,
>>> + .hdisplay = 1024,
>>> + .hsync_start = 1024 + 156,
>>> + .hsync_end = 1024 + 156 + 8,
>>> + .htotal = 1024 + 156 + 8 + 156,
>>> + .vdisplay = 600,
>>> + .vsync_start = 600 + 16,
>>> + .vsync_end = 600 + 16 + 6,
>>> + .vtotal = 600 + 16 + 6 + 16,
>>> + .vrefresh = 60,
>>> +};
>>
>> Instead of hardcoding the modes in the driver, which would then require to be
>> updated for every new simple panel model (and we know there are lots of them),
>> why don't you specify the modes in the panel DT node ? The simple panel driver
>> would then become much more generic. It would also allow board designers to
>> tweak h/v sync timings depending on the system requirements.
>
> Sigh... we keep second-guessing ourselves. Back at the time when power
> sequences were designed (and NAKed at the last minute), we all decided
> that the right thing to do would be to use specific compatible values
> for individual panels, because that would allow us to encode the power
> sequencing within the driver. And when we already have the panel model
> encoded in the compatible value, we might just as well encode the mode
> within the driver for that panel. Otherwise we'll have to keep adding
> the same mode timings for every board that uses the same panel.
Oh, I didn't feel "we all decided that the right thing..." =).
> I do agree though that it might be useful to tweak the mode in case the
> default one doesn't work. How about we provide a means to override the
> mode encoded in the driver using one specified in the DT? That's
> obviously a backwards-compatible change, so it could be added if or when
> it becomes necessary.
This sounds good to me.
Although maybe it'd be good to have the driver compatible with something
like "generic-panel", so that you could have:
compatible = "foo,specific-panel", "generic-panel";
and if there's no need for any power/gpio setup for your board, you may
skip adding "foo,specific-panel" support to the panel driver. Later,
somebody else may need to implement fine grained power/gpio support for
"foo,specific-panel", and it would just work.
Maybe that would help us with the problem of adding support in the
driver for a hundred different simple panels each used only once on a
specific board.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-17 10:48 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Dave Airlie, Laurent Pinchart, Laurent Pinchart, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <525FB368.6020003-l0cyMroinI0@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5264 bytes --]
On Thu, Oct 17, 2013 at 12:52:40PM +0300, Tomi Valkeinen wrote:
> On 17/10/13 12:16, Thierry Reding wrote:
> > On Thu, Oct 17, 2013 at 11:49:52AM +0300, Tomi Valkeinen wrote:
>
> >> Just my opinion, but yes, DT is that inflexible. DT bindings are like a
> >> syscall. Once they are there, you need to support them forever. That
> >> support can, of course, include some kind of DT data converters or such,
> >> that try to convert old bindings to new bindings at kernel boot.
> >
> > That's not entirely true. DT bindings are allowed to change, the only
> > restriction being that they don't break backwards compatibility. So if
>
> True, but afaik the same goes for syscalls. But yes, it's probably
> easier to add, say, new properties to DT bindings than adding new flags
> to a syscall.
>
> > Am I the only one refusing to accept that we can't provide even the most
> > basic functionality simply because we haven't been able to come up with
> > the ultimately "perfect" DT binding yet? By definition it's not very
> > likely that any binding will ever be perfect.
>
> With "perfect" I meant without any obvious features missing. I agree
> that DT bindings will never be perfect, but I at least would like them
> to be good enough to cover all the cases we know of.
As far as the simple panel device tree binding is concerned, it covers
all the cases I know of. I can't seriously be expected to do any better
than that. These panels are dumb. They only come with a power supply
that needs to be switched on and an additional GPIO to enable it once
powered up. All three panels just work with that set of properties. So
anything that doesn't can arguably be covered by some other, not-so-
simple binding.
> > I don't think we're doing ourselves any good by letting DT actively
> > hinder us in merging any of these features. I would personally prefer
> > having to support several bindings rather than not be able to provide
> > the functionality at all.
>
> I'd say the driver writer is of course free to create basic bindings for
> the device in question as long as the bindings are sane, and he agrees
> to later create any needed converters. I don't see any problem with
> having a driver supporting several versions of the bindings.
>
> But I don't think it's fair to push a driver with basic bindings,
> knowing that the bindings won't be enough in the future, and leave all
> the converter-mess to other people (not saying that's the case here).
>
> As a fbdev maintainer, I'm ok with adding DT bindings to device specific
> fb drivers, as those can be just left as they are. Even when CDF is
> merged, the specific fb drivers just work with the old bindings. If the
> driver maintainer wants to use CDF, it's up to him to create any needed
> binding-converters.
>
> But a generic driver for panels is more problematic, as that one should
> be maintained and refreshed to new features like CDF. If such was
> proposed to be merged to fbdev, I'd be very reluctant to merge it if
> there were any concerns about it.
I'm still missing the point here. For one, I don't see any reason that
this driver would ever need to gain CDF support. At the same time, CDF
could easily be supported by just adding a few required properties and
it should be easy to support any non-CDF cases just as well to remain
backwards-compatible.
> I guess one option there would be to implement a new driver for the same
> panel devices, one that supports CDF, and leave the old one as is. Not
> so nice option, though.
As I said, anything that really needs a CDF binding to work likely isn't
"simple" anymore, therefore a separate driver can easily be justified.
> > For crying out loud, we can use the 3D engine to render to a framebuffer
> > but we can't look at the result because we can't make the bloody panel
> > light up! Seriously!?
>
> Ah, but do you have DT bindings for the 3D engine yet, which represent
> how different internal blocks in the 3D engine are connected? If not,
> better start designing it! Then the problem is solved, you won't even
> have working 3D engine. ;)
>
> But seriously speaking, I'm with you. I don't get this DT stuff.
>
> And the funny thing is, DT should just represent the hardware, it
> doesn't matter what kind of SW drivers there are, so in a sense talking
> about DT problems with a SW framework like CDF is a bit silly. How hard
> can it be to describe the hardware properly?
>
> Very, obviously =).
Indeed. The fundamental point here seems to be that we don't represent
things in DT which we don't need. In the same way that we don't add
device nodes for enumerable devices, why would we need to explicitly add
information about connections between devices that cannot be controlled
anyway. If the board connects an RGB/LVDS output to a panel directly, it
is only required that the output knows what it is connected to because
there is no other way besides DT to obtain any information about the
panel. Therefore a unidirectional connection is more than enough. Using
that the output can access the panel and query it for information such
as the mode timings as well as switch it on or off as required.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ 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