* [PATCH/RFC v3 14/19] ARM: shmobile: r8a7790: Add DU clocks for DT
From: Laurent Pinchart @ 2013-08-09 23:03 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-media
In-Reply-To: <1376089398-13322-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
arch/arm/mach-shmobile/clock-r8a7790.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
index d99b87b..7229f96 100644
--- a/arch/arm/mach-shmobile/clock-r8a7790.c
+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
@@ -257,10 +257,15 @@ static struct clk_lookup lookups[] = {
/* MSTP */
CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]),
+ CLKDEV_ICK_ID("lvds.0", "feb00000.display", &mstp_clks[MSTP726]),
CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]),
+ CLKDEV_ICK_ID("lvds.1", "feb00000.display", &mstp_clks[MSTP725]),
CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]),
+ CLKDEV_ICK_ID("du.0", "feb00000.display", &mstp_clks[MSTP724]),
CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]),
+ CLKDEV_ICK_ID("du.1", "feb00000.display", &mstp_clks[MSTP723]),
CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]),
+ CLKDEV_ICK_ID("du.2", "feb00000.display", &mstp_clks[MSTP722]),
CLKDEV_DEV_ID("sh-sci.0", &mstp_clks[MSTP204]),
CLKDEV_DEV_ID("sh-sci.1", &mstp_clks[MSTP203]),
CLKDEV_DEV_ID("sh-sci.2", &mstp_clks[MSTP206]),
--
1.8.1.5
^ permalink raw reply related
* [PATCH/RFC v3 15/19] ARM: shmobile: r8a7790: Add DU device node to device tree
From: Laurent Pinchart @ 2013-08-09 23:03 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-media
In-Reply-To: <1376089398-13322-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
arch/arm/boot/dts/r8a7790.dtsi | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index ab0582c..b63f1a6 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -186,4 +186,37 @@
reg = <0 0xe6060000 0 0x250>;
#gpio-range-cells = <3>;
};
+
+ du: display@feb00000 {
+ compatible = "renesas,du-r8a7790";
+ reg = <0 0xfeb00000 0 0x70000>,
+ <0 0xfeb90000 0 0x1c>,
+ <0 0xfeb94000 0 0x1c>;
+ reg-names = "core", "lvds.0", "lvds.1";
+ interrupt-parent = <&gic>;
+ interrupts = <0 256 4
+ 0 268 4
+ 0 269 4>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ du_out_rgb: endpoint {
+ };
+ };
+ port@1 {
+ reg = <1>;
+ du_out_lvds0: endpoint {
+ };
+ };
+ port@2 {
+ reg = <2>;
+ du_out_lvds1: endpoint {
+ };
+ };
+ };
+ };
};
--
1.8.1.5
^ permalink raw reply related
* [PATCH/RFC v3 16/19] ARM: shmobile: marzen: Port DU platform data to CDF
From: Laurent Pinchart @ 2013-08-09 23:03 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-media
In-Reply-To: <1376089398-13322-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
arch/arm/mach-shmobile/board-marzen.c | 77 ++++++++++++++++++++++++-----------
1 file changed, 53 insertions(+), 24 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-marzen.c b/arch/arm/mach-shmobile/board-marzen.c
index 6499f1a..be1b7cb 100644
--- a/arch/arm/mach-shmobile/board-marzen.c
+++ b/arch/arm/mach-shmobile/board-marzen.c
@@ -39,6 +39,8 @@
#include <linux/mmc/host.h>
#include <linux/mmc/sh_mobile_sdhi.h>
#include <linux/mfd/tmio.h>
+#include <video/panel-dpi.h>
+#include <video/videomode.h>
#include <mach/r8a7779.h>
#include <mach/common.h>
#include <mach/irqs.h>
@@ -174,35 +176,56 @@ static struct platform_device hspi_device = {
* The panel only specifies the [hv]display and [hv]total values. The position
* and width of the sync pulses don't matter, they're copied from VESA timings.
*/
-static struct rcar_du_encoder_data du_encoders[] = {
+static const struct videomode marzen_panel_mode = {
+ .pixelclock = 65000000,
+ .hactive = 1024,
+ .hfront_porch = 24,
+ .hback_porch = 160,
+ .hsync_len = 136,
+ .vactive = 768,
+ .vfront_porch = 3,
+ .vback_porch = 29,
+ .vsync_len = 6,
+ .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW,
+};
+
+static const struct panel_dpi_platform_data marzen_panel_data = {
+ .width = 210,
+ .height = 158,
+ .mode = &marzen_panel_mode,
+};
+
+static const struct display_entity_graph_data marzen_du_entities[] = {
{
- .type = RCAR_DU_ENCODER_VGA,
- .output = RCAR_DU_OUTPUT_DPAD0,
+ .name = "adv7123",
+ .sources = (const struct display_entity_source_data[]) {
+ {
+ .name = "rcar-du",
+ .port = 0,
+ },
+ },
}, {
- .type = RCAR_DU_ENCODER_LVDS,
- .output = RCAR_DU_OUTPUT_DPAD1,
- .connector.lvds.panel = {
- .width_mm = 210,
- .height_mm = 158,
- .mode = {
- .clock = 65000,
- .hdisplay = 1024,
- .hsync_start = 1048,
- .hsync_end = 1184,
- .htotal = 1344,
- .vdisplay = 768,
- .vsync_start = 771,
- .vsync_end = 777,
- .vtotal = 806,
- .flags = 0,
+ .name = "con-vga",
+ .sources = (const struct display_entity_source_data[]) {
+ {
+ .name = "adv7123",
+ .port = 1,
},
},
+ }, {
+ .name = "panel-dpi",
+ .sources = (const struct display_entity_source_data[]) {
+ {
+ .name = "rcar-du",
+ .port = 1,
+ },
+ },
+ }, {
},
};
-static const struct rcar_du_platform_data du_pdata __initconst = {
- .encoders = du_encoders,
- .num_encoders = ARRAY_SIZE(du_encoders),
+static const struct rcar_du_platform_data marzen_du_pdata __initconst = {
+ .graph = marzen_du_entities,
};
static const struct resource du_resources[] __initconst = {
@@ -217,8 +240,8 @@ static void __init marzen_add_du_device(void)
.id = -1,
.res = du_resources,
.num_res = ARRAY_SIZE(du_resources),
- .data = &du_pdata,
- .size_data = sizeof(du_pdata),
+ .data = &marzen_du_pdata,
+ .size_data = sizeof(marzen_du_pdata),
.dma_mask = DMA_BIT_MASK(32),
};
@@ -327,6 +350,12 @@ static void __init marzen_init(void)
r8a7779_add_standard_devices();
platform_add_devices(marzen_devices, ARRAY_SIZE(marzen_devices));
marzen_add_du_device();
+
+ platform_device_register_simple("adv7123", -1, NULL, 0);
+ platform_device_register_simple("con-vga", -1, NULL, 0);
+ platform_device_register_data(&platform_bus, "panel-dpi", -1,
+ &marzen_panel_data,
+ sizeof(marzen_panel_data));
}
static const char *marzen_boards_compat_dt[] __initdata = {
--
1.8.1.5
^ permalink raw reply related
* [PATCH/RFC v3 17/19] ARM: shmobile: lager: Port DU platform data to CDF
From: Laurent Pinchart @ 2013-08-09 23:03 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-media
In-Reply-To: <1376089398-13322-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
arch/arm/mach-shmobile/board-lager.c | 76 +++++++++++++++++++++++++-----------
1 file changed, 53 insertions(+), 23 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-lager.c b/arch/arm/mach-shmobile/board-lager.c
index 75a01bc..d61b892 100644
--- a/arch/arm/mach-shmobile/board-lager.c
+++ b/arch/arm/mach-shmobile/board-lager.c
@@ -33,6 +33,8 @@
#include <linux/regulator/fixed.h>
#include <linux/regulator/machine.h>
#include <linux/sh_eth.h>
+#include <video/panel-dpi.h>
+#include <video/videomode.h>
#include <mach/common.h>
#include <mach/irqs.h>
#include <mach/r8a7790.h>
@@ -40,35 +42,56 @@
#include <asm/mach/arch.h>
/* DU */
-static struct rcar_du_encoder_data lager_du_encoders[] = {
+static const struct videomode lager_panel_mode = {
+ .pixelclock = 65000000,
+ .hactive = 1024,
+ .hfront_porch = 24,
+ .hback_porch = 160,
+ .hsync_len = 136,
+ .vactive = 768,
+ .vfront_porch = 3,
+ .vback_porch = 29,
+ .vsync_len = 6,
+ .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW,
+};
+
+static const struct panel_dpi_platform_data lager_panel_data = {
+ .width = 210,
+ .height = 158,
+ .mode = &lager_panel_mode,
+};
+
+static const struct display_entity_graph_data lager_du_entities[] = {
{
- .type = RCAR_DU_ENCODER_VGA,
- .output = RCAR_DU_OUTPUT_DPAD0,
+ .name = "adv7123",
+ .sources = (const struct display_entity_source_data[]) {
+ {
+ .name = "rcar-du",
+ .port = 0,
+ },
+ },
}, {
- .type = RCAR_DU_ENCODER_NONE,
- .output = RCAR_DU_OUTPUT_LVDS1,
- .connector.lvds.panel = {
- .width_mm = 210,
- .height_mm = 158,
- .mode = {
- .clock = 65000,
- .hdisplay = 1024,
- .hsync_start = 1048,
- .hsync_end = 1184,
- .htotal = 1344,
- .vdisplay = 768,
- .vsync_start = 771,
- .vsync_end = 777,
- .vtotal = 806,
- .flags = 0,
+ .name = "con-vga",
+ .sources = (const struct display_entity_source_data[]) {
+ {
+ .name = "adv7123",
+ .port = 1,
},
},
+ }, {
+ .name = "panel-dpi",
+ .sources = (const struct display_entity_source_data[]) {
+ {
+ .name = "rcar-du",
+ .port = 2,
+ },
+ },
+ }, {
},
};
static const struct rcar_du_platform_data lager_du_pdata __initconst = {
- .encoders = lager_du_encoders,
- .num_encoders = ARRAY_SIZE(lager_du_encoders),
+ .graph = lager_du_entities,
};
static const struct resource du_resources[] __initconst = {
@@ -87,8 +110,8 @@ static void __init lager_add_du_device(void)
.id = -1,
.res = du_resources,
.num_res = ARRAY_SIZE(du_resources),
- .data = &du_resources,
- .size_data = sizeof(du_resources),
+ .data = &lager_du_pdata,
+ .size_data = sizeof(lager_du_pdata),
.dma_mask = DMA_BIT_MASK(32),
};
@@ -202,6 +225,7 @@ static void __init lager_add_standard_devices(void)
r8a7790_pinmux_init();
r8a7790_add_standard_devices();
+
platform_device_register_data(&platform_bus, "leds-gpio", -1,
&lager_leds_pdata,
sizeof(lager_leds_pdata));
@@ -220,6 +244,12 @@ static void __init lager_add_standard_devices(void)
ðer_pdata, sizeof(ether_pdata));
lager_add_du_device();
+
+ platform_device_register_simple("adv7123", -1, NULL, 0);
+ platform_device_register_simple("con-vga", -1, NULL, 0);
+ platform_device_register_data(&platform_bus, "panel-dpi", -1,
+ &lager_panel_data,
+ sizeof(lager_panel_data));
}
static const char * const lager_boards_compat_dt[] __initconst = {
--
1.8.1.5
^ permalink raw reply related
* [PATCH/RFC v3 18/19] ARM: shmobile: lager-reference: Add display device nodes to device tree
From: Laurent Pinchart @ 2013-08-09 23:03 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-media
In-Reply-To: <1376089398-13322-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
arch/arm/boot/dts/r8a7790-lager-reference.dts | 92 +++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/arch/arm/boot/dts/r8a7790-lager-reference.dts b/arch/arm/boot/dts/r8a7790-lager-reference.dts
index d9a25d5..ba2469b 100644
--- a/arch/arm/boot/dts/r8a7790-lager-reference.dts
+++ b/arch/arm/boot/dts/r8a7790-lager-reference.dts
@@ -42,6 +42,98 @@
gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>;
};
};
+
+ adv7123 {
+ compatible = "adi,adv7123";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ adv7123_in: endpoint {
+ remote-endpoint = <&du_out_rgb>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ adv7123_out: endpoint {
+ remote-endpoint = <&con_vga_in>;
+ };
+ };
+ };
+ };
+
+ con-vga {
+ compatible = "con-vga";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ con_vga_in: endpoint {
+ remote-endpoint = <&adv7123_out>;
+ };
+ };
+ };
+ };
+
+ panel-dpi {
+ compatible = "panel-dpi";
+
+ width-mm = <210>;
+ height-mm = <158>;
+
+ display-timings {
+ timing {
+ /* 1024x768 @65Hz */
+ clock-frequency = <65000000>;
+ hactive = <1024>;
+ vactive = <768>;
+ hsync-len = <136>;
+ hfront-porch = <20>;
+ hback-porch = <160>;
+ vfront-porch = <3>;
+ vback-porch = <29>;
+ vsync-len = <6>;
+ };
+ };
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ panel_in: endpoint {
+ remote-endpoint = <&du_out_lvds1>;
+ };
+ };
+ };
+ };
+};
+
+&du {
+ pinctrl-0 = <&du_pins>;
+ pinctrl-names = "default";
+};
+
+&du_out_rgb {
+ remote-endpoint = <&adv7123_in>;
+};
+
+&du_out_lvds1 {
+ remote-endpoint = <&panel_in>;
+};
+
+&pfc {
+ du_pins: du {
+ renesas,groups = "du_rgb666", "du_sync_1", "du_clk_out_0";
+ renesas,function = "du";
+ };
};
&pfc {
--
1.8.1.5
^ permalink raw reply related
* [PATCH/RFC v3 19/19] drm/rcar-du: Port to the Common Display Framework
From: Laurent Pinchart @ 2013-08-09 23:03 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-media
In-Reply-To: <1376089398-13322-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/gpu/drm/rcar-du/Kconfig | 3 +-
drivers/gpu/drm/rcar-du/Makefile | 7 +-
drivers/gpu/drm/rcar-du/rcar_du_connector.c | 164 ++++++++++++++++
drivers/gpu/drm/rcar-du/rcar_du_connector.h | 36 ++++
drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 279 ++++++++++++++++++++++++----
drivers/gpu/drm/rcar-du/rcar_du_drv.h | 28 ++-
drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 87 ++++-----
drivers/gpu/drm/rcar-du/rcar_du_encoder.h | 22 +--
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 116 ++++++++++--
drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 131 -------------
drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 25 ---
drivers/gpu/drm/rcar-du/rcar_du_vgacon.c | 96 ----------
drivers/gpu/drm/rcar-du/rcar_du_vgacon.h | 23 ---
include/linux/platform_data/rcar-du.h | 55 +-----
15 files changed, 611 insertions(+), 463 deletions(-)
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_connector.c
create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_connector.h
delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h
delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_vgacon.h
diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index c590cd9..a54eeba 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -1,9 +1,10 @@
config DRM_RCAR_DU
tristate "DRM Support for R-Car Display Unit"
- depends on DRM && ARM
+ depends on DRM && ARM && OF
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
select DRM_GEM_CMA_HELPER
+ select VIDEOMODE_HELPERS
help
Choose this option if you have an R-Car chipset.
If M is selected the module will be called rcar-du-drm.
diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index 12b8d44..3ac8566 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -1,11 +1,10 @@
-rcar-du-drm-y := rcar_du_crtc.o \
+rcar-du-drm-y := rcar_du_connector.o \
+ rcar_du_crtc.o \
rcar_du_drv.o \
rcar_du_encoder.o \
rcar_du_group.o \
rcar_du_kms.o \
- rcar_du_lvdscon.o \
- rcar_du_plane.o \
- rcar_du_vgacon.o
+ rcar_du_plane.o
rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS) += rcar_du_lvdsenc.o
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_connector.c b/drivers/gpu/drm/rcar-du/rcar_du_connector.c
new file mode 100644
index 0000000..a09aada
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_connector.c
@@ -0,0 +1,164 @@
+/*
+ * rcar_du_connector.c -- R-Car Display Unit Connector
+ *
+ * Copyright (C) 2013 Renesas Corporation
+ *
+ * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/export.h>
+#include <video/videomode.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+
+#include "rcar_du_drv.h"
+#include "rcar_du_connector.h"
+#include "rcar_du_encoder.h"
+#include "rcar_du_kms.h"
+
+static int rcar_du_connector_get_modes(struct drm_connector *connector)
+{
+ struct rcar_du_connector *rcon = to_rcar_connector(connector);
+ struct display_entity *entity = to_display_entity(rcon->pad->entity);
+ const struct videomode *modes;
+ int ret;
+ int i;
+
+ ret = display_entity_get_modes(entity, rcon->pad->index, &modes);
+ if (ret <= 0)
+ return drm_add_modes_noedid(connector, 1280, 768);
+
+ for (i = 0; i < ret; ++i) {
+ struct drm_display_mode *mode;
+
+ mode = drm_mode_create(connector->dev);
+ if (mode = NULL)
+ break;
+
+ mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
+ drm_display_mode_from_videomode(&modes[i], mode);
+ drm_mode_probed_add(connector, mode);
+ }
+
+ return i;
+}
+
+static int rcar_du_connector_mode_valid(struct drm_connector *connector,
+ struct drm_display_mode *mode)
+{
+ return MODE_OK;
+}
+
+static struct drm_encoder *
+rcar_du_connector_best_encoder(struct drm_connector *connector)
+{
+ struct rcar_du_connector *rcon = to_rcar_connector(connector);
+
+ return &rcon->encoder->encoder;
+}
+
+static const struct drm_connector_helper_funcs connector_helper_funcs = {
+ .get_modes = rcar_du_connector_get_modes,
+ .mode_valid = rcar_du_connector_mode_valid,
+ .best_encoder = rcar_du_connector_best_encoder,
+};
+
+static void rcar_du_connector_destroy(struct drm_connector *connector)
+{
+ drm_sysfs_connector_remove(connector);
+ drm_connector_cleanup(connector);
+}
+
+static enum drm_connector_status
+rcar_du_connector_detect(struct drm_connector *connector, bool force)
+{
+ return connector_status_connected;
+}
+
+static const struct drm_connector_funcs connector_funcs = {
+ .dpms = drm_helper_connector_dpms,
+ .detect = rcar_du_connector_detect,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = rcar_du_connector_destroy,
+};
+
+struct rcar_du_connector *
+rcar_du_connector_create(struct rcar_du_device *rcdu,
+ struct rcar_du_encoder *renc, struct media_pad *pad)
+{
+ struct display_entity *entity = to_display_entity(pad->entity);
+ struct display_entity_interface_params params;
+ struct rcar_du_connector *rcon;
+ struct drm_connector *connector;
+ int connector_type;
+ unsigned int width;
+ unsigned int height;
+ int ret;
+
+ rcon = devm_kzalloc(rcdu->dev, sizeof(*rcon), GFP_KERNEL);
+ if (rcon = NULL)
+ return ERR_PTR(-ENOMEM);
+
+ rcon->encoder = renc;
+ rcon->pad = pad;
+
+ connector = &rcon->connector;
+
+ ret = display_entity_get_size(entity, &width, &height);
+ if (ret < 0) {
+ connector->display_info.width_mm = 0;
+ connector->display_info.height_mm = 0;
+ } else {
+ connector->display_info.width_mm = width;
+ connector->display_info.height_mm = height;
+ }
+
+ ret = display_entity_get_params(entity, pad->index, ¶ms);
+ if (ret < 0) {
+ dev_dbg(rcdu->dev,
+ "failed to retrieve connector %s parameters\n",
+ entity->name);
+ return ERR_PTR(ret);
+ }
+
+ switch (params.type) {
+ case DISPLAY_ENTITY_INTERFACE_VGA:
+ connector_type = DRM_MODE_CONNECTOR_VGA;
+ break;
+ case DISPLAY_ENTITY_INTERFACE_LVDS:
+ connector_type = DRM_MODE_CONNECTOR_LVDS;
+ break;
+ default:
+ connector_type = DRM_MODE_CONNECTOR_Unknown;
+ break;
+ }
+
+ ret = drm_connector_init(rcdu->ddev, connector, &connector_funcs,
+ connector_type);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ drm_connector_helper_add(connector, &connector_helper_funcs);
+ ret = drm_sysfs_connector_add(connector);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
+ drm_object_property_set_value(&connector->base,
+ rcdu->ddev->mode_config.dpms_property, DRM_MODE_DPMS_OFF);
+
+ ret = drm_mode_connector_attach_encoder(connector, &renc->encoder);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ connector->encoder = &renc->encoder;
+
+ return rcon;
+}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_connector.h b/drivers/gpu/drm/rcar-du/rcar_du_connector.h
new file mode 100644
index 0000000..b9a0833
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_du_connector.h
@@ -0,0 +1,36 @@
+/*
+ * rcar_du_connector.h -- R-Car Display Unit Connector
+ *
+ * Copyright (C) 2013 Renesas Corporation
+ *
+ * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __RCAR_DU_CONNECTOR_H__
+#define __RCAR_DU_CONNECTOR_H__
+
+#include <drm/drm_crtc.h>
+
+struct media_pad;
+struct rcar_du_device;
+struct rcar_du_encoder;
+
+struct rcar_du_connector {
+ struct drm_connector connector;
+ struct rcar_du_encoder *encoder;
+ struct media_pad *pad;
+};
+
+#define to_rcar_connector(c) \
+ container_of(c, struct rcar_du_connector, connector)
+
+struct rcar_du_connector *
+rcar_du_connector_create(struct rcar_du_device *rcdu,
+ struct rcar_du_encoder *renc, struct media_pad *pad);
+
+#endif /* __RCAR_DU_CONNECTOR_H__ */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 43e7575..4e8bfff 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -15,11 +15,11 @@
#define __RCAR_DU_CRTC_H__
#include <linux/mutex.h>
-#include <linux/platform_data/rcar-du.h>
#include <drm/drmP.h>
#include <drm/drm_crtc.h>
+enum rcar_du_output;
struct rcar_du_group;
struct rcar_du_plane;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 0a9f1bb..eda9ca9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -15,6 +15,8 @@
#include <linux/io.h>
#include <linux/mm.h>
#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_data/rcar-du.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
#include <linux/slab.h>
@@ -30,6 +32,153 @@
#include "rcar_du_regs.h"
/* -----------------------------------------------------------------------------
+ * Display Entities
+ */
+
+static int rcdu_entity_set_stream(struct display_entity *ent, unsigned int port,
+ enum display_entity_stream_state state)
+{
+ return 0;
+}
+
+static const struct display_entity_video_ops rcdu_entity_video_ops = {
+ .set_stream = rcdu_entity_set_stream,
+};
+
+static const struct display_entity_ops rcdu_entity_ops = {
+ .video = &rcdu_entity_video_ops,
+};
+
+static struct media_pad *rcar_du_find_remote_pad(struct rcar_du_device *rcdu,
+ unsigned int local_pad)
+{
+ struct display_entity *entity;
+ unsigned int i;
+
+ list_for_each_entry(entity, &rcdu->notifier.done, list) {
+ const struct display_entity_graph_data *graph + entity->match->data;
+ const struct display_entity_source_data *source + graph->sources;
+
+ for (i = 0; i < entity->entity.num_pads; ++i) {
+ struct media_pad *pad = &entity->entity.pads[i];
+
+ if (pad->flags & MEDIA_PAD_FL_SOURCE)
+ continue;
+
+ if (!strcmp(source->name, "rcar-du") &&
+ source->port = local_pad)
+ return pad;
+ }
+ }
+
+ return NULL;
+}
+
+static int rcar_du_create_local_links(struct rcar_du_device *rcdu)
+{
+ u32 link_flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED;
+ unsigned int i;
+
+ for (i = 0; i < rcdu->entity.entity.num_pads; ++i) {
+ struct media_pad *pad;
+ int ret;
+
+ pad = rcar_du_find_remote_pad(rcdu, i);
+ if (pad = NULL)
+ continue;
+
+ ret = media_entity_create_link(&rcdu->entity.entity, i,
+ pad->entity, pad->index,
+ link_flags);
+ if (ret < 0) {
+ dev_err(rcdu->dev,
+ "failed to create %s:%u -> %s:%u link\n",
+ rcdu->entity.entity.name, i,
+ pad->entity->name, pad->index);
+ return ret;
+ }
+ }
+
+ return 0;
+};
+
+static int rcar_du_graph_init(struct rcar_du_device *rcdu)
+{
+ struct display_entity *entity;
+ unsigned int num_pads;
+ unsigned int i;
+ int ret;
+
+ /* Count the number of output pads and initialize the DU entity. */
+ for (i = 0, num_pads = 0; rcdu->info->routes[i].output; ++i)
+ num_pads++;
+
+ rcdu->entity.dev = rcdu->dev;
+ rcdu->entity.ops = &rcdu_entity_ops;
+ strcpy(rcdu->entity.name, "R-Car DU");
+
+ ret = display_entity_init(&rcdu->entity, 0, num_pads);
+ if (ret < 0)
+ return ret;
+
+ /* Create links between all entities. In the non-DT case, this is a two
+ * steps process, we first create links between all external entities,
+ * and then between the DU entity and the external entities.
+ */
+ if (rcdu->dev->of_node) {
+ ret = display_of_entity_link_graph(rcdu->dev,
+ &rcdu->notifier.done,
+ &rcdu->entity);
+ } else {
+ ret = display_entity_link_graph(rcdu->dev,
+ &rcdu->notifier.done);
+ if (!ret)
+ ret = rcar_du_create_local_links(rcdu);
+ }
+
+ if (ret < 0) {
+ dev_err(rcdu->dev, "unable to link graph\n");
+ return ret;
+ }
+
+ /* Register the media device and all entities. */
+ rcdu->mdev.dev = rcdu->dev;
+ strlcpy(rcdu->mdev.model, dev_name(rcdu->dev),
+ sizeof(rcdu->mdev.model));
+
+ ret = media_device_register(&rcdu->mdev);
+ if (ret < 0)
+ return ret;
+
+ list_for_each_entry(entity, &rcdu->notifier.done, list) {
+ ret = display_entity_register(&rcdu->mdev, entity);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = display_entity_register(&rcdu->mdev, &rcdu->entity);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static void rcar_du_graph_cleanup(struct rcar_du_device *rcdu)
+{
+ struct display_entity *entity;
+
+ list_for_each_entry(entity, &rcdu->notifier.done, list)
+ display_entity_unregister(entity);
+
+ display_entity_unregister(&rcdu->entity);
+ display_entity_cleanup(&rcdu->entity);
+
+ media_device_unregister(&rcdu->mdev);
+}
+
+/* -----------------------------------------------------------------------------
* DRM operations
*/
@@ -44,6 +193,8 @@ static int rcar_du_unload(struct drm_device *dev)
drm_mode_config_cleanup(dev);
drm_vblank_cleanup(dev);
+ rcar_du_graph_cleanup(rcdu);
+
dev->irq_enabled = 0;
dev->dev_private = NULL;
@@ -53,25 +204,10 @@ static int rcar_du_unload(struct drm_device *dev)
static int rcar_du_load(struct drm_device *dev, unsigned long flags)
{
struct platform_device *pdev = dev->platformdev;
- struct rcar_du_platform_data *pdata = pdev->dev.platform_data;
- struct rcar_du_device *rcdu;
+ struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
struct resource *mem;
int ret;
- if (pdata = NULL) {
- dev_err(dev->dev, "no platform data\n");
- return -ENODEV;
- }
-
- rcdu = devm_kzalloc(&pdev->dev, sizeof(*rcdu), GFP_KERNEL);
- if (rcdu = NULL) {
- dev_err(dev->dev, "failed to allocate private data\n");
- return -ENOMEM;
- }
-
- rcdu->dev = &pdev->dev;
- rcdu->pdata = pdata;
- rcdu->info = (struct rcar_du_device_info *)pdev->id_entry->driver_data;
rcdu->ddev = dev;
dev->dev_private = rcdu;
@@ -81,6 +217,13 @@ static int rcar_du_load(struct drm_device *dev, unsigned long flags)
if (IS_ERR(rcdu->mmio))
return PTR_ERR(rcdu->mmio);
+ /* Display Entities */
+ ret = rcar_du_graph_init(rcdu);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to initialize display entities\n");
+ goto done;
+ }
+
/* DRM/KMS objects */
ret = rcar_du_modeset_init(rcdu);
if (ret < 0) {
@@ -97,8 +240,6 @@ static int rcar_du_load(struct drm_device *dev, unsigned long flags)
dev->irq_enabled = 1;
- platform_set_drvdata(pdev, rcdu);
-
done:
if (ret)
rcar_du_unload(dev);
@@ -218,33 +359,23 @@ static const struct dev_pm_ops rcar_du_pm_ops = {
* Platform driver
*/
-static int rcar_du_probe(struct platform_device *pdev)
-{
- return drm_platform_init(&rcar_du_driver, pdev);
-}
-
-static int rcar_du_remove(struct platform_device *pdev)
-{
- drm_platform_exit(&rcar_du_driver, pdev);
-
- return 0;
-}
-
static const struct rcar_du_device_info rcar_du_r8a7779_info = {
.features = 0,
.num_crtcs = 2,
- .routes = {
+ .routes = (const struct rcar_du_output_routing[]) {
/* R8A7779 has two RGB outputs and one (currently unsupported)
* TCON output.
*/
- [RCAR_DU_OUTPUT_DPAD0] = {
+ {
+ .output = RCAR_DU_OUTPUT_DPAD0,
.possible_crtcs = BIT(0),
.encoder_type = DRM_MODE_ENCODER_NONE,
- },
- [RCAR_DU_OUTPUT_DPAD1] = {
+ }, {
+ .output = RCAR_DU_OUTPUT_DPAD1,
.possible_crtcs = BIT(1) | BIT(0),
.encoder_type = DRM_MODE_ENCODER_NONE,
},
+ { },
},
.num_lvds = 0,
};
@@ -253,22 +384,24 @@ static const struct rcar_du_device_info rcar_du_r8a7790_info = {
.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK | RCAR_DU_FEATURE_ALIGN_128B
| RCAR_DU_FEATURE_DEFR8,
.num_crtcs = 3,
- .routes = {
+ .routes = (const struct rcar_du_output_routing[]) {
/* R8A7790 has one RGB output, two LVDS outputs and one
* (currently unsupported) TCON output.
*/
- [RCAR_DU_OUTPUT_DPAD0] = {
+ {
+ .output = RCAR_DU_OUTPUT_DPAD0,
.possible_crtcs = BIT(2) | BIT(1) | BIT(0),
.encoder_type = DRM_MODE_ENCODER_NONE,
- },
- [RCAR_DU_OUTPUT_LVDS0] = {
+ }, {
+ .output = RCAR_DU_OUTPUT_LVDS0,
.possible_crtcs = BIT(0),
.encoder_type = DRM_MODE_ENCODER_LVDS,
- },
- [RCAR_DU_OUTPUT_LVDS1] = {
+ }, {
+ .output = RCAR_DU_OUTPUT_LVDS1,
.possible_crtcs = BIT(2) | BIT(1),
.encoder_type = DRM_MODE_ENCODER_LVDS,
},
+ { },
},
.num_lvds = 2,
};
@@ -278,9 +411,72 @@ static const struct platform_device_id rcar_du_id_table[] = {
{ "rcar-du-r8a7790", (kernel_ulong_t)&rcar_du_r8a7790_info },
{ }
};
-
MODULE_DEVICE_TABLE(platform, rcar_du_id_table);
+static const struct of_device_id rcar_du_of_table[] = {
+ { .compatible = "renesas,du-r8a7779", .data = &rcar_du_r8a7779_info },
+ { .compatible = "renesas,du-r8a7790", .data = &rcar_du_r8a7790_info },
+ { }
+};
+MODULE_DEVICE_TABLE(of, rcar_du_of_table);
+
+static int rcar_du_notifier_complete(struct display_entity_notifier *notifier)
+{
+ struct platform_device *pdev = to_platform_device(notifier->dev);
+
+ return drm_platform_init(&rcar_du_driver, pdev);
+}
+
+static int rcar_du_probe(struct platform_device *pdev)
+{
+ struct rcar_du_platform_data *pdata = pdev->dev.platform_data;
+ struct device_node *np = pdev->dev.of_node;
+ struct rcar_du_device *rcdu;
+ int ret;
+
+ rcdu = devm_kzalloc(&pdev->dev, sizeof(*rcdu), GFP_KERNEL);
+ if (rcdu = NULL)
+ return -ENOMEM;
+
+ rcdu->dev = &pdev->dev;
+
+ if (np)
+ rcdu->info = of_match_device(rcar_du_of_table, rcdu->dev)->data;
+ else
+ rcdu->info = (void *)platform_get_device_id(pdev)->driver_data;
+
+ platform_set_drvdata(pdev, rcdu);
+
+ rcdu->notifier.dev = rcdu->dev;
+ rcdu->notifier.complete = rcar_du_notifier_complete;
+
+ if (np) {
+ ret = display_of_entity_build_notifier(&rcdu->notifier, np);
+ } else if (pdata) {
+ ret = display_entity_build_notifier(&rcdu->notifier,
+ pdata->graph);
+ } else {
+ dev_err(&pdev->dev, "no platform data");
+ ret = -ENXIO;
+ }
+
+ if (ret < 0)
+ return ret;
+
+ return display_entity_register_notifier(&rcdu->notifier);
+}
+
+static int rcar_du_remove(struct platform_device *pdev)
+{
+ struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
+
+ display_entity_unregister_notifier(&rcdu->notifier);
+
+ drm_platform_exit(&rcar_du_driver, pdev);
+
+ return 0;
+}
+
static struct platform_driver rcar_du_platform_driver = {
.probe = rcar_du_probe,
.remove = rcar_du_remove,
@@ -288,6 +484,7 @@ static struct platform_driver rcar_du_platform_driver = {
.owner = THIS_MODULE,
.name = "rcar-du",
.pm = &rcar_du_pm_ops,
+ .of_match_table = of_match_ptr(rcar_du_of_table),
},
.id_table = rcar_du_id_table,
};
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index 65d2d63..49338e1 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -15,7 +15,8 @@
#define __RCAR_DU_DRV_H__
#include <linux/kernel.h>
-#include <linux/platform_data/rcar-du.h>
+#include <media/media-device.h>
+#include <video/display.h>
#include "rcar_du_crtc.h"
#include "rcar_du_group.h"
@@ -31,16 +32,28 @@ struct rcar_du_lvdsenc;
#define RCAR_DU_FEATURE_ALIGN_128B (1 << 1) /* Align pitches to 128 bytes */
#define RCAR_DU_FEATURE_DEFR8 (1 << 2) /* Has DEFR8 register */
+enum rcar_du_output {
+ RCAR_DU_OUTPUT_NONE,
+ RCAR_DU_OUTPUT_DPAD0,
+ RCAR_DU_OUTPUT_DPAD1,
+ RCAR_DU_OUTPUT_LVDS0,
+ RCAR_DU_OUTPUT_LVDS1,
+ RCAR_DU_OUTPUT_TCON,
+ RCAR_DU_OUTPUT_MAX,
+};
+
/*
* struct rcar_du_output_routing - Output routing specification
+ * @output: DU output
* @possible_crtcs: bitmask of possible CRTCs for the output
* @encoder_type: DRM type of the internal encoder associated with the output
*
* The DU has 5 possible outputs (DPAD0/1, LVDS0/1, TCON). Output routing data
- * specify the valid SoC outputs, which CRTCs can drive the output, and the type
- * of in-SoC encoder for the output.
+ * specify an SoC output, which CRTCs can drive it, and the type of in-SoC
+ * encoder for the output.
*/
struct rcar_du_output_routing {
+ enum rcar_du_output output;
unsigned int possible_crtcs;
unsigned int encoder_type;
};
@@ -49,23 +62,26 @@ struct rcar_du_output_routing {
* struct rcar_du_device_info - DU model-specific information
* @features: device features (RCAR_DU_FEATURE_*)
* @num_crtcs: total number of CRTCs
- * @routes: array of CRTC to output routes, indexed by output (RCAR_DU_OUTPUT_*)
+ * @routes: array of CRTC to output routes, indexed by port number
* @num_lvds: number of internal LVDS encoders
*/
struct rcar_du_device_info {
unsigned int features;
unsigned int num_crtcs;
- struct rcar_du_output_routing routes[RCAR_DU_OUTPUT_MAX];
+ const struct rcar_du_output_routing *routes;
unsigned int num_lvds;
};
struct rcar_du_device {
struct device *dev;
- const struct rcar_du_platform_data *pdata;
const struct rcar_du_device_info *info;
void __iomem *mmio;
+ struct media_device mdev;
+ struct display_entity entity;
+ struct display_entity_notifier notifier;
+
struct drm_device *ddev;
struct drm_fbdev_cma *fbdev;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 3daa7a1..9991fb0 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -20,25 +20,7 @@
#include "rcar_du_drv.h"
#include "rcar_du_encoder.h"
#include "rcar_du_kms.h"
-#include "rcar_du_lvdscon.h"
#include "rcar_du_lvdsenc.h"
-#include "rcar_du_vgacon.h"
-
-/* -----------------------------------------------------------------------------
- * Common connector functions
- */
-
-struct drm_encoder *
-rcar_du_connector_best_encoder(struct drm_connector *connector)
-{
- struct rcar_du_connector *rcon = to_rcar_connector(connector);
-
- return &rcon->encoder->encoder;
-}
-
-/* -----------------------------------------------------------------------------
- * Encoder
- */
static void rcar_du_encoder_dpms(struct drm_encoder *encoder, int mode)
{
@@ -139,10 +121,9 @@ static const struct drm_encoder_funcs encoder_funcs = {
.destroy = drm_encoder_cleanup,
};
-int rcar_du_encoder_init(struct rcar_du_device *rcdu,
- enum rcar_du_encoder_type type,
- enum rcar_du_output output,
- const struct rcar_du_encoder_data *data)
+struct rcar_du_encoder *
+rcar_du_encoder_create(struct rcar_du_device *rcdu, unsigned int port,
+ struct media_pad *pad)
{
struct rcar_du_encoder *renc;
unsigned int encoder_type;
@@ -150,11 +131,13 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
renc = devm_kzalloc(rcdu->dev, sizeof(*renc), GFP_KERNEL);
if (renc = NULL)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
- renc->output = output;
+ renc->port = port;
+ renc->output = rcdu->info->routes[port].output;
- switch (output) {
+ /* Do we have an internal LVDS encoder? */
+ switch (renc->output) {
case RCAR_DU_OUTPUT_LVDS0:
renc->lvds = rcdu->lvds[0];
break;
@@ -167,36 +150,44 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
break;
}
- switch (type) {
- case RCAR_DU_ENCODER_VGA:
- encoder_type = DRM_MODE_ENCODER_DAC;
- break;
- case RCAR_DU_ENCODER_LVDS:
- encoder_type = DRM_MODE_ENCODER_LVDS;
- break;
- case RCAR_DU_ENCODER_NONE:
- default:
+ /* Find the encoder type. */
+ if (pad) {
+ struct display_entity *entity = to_display_entity(pad->entity);
+ struct display_entity_interface_params params;
+
+ ret = display_entity_get_params(entity, pad->index, ¶ms);
+ if (ret < 0) {
+ dev_dbg(rcdu->dev,
+ "failed to retrieve encoder %s parameters\n",
+ entity->name);
+ return ERR_PTR(ret);
+ }
+
+ switch (params.type) {
+ case DISPLAY_ENTITY_INTERFACE_DPI:
+ case DISPLAY_ENTITY_INTERFACE_DBI:
+ default:
+ encoder_type = DRM_MODE_ENCODER_NONE;
+ break;
+ case DISPLAY_ENTITY_INTERFACE_LVDS:
+ encoder_type = DRM_MODE_ENCODER_DAC;
+ break;
+ case DISPLAY_ENTITY_INTERFACE_VGA:
+ encoder_type = DRM_MODE_ENCODER_DAC;
+ break;
+ }
+ } else {
/* No external encoder, use the internal encoder type. */
- encoder_type = rcdu->info->routes[output].encoder_type;
- break;
+ encoder_type = rcdu->info->routes[port].encoder_type;
}
+ /* Initialize the encoder. */
ret = drm_encoder_init(rcdu->ddev, &renc->encoder, &encoder_funcs,
encoder_type);
if (ret < 0)
- return ret;
+ return ERR_PTR(ret);
drm_encoder_helper_add(&renc->encoder, &encoder_helper_funcs);
- switch (encoder_type) {
- case DRM_MODE_ENCODER_LVDS:
- return rcar_du_lvds_connector_init(rcdu, renc,
- &data->connector.lvds.panel);
-
- case DRM_MODE_ENCODER_DAC:
- return rcar_du_vga_connector_init(rcdu, renc);
-
- default:
- return -EINVAL;
- }
+ return renc;
}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
index 0e5a65e..7a19d5b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
@@ -14,15 +14,15 @@
#ifndef __RCAR_DU_ENCODER_H__
#define __RCAR_DU_ENCODER_H__
-#include <linux/platform_data/rcar-du.h>
-
#include <drm/drm_crtc.h>
+struct media_pad;
struct rcar_du_device;
struct rcar_du_lvdsenc;
struct rcar_du_encoder {
struct drm_encoder encoder;
+ unsigned int port;
enum rcar_du_output output;
struct rcar_du_lvdsenc *lvds;
};
@@ -30,20 +30,8 @@ struct rcar_du_encoder {
#define to_rcar_encoder(e) \
container_of(e, struct rcar_du_encoder, encoder)
-struct rcar_du_connector {
- struct drm_connector connector;
- struct rcar_du_encoder *encoder;
-};
-
-#define to_rcar_connector(c) \
- container_of(c, struct rcar_du_connector, connector)
-
-struct drm_encoder *
-rcar_du_connector_best_encoder(struct drm_connector *connector);
-
-int rcar_du_encoder_init(struct rcar_du_device *rcdu,
- enum rcar_du_encoder_type type,
- enum rcar_du_output output,
- const struct rcar_du_encoder_data *data);
+struct rcar_du_encoder *
+rcar_du_encoder_create(struct rcar_du_device *rcdu, unsigned int port,
+ struct media_pad *pad);
#endif /* __RCAR_DU_ENCODER_H__ */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index b31ac08..2dc9e80 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -17,6 +17,7 @@
#include <drm/drm_fb_cma_helper.h>
#include <drm/drm_gem_cma_helper.h>
+#include "rcar_du_connector.h"
#include "rcar_du_crtc.h"
#include "rcar_du_drv.h"
#include "rcar_du_encoder.h"
@@ -179,6 +180,89 @@ static const struct drm_mode_config_funcs rcar_du_mode_config_funcs = {
.output_poll_changed = rcar_du_output_poll_changed,
};
+/* -----------------------------------------------------------------------------
+ * Pipeline and Mode Setting Initialization
+ */
+
+static int rcar_du_pipe_init(struct rcar_du_device *rcdu, unsigned int port,
+ struct media_pad *remote)
+{
+ struct media_pad *pad_encoder = NULL;
+ struct media_pad *pad_connector = remote;
+ struct rcar_du_encoder *renc;
+ struct rcar_du_connector *rcon;
+ struct media_entity *entity;
+
+ /* We need to expose one KMS encoder and one KMS connector for an
+ * arbitrarily long chain of external entities. Walk the chain and map
+ * the last entity to the connector, and the next-to-last entity to the
+ * encoder.
+ */
+
+ /* Start at the entity connected to the DU output. */
+ entity = remote->entity;
+
+ dev_dbg(rcdu->dev, "%s: starting at entity %s pad %u\n", __func__,
+ entity->name, remote->index);
+
+ while (1) {
+ struct media_link *next = NULL;
+ unsigned int i;
+
+ /* Search the entity for an output link. As we only support
+ * linear pipelines, return an error if multiple output links
+ * are found.
+ */
+ dev_dbg(rcdu->dev,
+ "%s: searching for an output link on entity %s\n",
+ __func__, entity->name);
+
+ for (i = 0; i < entity->num_links; ++i) {
+ struct media_link *link = &entity->links[i];
+
+ if (link->source->entity != entity)
+ continue;
+
+ if (next)
+ return -EPIPE;
+
+ next = link;
+ }
+
+ if (next = NULL) {
+ dev_dbg(rcdu->dev, "%s: not output link found\n",
+ __func__);
+ break;
+ }
+
+ dev_dbg(rcdu->dev,
+ "%s: output link %s:%u -> %s:%u found\n", __func__,
+ next->source->entity->name, next->source->index,
+ next->sink->entity->name, next->sink->index);
+
+ pad_encoder = next->source;
+ pad_connector = next->sink;
+ entity = pad_connector->entity;
+ }
+
+ dev_dbg(rcdu->dev,
+ "%s: encoder pad %s/%u connector pad %s/%u\n", __func__,
+ pad_encoder ? pad_encoder->entity->name : NULL,
+ pad_encoder ? pad_encoder->index : 0,
+ pad_connector->entity->name, pad_connector->index);
+
+ /* Create the encoder and connector. */
+ renc = rcar_du_encoder_create(rcdu, port, pad_encoder);
+ if (IS_ERR(renc))
+ return PTR_ERR(renc);
+
+ rcon = rcar_du_connector_create(rcdu, renc, pad_connector);
+ if (IS_ERR(rcon))
+ return PTR_ERR(rcon);
+
+ return 0;
+}
+
int rcar_du_modeset_init(struct rcar_du_device *rcdu)
{
static const unsigned int mmio_offsets[] = {
@@ -188,6 +272,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
struct drm_device *dev = rcdu->ddev;
struct drm_encoder *encoder;
struct drm_fbdev_cma *fbdev;
+ unsigned int num_encoders = 0;
unsigned int num_groups;
unsigned int i;
int ret;
@@ -226,29 +311,26 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
return ret;
}
- /* Initialize the encoders. */
+ /* Initialize the internal encoders. */
ret = rcar_du_lvdsenc_init(rcdu);
if (ret < 0)
return ret;
- for (i = 0; i < rcdu->pdata->num_encoders; ++i) {
- const struct rcar_du_encoder_data *pdata - &rcdu->pdata->encoders[i];
- const struct rcar_du_output_routing *route - &rcdu->info->routes[pdata->output];
+ /* Create an encoder and a connector for each output connected to
+ * external entities.
+ */
+ for (i = 0; i < rcdu->entity.entity.num_pads; ++i) {
+ struct media_pad *pad;
- if (pdata->type = RCAR_DU_ENCODER_UNUSED)
+ pad = media_entity_remote_pad(&rcdu->entity.entity.pads[i]);
+ if (pad = NULL)
continue;
- if (pdata->output >= RCAR_DU_OUTPUT_MAX ||
- route->possible_crtcs = 0) {
- dev_warn(rcdu->dev,
- "encoder %u references unexisting output %u, skipping\n",
- i, pdata->output);
- continue;
- }
+ ret = rcar_du_pipe_init(rcdu, i, pad);
+ if (ret < 0)
+ return ret;
- rcar_du_encoder_init(rcdu, pdata->type, pdata->output, pdata);
+ num_encoders++;
}
/* Set the possible CRTCs and possible clones. There's always at least
@@ -258,10 +340,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
const struct rcar_du_output_routing *route - &rcdu->info->routes[renc->output];
+ &rcdu->info->routes[renc->port];
encoder->possible_crtcs = route->possible_crtcs;
- encoder->possible_clones = (1 << rcdu->pdata->num_encoders) - 1;
+ encoder->possible_clones = (1 << num_encoders) - 1;
}
/* Now that the CRTCs have been initialized register the planes. */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
deleted file mode 100644
index 4f3ba93..0000000
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
+++ /dev/null
@@ -1,131 +0,0 @@
-/*
- * rcar_du_lvdscon.c -- R-Car Display Unit LVDS Connector
- *
- * Copyright (C) 2013 Renesas Corporation
- *
- * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <drm/drmP.h>
-#include <drm/drm_crtc.h>
-#include <drm/drm_crtc_helper.h>
-
-#include "rcar_du_drv.h"
-#include "rcar_du_encoder.h"
-#include "rcar_du_kms.h"
-#include "rcar_du_lvdscon.h"
-
-struct rcar_du_lvds_connector {
- struct rcar_du_connector connector;
-
- const struct rcar_du_panel_data *panel;
-};
-
-#define to_rcar_lvds_connector(c) \
- container_of(c, struct rcar_du_lvds_connector, connector.connector)
-
-static int rcar_du_lvds_connector_get_modes(struct drm_connector *connector)
-{
- struct rcar_du_lvds_connector *lvdscon - to_rcar_lvds_connector(connector);
- struct drm_display_mode *mode;
-
- mode = drm_mode_create(connector->dev);
- if (mode = NULL)
- return 0;
-
- mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
- mode->clock = lvdscon->panel->mode.clock;
- mode->hdisplay = lvdscon->panel->mode.hdisplay;
- mode->hsync_start = lvdscon->panel->mode.hsync_start;
- mode->hsync_end = lvdscon->panel->mode.hsync_end;
- mode->htotal = lvdscon->panel->mode.htotal;
- mode->vdisplay = lvdscon->panel->mode.vdisplay;
- mode->vsync_start = lvdscon->panel->mode.vsync_start;
- mode->vsync_end = lvdscon->panel->mode.vsync_end;
- mode->vtotal = lvdscon->panel->mode.vtotal;
- mode->flags = lvdscon->panel->mode.flags;
-
- drm_mode_set_name(mode);
- drm_mode_probed_add(connector, mode);
-
- return 1;
-}
-
-static int rcar_du_lvds_connector_mode_valid(struct drm_connector *connector,
- struct drm_display_mode *mode)
-{
- return MODE_OK;
-}
-
-static const struct drm_connector_helper_funcs connector_helper_funcs = {
- .get_modes = rcar_du_lvds_connector_get_modes,
- .mode_valid = rcar_du_lvds_connector_mode_valid,
- .best_encoder = rcar_du_connector_best_encoder,
-};
-
-static void rcar_du_lvds_connector_destroy(struct drm_connector *connector)
-{
- drm_sysfs_connector_remove(connector);
- drm_connector_cleanup(connector);
-}
-
-static enum drm_connector_status
-rcar_du_lvds_connector_detect(struct drm_connector *connector, bool force)
-{
- return connector_status_connected;
-}
-
-static const struct drm_connector_funcs connector_funcs = {
- .dpms = drm_helper_connector_dpms,
- .detect = rcar_du_lvds_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = rcar_du_lvds_connector_destroy,
-};
-
-int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu,
- struct rcar_du_encoder *renc,
- const struct rcar_du_panel_data *panel)
-{
- struct rcar_du_lvds_connector *lvdscon;
- struct drm_connector *connector;
- int ret;
-
- lvdscon = devm_kzalloc(rcdu->dev, sizeof(*lvdscon), GFP_KERNEL);
- if (lvdscon = NULL)
- return -ENOMEM;
-
- lvdscon->panel = panel;
-
- connector = &lvdscon->connector.connector;
- connector->display_info.width_mm = panel->width_mm;
- connector->display_info.height_mm = panel->height_mm;
-
- ret = drm_connector_init(rcdu->ddev, connector, &connector_funcs,
- DRM_MODE_CONNECTOR_LVDS);
- if (ret < 0)
- return ret;
-
- drm_connector_helper_add(connector, &connector_helper_funcs);
- ret = drm_sysfs_connector_add(connector);
- if (ret < 0)
- return ret;
-
- drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
- drm_object_property_set_value(&connector->base,
- rcdu->ddev->mode_config.dpms_property, DRM_MODE_DPMS_OFF);
-
- ret = drm_mode_connector_attach_encoder(connector, &renc->encoder);
- if (ret < 0)
- return ret;
-
- connector->encoder = &renc->encoder;
- lvdscon->connector.encoder = renc;
-
- return 0;
-}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h
deleted file mode 100644
index bff8683..0000000
--- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * rcar_du_lvdscon.h -- R-Car Display Unit LVDS Connector
- *
- * Copyright (C) 2013 Renesas Corporation
- *
- * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#ifndef __RCAR_DU_LVDSCON_H__
-#define __RCAR_DU_LVDSCON_H__
-
-struct rcar_du_device;
-struct rcar_du_encoder;
-struct rcar_du_panel_data;
-
-int rcar_du_lvds_connector_init(struct rcar_du_device *rcdu,
- struct rcar_du_encoder *renc,
- const struct rcar_du_panel_data *panel);
-
-#endif /* __RCAR_DU_LVDSCON_H__ */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vgacon.c b/drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
deleted file mode 100644
index 72312f7..0000000
--- a/drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
+++ /dev/null
@@ -1,96 +0,0 @@
-/*
- * rcar_du_vgacon.c -- R-Car Display Unit VGA Connector
- *
- * Copyright (C) 2013 Renesas Corporation
- *
- * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <drm/drmP.h>
-#include <drm/drm_crtc.h>
-#include <drm/drm_crtc_helper.h>
-
-#include "rcar_du_drv.h"
-#include "rcar_du_encoder.h"
-#include "rcar_du_kms.h"
-#include "rcar_du_vgacon.h"
-
-static int rcar_du_vga_connector_get_modes(struct drm_connector *connector)
-{
- return drm_add_modes_noedid(connector, 1280, 768);
-}
-
-static int rcar_du_vga_connector_mode_valid(struct drm_connector *connector,
- struct drm_display_mode *mode)
-{
- return MODE_OK;
-}
-
-static const struct drm_connector_helper_funcs connector_helper_funcs = {
- .get_modes = rcar_du_vga_connector_get_modes,
- .mode_valid = rcar_du_vga_connector_mode_valid,
- .best_encoder = rcar_du_connector_best_encoder,
-};
-
-static void rcar_du_vga_connector_destroy(struct drm_connector *connector)
-{
- drm_sysfs_connector_remove(connector);
- drm_connector_cleanup(connector);
-}
-
-static enum drm_connector_status
-rcar_du_vga_connector_detect(struct drm_connector *connector, bool force)
-{
- return connector_status_connected;
-}
-
-static const struct drm_connector_funcs connector_funcs = {
- .dpms = drm_helper_connector_dpms,
- .detect = rcar_du_vga_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = rcar_du_vga_connector_destroy,
-};
-
-int rcar_du_vga_connector_init(struct rcar_du_device *rcdu,
- struct rcar_du_encoder *renc)
-{
- struct rcar_du_connector *rcon;
- struct drm_connector *connector;
- int ret;
-
- rcon = devm_kzalloc(rcdu->dev, sizeof(*rcon), GFP_KERNEL);
- if (rcon = NULL)
- return -ENOMEM;
-
- connector = &rcon->connector;
- connector->display_info.width_mm = 0;
- connector->display_info.height_mm = 0;
-
- ret = drm_connector_init(rcdu->ddev, connector, &connector_funcs,
- DRM_MODE_CONNECTOR_VGA);
- if (ret < 0)
- return ret;
-
- drm_connector_helper_add(connector, &connector_helper_funcs);
- ret = drm_sysfs_connector_add(connector);
- if (ret < 0)
- return ret;
-
- drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF);
- drm_object_property_set_value(&connector->base,
- rcdu->ddev->mode_config.dpms_property, DRM_MODE_DPMS_OFF);
-
- ret = drm_mode_connector_attach_encoder(connector, &renc->encoder);
- if (ret < 0)
- return ret;
-
- connector->encoder = &renc->encoder;
- rcon->encoder = renc;
-
- return 0;
-}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vgacon.h b/drivers/gpu/drm/rcar-du/rcar_du_vgacon.h
deleted file mode 100644
index b12b0cf..0000000
--- a/drivers/gpu/drm/rcar-du/rcar_du_vgacon.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * rcar_du_vgacon.h -- R-Car Display Unit VGA Connector
- *
- * Copyright (C) 2013 Renesas Corporation
- *
- * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#ifndef __RCAR_DU_VGACON_H__
-#define __RCAR_DU_VGACON_H__
-
-struct rcar_du_device;
-struct rcar_du_encoder;
-
-int rcar_du_vga_connector_init(struct rcar_du_device *rcdu,
- struct rcar_du_encoder *renc);
-
-#endif /* __RCAR_DU_VGACON_H__ */
diff --git a/include/linux/platform_data/rcar-du.h b/include/linux/platform_data/rcar-du.h
index 1a2e990..b424b75 100644
--- a/include/linux/platform_data/rcar-du.h
+++ b/include/linux/platform_data/rcar-du.h
@@ -14,61 +14,10 @@
#ifndef __RCAR_DU_H__
#define __RCAR_DU_H__
-#include <drm/drm_mode.h>
-
-enum rcar_du_output {
- RCAR_DU_OUTPUT_DPAD0,
- RCAR_DU_OUTPUT_DPAD1,
- RCAR_DU_OUTPUT_LVDS0,
- RCAR_DU_OUTPUT_LVDS1,
- RCAR_DU_OUTPUT_TCON,
- RCAR_DU_OUTPUT_MAX,
-};
-
-enum rcar_du_encoder_type {
- RCAR_DU_ENCODER_UNUSED = 0,
- RCAR_DU_ENCODER_NONE,
- RCAR_DU_ENCODER_VGA,
- RCAR_DU_ENCODER_LVDS,
-};
-
-struct rcar_du_panel_data {
- unsigned int width_mm; /* Panel width in mm */
- unsigned int height_mm; /* Panel height in mm */
- struct drm_mode_modeinfo mode;
-};
-
-struct rcar_du_connector_lvds_data {
- struct rcar_du_panel_data panel;
-};
-
-struct rcar_du_connector_vga_data {
- /* TODO: Add DDC information for EDID retrieval */
-};
-
-/*
- * struct rcar_du_encoder_data - Encoder platform data
- * @type: the encoder type (RCAR_DU_ENCODER_*)
- * @output: the DU output the connector is connected to (RCAR_DU_OUTPUT_*)
- * @connector.lvds: platform data for LVDS connectors
- * @connector.vga: platform data for VGA connectors
- *
- * Encoder platform data describes an on-board encoder, its associated DU SoC
- * output, and the connector.
- */
-struct rcar_du_encoder_data {
- enum rcar_du_encoder_type type;
- enum rcar_du_output output;
-
- union {
- struct rcar_du_connector_lvds_data lvds;
- struct rcar_du_connector_vga_data vga;
- } connector;
-};
+#include <video/display.h>
struct rcar_du_platform_data {
- struct rcar_du_encoder_data *encoders;
- unsigned int num_encoders;
+ const struct display_entity_graph_data *graph;
};
#endif /* __RCAR_DU_H__ */
--
1.8.1.5
^ permalink raw reply related
* [PATCH v2] video: imxfb: Fix retrieve values from DT
From: Alexander Shiyan @ 2013-08-10 7:33 UTC (permalink / raw)
To: linux-arm-kernel
Display settings should be retrieved from "display" node, not from
root fb node. This patch fix this bug.
Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
drivers/video/imxfb.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 38733ac..8e104c4 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -753,12 +753,12 @@ static int imxfb_resume(struct platform_device *dev)
#define imxfb_resume NULL
#endif
-static int imxfb_init_fbinfo(struct platform_device *pdev)
+static int imxfb_init_fbinfo(struct platform_device *pdev,
+ struct device_node *np)
{
struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
struct fb_info *info = dev_get_drvdata(&pdev->dev);
struct imxfb_info *fbi = info->par;
- struct device_node *np;
pr_debug("%s\n",__func__);
@@ -799,7 +799,6 @@ static int imxfb_init_fbinfo(struct platform_device *pdev)
fbi->lcd_power = pdata->lcd_power;
fbi->backlight_power = pdata->backlight_power;
} else {
- np = pdev->dev.of_node;
info->var.grayscale = of_property_read_bool(np,
"cmap-greyscale");
fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
@@ -858,6 +857,7 @@ static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
static int imxfb_probe(struct platform_device *pdev)
{
+ struct device_node *display_np = NULL;
struct imxfb_info *fbi;
struct fb_info *info;
struct imx_fb_platform_data *pdata;
@@ -887,7 +887,17 @@ static int imxfb_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, info);
- ret = imxfb_init_fbinfo(pdev);
+ if (pdev->dev.of_node) {
+ display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
+ if (!display_np) {
+ dev_err(&pdev->dev,
+ "No display defined in devicetree\n");
+ ret = -EINVAL;
+ goto failed_init;
+ }
+ }
+
+ ret = imxfb_init_fbinfo(pdev, display_np);
if (ret < 0)
goto failed_init;
@@ -898,16 +908,8 @@ static int imxfb_probe(struct platform_device *pdev)
fbi->mode = pdata->mode;
fbi->num_modes = pdata->num_modes;
} else {
- struct device_node *display_np;
fb_mode = NULL;
- display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
- if (!display_np) {
- dev_err(&pdev->dev, "No display defined in devicetree\n");
- ret = -EINVAL;
- goto failed_of_parse;
- }
-
/*
* imxfb does not support more modes, we choose only the native
* mode.
--
1.8.1.5
^ permalink raw reply related
* Re: [RFC 1/1] drm/pl111: Initial drm/kms driver for pl111
From: Rob Clark @ 2013-08-10 12:30 UTC (permalink / raw)
To: Tom Cooksey; +Cc: dri-devel, linux-fbdev, Pawel Moll, linux-arm-kernel
In-Reply-To: <5205277e.84320f0a.1cdf.ffff8816SMTPIN_ADDED_BROKEN@mx.google.com>
On Fri, Aug 9, 2013 at 1:31 PM, Tom Cooksey <tom.cooksey@arm.com> wrote:
>> > > So in the above, after X receives the second DRI2SwapBuffers, it
>> > > doesn't need to get scheduled again for the next frame to be both
>> > > rendered by the GPU and issued to the display for scanout.
>> >
>> > well, this is really only an issue if you are so loaded that you
>> > don't get a chance to schedule for ~16ms.. which is pretty long time.
>
> Yes - it really is 16ms (minus interrupt/workqueue latency) isn't it?
> Hmmm, that does sound very long. Will try out some experiments and see.
>
yeah
>
>> > If you are triple buffering, it should not end up in the critical
>> > path (since the gpu already has the 3rd buffer to start on the next
>> > frame). And, well, if you do it all in the kernel you probably need
>> > to toss things over to a workqueue anyways.
>>
>> Just a quick comment on the kernel flip queue issue.
>>
>> 16 ms scheduling latency sounds awful but totally doable with a less
>> than stellar ddx driver going into limbo land and so preventing your
>> single threaded X from doing more useful stuff. Is this really the
>> linux scheduler being stupid?
>
> Ahahhaaa!! Yes!!! Really good point. We generally don't have 2D HW and
> so rely on pixman to perform all 2D operations which does indeed tie
> up that thread for fairly long periods of time.
>
> We've had internal discussions about introducing a thread (gulp) in
> the DDX to off-load drawing operations to. I think we were all a bit
> scared by that idea though.
>
thread does sound a bit scary.. it probably could be done if you treat
it like a virtual cpu and have WaitMarker or PrepareAccess for sw
fallbacks synchronize properly..
I bet you'd be much better off just making non-scanout pixmaps cached
and doing cache sync ops when needed for dri2 buffers. Sw fallbacks
on uncached buffers probably aren't exactly the hot ticket.
>
> BTW: I wasn't suggesting it was the linux scheduler being stupid, just
> that there is sometimes lots of contention over the CPU cores and X
> is just one thread among many wanting to run.
>
>
>> At least my impression was that the hw/kernel flip queue is to save
>> power so that you can queue up a few frames and everything goes to
>> sleep for half a second or so (at 24fps or whatever movie your
>> showing). Needing to schedule 5 frames ahead with pageflips under
>> load is just guaranteed to result in really horrible interactivity
>> and so awful user experience
>
> Agreed. There's always a tradeoff between tolerance to variable frame
> rendering time/system latency (lot of buffers) and UI latency (few
> buffers).
>
> As a side note, video playback is one use-case for explicit sync
> objects which implicit/buffer-based sync doesn't handle: Queue up lots
> of video frames for display, but mark those "display buffer"
> operations as depending on explicit sync objects which get signalled
> by the audio clock. Not sure Android actually does that yet though.
> Anyway, off topic.
>
w/ dmafence, rather than explicit fences, I suppose you could add some
way to queue the buffer to the audio device and have the audio device
signal the fence. I suppose it does sound a bit funny for ALSA to
have a DMA_BUF_AV_SYNC ioctl for this sort of case?
I don't think there is anything like it in EGL, but there is
oml_sync_control extension for more precise control of presentation
time. But this is all implemented in userspace and doesn't really
work out w/ >double buffering. This is part of the reason for the
timing information in vblank events. Of course it doesn't have any
tie in to audio subsystem, but in practice this really shouldn't be
needed. Audio samples are either rendered at a very predictable rate,
or sound like sh** with lots of pops and cut outs.
BR,
-R
>
> Cheers,
>
> Tom
>
>
>
>
>
^ permalink raw reply
* EDID modes unavailable when no connector/crtc available at boot
From: Tony Prisk @ 2013-08-11 4:41 UTC (permalink / raw)
To: dri-devel, David Airlie, linux-fbdev@vger.kernel.org
I am working on the HDMI driver for the i.MX6 as part of the larger DRM
driver written by Sascha Hauer and need a little advice. I seem to be
missing one important part of the subsystem that I haven't been able to
resolve.
In my testing, powering on the system with only a HDMI cable connected
works perfectly. Resolution is set to 1920x1080 as requested (with the
framebuffer console at the same resolution).
If I boot up the system without an HDMI cable connected (or any other
cable), I get the expected:
[ 1.470273] imx_hdmi_connector_detect
[ 1.470276] returned cable DISCONNECTED
[ 1.470289] imx-drm imx-drm: No connectors reported connected with modes
[ 1.470297] [drm] Cannot find any crtc or sizes - going 1024x768
When I connect the cable at a later time, all EDID detected modes are
'truncated' to '<= 1024x768', but because I have a video=xxx in
bootargs, the driver still seems to initialize to 1920x1080, but with a
framebuffer of 1024x768.
I assume this is a mistake on my part, but I don't know what I'm missing
that would cause it.
I see in drm_crtc_helper.c:drm_helper_mode_fill_fb_struct that the
width/height are set to the initialized mode.
Later in drm_fb_helper.c:drm_fb_helper_hotplug_event, max_width if set
to fb->width.
This is what causes the modes to be truncated when the HDMI connector is
finally connected (max_width is now limited to 1024).
Suggestions appreciated.
Regards
Tony Prisk
^ permalink raw reply
* Re: EDID modes unavailable when no connector/crtc available at boot
From: Dave Airlie @ 2013-08-11 8:42 UTC (permalink / raw)
To: Tony Prisk; +Cc: linux-fbdev@vger.kernel.org, dri-devel
In-Reply-To: <5207160A.6070803@prisktech.co.nz>
On Sun, Aug 11, 2013 at 2:41 PM, Tony Prisk <linux@prisktech.co.nz> wrote:
> I am working on the HDMI driver for the i.MX6 as part of the larger DRM
> driver written by Sascha Hauer and need a little advice. I seem to be
> missing one important part of the subsystem that I haven't been able to
> resolve.
>
fbcon is limited by boot sizes as at least with dynamic memory
management and how fbdev works resizing the allocation is nearly
impossible to do race free, since fbdev will hand out mmaps to
userspace and that stops you from ever moving anything once the device
is open.
But this is only for the fbdev emulation, a real kms application
should be able to use a larger size no problems.
Dave.
^ permalink raw reply
* Re: EDID modes unavailable when no connector/crtc available at boot
From: Tony Prisk @ 2013-08-11 9:42 UTC (permalink / raw)
To: Dave Airlie; +Cc: linux-fbdev@vger.kernel.org, dri-devel
In-Reply-To: <CAPM=9tzShWxZ-f=641r+-uc21JkY3kTeM2K=tsiB+OotNCnrQA@mail.gmail.com>
On 11/08/13 20:42, Dave Airlie wrote:
> On Sun, Aug 11, 2013 at 2:41 PM, Tony Prisk <linux@prisktech.co.nz> wrote:
>> I am working on the HDMI driver for the i.MX6 as part of the larger DRM
>> driver written by Sascha Hauer and need a little advice. I seem to be
>> missing one important part of the subsystem that I haven't been able to
>> resolve.
>>
> fbcon is limited by boot sizes as at least with dynamic memory
> management and how fbdev works resizing the allocation is nearly
> impossible to do race free, since fbdev will hand out mmaps to
> userspace and that stops you from ever moving anything once the device
> is open.
>
> But this is only for the fbdev emulation, a real kms application
> should be able to use a larger size no problems.
>
> Dave.
It seems to be worse than just a fbcon issue as far as I can tell.
I am making an assumption, but I believe
'/sys/class/drm/card0-HDMI-A-1/modes' should list all the supported
modes of the connector (regardless of fbcon).
Using 'cat /sys/class/drm/card0-HDMI-A-1/modes', it appears the
supported modes are being limited by fbcon
1) HDMI Cable connected at bootup (fb @ 1920x1080)
cat /sys/class/drm/card0-HDMI-A-1/modes
1920x1080
1280x720
1280x720
720x576
720x480
640x480
2) HDMI Cable NOT connected at bootup (fb @ 1024x768), cable is then
connected after userspace has started (still in console)
cat /sys/class/drm/card0-HDMI-A-1/modes
720x576
720x480
640x480
Following back through the source:
static struct drm_connector_funcs imx_hdmi_connector_funcs = {
.fill_modes = drm_helper_probe_single_connector_modes,
...
};
static struct drm_connector_helper_funcs imx_hdmi_connector_helper_funcs = {
.get_modes = imx_hdmi_connector_get_modes,
.mode_valid = imx_hdmi_connector_mode_valid,
...
};
It appears that only drm_helper_probe_single_connector_modes() calls
.get_modes() and .mode_valid()
.fill_modes() is called from drm_fb_helper_probe_connector_modes(),
which is called from drm_fb_helper_hotplug_event()
drm_fb_helper_hotplug_event() sets max_width to fb_helper->fb->width,
and max_height to fb_helper->fb->height.
fb->width is 1024 if booted without the cable connected, hence the
clipping of the values.
Regards
Tony Prisk
^ permalink raw reply
* Re: EDID modes unavailable when no connector/crtc available at boot
From: Dave Airlie @ 2013-08-11 10:06 UTC (permalink / raw)
To: Tony Prisk; +Cc: linux-fbdev@vger.kernel.org, dri-devel
In-Reply-To: <52075C9E.6040901@prisktech.co.nz>
On Sun, Aug 11, 2013 at 7:42 PM, Tony Prisk <linux@prisktech.co.nz> wrote:
> On 11/08/13 20:42, Dave Airlie wrote:
>>
>> On Sun, Aug 11, 2013 at 2:41 PM, Tony Prisk <linux@prisktech.co.nz> wrote:
>>>
>>> I am working on the HDMI driver for the i.MX6 as part of the larger DRM
>>> driver written by Sascha Hauer and need a little advice. I seem to be
>>> missing one important part of the subsystem that I haven't been able to
>>> resolve.
>>>
>> fbcon is limited by boot sizes as at least with dynamic memory
>> management and how fbdev works resizing the allocation is nearly
>> impossible to do race free, since fbdev will hand out mmaps to
>> userspace and that stops you from ever moving anything once the device
>> is open.
>>
>> But this is only for the fbdev emulation, a real kms application
>> should be able to use a larger size no problems.
>>
>> Dave.
>
> It seems to be worse than just a fbcon issue as far as I can tell.
>
> I am making an assumption, but I believe
> '/sys/class/drm/card0-HDMI-A-1/modes' should list all the supported modes of
> the connector (regardless of fbcon).
> Using 'cat /sys/class/drm/card0-HDMI-A-1/modes', it appears the supported
> modes are being limited by fbcon
>
> 1) HDMI Cable connected at bootup (fb @ 1920x1080)
> cat /sys/class/drm/card0-HDMI-A-1/modes
> 1920x1080
> 1280x720
> 1280x720
> 720x576
> 720x480
> 640x480
>
> 2) HDMI Cable NOT connected at bootup (fb @ 1024x768), cable is then
> connected after userspace has started (still in console)
> cat /sys/class/drm/card0-HDMI-A-1/modes
> 720x576
> 720x480
> 640x480
>
>
> Following back through the source:
>
> static struct drm_connector_funcs imx_hdmi_connector_funcs = {
> .fill_modes = drm_helper_probe_single_connector_modes,
> ...
> };
>
> static struct drm_connector_helper_funcs imx_hdmi_connector_helper_funcs = {
> .get_modes = imx_hdmi_connector_get_modes,
> .mode_valid = imx_hdmi_connector_mode_valid,
> ...
> };
>
>
> It appears that only drm_helper_probe_single_connector_modes() calls
> .get_modes() and .mode_valid()
> .fill_modes() is called from drm_fb_helper_probe_connector_modes(), which is
> called from drm_fb_helper_hotplug_event()
> drm_fb_helper_hotplug_event() sets max_width to fb_helper->fb->width, and
> max_height to fb_helper->fb->height.
> fb->width is 1024 if booted without the cable connected, hence the clipping
> of the values.
fill_modes is also called from the drm_crtc.c userspace interface, all
the functions in drm_fb_helper are for fbdev/con use, the fact sysfs
is wrong is only a side effect.
Userspace should get a full list of modes, try using the modetest
application which I think should work.
Dave.
^ permalink raw reply
* About buffer sychronization mechanism and cache operation
From: Inki Dae @ 2013-08-12 9:55 UTC (permalink / raw)
To: linux-fbdev
Hello all,
The purpose of this email is to get other opinions and advices to buffer synchronization mechanism, and coupling cache operation feature with the buffer synchronization mechanism. First of all, I am not a native English speaker so I'm not sure that I can convey my intention to you. And I'm not a specialist in Linux than other people so also there might be my missing points.
I had posted the buffer synchronization mechanism called dmabuf sync framework like below,
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/177045.html
And I'm sending this email before posting next version with more stable patch set and features. The purpose of this framework is to provide not only buffer access control to CPU and DMA but also easy-to-use interfaces for device drivers and user application. This framework can be used for all DMA devices using system memory as DMA buffer, especially for most ARM based SoCs.
There are two cases we are using this buffer synchronization framework. One is to primarily enhance GPU rendering performance on Tizen platform in case of 3d app with compositing mode that the 3d app draws something in off-screen buffer.
And other is to couple buffer access control and cache operation between CPU and DMA; the cache operation is done by the dmabuf sync framework in kernel side.
Why do we need buffer access control between CPU and DMA?
---------------------------------------------------------
The below shows simple 3D software layers,
3D Application
-----------------------------------------
Standard OpenGL ES and EGL Interfaces ------- [A]
-----------------------------------------
MALI OpenGL ES and EGL Native modules --------- [B]
-----------------------------------------
Exynos DRM Driver | GPU Driver ---------- [C]
3d application requests 3d rendering through A. And then B fills a 3d command buffer with the requests from A. And then 3D application calls glFlush so that the 3d command buffer can be submitted to C, and rendered by GPU hardware. Internally, glFlush(also glFinish) will call a function of native module[B] glFinish blocks caller's task until all GL execution is complete. On the other hand, glFlush forces execution of GL commands but doesn't block the caller's task until the completion.
In composting mode, in case of using glFinish, the 3d rendering performance is quite lower than using glFlush because glFinish makes CPU blocked until the execution of the 3d commands is completed. However, the use of glFlush has one issue that the shared buffer with GPU could be broken when CPU accesses the shared buffer at once after glFlush because CPU cannot be aware of the completion of GPU access to the shared buffer: actually, Tizen platform internally calls only eglSwapBuffer instead of glFlush and glFinish, and whether flushing or finishing is decided according to compositing mode or not. So in such case, we would need buffer access control between CPU and DMA for more performance.
About cache operation
---------------------
The dmabuf sync framework can include cache operation feature, and the below shows how the cache operation based on dmabuf sync framework is performed,
device driver in kernel or fctrl in user land
dmabuf_sync_lock or dmabuf_sync_single_lock
check before and after buffer access
dma_buf_begin_cpu_access or dma_buf_end_cpu_access
begin_cpu_access or end_cpu_access of exporter
dma_sync_sg_for_device or dma_sync_sg_for_cpu
In case that using dmabuf sync framework, kernel can be aware of when CPU and DMA access to a shared buffer is completed so we can do cache operation in kernel so that way, we can couple buffer access control and cache operation. So with this, we can avoid that user land overuses cache operation.
I guess most Linux based platforms are using cachable mapped buffer for more performance: in case that CPU frequently accesses the shared buffer which is shared with DMA, the use of cachable mapped buffer is more fast than the use of non-cachable. However, this way could make cache operation overused by user land because only user land can be aware of the completion of CPU or DMA access to the shared buffer so user land could request cache operations every time it wants even the cache operation is unnecessary. That is how user land could overuse cache operations.
To Android, Chrome OS, and other forks,
Are there other cases that buffer access control between CPU and DMA is needed? I know Android sync driver and KDS are already being used for Android, Chrome OS, and so on.
How does your platform do cache operation? And How do you think about coupling buffer access control and cache operation between CPU and DMA?.
Lastly, I think we may need Linux generic buffer synchronization mechanism that uses only Linux standard interfaces (dmabuf) including user land interfaces (fcntl and select system calls), and the dmabuf sync framework could meet it.
Thanks,
Inki Dae
^ permalink raw reply
* Re: [V4 1/5] video: mmp: rb swap setting update for mmp display
From: jett zhou @ 2013-08-12 11:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1375077384-7467-1-git-send-email-jtzhou@marvell.com>
Hi Jean
Would you please help to review these 5 patches when you have
time, thanks very much.
Thanks
2013/7/29 Jett.Zhou <jtzhou@marvell.com>:
> From: Guoqing Li <ligq@marvell.com>
>
> We could set rb swap in two modules: DMA controler input part and
> dsi interface output part.
> DMA input part is based on pix_fmt to set rbswap, dsi output interface
> part will set rbswap based on platform dsi_rbswap configuration.
>
> This patch include below change and enhancement:
>
> 1) The input format which support rbswap is based on RGB format,
> eg. RGB565 indicates the source data in memory is that Red is [15~11],
> Green is [10~5], Blue is [4:0], Red is MSB, Blue is LSB, but for the
> display dma input default setting(rbswap = 0), it only support Blue
> is [15~11], Green is [10~5], Red is [4:0], Red is LSB, Blue is MSB,
> so for this format(RGB565), display controller need to set rbswap
> = 1 and it can support the MSB/LSB correctly.
> BGR/YUV format will not set it in mmp display driver.
>
> 2) The dsi output part of rbswap is depend on dsi_rbswap which is
> defined in specific platfrom. For output dsi interface, it has this
> feature to do rbswap again if it needs specifc byte sequence of RGB
> byte for DSI panel.
> eg. If display content is set RGB565 in memory and DMA input part set
> rbswap in driver to support Red as MSB , Blue LSB, but dsi panel only
> support Red as LSB, Blue as MSB, then it can use this feature.
> If there is no this requirement of panel, this dsi output part is not
> needed.
>
> Signed-off-by: Guoqing Li <ligq@marvell.com>
> Signed-off-by: Jett.Zhou <jtzhou@marvell.com>
> Reviewed-by: Daniel Drake <dsd@laptop.org>
> ---
> drivers/video/mmp/hw/mmp_ctrl.c | 19 +++++++++++--------
> drivers/video/mmp/hw/mmp_ctrl.h | 5 +++++
> include/video/mmp_disp.h | 1 +
> 3 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
> index 75dca19..a40d95a 100644
> --- a/drivers/video/mmp/hw/mmp_ctrl.c
> +++ b/drivers/video/mmp/hw/mmp_ctrl.c
> @@ -60,8 +60,7 @@ static irqreturn_t ctrl_handle_irq(int irq, void *dev_id)
>
> static u32 fmt_to_reg(struct mmp_overlay *overlay, int pix_fmt)
> {
> - u32 link_config = path_to_path_plat(overlay->path)->link_config;
> - u32 rbswap, uvswap = 0, yuvswap = 0,
> + u32 rbswap = 0, uvswap = 0, yuvswap = 0,
> csc_en = 0, val = 0,
> vid = overlay_is_vid(overlay);
>
> @@ -71,27 +70,23 @@ static u32 fmt_to_reg(struct mmp_overlay *overlay, int pix_fmt)
> case PIXFMT_RGB888PACK:
> case PIXFMT_RGB888UNPACK:
> case PIXFMT_RGBA888:
> - rbswap = !(link_config & 0x1);
> + rbswap = 1;
> break;
> case PIXFMT_VYUY:
> case PIXFMT_YVU422P:
> case PIXFMT_YVU420P:
> - rbswap = link_config & 0x1;
> uvswap = 1;
> break;
> case PIXFMT_YUYV:
> - rbswap = link_config & 0x1;
> yuvswap = 1;
> break;
> default:
> - rbswap = link_config & 0x1;
> break;
> }
>
> switch (pix_fmt) {
> case PIXFMT_RGB565:
> case PIXFMT_BGR565:
> - val = 0;
> break;
> case PIXFMT_RGB1555:
> case PIXFMT_BGR1555:
> @@ -248,7 +243,8 @@ static void path_set_mode(struct mmp_path *path, struct mmp_mode *mode)
> {
> struct lcd_regs *regs = path_regs(path);
> u32 total_x, total_y, vsync_ctrl, tmp, sclk_src, sclk_div,
> - link_config = path_to_path_plat(path)->link_config;
> + link_config = path_to_path_plat(path)->link_config,
> + dsi_rbswap = path_to_path_plat(path)->link_config;
>
> /* FIXME: assert videomode supported */
> memcpy(&path->mode, mode, sizeof(struct mmp_mode));
> @@ -263,6 +259,12 @@ static void path_set_mode(struct mmp_path *path, struct mmp_mode *mode)
> tmp |= CFG_DUMB_ENA(1);
> writel_relaxed(tmp, ctrl_regs(path) + intf_ctrl(path->id));
>
> + /* interface rb_swap setting */
> + tmp = readl_relaxed(ctrl_regs(path) + intf_rbswap_ctrl(path->id)) &
> + (~(CFG_INTFRBSWAP_MASK));
> + tmp |= dsi_rbswap & CFG_INTFRBSWAP_MASK;
> + writel_relaxed(tmp, ctrl_regs(path) + intf_rbswap_ctrl(path->id));
> +
> writel_relaxed((mode->yres << 16) | mode->xres, ®s->screen_active);
> writel_relaxed((mode->left_margin << 16) | mode->right_margin,
> ®s->screen_h_porch);
> @@ -419,6 +421,7 @@ static int path_init(struct mmphw_path_plat *path_plat,
> path_plat->path = path;
> path_plat->path_config = config->path_config;
> path_plat->link_config = config->link_config;
> + path_plat->dsi_rbswap = config->dsi_rbswap;
> path_set_default(path);
>
> kfree(path_info);
> diff --git a/drivers/video/mmp/hw/mmp_ctrl.h b/drivers/video/mmp/hw/mmp_ctrl.h
> index edd2002..53301cf 100644
> --- a/drivers/video/mmp/hw/mmp_ctrl.h
> +++ b/drivers/video/mmp/hw/mmp_ctrl.h
> @@ -163,6 +163,8 @@ struct lcd_regs {
>
> #define LCD_SCLK(path) ((PATH_PN = path->id) ? LCD_CFG_SCLK_DIV :\
> ((PATH_TV = path->id) ? LCD_TCLK_DIV : LCD_PN2_SCLK_DIV))
> +#define intf_rbswap_ctrl(id) ((id) ? (((id) & 1) ? LCD_TVIF_CTRL : \
> + PN2_IOPAD_CONTROL) : LCD_TOP_CTRL)
>
> /* dither configure */
> #ifdef CONFIG_CPU_PXA988
> @@ -615,6 +617,8 @@ struct lcd_regs {
> #define LCD_SPU_DUMB_CTRL 0x01B8
> #define CFG_DUMBMODE(mode) ((mode)<<28)
> #define CFG_DUMBMODE_MASK 0xF0000000
> +#define CFG_INTFRBSWAP(mode) ((mode)<<24)
> +#define CFG_INTFRBSWAP_MASK 0x0F000000
> #define CFG_LCDGPIO_O(data) ((data)<<20)
> #define CFG_LCDGPIO_O_MASK 0x0FF00000
> #define CFG_LCDGPIO_ENA(gpio) ((gpio)<<12)
> @@ -1427,6 +1431,7 @@ struct mmphw_path_plat {
> struct mmp_path *path;
> u32 path_config;
> u32 link_config;
> + u32 dsi_rbswap;
> };
>
> /* mmp ctrl describes mmp controller related info */
> diff --git a/include/video/mmp_disp.h b/include/video/mmp_disp.h
> index b9dd1fb..32094c0 100644
> --- a/include/video/mmp_disp.h
> +++ b/include/video/mmp_disp.h
> @@ -334,6 +334,7 @@ struct mmp_mach_path_config {
> int output_type;
> u32 path_config;
> u32 link_config;
> + u32 dsi_rbswap;
> };
>
> struct mmp_mach_plat_info {
> --
> 1.7.0.4
>
--
----------------------------------
Best Regards
Jett Zhou
^ permalink raw reply
* Re: [RFC 0/1] Adding DT support to video/da8xx-fb.c
From: Darren Etheridge @ 2013-08-12 19:11 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: devicetree, plagnioj, linux-fbdev, afzal
In-Reply-To: <52052D10.7060803@ti.com>
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Fri [2013-Aug-09 20:55:28 +0300]:
> Hi,
>
> On 08/08/13 23:15, Darren Etheridge wrote:
> > This is part of a larger series of patches to upgrade the da8xx-fb.c driver
> > to support the Texas Instruments AM335x device. As part of this upgrade
> > we also want to add devicetree support for both the original da8xx and the
> > am335x. Tomi Valkeinen has reviewed the fbdev changes but he suggested
> > that it was prudent to extract the dt pieces and run it through the
> > devicetree mailing list for review.
> >
> > Thanks,
> > Darren
> >
> > Darren Etheridge (1):
> > video: da8xx-fb: adding dt support
> >
> > .../devicetree/bindings/video/fb-da8xx.txt | 37 +++++++++++
> > drivers/video/da8xx-fb.c | 67 +++++++++++++++++++-
> > 2 files changed, 101 insertions(+), 3 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt
>
> I just realized something. We have both drm and fb drivers for the LCDC
> IP. The hardware is the same, thus, there should only be one set of DT
> bindings, used by both drivers.
>
> I don't know why this didn't occur to me earlier, as it's the same
> situation with OMAP DSS.
>
OK that makes sense. So in this case what I need to do is change the
.compatible to match the gpu/drm/driver/tilcdc driver which has the
existing binding so it would become ti,am33xx-tilcdc instead of
ti,am3352-lcdc. This fbdev driver also supports da8xx class devices
so I would just rename this second .compatible to be ti,da8xx-tilcdc
to be consistent with the other. The fbdev driver doesn't support
some of the options that the tilcdc/drm driver supports, but that
shouldn't be a problem for the fbdev driver.
I will also change the name of the documentation file to be
da8xx-fb.txt to match the driver name.
Darren
^ permalink raw reply
* [PATCH] pwm-backlight: add "max-brightness" property
From: Andrew Bresticker @ 2013-08-12 22:04 UTC (permalink / raw)
To: Richard Purdie, Jingoo Han
Cc: Olof Johansson, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Thierry Reding,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, devicetree,
linux-doc, linux-kernel, linux-pwm, linux-fbdev,
Andrew Bresticker
Specifying each individual brightness value via the "brightness-levels"
property can be a pain if we want to use a large continuous range of
brightness values. Add the property "max-brightness", which can be
given in place of "brightness-levels", that specifies that all values
between 0 and the given value can be used.
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
.../bindings/video/backlight/pwm-backlight.txt | 22 +++++++++---
drivers/video/backlight/pwm_bl.c | 39 +++++++++++++---------
2 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 1e4fc72..856dfc9 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -3,15 +3,21 @@ pwm-backlight bindings
Required properties:
- compatible: "pwm-backlight"
- pwms: OF device-tree PWM specification (see PWM binding[0])
+ - one of "brightness-levels" or "max-brightness", described below
+ - default-brightness-level: the default brightness level (index into the array
+ defined by the "brightness-levels" property or a value between 0 and
+ "max-brightness")
+
+Optional properties:
- brightness-levels: Array of distinct brightness levels. Typically these
are in the range from 0 to 255, but any range starting at 0 will do.
The actual brightness level (PWM duty cycle) will be interpolated
from these values. 0 means a 0% duty cycle (darkest/off), while the
last value in the array represents a 100% duty cycle (brightest).
- - default-brightness-level: the default brightness level (index into the
- array defined by the "brightness-levels" property)
-
-Optional properties:
+ - max-brightness: Instead of specifying a complete set of brightness
+ levels, a single maximum brightness value may be given. This indicates
+ that all integers between 0 and max-brightness are valid brightness
+ values.
- pwm-names: a list of names for the PWM devices specified in the
"pwms" property (see PWM binding[0])
@@ -26,3 +32,11 @@ Example:
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <6>;
};
+or:
+ backlight {
+ compatible = "pwm-backlight";
+ pwms = <&pwm 0 1000000 0>;
+
+ max-brightness = <1024>;
+ default-brightness-level = <700>;
+ };
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 1fea627..92973b1 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -98,7 +98,6 @@ static int pwm_backlight_parse_dt(struct device *dev,
struct platform_pwm_backlight_data *data)
{
struct device_node *node = dev->of_node;
- struct property *prop;
int length;
u32 value;
int ret;
@@ -108,34 +107,44 @@ static int pwm_backlight_parse_dt(struct device *dev,
memset(data, 0, sizeof(*data));
- /* determine the number of brightness levels */
- prop = of_find_property(node, "brightness-levels", &length);
- if (!prop)
- return -EINVAL;
+ if (of_find_property(node, "brightness-levels", &length)) {
+ data->max_brightness = length / sizeof(u32);
- data->max_brightness = length / sizeof(u32);
+ /* read brightness levels from DT property */
+ if (data->max_brightness > 0) {
+ size_t size = sizeof(*data->levels) *
+ data->max_brightness;
- /* read brightness levels from DT property */
- if (data->max_brightness > 0) {
- size_t size = sizeof(*data->levels) * data->max_brightness;
-
- data->levels = devm_kzalloc(dev, size, GFP_KERNEL);
- if (!data->levels)
- return -ENOMEM;
+ data->levels = devm_kzalloc(dev, size, GFP_KERNEL);
+ if (!data->levels)
+ return -ENOMEM;
- ret = of_property_read_u32_array(node, "brightness-levels",
+ ret = of_property_read_u32_array(node,
+ "brightness-levels",
data->levels,
data->max_brightness);
+ if (ret < 0)
+ return ret;
+
+ data->max_brightness--;
+ }
+ } else {
+ ret = of_property_read_u32(node, "max-brightness",
+ &value);
if (ret < 0)
return ret;
+ /* brightness values are 0 to max-brightness */
+ data->max_brightness = value;
+ }
+
+ if (data->max_brightness > 0) {
ret = of_property_read_u32(node, "default-brightness-level",
&value);
if (ret < 0)
return ret;
data->dft_brightness = value;
- data->max_brightness--;
}
/*
--
1.8.3
^ permalink raw reply related
* Re: [PATCH] pwm-backlight: add "max-brightness" property
From: Stephen Warren @ 2013-08-12 22:43 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Richard Purdie, Jingoo Han, Olof Johansson, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Rob Landley,
Thierry Reding, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
devicetree, linux-doc, linux-kernel, linux-pwm, linux-fbdev
In-Reply-To: <1376345057-29895-1-git-send-email-abrestic@chromium.org>
On 08/12/2013 04:04 PM, Andrew Bresticker wrote:
> Specifying each individual brightness value via the "brightness-levels"
> property can be a pain if we want to use a large continuous range of
> brightness values. Add the property "max-brightness", which can be
> given in place of "brightness-levels", that specifies that all values
> between 0 and the given value can be used.
What about the non-linear nature of PWM duty cycle <-> (perceived)
brightness level? That's why the values are typically enumerated. I
guess if you use this new property, you'd use a value of say 16;
exposing levels 0..255 to a user is probably more than they want?
^ permalink raw reply
* Re: [PATCH] pwm-backlight: add "max-brightness" property
From: Thierry Reding @ 2013-08-13 7:34 UTC (permalink / raw)
To: Andrew Bresticker
Cc: Richard Purdie, Jingoo Han, Olof Johansson, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
Rob Landley, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
devicetree, linux-doc, linux-kernel, linux-pwm, linux-fbdev
In-Reply-To: <1376345057-29895-1-git-send-email-abrestic@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 829 bytes --]
On Mon, Aug 12, 2013 at 03:04:17PM -0700, Andrew Bresticker wrote:
> Specifying each individual brightness value via the "brightness-levels"
> property can be a pain if we want to use a large continuous range of
> brightness values. Add the property "max-brightness", which can be
> given in place of "brightness-levels", that specifies that all values
> between 0 and the given value can be used.
This was actually part of my first attempt at a DT binding for this
driver as well. Some discussion ensued and we ended up changing the
binding to what it is now. And I think there was even an attempt already
at adding the same thing you propose (albeit under a different name). So
instead of repeating myself I'll just point you at the earlier thread:
https://groups.google.com/forum/#!topic/linux.kernel/UkXktWt6zI0
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 00/22] OMAPDSS: DT support
From: Tony Lindgren @ 2013-08-13 7:54 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
Laurent Pinchart, Nishanth Menon, Felipe Balbi, Santosh Shilimkar
In-Reply-To: <1376037547-10859-1-git-send-email-tomi.valkeinen@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [130809 01:46]:
>
> So as is evident, I have things in my mind that should be improved. Maybe
> the most important question for short term future is:
>
> Can we add DSS DT bindings for OMAP4 as unstable bindings? It would give us
> some proper testing of the related code, and would also allow us to remove
> the related hacks (which don't even work quite right). However, I have no
> idea yet when the unstable DSS bindings would turn stable.
>
> If we shouldn't add the bindings as unstable, when should the bindings be
> added? Wait until CDF is in the mainline, and use that?
I don't think we should add any temporary bindings as it's going to be
a pain to support those in the long run. I suggest you initially just
stick to established bindings for the basic hardware IO address and
interrupts etc, then those should still be valid with the generic panel
bindings later on.
Regards,
Tony
^ permalink raw reply
* [RFC PATCH v6 0/2] Introduce buffer synchronization framework
From: Inki Dae @ 2013-08-13 9:19 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
Hi all,
This patch set introduces a buffer synchronization framework based
on DMA BUF[1] and based on ww-mutexes[2] for lock mechanism, and
may be final RFC.
The purpose of this framework is to provide not only buffer access
control to CPU and CPU, and CPU and DMA, and DMA and DMA but also
easy-to-use interfaces for device drivers and user application.
In addtion, this patch set suggests a way for enhancing performance.
For generic user mode interface, we have used fcntl and select system
call[3]. As you know, user application sees a buffer object as a dma-buf
file descriptor. So fcntl() call with the file descriptor means to lock
some buffer region being managed by the dma-buf object. And select() call
means to wait for the completion of CPU or DMA access to the dma-buf
without locking. For more detail, you can refer to the dma-buf-sync.txt
in Documentation/
There are some cases we should use this buffer synchronization framework.
One of which is to primarily enhance GPU rendering performance on Tizen
platform in case of 3d app with compositing mode that 3d app draws
something in off-screen buffer, and Web app.
In case of 3d app with compositing mode which is not a full screen mode,
the app calls glFlush to submit 3d commands to GPU driver instead of
glFinish for more performance. The reason we call glFlush is that glFinish
blocks caller's task until the execution of the 2d commands is completed.
Thus, that makes GPU and CPU more idle. As result, 3d rendering performance
with glFinish is quite lower than glFlush. However, the use of glFlush has
one issue that the a buffer shared with GPU could be broken when CPU
accesses the buffer at once after glFlush because CPU cannot be aware of
the completion of GPU access to the buffer. Of course, the app can be aware
of that time using eglWaitGL but this function is valid only in case of the
same process.
In case of Tizen, there are some applications that one process draws
something in its own off-screen buffer (pixmap buffer) using CPU, and other
process gets a off-screen buffer (window buffer) from Xorg using
DRI2GetBuffers, and then composites the pixmap buffer with the window buffer
using GPU, and finally page flip.
Web app based on HTML5 also has the same issue. Web browser and its web app
are different process. The web app draws something in its own pixmap buffer,
and then the web browser gets a window buffer from Xorg, and then composites
the pixmap buffer with the window buffer. And finally, page flip.
Thus, in such cases, a shared buffer could be broken as one process draws
something in pixmap buffer using CPU, when other process composites the
pixmap buffer with window buffer using GPU without any locking mechanism.
That is why we need user land locking interface, fcntl system call.
And last one is a deferred page flip issue. This issue is that a window
buffer rendered can be displayed on screen in about 32ms in worst case:
assume that the gpu rendering is completed within 16ms.
That can be incurred when compositing a pixmap buffer with a window buffer
using GPU and when vsync is just started. At this time, Xorg waits for
a vblank event to get a window buffer so 3d rendering will be delayed
up to about 16ms. As a result, the window buffer would be displayed in
about two vsyncs (about 32ms) and in turn, that would show slow
responsiveness.
For this, we could enhance the responsiveness with locking
mechanism: skipping one vblank wait. I guess in the similar reason,
Android, Chrome OS, and other platforms are using their own locking
mechanisms; Android sync driver, KDS, and DMA fence.
The below shows the deferred page flip issue in worst case,
|------------ <- vsync signal
|<------ DRI2GetBuffers
|
|
|
|------------ <- vsync signal
|<------ Request gpu rendering
time |
|
|<------ Request page flip (deferred)
|------------ <- vsync signal
|<------ Displayed on screen
|
|
|
|------------ <- vsync signal
Thanks,
Inki Dae
References:
[1] http://lwn.net/Articles/470339/
[2] https://patchwork.kernel.org/patch/2625361/
[3] http://linux.die.net/man/2/fcntl
Inki Dae (2):
[RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
[RFC PATCH v2] dma-buf: Add user interfaces for dmabuf sync support.
Documentation/dma-buf-sync.txt | 285 +++++++++++++++++
drivers/base/Kconfig | 7 +
drivers/base/Makefile | 1 +
drivers/base/dma-buf.c | 85 +++++
drivers/base/dmabuf-sync.c | 678 ++++++++++++++++++++++++++++++++++++++++
include/linux/dma-buf.h | 16 +
include/linux/dmabuf-sync.h | 191 +++++++++++
7 files changed, 1263 insertions(+), 0 deletions(-)
create mode 100644 Documentation/dma-buf-sync.txt
create mode 100644 drivers/base/dmabuf-sync.c
create mode 100644 include/linux/dmabuf-sync.h
--
1.7.5.4
^ permalink raw reply
* [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
From: Inki Dae @ 2013-08-13 9:19 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
In-Reply-To: <1376385576-9039-1-git-send-email-inki.dae@samsung.com>
This patch adds a buffer synchronization framework based on DMA BUF[1]
and and based on ww-mutexes[2] for lock mechanism.
The purpose of this framework is to provide not only buffer access control
to CPU and DMA but also easy-to-use interfaces for device drivers and
user application. This framework can be used for all dma devices using
system memory as dma buffer, especially for most ARM based SoCs.
Changelog v6:
- Fix sync lock to multiple reads.
- Add select system call support.
. Wake up poll_wait when a dmabuf is unlocked.
- Remove unnecessary the use of mutex lock.
- Add private backend ops callbacks.
. This ops has one callback for device drivers to clean up their
sync object resource when the sync object is freed. For this,
device drivers should implement the free callback properly.
- Update document file.
Changelog v5:
- Rmove a dependence on reservation_object: the reservation_object is used
to hook up to ttm and dma-buf for easy sharing of reservations across
devices. However, the dmabuf sync can be used for all dma devices; v4l2
and drm based drivers, so doesn't need the reservation_object anymore.
With regared to this, it adds 'void *sync' to dma_buf structure.
- All patches are rebased on mainline, Linux v3.10.
Changelog v4:
- Add user side interface for buffer synchronization mechanism and update
descriptions related to the user side interface.
Changelog v3:
- remove cache operation relevant codes and update document file.
Changelog v2:
- use atomic_add_unless to avoid potential bug.
- add a macro for checking valid access type.
- code clean.
The mechanism of this framework has the following steps,
1. Register dmabufs to a sync object - A task gets a new sync object and
can add one or more dmabufs that the task wants to access.
This registering should be performed when a device context or an event
context such as a page flip event is created or before CPU accesses a shared
buffer.
dma_buf_sync_get(a sync object, a dmabuf);
2. Lock a sync object - A task tries to lock all dmabufs added in its own
sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid dead
lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
and DMA. Taking a lock means that others cannot access all locked dmabufs
until the task that locked the corresponding dmabufs, unlocks all the locked
dmabufs.
This locking should be performed before DMA or CPU accesses these dmabufs.
dma_buf_sync_lock(a sync object);
3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
object. The unlock means that the DMA or CPU accesses to the dmabufs have
been completed so that others may access them.
This unlocking should be performed after DMA or CPU has completed accesses
to the dmabufs.
dma_buf_sync_unlock(a sync object);
4. Unregister one or all dmabufs from a sync object - A task unregisters
the given dmabufs from the sync object. This means that the task dosen't
want to lock the dmabufs.
The unregistering should be performed after DMA or CPU has completed
accesses to the dmabufs or when dma_buf_sync_lock() is failed.
dma_buf_sync_put(a sync object, a dmabuf);
dma_buf_sync_put_all(a sync object);
The described steps may be summarized as:
get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
This framework includes the following two features.
1. read (shared) and write (exclusive) locks - A task is required to declare
the access type when the task tries to register a dmabuf;
READ, WRITE, READ DMA, or WRITE DMA.
The below is example codes,
struct dmabuf_sync *sync;
sync = dmabuf_sync_init(...);
...
dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
...
And the below can be used as access types:
DMA_BUF_ACCESS_R - CPU will access a buffer for read.
DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or
write.
2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
A task may never try to unlock a buffer after taking a lock to the buffer.
In this case, a timer handler to the corresponding sync object is called
in five (default) seconds and then the timed-out buffer is unlocked by work
queue handler to avoid lockups and to enforce resources of the buffer.
The below is how to use interfaces for device driver:
1. Allocate and Initialize a sync object:
static void xxx_dmabuf_sync_free(void *priv)
{
struct xxx_context *ctx = priv;
if (!ctx)
return;
ctx->sync = NULL;
}
...
static struct dmabuf_sync_priv_ops driver_specific_ops = {
.free = xxx_dmabuf_sync_free,
};
...
struct dmabuf_sync *sync;
sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
...
2. Add a dmabuf to the sync object when setting up dma buffer relevant
registers:
dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
...
3. Lock all dmabufs of the sync object before DMA or CPU accesses
the dmabufs:
dmabuf_sync_lock(sync);
...
4. Now CPU or DMA can access all dmabufs locked in step 3.
5. Unlock all dmabufs added in a sync object after DMA or CPU access
to these dmabufs is completed:
dmabuf_sync_unlock(sync);
And call the following functions to release all resources,
dmabuf_sync_put_all(sync);
dmabuf_sync_fini(sync);
You can refer to actual example codes:
"drm/exynos: add dmabuf sync support for g2d driver" and
"drm/exynos: add dmabuf sync support for kms framework" from
https://git.kernel.org/cgit/linux/kernel/git/daeinki/
drm-exynos.git/log/?h=dmabuf-sync
And this framework includes fcntl system call[3] as interfaces exported
to user. As you know, user sees a buffer object as a dma-buf file descriptor.
So fcntl() call with the file descriptor means to lock some buffer region being
managed by the dma-buf object.
The below is how to use interfaces for user application:
fcntl system call:
struct flock filelock;
1. Lock a dma buf:
filelock.l_type = F_WRLCK or F_RDLCK;
/* lock entire region to the dma buf. */
filelock.lwhence = SEEK_CUR;
filelock.l_start = 0;
filelock.l_len = 0;
fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
...
CPU access to the dma buf
2. Unlock a dma buf:
filelock.l_type = F_UNLCK;
fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
close(dmabuf fd) call would also unlock the dma buf. And for more
detail, please refer to [3]
select system call:
fd_set wdfs or rdfs;
FD_ZERO(&wdfs or &rdfs);
FD_SET(fd, &wdfs or &rdfs);
select(fd + 1, &rdfs, NULL, NULL, NULL);
or
select(fd + 1, NULL, &wdfs, NULL, NULL);
Every time select system call is called, a caller will wait for
the completion of DMA or CPU access to a shared buffer if there
is someone accessing the shared buffer; locked the shared buffer.
However, if no anyone then select system call will be returned
at once.
References:
[1] http://lwn.net/Articles/470339/
[2] https://patchwork.kernel.org/patch/2625361/
[3] http://linux.die.net/man/2/fcntl
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Documentation/dma-buf-sync.txt | 285 +++++++++++++++++
drivers/base/Kconfig | 7 +
drivers/base/Makefile | 1 +
drivers/base/dma-buf.c | 4 +
drivers/base/dmabuf-sync.c | 678 ++++++++++++++++++++++++++++++++++++++++
include/linux/dma-buf.h | 16 +
include/linux/dmabuf-sync.h | 190 +++++++++++
7 files changed, 1181 insertions(+), 0 deletions(-)
create mode 100644 Documentation/dma-buf-sync.txt
create mode 100644 drivers/base/dmabuf-sync.c
create mode 100644 include/linux/dmabuf-sync.h
diff --git a/Documentation/dma-buf-sync.txt b/Documentation/dma-buf-sync.txt
new file mode 100644
index 0000000..8023d06
--- /dev/null
+++ b/Documentation/dma-buf-sync.txt
@@ -0,0 +1,285 @@
+ DMA Buffer Synchronization Framework
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+ Inki Dae
+ <inki dot dae at samsung dot com>
+ <daeinki at gmail dot com>
+
+This document is a guide for device-driver writers describing the DMA buffer
+synchronization API. This document also describes how to use the API to
+use buffer synchronization mechanism between DMA and DMA, CPU and DMA, and
+CPU and CPU.
+
+The DMA Buffer synchronization API provides buffer synchronization mechanism;
+i.e., buffer access control to CPU and DMA, and easy-to-use interfaces for
+device drivers and user application. And this API can be used for all dma
+devices using system memory as dma buffer, especially for most ARM based SoCs.
+
+
+Motivation
+----------
+
+Buffer synchronization issue between DMA and DMA:
+ Sharing a buffer, a device cannot be aware of when the other device
+ will access the shared buffer: a device may access a buffer containing
+ wrong data if the device accesses the shared buffer while another
+ device is still accessing the shared buffer.
+ Therefore, a user process should have waited for the completion of DMA
+ access by another device before a device tries to access the shared
+ buffer.
+
+Buffer synchronization issue between CPU and DMA:
+ A user process should consider that when having to send a buffer, filled
+ by CPU, to a device driver for the device driver to access the buffer as
+ a input buffer while CPU and DMA are sharing the buffer.
+ This means that the user process needs to understand how the device
+ driver is worked. Hence, the conventional mechanism not only makes
+ user application complicated but also incurs performance overhead.
+
+Buffer synchronization issue between CPU and CPU:
+ In case that two processes share one buffer; shared with DMA also,
+ they may need some mechanism to allow process B to access the shared
+ buffer after the completion of CPU access by process A.
+ Therefore, process B should have waited for the completion of CPU access
+ by process A using the mechanism before trying to access the shared
+ buffer.
+
+What is the best way to solve these buffer synchronization issues?
+ We may need a common object that a device driver and a user process
+ notify the common object of when they try to access a shared buffer.
+ That way we could decide when we have to allow or not to allow for CPU
+ or DMA to access the shared buffer through the common object.
+ If so, what could become the common object? Right, that's a dma-buf[1].
+ Now we have already been using the dma-buf to share one buffer with
+ other drivers.
+
+
+Basic concept
+-------------
+
+The mechanism of this framework has the following steps,
+ 1. Register dmabufs to a sync object - A task gets a new sync object and
+ can add one or more dmabufs that the task wants to access.
+ This registering should be performed when a device context or an event
+ context such as a page flip event is created or before CPU accesses a shared
+ buffer.
+
+ dma_buf_sync_get(a sync object, a dmabuf);
+
+ 2. Lock a sync object - A task tries to lock all dmabufs added in its own
+ sync object. Basically, the lock mechanism uses ww-mutexes[2] to avoid dead
+ lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
+ and DMA. Taking a lock means that others cannot access all locked dmabufs
+ until the task that locked the corresponding dmabufs, unlocks all the locked
+ dmabufs.
+ This locking should be performed before DMA or CPU accesses these dmabufs.
+
+ dma_buf_sync_lock(a sync object);
+
+ 3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
+ object. The unlock means that the DMA or CPU accesses to the dmabufs have
+ been completed so that others may access them.
+ This unlocking should be performed after DMA or CPU has completed accesses
+ to the dmabufs.
+
+ dma_buf_sync_unlock(a sync object);
+
+ 4. Unregister one or all dmabufs from a sync object - A task unregisters
+ the given dmabufs from the sync object. This means that the task dosen't
+ want to lock the dmabufs.
+ The unregistering should be performed after DMA or CPU has completed
+ accesses to the dmabufs or when dma_buf_sync_lock() is failed.
+
+ dma_buf_sync_put(a sync object, a dmabuf);
+ dma_buf_sync_put_all(a sync object);
+
+ The described steps may be summarized as:
+ get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
+
+This framework includes the following two features.
+ 1. read (shared) and write (exclusive) locks - A task is required to declare
+ the access type when the task tries to register a dmabuf;
+ READ, WRITE, READ DMA, or WRITE DMA.
+
+ The below is example codes,
+ struct dmabuf_sync *sync;
+
+ sync = dmabuf_sync_init(NULL, "test sync");
+
+ dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
+ ...
+
+ 2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
+ A task may never try to unlock a buffer after taking a lock to the buffer.
+ In this case, a timer handler to the corresponding sync object is called
+ in five (default) seconds and then the timed-out buffer is unlocked by work
+ queue handler to avoid lockups and to enforce resources of the buffer.
+
+
+Access types
+------------
+
+DMA_BUF_ACCESS_R - CPU will access a buffer for read.
+DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
+DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
+DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or write.
+
+
+Generic user interfaces
+-----------------------
+
+And this framework includes fcntl system call[3] as interfaces exported
+to user. As you know, user sees a buffer object as a dma-buf file descriptor.
+So fcntl() call with the file descriptor means to lock some buffer region being
+managed by the dma-buf object.
+
+
+API set
+-------
+
+bool is_dmabuf_sync_supported(void)
+ - Check if dmabuf sync is supported or not.
+
+struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void priv*)
+ - Allocate and initialize a new sync object. The caller can get a new
+ sync object for buffer synchronization. ops is used for device driver
+ to clean up its own sync object. For this, each device driver should
+ implement a free callback. priv is used for device driver to get its
+ device context when free callback is called.
+
+void dmabuf_sync_fini(struct dmabuf_sync *sync)
+ - Release all resources to the sync object.
+
+int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
+ unsigned int type)
+ - Get dmabuf sync object. Internally, this function allocates
+ a dmabuf_sync object and adds a given dmabuf to it, and also takes
+ a reference to the dmabuf. The caller can tie up multiple dmabufs
+ into one sync object by calling this function several times.
+
+void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
+ - Put dmabuf sync object to a given dmabuf. Internally, this function
+ removes a given dmabuf from a sync object and remove the sync object.
+ At this time, the dmabuf is putted.
+
+void dmabuf_sync_put_all(struct dmabuf_sync *sync)
+ - Put dmabuf sync object to dmabufs. Internally, this function removes
+ all dmabufs from a sync object and remove the sync object.
+ At this time, all dmabufs are putted.
+
+int dmabuf_sync_lock(struct dmabuf_sync *sync)
+ - Lock all dmabufs added in a sync object. The caller should call this
+ function prior to CPU or DMA access to the dmabufs so that others can
+ not access the dmabufs. Internally, this function avoids dead lock
+ issue with ww-mutexes.
+
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf)
+ - Lock a dmabuf. The caller should call this
+ function prior to CPU or DMA access to the dmabuf so that others can
+ not access the dmabuf.
+
+int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+ - Unlock all dmabufs added in a sync object. The caller should call
+ this function after CPU or DMA access to the dmabufs is completed so
+ that others can access the dmabufs.
+
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+ - Unlock a dmabuf. The caller should call this function after CPU or
+ DMA access to the dmabuf is completed so that others can access
+ the dmabuf.
+
+
+Tutorial for device driver
+--------------------------
+
+1. Allocate and Initialize a sync object:
+ static void xxx_dmabuf_sync_free(void *priv)
+ {
+ struct xxx_context *ctx = priv;
+
+ if (!ctx)
+ return;
+
+ ctx->sync = NULL;
+ }
+ ...
+
+ static struct dmabuf_sync_priv_ops driver_specific_ops = {
+ .free = xxx_dmabuf_sync_free,
+ };
+ ...
+
+ struct dmabuf_sync *sync;
+
+ sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
+ ...
+
+2. Add a dmabuf to the sync object when setting up dma buffer relevant registers:
+ dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
+ ...
+
+3. Lock all dmabufs of the sync object before DMA or CPU accesses the dmabufs:
+ dmabuf_sync_lock(sync);
+ ...
+
+4. Now CPU or DMA can access all dmabufs locked in step 3.
+
+5. Unlock all dmabufs added in a sync object after DMA or CPU access to these
+ dmabufs is completed:
+ dmabuf_sync_unlock(sync);
+
+ And call the following functions to release all resources,
+ dmabuf_sync_put_all(sync);
+ dmabuf_sync_fini(sync);
+
+
+Tutorial for user application
+-----------------------------
+fcntl system call:
+
+ struct flock filelock;
+
+1. Lock a dma buf:
+ filelock.l_type = F_WRLCK or F_RDLCK;
+
+ /* lock entire region to the dma buf. */
+ filelock.lwhence = SEEK_CUR;
+ filelock.l_start = 0;
+ filelock.l_len = 0;
+
+ fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
+ ...
+ CPU access to the dma buf
+
+2. Unlock a dma buf:
+ filelock.l_type = F_UNLCK;
+
+ fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
+
+ close(dmabuf fd) call would also unlock the dma buf. And for more
+ detail, please refer to [3]
+
+
+select system call:
+
+ fd_set wdfs or rdfs;
+
+ FD_ZERO(&wdfs or &rdfs);
+ FD_SET(fd, &wdfs or &rdfs);
+
+ select(fd + 1, &rdfs, NULL, NULL, NULL);
+ or
+ select(fd + 1, NULL, &wdfs, NULL, NULL);
+
+ Every time select system call is called, a caller will wait for
+ the completion of DMA or CPU access to a shared buffer if there is
+ someone accessing the shared buffer; locked the shared buffer.
+ However, if no anyone then select system call will be returned
+ at once.
+
+References:
+[1] http://lwn.net/Articles/470339/
+[2] https://patchwork.kernel.org/patch/2625361/
+[3] http://linux.die.net/man/2/fcntl
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 5daa259..35e1518 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -200,6 +200,13 @@ config DMA_SHARED_BUFFER
APIs extension; the file's descriptor can then be passed on to other
driver.
+config DMABUF_SYNC
+ bool "DMABUF Synchronization Framework"
+ depends on DMA_SHARED_BUFFER
+ help
+ This option enables dmabuf sync framework for buffer synchronization between
+ DMA and DMA, CPU and DMA, and CPU and CPU.
+
config CMA
bool "Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 48029aa..e06a5d7 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -11,6 +11,7 @@ obj-y += power/
obj-$(CONFIG_HAS_DMA) += dma-mapping.o
obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o reservation.o
+obj-$(CONFIG_DMABUF_SYNC) += dmabuf-sync.o
obj-$(CONFIG_ISA) += isa.o
obj-$(CONFIG_FW_LOADER) += firmware_class.o
obj-$(CONFIG_NUMA) += node.o
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 6687ba7..4aca57a 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -29,6 +29,7 @@
#include <linux/export.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/dmabuf-sync.h>
static inline int is_dma_buf_file(struct file *);
@@ -56,6 +57,8 @@ static int dma_buf_release(struct inode *inode, struct file *file)
list_del(&dmabuf->list_node);
mutex_unlock(&db_list.lock);
+ dmabuf_sync_reservation_fini(dmabuf);
+
kfree(dmabuf);
return 0;
}
@@ -134,6 +137,7 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
+ dmabuf_sync_reservation_init(dmabuf);
dmabuf->file = file;
mutex_init(&dmabuf->lock);
diff --git a/drivers/base/dmabuf-sync.c b/drivers/base/dmabuf-sync.c
new file mode 100644
index 0000000..fbe711c
--- /dev/null
+++ b/drivers/base/dmabuf-sync.c
@@ -0,0 +1,678 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics Co.Ltd
+ * Authors:
+ * Inki Dae <inki.dae@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/debugfs.h>
+#include <linux/uaccess.h>
+
+#include <linux/dmabuf-sync.h>
+
+#define MAX_SYNC_TIMEOUT 5 /* Second. */
+
+int dmabuf_sync_enabled = 1;
+
+MODULE_PARM_DESC(enabled, "Check if dmabuf sync is supported or not");
+module_param_named(enabled, dmabuf_sync_enabled, int, 0444);
+
+DEFINE_WW_CLASS(dmabuf_sync_ww_class);
+EXPORT_SYMBOL(dmabuf_sync_ww_class);
+
+static void dmabuf_sync_timeout_worker(struct work_struct *work)
+{
+ struct dmabuf_sync *sync = container_of(work, struct dmabuf_sync, work);
+ struct dmabuf_sync_object *sobj;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ BUG_ON(!sobj->robj);
+
+ mutex_lock(&sobj->robj->lock);
+
+ printk(KERN_WARNING "%s: timeout = 0x%x [type = %d:%d, " \
+ "refcnt = %d, locked = %d]\n",
+ sync->name, (u32)sobj->dmabuf,
+ sobj->robj->accessed_type,
+ sobj->access_type,
+ atomic_read(&sobj->robj->shared_cnt),
+ sobj->robj->locked);
+
+ /* unlock only valid sync object. */
+ if (!sobj->robj->locked) {
+ mutex_unlock(&sobj->robj->lock);
+ continue;
+ }
+
+ if (sobj->robj->polled) {
+ sobj->robj->poll_event = true;
+ sobj->robj->polled = false;
+ wake_up_interruptible(&sobj->robj->poll_wait);
+ }
+
+ if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
+ mutex_unlock(&sobj->robj->lock);
+ continue;
+ }
+
+ mutex_unlock(&sobj->robj->lock);
+
+ ww_mutex_unlock(&sobj->robj->sync_lock);
+
+ mutex_lock(&sobj->robj->lock);
+ sobj->robj->locked = false;
+
+ if (sobj->access_type & DMA_BUF_ACCESS_R)
+ printk(KERN_WARNING "%s: r-unlocked = 0x%x\n",
+ sync->name, (u32)sobj->dmabuf);
+ else
+ printk(KERN_WARNING "%s: w-unlocked = 0x%x\n",
+ sync->name, (u32)sobj->dmabuf);
+
+ mutex_unlock(&sobj->robj->lock);
+ }
+
+ sync->status = 0;
+ mutex_unlock(&sync->lock);
+
+ dmabuf_sync_put_all(sync);
+ dmabuf_sync_fini(sync);
+}
+
+static void dmabuf_sync_lock_timeout(unsigned long arg)
+{
+ struct dmabuf_sync *sync = (struct dmabuf_sync *)arg;
+
+ schedule_work(&sync->work);
+}
+
+static int dmabuf_sync_lock_objs(struct dmabuf_sync *sync,
+ struct ww_acquire_ctx *ctx)
+{
+ struct dmabuf_sync_object *contended_sobj = NULL;
+ struct dmabuf_sync_object *res_sobj = NULL;
+ struct dmabuf_sync_object *sobj = NULL;
+ int ret;
+
+ if (ctx)
+ ww_acquire_init(ctx, &dmabuf_sync_ww_class);
+
+retry:
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ if (WARN_ON(!sobj->robj))
+ continue;
+
+ mutex_lock(&sobj->robj->lock);
+
+ /* Don't lock in case of read and read. */
+ if (sobj->robj->accessed_type & DMA_BUF_ACCESS_R &&
+ sobj->access_type & DMA_BUF_ACCESS_R) {
+ atomic_inc(&sobj->robj->shared_cnt);
+ mutex_unlock(&sobj->robj->lock);
+ continue;
+ }
+
+ if (sobj = res_sobj) {
+ res_sobj = NULL;
+ mutex_unlock(&sobj->robj->lock);
+ continue;
+ }
+
+ mutex_unlock(&sobj->robj->lock);
+
+ ret = ww_mutex_lock(&sobj->robj->sync_lock, ctx);
+ if (ret < 0) {
+ contended_sobj = sobj;
+
+ if (ret = -EDEADLK)
+ printk(KERN_WARNING"%s: deadlock = 0x%x\n",
+ sync->name, (u32)sobj->dmabuf);
+ goto err;
+ }
+
+ mutex_lock(&sobj->robj->lock);
+ sobj->robj->locked = true;
+
+ mutex_unlock(&sobj->robj->lock);
+ }
+
+ if (ctx)
+ ww_acquire_done(ctx);
+
+ init_timer(&sync->timer);
+
+ sync->timer.data = (unsigned long)sync;
+ sync->timer.function = dmabuf_sync_lock_timeout;
+ sync->timer.expires = jiffies + (HZ * MAX_SYNC_TIMEOUT);
+
+ add_timer(&sync->timer);
+
+ return 0;
+
+err:
+ list_for_each_entry_continue_reverse(sobj, &sync->syncs, head) {
+ mutex_lock(&sobj->robj->lock);
+
+ /* Don't need to unlock in case of read and read. */
+ if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
+ mutex_unlock(&sobj->robj->lock);
+ continue;
+ }
+
+ ww_mutex_unlock(&sobj->robj->sync_lock);
+ sobj->robj->locked = false;
+
+ mutex_unlock(&sobj->robj->lock);
+ }
+
+ if (res_sobj) {
+ mutex_lock(&res_sobj->robj->lock);
+
+ if (!atomic_add_unless(&res_sobj->robj->shared_cnt, -1, 1)) {
+ ww_mutex_unlock(&res_sobj->robj->sync_lock);
+ res_sobj->robj->locked = false;
+ }
+
+ mutex_unlock(&res_sobj->robj->lock);
+ }
+
+ if (ret = -EDEADLK) {
+ ww_mutex_lock_slow(&contended_sobj->robj->sync_lock, ctx);
+ res_sobj = contended_sobj;
+
+ goto retry;
+ }
+
+ if (ctx)
+ ww_acquire_fini(ctx);
+
+ return ret;
+}
+
+static void dmabuf_sync_unlock_objs(struct dmabuf_sync *sync,
+ struct ww_acquire_ctx *ctx)
+{
+ struct dmabuf_sync_object *sobj;
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ mutex_lock(&sobj->robj->lock);
+
+ if (sobj->robj->polled) {
+ sobj->robj->poll_event = true;
+ sobj->robj->polled = false;
+ wake_up_interruptible(&sobj->robj->poll_wait);
+ }
+
+ if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
+ mutex_unlock(&sobj->robj->lock);
+ continue;
+ }
+
+ mutex_unlock(&sobj->robj->lock);
+
+ ww_mutex_unlock(&sobj->robj->sync_lock);
+
+ mutex_lock(&sobj->robj->lock);
+ sobj->robj->locked = false;
+ mutex_unlock(&sobj->robj->lock);
+ }
+
+ mutex_unlock(&sync->lock);
+
+ if (ctx)
+ ww_acquire_fini(ctx);
+
+ del_timer(&sync->timer);
+}
+
+/**
+ * is_dmabuf_sync_supported - Check if dmabuf sync is supported or not.
+ */
+bool is_dmabuf_sync_supported(void)
+{
+ return dmabuf_sync_enabled = 1;
+}
+EXPORT_SYMBOL(is_dmabuf_sync_supported);
+
+/**
+ * dmabuf_sync_init - Allocate and initialize a dmabuf sync.
+ *
+ * @priv: A device private data.
+ * @name: A sync object name.
+ *
+ * This function should be called when a device context or an event
+ * context such as a page flip event is created. And the created
+ * dmabuf_sync object should be set to the context.
+ * The caller can get a new sync object for buffer synchronization
+ * through this function.
+ */
+struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv)
+{
+ struct dmabuf_sync *sync;
+
+ sync = kzalloc(sizeof(*sync), GFP_KERNEL);
+ if (!sync)
+ return ERR_PTR(-ENOMEM);
+
+ strncpy(sync->name, name, ARRAY_SIZE(sync->name) - 1);
+
+ sync->ops = ops;
+ sync->priv = priv;
+ INIT_LIST_HEAD(&sync->syncs);
+ mutex_init(&sync->lock);
+ INIT_WORK(&sync->work, dmabuf_sync_timeout_worker);
+
+ return sync;
+}
+EXPORT_SYMBOL(dmabuf_sync_init);
+
+/**
+ * dmabuf_sync_fini - Release a given dmabuf sync.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_init call to release relevant resources, and after
+ * dmabuf_sync_unlock function is called.
+ */
+void dmabuf_sync_fini(struct dmabuf_sync *sync)
+{
+ if (WARN_ON(!sync))
+ return;
+
+ if (sync->ops && sync->ops->free)
+ sync->ops->free(sync->priv);
+
+ kfree(sync);
+}
+EXPORT_SYMBOL(dmabuf_sync_fini);
+
+/*
+ * dmabuf_sync_get_obj - Add a given object to syncs list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @dmabuf: An object to dma_buf structure.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can be
+ * combined.
+ *
+ * This function creates and initializes a new dmabuf sync object and it adds
+ * the dmabuf sync object to syncs list to track and manage all dmabufs.
+ */
+static int dmabuf_sync_get_obj(struct dmabuf_sync *sync, struct dma_buf *dmabuf,
+ unsigned int type)
+{
+ struct dmabuf_sync_object *sobj;
+
+ if (!dmabuf->sync) {
+ WARN_ON(1);
+ return -EFAULT;
+ }
+
+ if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type))
+ return -EINVAL;
+
+ if ((type & DMA_BUF_ACCESS_RW) = DMA_BUF_ACCESS_RW)
+ type &= ~DMA_BUF_ACCESS_R;
+
+ sobj = kzalloc(sizeof(*sobj), GFP_KERNEL);
+ if (!sobj) {
+ WARN_ON(1);
+ return -ENOMEM;
+ }
+
+ get_dma_buf(dmabuf);
+
+ sobj->dmabuf = dmabuf;
+ sobj->robj = dmabuf->sync;
+ sobj->access_type = type;
+
+ mutex_lock(&sync->lock);
+ list_add_tail(&sobj->head, &sync->syncs);
+ mutex_unlock(&sync->lock);
+
+ return 0;
+}
+
+/*
+ * dmabuf_sync_put_obj - Release a given sync object.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_get_obj call to release a given sync object.
+ */
+static void dmabuf_sync_put_obj(struct dmabuf_sync *sync,
+ struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_object *sobj;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ if (sobj->dmabuf != dmabuf)
+ continue;
+
+ dma_buf_put(sobj->dmabuf);
+
+ list_del_init(&sobj->head);
+ kfree(sobj);
+ break;
+ }
+
+ if (list_empty(&sync->syncs))
+ sync->status = 0;
+
+ mutex_unlock(&sync->lock);
+}
+
+/*
+ * dmabuf_sync_put_objs - Release all sync objects of dmabuf_sync.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_get_obj call to release all sync objects.
+ */
+static void dmabuf_sync_put_objs(struct dmabuf_sync *sync)
+{
+ struct dmabuf_sync_object *sobj, *next;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry_safe(sobj, next, &sync->syncs, head) {
+ dma_buf_put(sobj->dmabuf);
+
+ list_del_init(&sobj->head);
+ kfree(sobj);
+ }
+
+ mutex_unlock(&sync->lock);
+
+ sync->status = 0;
+}
+
+/**
+ * dmabuf_sync_lock - lock all dmabufs added to syncs list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * The caller should call this function prior to CPU or DMA access to
+ * the dmabufs so that others can not access the dmabufs.
+ * Internally, this function avoids dead lock issue with ww-mutex.
+ */
+int dmabuf_sync_lock(struct dmabuf_sync *sync)
+{
+ int ret;
+
+ if (!sync) {
+ WARN_ON(1);
+ return -EFAULT;
+ }
+
+ if (list_empty(&sync->syncs))
+ return -EINVAL;
+
+ if (sync->status != DMABUF_SYNC_GOT)
+ return -EINVAL;
+
+ ret = dmabuf_sync_lock_objs(sync, &sync->ctx);
+ if (ret < 0) {
+ WARN_ON(1);
+ return ret;
+ }
+
+ sync->status = DMABUF_SYNC_LOCKED;
+
+ return ret;
+}
+EXPORT_SYMBOL(dmabuf_sync_lock);
+
+/**
+ * dmabuf_sync_unlock - unlock all objects added to syncs list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * The caller should call this function after CPU or DMA access to
+ * the dmabufs is completed so that others can access the dmabufs.
+ */
+int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+{
+ if (!sync) {
+ WARN_ON(1);
+ return -EFAULT;
+ }
+
+ /* If current dmabuf sync object wasn't reserved then just return. */
+ if (sync->status != DMABUF_SYNC_LOCKED)
+ return -EAGAIN;
+
+ dmabuf_sync_unlock_objs(sync, &sync->ctx);
+
+ return 0;
+}
+EXPORT_SYMBOL(dmabuf_sync_unlock);
+
+/**
+ * dmabuf_sync_single_lock - lock a dma buf.
+ *
+ * @dmabuf: A dma buf object that tries to lock.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can
+ * be combined with other.
+ * @wait: Indicate whether caller is blocked or not.
+ * true means that caller will be blocked, and false means that this
+ * function will return -EAGAIN if this caller can't take the lock
+ * right now.
+ *
+ * The caller should call this function prior to CPU or DMA access to the dmabuf
+ * so that others cannot access the dmabuf.
+ */
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
+ bool wait)
+{
+ struct dmabuf_sync_reservation *robj;
+
+ if (!dmabuf->sync) {
+ WARN_ON(1);
+ return -EFAULT;
+ }
+
+ if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type)) {
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
+ get_dma_buf(dmabuf);
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ /* Don't lock in case of read and read. */
+ if (robj->accessed_type & DMA_BUF_ACCESS_R && type & DMA_BUF_ACCESS_R) {
+ atomic_inc(&robj->shared_cnt);
+ mutex_unlock(&robj->lock);
+ return 0;
+ }
+
+ /*
+ * In case of F_SETLK, just return -EAGAIN if this dmabuf has already
+ * been locked.
+ */
+ if (!wait && robj->locked) {
+ mutex_unlock(&robj->lock);
+ dma_buf_put(dmabuf);
+ return -EAGAIN;
+ }
+
+ mutex_unlock(&robj->lock);
+
+ mutex_lock(&robj->sync_lock.base);
+
+ mutex_lock(&robj->lock);
+ robj->locked = true;
+ mutex_unlock(&robj->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(dmabuf_sync_single_lock);
+
+/**
+ * dmabuf_sync_single_unlock - unlock a dma buf.
+ *
+ * @dmabuf: A dma buf object that tries to unlock.
+ *
+ * The caller should call this function after CPU or DMA access to
+ * the dmabuf is completed so that others can access the dmabuf.
+ */
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *robj;
+
+ if (!dmabuf->sync) {
+ WARN_ON(1);
+ return;
+ }
+
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ if (robj->polled) {
+ robj->poll_event = true;
+ robj->polled = false;
+ wake_up_interruptible(&robj->poll_wait);
+ }
+
+ if (atomic_add_unless(&robj->shared_cnt, -1 , 1)) {
+ mutex_unlock(&robj->lock);
+ dma_buf_put(dmabuf);
+ return;
+ }
+
+ mutex_unlock(&robj->lock);
+
+ mutex_unlock(&robj->sync_lock.base);
+
+ mutex_lock(&robj->lock);
+ robj->locked = false;
+ mutex_unlock(&robj->lock);
+
+ dma_buf_put(dmabuf);
+
+ return;
+}
+EXPORT_SYMBOL(dmabuf_sync_single_unlock);
+
+/**
+ * dmabuf_sync_get - Get dmabuf sync object.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @sync_buf: A dmabuf object to be synchronized with others.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can
+ * be combined with other.
+ *
+ * This function should be called after dmabuf_sync_init function is called.
+ * The caller can tie up multiple dmabufs into one sync object by calling this
+ * function several times. Internally, this function allocates
+ * a dmabuf_sync_object and adds a given dmabuf to it, and also takes
+ * a reference to a dmabuf.
+ */
+int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf, unsigned int type)
+{
+ int ret;
+
+ if (!sync || !sync_buf) {
+ WARN_ON(1);
+ return -EFAULT;
+ }
+
+ ret = dmabuf_sync_get_obj(sync, sync_buf, type);
+ if (ret < 0) {
+ WARN_ON(1);
+ return ret;
+ }
+
+ sync->status = DMABUF_SYNC_GOT;
+
+ return 0;
+}
+EXPORT_SYMBOL(dmabuf_sync_get);
+
+/**
+ * dmabuf_sync_put - Put dmabuf sync object to a given dmabuf.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @dmabuf: An dmabuf object.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_get function is called to release the dmabuf, or
+ * dmabuf_sync_unlock function is called. Internally, this function
+ * removes a given dmabuf from a sync object and remove the sync object.
+ * At this time, the dmabuf is putted.
+ */
+void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
+{
+ if (!sync || !dmabuf) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ dmabuf_sync_put_obj(sync, dmabuf);
+}
+EXPORT_SYMBOL(dmabuf_sync_put);
+
+/**
+ * dmabuf_sync_put_all - Put dmabuf sync object to dmabufs.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_get function is called to release all sync objects, or
+ * dmabuf_sync_unlock function is called. Internally, this function
+ * removes dmabufs from a sync object and remove the sync object.
+ * At this time, all dmabufs are putted.
+ */
+void dmabuf_sync_put_all(struct dmabuf_sync *sync)
+{
+ if (!sync) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ dmabuf_sync_put_objs(sync);
+}
+EXPORT_SYMBOL(dmabuf_sync_put_all);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index dfac5ed..0109673 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -115,6 +115,7 @@ struct dma_buf_ops {
* @exp_name: name of the exporter; useful for debugging.
* @list_node: node for dma_buf accounting and debugging.
* @priv: exporter specific private data for this buffer object.
+ * @sync: sync object linked to this dma-buf
*/
struct dma_buf {
size_t size;
@@ -128,6 +129,7 @@ struct dma_buf {
const char *exp_name;
struct list_head list_node;
void *priv;
+ void *sync;
};
/**
@@ -148,6 +150,20 @@ struct dma_buf_attachment {
void *priv;
};
+#define DMA_BUF_ACCESS_R 0x1
+#define DMA_BUF_ACCESS_W 0x2
+#define DMA_BUF_ACCESS_DMA 0x4
+#define DMA_BUF_ACCESS_RW (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_W)
+#define DMA_BUF_ACCESS_DMA_R (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_DMA)
+#define DMA_BUF_ACCESS_DMA_W (DMA_BUF_ACCESS_W | DMA_BUF_ACCESS_DMA)
+#define DMA_BUF_ACCESS_DMA_RW (DMA_BUF_ACCESS_DMA_R | DMA_BUF_ACCESS_DMA_W)
+#define IS_VALID_DMA_BUF_ACCESS_TYPE(t) (t = DMA_BUF_ACCESS_R || \
+ t = DMA_BUF_ACCESS_W || \
+ t = DMA_BUF_ACCESS_DMA_R || \
+ t = DMA_BUF_ACCESS_DMA_W || \
+ t = DMA_BUF_ACCESS_RW || \
+ t = DMA_BUF_ACCESS_DMA_RW)
+
/**
* get_dma_buf - convenience wrapper for get_file.
* @dmabuf: [in] pointer to dma_buf
diff --git a/include/linux/dmabuf-sync.h b/include/linux/dmabuf-sync.h
new file mode 100644
index 0000000..9a3afc4
--- /dev/null
+++ b/include/linux/dmabuf-sync.h
@@ -0,0 +1,190 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics Co.Ltd
+ * Authors:
+ * Inki Dae <inki.dae@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/mutex.h>
+#include <linux/sched.h>
+#include <linux/dma-buf.h>
+
+enum dmabuf_sync_status {
+ DMABUF_SYNC_GOT = 1,
+ DMABUF_SYNC_LOCKED,
+};
+
+struct dmabuf_sync_reservation {
+ struct ww_mutex sync_lock;
+ struct mutex lock;
+ wait_queue_head_t poll_wait;
+ unsigned int poll_event;
+ unsigned int polled;
+ atomic_t shared_cnt;
+ unsigned int accessed_type;
+ unsigned int locked;
+};
+
+/*
+ * A structure for dmabuf_sync_object.
+ *
+ * @head: A list head to be added to syncs list.
+ * @robj: A reservation_object object.
+ * @dma_buf: A dma_buf object.
+ * @access_type: Indicate how a current task tries to access
+ * a given buffer.
+ */
+struct dmabuf_sync_object {
+ struct list_head head;
+ struct dmabuf_sync_reservation *robj;
+ struct dma_buf *dmabuf;
+ unsigned int access_type;
+};
+
+struct dmabuf_sync_priv_ops {
+ void (*free)(void *priv);
+};
+
+/*
+ * A structure for dmabuf_sync.
+ *
+ * @syncs: A list head to sync object and this is global to system.
+ * @list: A list entry used as committed list node
+ * @lock: A mutex lock to current sync object.
+ * @ctx: A current context for ww mutex.
+ * @work: A work struct to release resources at timeout.
+ * @priv: A private data.
+ * @name: A string to dmabuf sync owner.
+ * @timer: A timer list to avoid lockup and release resources.
+ * @status: Indicate current status (DMABUF_SYNC_GOT or DMABUF_SYNC_LOCKED).
+ */
+struct dmabuf_sync {
+ struct list_head syncs;
+ struct list_head list;
+ struct mutex lock;
+ struct ww_acquire_ctx ctx;
+ struct work_struct work;
+ void *priv;
+ struct dmabuf_sync_priv_ops *ops;
+ char name[64];
+ struct timer_list timer;
+ unsigned int status;
+};
+
+#ifdef CONFIG_DMABUF_SYNC
+
+extern struct ww_class dmabuf_sync_ww_class;
+
+static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *obj;
+
+ obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+ if (!obj)
+ return;
+
+ dmabuf->sync = obj;
+
+ ww_mutex_init(&obj->sync_lock, &dmabuf_sync_ww_class);
+
+ mutex_init(&obj->lock);
+ atomic_set(&obj->shared_cnt, 1);
+
+ init_waitqueue_head(&obj->poll_wait);
+}
+
+static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *obj;
+
+ if (!dmabuf->sync)
+ return;
+
+ obj = dmabuf->sync;
+
+ ww_mutex_destroy(&obj->sync_lock);
+
+ kfree(obj);
+}
+
+extern bool is_dmabuf_sync_supported(void);
+
+extern struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv);
+
+extern void dmabuf_sync_fini(struct dmabuf_sync *sync);
+
+extern int dmabuf_sync_lock(struct dmabuf_sync *sync);
+
+extern int dmabuf_sync_unlock(struct dmabuf_sync *sync);
+
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
+ bool wait);
+
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf);
+
+extern int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
+ unsigned int type);
+
+extern void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf);
+
+extern void dmabuf_sync_put_all(struct dmabuf_sync *sync);
+
+#else
+
+static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf) { }
+
+static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf) { }
+
+static inline bool is_dmabuf_sync_supported(void) { return false; }
+
+static inline struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv)
+{
+ return ERR_PTR(0);
+}
+
+static inline void dmabuf_sync_fini(struct dmabuf_sync *sync) { }
+
+static inline int dmabuf_sync_lock(struct dmabuf_sync *sync)
+{
+ return 0;
+}
+
+static inline int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+{
+ return 0;
+}
+
+static inline int dmabuf_sync_single_lock(struct dma_buf *dmabuf,
+ unsigned int type,
+ bool wait)
+{
+ return 0;
+}
+
+static inline void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+{
+ return;
+}
+
+static inline int dmabuf_sync_get(struct dmabuf_sync *sync,
+ void *sync_buf,
+ unsigned int type)
+{
+ return 0;
+}
+
+static inline void dmabuf_sync_put(struct dmabuf_sync *sync,
+ struct dma_buf *dmabuf) { }
+
+static inline void dmabuf_sync_put_all(struct dmabuf_sync *sync) { }
+
+#endif
--
1.7.5.4
^ permalink raw reply related
* [PATCH 2/2] [RFC PATCH v2] dma-buf: Add user interfaces for dmabuf sync support.
From: Inki Dae @ 2013-08-13 9:19 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
In-Reply-To: <1376385576-9039-1-git-send-email-inki.dae@samsung.com>
This patch adds lock and poll callbacks to dma buf file operations,
and these callbacks will be called by fcntl and select system calls.
fcntl and select system calls can be used to wait for the completion
of DMA or CPU access to a shared dmabuf. The difference of them is
fcntl system call takes a lock after the completion but select system
call doesn't. So in case of fcntl system call, it's useful when a task
wants to access a shared dmabuf without any broken. On the other hand,
it's useful when a task wants to just wait for the completion.
Changelog v2:
- Add select system call support.
. The purpose of this feature is to wait for the completion of DMA or
CPU access to a dmabuf without that caller locks the dmabuf again
after the completion.
That is useful when caller wants to be aware of the completion of
DMA access to the dmabuf, and the caller doesn't use intefaces for
the DMA device driver.
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/base/dma-buf.c | 81 +++++++++++++++++++++++++++++++++++++++++++
include/linux/dmabuf-sync.h | 1 +
2 files changed, 82 insertions(+), 0 deletions(-)
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 4aca57a..f16a396 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -29,6 +29,7 @@
#include <linux/export.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/poll.h>
#include <linux/dmabuf-sync.h>
static inline int is_dma_buf_file(struct file *);
@@ -80,9 +81,89 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
return dmabuf->ops->mmap(dmabuf, vma);
}
+static unsigned int dma_buf_poll(struct file *filp,
+ struct poll_table_struct *poll)
+{
+ struct dma_buf *dmabuf;
+ struct dmabuf_sync_reservation *robj;
+ int ret = 0;
+
+ if (!is_dma_buf_file(filp))
+ return POLLERR;
+
+ dmabuf = filp->private_data;
+ if (!dmabuf || !dmabuf->sync)
+ return POLLERR;
+
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ robj->polled = true;
+
+ /*
+ * CPU or DMA access to this buffer has been completed, and
+ * the blocked task has been waked up. Return poll event
+ * so that the task can get out of select().
+ */
+ if (robj->poll_event) {
+ robj->poll_event = false;
+ mutex_unlock(&robj->lock);
+ return POLLIN | POLLOUT;
+ }
+
+ /*
+ * There is no anyone accessing this buffer so just return.
+ */
+ if (!robj->locked) {
+ mutex_unlock(&robj->lock);
+ return POLLIN | POLLOUT;
+ }
+
+ poll_wait(filp, &robj->poll_wait, poll);
+
+ mutex_unlock(&robj->lock);
+
+ return ret;
+}
+
+static int dma_buf_lock(struct file *file, int cmd, struct file_lock *fl)
+{
+ struct dma_buf *dmabuf;
+ unsigned int type;
+ bool wait = false;
+
+ if (!is_dma_buf_file(file))
+ return -EINVAL;
+
+ dmabuf = file->private_data;
+
+ if ((fl->fl_type & F_UNLCK) = F_UNLCK) {
+ dmabuf_sync_single_unlock(dmabuf);
+ return 0;
+ }
+
+ /* convert flock type to dmabuf sync type. */
+ if ((fl->fl_type & F_WRLCK) = F_WRLCK)
+ type = DMA_BUF_ACCESS_W;
+ else if ((fl->fl_type & F_RDLCK) = F_RDLCK)
+ type = DMA_BUF_ACCESS_R;
+ else
+ return -EINVAL;
+
+ if (fl->fl_flags & FL_SLEEP)
+ wait = true;
+
+ /* TODO. the locking to certain region should also be considered. */
+
+ return dmabuf_sync_single_lock(dmabuf, type, wait);
+}
+
static const struct file_operations dma_buf_fops = {
.release = dma_buf_release,
.mmap = dma_buf_mmap_internal,
+ .poll = dma_buf_poll,
+ .lock = dma_buf_lock,
};
/*
diff --git a/include/linux/dmabuf-sync.h b/include/linux/dmabuf-sync.h
index 9a3afc4..0316f68 100644
--- a/include/linux/dmabuf-sync.h
+++ b/include/linux/dmabuf-sync.h
@@ -11,6 +11,7 @@
*/
#include <linux/mutex.h>
+#include <linux/ww_mutex.h>
#include <linux/sched.h>
#include <linux/dma-buf.h>
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-13 10:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130731061538.GC13289@radagast>
Hi,
On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote:
> Hi,
>
> On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I wrote:
>>>>>>> IMHO we need a lookup method for PHYs, just like for clocks,
>>>>>>> regulators, PWMs or even i2c busses because there are complex cases
>>>>>>> when passing just a name using platform data will not work. I would
>>>>>>> second what Stephen said [1] and define a structure doing things in a
>>>>>>> DT-like way.
>>>>>>>
>>>>>>> Example;
>>>>>>>
>>>>>>> [platform code]
>>>>>>>
>>>>>>> static const struct phy_lookup my_phy_lookup[] = {
>>>>>>>
>>>>>>> PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"),
>>>>>>
>>>>>> The only problem here is that if *PLATFORM_DEVID_AUTO* is used while
>>>>>> creating the device, the ids in the device name would change and
>>>>>> PHY_LOOKUP wont be useful.
>>>>>
>>>>> I don't think this is a problem. All the existing lookup methods already
>>>>> use ID to identify devices (see regulators, clkdev, PWMs, i2c, ...). You
>>>>> can simply add a requirement that the ID must be assigned manually,
>>>>> without using PLATFORM_DEVID_AUTO to use PHY lookup.
>>>>
>>>> And I'm saying that this idea, of using a specific name and id, is
>>>> frought with fragility and will break in the future in various ways when
>>>> devices get added to systems, making these strings constantly have to be
>>>> kept up to date with different board configurations.
>>>>
>>>> People, NEVER, hardcode something like an id. The fact that this
>>>> happens today with the clock code, doesn't make it right, it makes the
>>>> clock code wrong. Others have already said that this is wrong there as
>>>> well, as systems change and dynamic ids get used more and more.
>>>>
>>>> Let's not repeat the same mistakes of the past just because we refuse to
>>>> learn from them...
>>>>
>>>> So again, the "find a phy by a string" functions should be removed, the
>>>> device id should be automatically created by the phy core just to make
>>>> things unique in sysfs, and no driver code should _ever_ be reliant on
>>>> the number that is being created, and the pointer to the phy structure
>>>> should be used everywhere instead.
>>>>
>>>> With those types of changes, I will consider merging this subsystem, but
>>>> without them, sorry, I will not.
>>>
>>> I'll agree with Greg here, the very fact that we see people trying to
>>> add a requirement of *NOT* using PLATFORM_DEVID_AUTO already points to a
>>> big problem in the framework.
>>>
>>> The fact is that if we don't allow PLATFORM_DEVID_AUTO we will end up
>>> adding similar infrastructure to the driver themselves to make sure we
>>> don't end up with duplicate names in sysfs in case we have multiple
>>> instances of the same IP in the SoC (or several of the same PCIe card).
>>> I really don't want to go back to that.
>>
>> If we are using PLATFORM_DEVID_AUTO, then I dont see any way we can give the
>> correct binding information to the PHY framework. I think we can drop having
>> this non-dt support in PHY framework? I see only one platform (OMAP3) going to
>> be needing this non-dt support and we can use the USB PHY library for it.
>
> you shouldn't drop support for non-DT platform, in any case we lived
> without DT (and still do) for years. Gotta find a better way ;-)
hmm..
how about passing the device names of PHY in platform data of the controller?
It should be deterministic as the PHY framework assigns its own id and we
*don't* want to add any requirement that the ID must be assigned manually
without using PLATFORM_DEVID_AUTO. We can get rid of *phy_init_data* in the v10
patch series.
Thanks
Kishon
>
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Tomasz Figa @ 2013-08-13 11:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <520A0E1C.5000306@ti.com>
On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote:
> > Hi,
> >
> > On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I wrote:
> >>>>>>> IMHO we need a lookup method for PHYs, just like for clocks,
> >>>>>>> regulators, PWMs or even i2c busses because there are complex
> >>>>>>> cases
> >>>>>>> when passing just a name using platform data will not work. I
> >>>>>>> would
> >>>>>>> second what Stephen said [1] and define a structure doing things
> >>>>>>> in a
> >>>>>>> DT-like way.
> >>>>>>>
> >>>>>>> Example;
> >>>>>>>
> >>>>>>> [platform code]
> >>>>>>>
> >>>>>>> static const struct phy_lookup my_phy_lookup[] = {
> >>>>>>>
> >>>>>>> PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"),
> >>>>>>
> >>>>>> The only problem here is that if *PLATFORM_DEVID_AUTO* is used
> >>>>>> while
> >>>>>> creating the device, the ids in the device name would change and
> >>>>>> PHY_LOOKUP wont be useful.
> >>>>>
> >>>>> I don't think this is a problem. All the existing lookup methods
> >>>>> already
> >>>>> use ID to identify devices (see regulators, clkdev, PWMs, i2c,
> >>>>> ...). You
> >>>>> can simply add a requirement that the ID must be assigned manually,
> >>>>> without using PLATFORM_DEVID_AUTO to use PHY lookup.
> >>>>
> >>>> And I'm saying that this idea, of using a specific name and id, is
> >>>> frought with fragility and will break in the future in various ways
> >>>> when
> >>>> devices get added to systems, making these strings constantly have
> >>>> to be
> >>>> kept up to date with different board configurations.
> >>>>
> >>>> People, NEVER, hardcode something like an id. The fact that this
> >>>> happens today with the clock code, doesn't make it right, it makes
> >>>> the
> >>>> clock code wrong. Others have already said that this is wrong there
> >>>> as
> >>>> well, as systems change and dynamic ids get used more and more.
> >>>>
> >>>> Let's not repeat the same mistakes of the past just because we
> >>>> refuse to
> >>>> learn from them...
> >>>>
> >>>> So again, the "find a phy by a string" functions should be removed,
> >>>> the
> >>>> device id should be automatically created by the phy core just to
> >>>> make
> >>>> things unique in sysfs, and no driver code should _ever_ be reliant
> >>>> on
> >>>> the number that is being created, and the pointer to the phy
> >>>> structure
> >>>> should be used everywhere instead.
> >>>>
> >>>> With those types of changes, I will consider merging this subsystem,
> >>>> but
> >>>> without them, sorry, I will not.
> >>>
> >>> I'll agree with Greg here, the very fact that we see people trying to
> >>> add a requirement of *NOT* using PLATFORM_DEVID_AUTO already points
> >>> to a
> >>> big problem in the framework.
> >>>
> >>> The fact is that if we don't allow PLATFORM_DEVID_AUTO we will end up
> >>> adding similar infrastructure to the driver themselves to make sure
> >>> we
> >>> don't end up with duplicate names in sysfs in case we have multiple
> >>> instances of the same IP in the SoC (or several of the same PCIe
> >>> card).
> >>> I really don't want to go back to that.
> >>
> >> If we are using PLATFORM_DEVID_AUTO, then I dont see any way we can
> >> give the correct binding information to the PHY framework. I think we
> >> can drop having this non-dt support in PHY framework? I see only one
> >> platform (OMAP3) going to be needing this non-dt support and we can
> >> use the USB PHY library for it.>
> > you shouldn't drop support for non-DT platform, in any case we lived
> > without DT (and still do) for years. Gotta find a better way ;-)
>
> hmm..
>
> how about passing the device names of PHY in platform data of the
> controller? It should be deterministic as the PHY framework assigns its
> own id and we *don't* want to add any requirement that the ID must be
> assigned manually without using PLATFORM_DEVID_AUTO. We can get rid of
> *phy_init_data* in the v10 patch series.
What about slightly altering the concept of v9 to pass a pointer to struct
device instead of device name inside phy_init_data?
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-13 12:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2034985.S0danJZqk4@amdc1227>
On Tuesday 13 August 2013 05:07 PM, Tomasz Figa wrote:
> On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I wrote:
>>>>>>>>> IMHO we need a lookup method for PHYs, just like for clocks,
>>>>>>>>> regulators, PWMs or even i2c busses because there are complex
>>>>>>>>> cases
>>>>>>>>> when passing just a name using platform data will not work. I
>>>>>>>>> would
>>>>>>>>> second what Stephen said [1] and define a structure doing things
>>>>>>>>> in a
>>>>>>>>> DT-like way.
>>>>>>>>>
>>>>>>>>> Example;
>>>>>>>>>
>>>>>>>>> [platform code]
>>>>>>>>>
>>>>>>>>> static const struct phy_lookup my_phy_lookup[] = {
>>>>>>>>>
>>>>>>>>> PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"),
>>>>>>>>
>>>>>>>> The only problem here is that if *PLATFORM_DEVID_AUTO* is used
>>>>>>>> while
>>>>>>>> creating the device, the ids in the device name would change and
>>>>>>>> PHY_LOOKUP wont be useful.
>>>>>>>
>>>>>>> I don't think this is a problem. All the existing lookup methods
>>>>>>> already
>>>>>>> use ID to identify devices (see regulators, clkdev, PWMs, i2c,
>>>>>>> ...). You
>>>>>>> can simply add a requirement that the ID must be assigned manually,
>>>>>>> without using PLATFORM_DEVID_AUTO to use PHY lookup.
>>>>>>
>>>>>> And I'm saying that this idea, of using a specific name and id, is
>>>>>> frought with fragility and will break in the future in various ways
>>>>>> when
>>>>>> devices get added to systems, making these strings constantly have
>>>>>> to be
>>>>>> kept up to date with different board configurations.
>>>>>>
>>>>>> People, NEVER, hardcode something like an id. The fact that this
>>>>>> happens today with the clock code, doesn't make it right, it makes
>>>>>> the
>>>>>> clock code wrong. Others have already said that this is wrong there
>>>>>> as
>>>>>> well, as systems change and dynamic ids get used more and more.
>>>>>>
>>>>>> Let's not repeat the same mistakes of the past just because we
>>>>>> refuse to
>>>>>> learn from them...
>>>>>>
>>>>>> So again, the "find a phy by a string" functions should be removed,
>>>>>> the
>>>>>> device id should be automatically created by the phy core just to
>>>>>> make
>>>>>> things unique in sysfs, and no driver code should _ever_ be reliant
>>>>>> on
>>>>>> the number that is being created, and the pointer to the phy
>>>>>> structure
>>>>>> should be used everywhere instead.
>>>>>>
>>>>>> With those types of changes, I will consider merging this subsystem,
>>>>>> but
>>>>>> without them, sorry, I will not.
>>>>>
>>>>> I'll agree with Greg here, the very fact that we see people trying to
>>>>> add a requirement of *NOT* using PLATFORM_DEVID_AUTO already points
>>>>> to a
>>>>> big problem in the framework.
>>>>>
>>>>> The fact is that if we don't allow PLATFORM_DEVID_AUTO we will end up
>>>>> adding similar infrastructure to the driver themselves to make sure
>>>>> we
>>>>> don't end up with duplicate names in sysfs in case we have multiple
>>>>> instances of the same IP in the SoC (or several of the same PCIe
>>>>> card).
>>>>> I really don't want to go back to that.
>>>>
>>>> If we are using PLATFORM_DEVID_AUTO, then I dont see any way we can
>>>> give the correct binding information to the PHY framework. I think we
>>>> can drop having this non-dt support in PHY framework? I see only one
>>>> platform (OMAP3) going to be needing this non-dt support and we can
>>>> use the USB PHY library for it.>
>>> you shouldn't drop support for non-DT platform, in any case we lived
>>> without DT (and still do) for years. Gotta find a better way ;-)
>>
>> hmm..
>>
>> how about passing the device names of PHY in platform data of the
>> controller? It should be deterministic as the PHY framework assigns its
>> own id and we *don't* want to add any requirement that the ID must be
>> assigned manually without using PLATFORM_DEVID_AUTO. We can get rid of
>> *phy_init_data* in the v10 patch series.
>
> What about slightly altering the concept of v9 to pass a pointer to struct
> device instead of device name inside phy_init_data?
The problem is device might be created very late. (For example in omap4, usb2
phy device gets created when ocp2scp bus is probed). And we have to pass the
init data in board file.
Thanks
Kishon
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox